From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web12.3271.1582807153925758752 for ; Thu, 27 Feb 2020 04:39:14 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=UalrggDN; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1582807153; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=QLHXvAfhJcsba6FaUfmlyA79SnTyODHLYVhsPdX9nKc=; b=UalrggDNMfr8h+/KJPSD/VFpN/7VoTrdEdpBc7SAaEm7wrRTCn6mW9jOEEYUmsBZe0bUoJ WbBRPoR8HLtAueKyxJvL0i0ToXQ6c68RA2RX+eGiab9kUDl0bi2CGonYCSbXN2O6nZNjpH PooqZOMMo9amwuSe7zakg5YfceN8g5E= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-417-rENqKvqkOrifkUIZczcjwg-1; Thu, 27 Feb 2020 07:39:06 -0500 X-MC-Unique: rENqKvqkOrifkUIZczcjwg-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id BAF12189F762; Thu, 27 Feb 2020 12:39:04 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-46.ams2.redhat.com [10.36.116.46]) by smtp.corp.redhat.com (Postfix) with ESMTP id B567B60BE2; Thu, 27 Feb 2020 12:39:02 +0000 (UTC) Subject: Re: [Patch 1/1][edk2-stable202002]BaseTools: Fixed a regression issue in Makefile for incremental build To: Bob Feng , Andrew Fish , Leif Lindholm , Michael D Kinney , Liming Gao Cc: devel@edk2.groups.io, Pierre Gondois References: <20200227094705.25404-1-bob.c.feng@intel.com> From: "Laszlo Ersek" Message-ID: Date: Thu, 27 Feb 2020 13:39:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200227094705.25404-1-bob.c.feng@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 02/27/20 10:47, Bob Feng wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2563 > > 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 GCC5. > 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/BaseTools/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 = self.module_autogen.BuildOption.get('MAKE', {}).get('PATH') > if not MakePath: > EdkLogger.error("build", PARAMETER_MISSING, Message="No Make path available.") > elif "nmake" in MakePath: > _INCLUDE_DEPS_TEMPLATE = 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 = TemplateString(''' > - ${BEGIN} > - -include ${deps_file} > - ${END} > +${BEGIN} > +-include ${deps_file} > +${END} > ''') > > try: > deps_include_str = _INCLUDE_DEPS_TEMPLATE.Replace(deps_file) > except Exception as e: > (1) I agree this should go into edk2-stable202002. Acked-by: Laszlo Ersek (2) Andrew, Leif, Mike: should we extend the hard freeze by a few days? (3) Bob, I'm sad that proper Bugzilla practices are not being followed. - 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! - 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. 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. - 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? - TianoCore#2563 did not receive the Regression keyword. - 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 words. I've fixed up these workflow warts for you now, but come on now! (4) Liming, can you please post the following two tags in response to the patch, on the list: Reviewed-by: Liming Gao Tested-by: Liming Gao 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. I don't have the slightest hope that GitHub.com PRs would fix any of this. No tooling can fix lack of caring. Laszlo