From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mx.groups.io with SMTP id smtpd.web12.4396.1582812758222920499 for ; Thu, 27 Feb 2020 06:12:38 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.65, mailfrom: liming.gao@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 27 Feb 2020 06:12:37 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,492,1574150400"; d="scan'208";a="438861223" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga006.fm.intel.com with ESMTP; 27 Feb 2020 06:12:36 -0800 Received: from shsmsx601.ccr.corp.intel.com (10.109.6.141) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 27 Feb 2020 06:12:36 -0800 Received: from shsmsx606.ccr.corp.intel.com (10.109.6.216) by SHSMSX601.ccr.corp.intel.com (10.109.6.141) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Thu, 27 Feb 2020 22:12:34 +0800 Received: from shsmsx606.ccr.corp.intel.com ([10.109.6.216]) by SHSMSX606.ccr.corp.intel.com ([10.109.6.216]) with mapi id 15.01.1713.004; Thu, 27 Feb 2020 22:12:34 +0800 From: "Liming Gao" To: "devel@edk2.groups.io" , "lersek@redhat.com" , "Feng, Bob C" , Andrew Fish , Leif Lindholm , "Kinney, Michael D" CC: Pierre Gondois Subject: Re: [edk2-devel] [Patch 1/1][edk2-stable202002]BaseTools: Fixed a regression issue in Makefile for incremental build Thread-Topic: [edk2-devel] [Patch 1/1][edk2-stable202002]BaseTools: Fixed a regression issue in Makefile for incremental build Thread-Index: AQHV7VLylv0Z2uL95U+Yee5J4ZDFFKgudQSAgACaA6A= Date: Thu, 27 Feb 2020 14:12:34 +0000 Message-ID: References: <20200227094705.25404-1-bob.c.feng@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-version: 11.2.0.6 dlp-product: dlpe-windows dlp-reaction: no-action x-originating-ip: [10.239.127.36] MIME-Version: 1.0 Return-Path: liming.gao@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo: Thanks for your quick response. I add my comments.=20 Thanks Liming > -----Original Message----- > From: devel@edk2.groups.io On Behalf Of Laszlo Er= sek > Sent: Thursday, February 27, 2020 8:39 PM > To: Feng, Bob C ; Andrew Fish ; L= eif Lindholm ; Kinney, Michael D > ; Gao, Liming > Cc: devel@edk2.groups.io; Pierre Gondois > Subject: Re: [edk2-devel] [Patch 1/1][edk2-stable202002]BaseTools: Fixed= a regression issue in Makefile for incremental build >=20 > On 02/27/20 10:47, Bob Feng wrote: > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2563 > > > > This patch is to fix a increametal build regression bug > > which happen when using nmake. That's introduced by 818283de3f6d. > > > > If there is white space before !INCLUDE instruction, nmake will not > > process it. Source code's dependent header files are listed in > > ${deps_file} file, if it's not included successfully, nmake will > > not detect the change of those header file. > > > > This patch has been verified in Windows with VS2015 and Linux with GCC= 5. > > The header file add/modify/delete can trig the incremental build with = this fix. > > There is no impact on the clean build. > > > > Cc: Andrew Fish > > Cc: Laszlo Ersek > > Cc: Leif Lindholm > > Cc: Michael D Kinney > > Cc: Pierre Gondois > > Signed-off-by: Bob Feng > > Reviewed-by: Liming Gao > > Tested-by: Liming Gao > > --- > > .../Source/Python/AutoGen/IncludesAutoGen.py | 16 ++++++++-------= - > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/BaseTools/Source/Python/AutoGen/IncludesAutoGen.py b/Base= Tools/Source/Python/AutoGen/IncludesAutoGen.py > > index 0a6314266f45..720d93395aaf 100644 > > --- a/BaseTools/Source/Python/AutoGen/IncludesAutoGen.py > > +++ b/BaseTools/Source/Python/AutoGen/IncludesAutoGen.py > > @@ -50,21 +50,21 @@ class IncludesAutoGen(): > > MakePath =3D self.module_autogen.BuildOption.get('MAKE', {}).= get('PATH') > > if not MakePath: > > EdkLogger.error("build", PARAMETER_MISSING, Message=3D"No= Make path available.") > > elif "nmake" in MakePath: > > _INCLUDE_DEPS_TEMPLATE =3D TemplateString(''' > > - ${BEGIN} > > - !IF EXIST(${deps_file}) > > - !INCLUDE ${deps_file} > > - !ENDIF > > - ${END} > > +${BEGIN} > > +!IF EXIST(${deps_file}) > > +!INCLUDE ${deps_file} > > +!ENDIF > > +${END} > > ''') > > else: > > _INCLUDE_DEPS_TEMPLATE =3D TemplateString(''' > > - ${BEGIN} > > - -include ${deps_file} > > - ${END} > > +${BEGIN} > > +-include ${deps_file} > > +${END} > > ''') > > > > try: > > deps_include_str =3D _INCLUDE_DEPS_TEMPLATE.Replace(deps_= file) > > except Exception as e: > > >=20 > (1) I agree this should go into edk2-stable202002. >=20 > Acked-by: Laszlo Ersek >=20 [Liming] I see this fix is to restore the original behavior before the com= mit 818283de3f6d for !INCLUDE style in Makefile. So, I think the fix risk i= s low. Because this regression causes the basic incremental build not work= with VS nmake tool, I also prefer to add it into this stable tag. > (2) Andrew, Leif, Mike: should we extend the hard freeze by a few days? >=20 [Liming] I will summary all pending patch lists and status into patch list= mail and collect the feedback.=20 > (3) Bob, I'm sad that proper Bugzilla practices are not being followed. >=20 > - Commit 818283de3f6d never referenced TianoCore#2481. If the submitter > forgets about adding such a reference, then it's the reviewer's / > maintainer's responsibility to point it out! >=20 > - In the same vein, when commit 818283de3f6d was pushed, TianoCore#2481 > was not closed. I've had to associate the BZ with the commit now (first: > I had to figure out the right BZ), and then close the BZ. [Liming] Thank your help. I agree that Maintainer and reviewer should remi= nder the submitter to follow BZ process. >=20 > NOTE: GitHub.com Pull Requests would not help *at all* in the face of > such sloppiness; even on GitHub.com, people have to at least *name* > issue numbers in commit messages. >=20 > - TianoCore#2563 (which tracks the regression) identifies *neither* the > BZ for which the regression was introduced (2481), *nor* the faulty > commit (818283de3f6d). You realize it's *completely useless* to file BZs > with such negligence, right? It has no more information than "stuff > broke, we need to fix it" -- but ain't that the general state of things, > at all times? Are you only trying to fill a BZ quota? >=20 > - TianoCore#2563 did not receive the Regression keyword. >=20 > - TianoCore#2563 was not moved to IN_PROGRESS status when the present > patch got posted; nor did it receive a pointer to the patch in the list > archive. (Not even the *subject* was listed in the BZ.) There are no wor= ds. >=20 > I've fixed up these workflow warts for you now, but come on now! >=20 [Liming] Thanks for your information. Now, we have no detail wiki page on = how to use report issue and update status. I would like to update wiki page https://github.com/tianocore/tianocore.gi= thub.io/wiki/Reporting-Issues. > (4) Liming, can you please post the following two tags in response to > the patch, on the list: >=20 [Liming] Sure. With the commit message is update, I give=20 Reviewed-by: Liming Gao Tested-by: Liming Gao Thanks Liming > Reviewed-by: Liming Gao > Tested-by: Liming Gao >=20 > There is no sign on the list that you have ever reviewed and/or tested > this patch, so Bob's including those tags in the commit message right > off the bat is unjustified. It's OK if you tested it off-list > beforehand, but you still need to state that on the list yourself, after > the patch has been posted. >=20 > I don't have the slightest hope that GitHub.com PRs would fix any of > this. No tooling can fix lack of caring. >=20 > Laszlo >=20 >=20 >=20