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 <af...@apple.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Leif Lindholm <l...@nuviainc.com> > Cc: Michael D Kinney <michael.d.kin...@intel.com> > Cc: Pierre Gondois <pierre.gond...@arm.com> > Signed-off-by: Bob Feng <bob.c.f...@intel.com> > Reviewed-by: Liming Gao <liming....@intel.com> > Tested-by: Liming Gao <liming....@intel.com> > --- > .../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 <ler...@redhat.com> (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 <liming....@intel.com> Tested-by: Liming Gao <liming....@intel.com> 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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54999): https://edk2.groups.io/g/devel/message/54999 Mute This Topic: https://groups.io/mt/71583896/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
