So the direction put forward by my team is that we should raise an error instead of a warning because a false positive successful build would be detrimental to a continuous integration environment. And it would also enforce the Spec requirements.
The point they are making is that a warning does not emphasizes that there is a false successful build as much as breaking the build. And for now it will be at a limited scope only with the hashing feature enabled so it won't catch people doing a normal build by surprise. Thanks, Christian >-----Original Message----- >From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >Christian Rodriguez >Sent: Monday, May 13, 2019 11:54 AM >To: Laszlo Ersek <ler...@redhat.com>; devel@edk2.groups.io; >fel...@ami.com >Cc: Feng, Bob C <bob.c.f...@intel.com>; Gao, Liming ><liming....@intel.com>; Zhu, Yonghong <yonghong....@intel.com> >Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned >in inf are not hashed > >I think a warning would be reasonable. > >I only mention the spec because it requires all headers to be in the sources >section of the inf, but it's not enforced strictly by BaseTools. Though the >hashing feature relies on this requirement. It not a big deal, I just wanted to >make sure false positive build successes were addressed. > >Thanks, >Christian > >>-----Original Message----- >>From: Laszlo Ersek [mailto:ler...@redhat.com] >>Sent: Monday, May 13, 2019 4:39 AM >>To: Rodriguez, Christian <christian.rodrig...@intel.com>; >>devel@edk2.groups.io; fel...@ami.com >>Cc: Feng, Bob C <bob.c.f...@intel.com>; Gao, Liming >><liming....@intel.com>; Zhu, Yonghong <yonghong....@intel.com> >>Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not >>mentioned in inf are not hashed >> >>On 05/10/19 21:45, Rodriguez, Christian wrote: >>> Hashing is not changing file format requirements as Basetools has no >>requirement on this even though the spec does have file requirements. >>That's why the initial patch was a workaround of sorts because it is >>allowed by Basetools to have local headers not in the sources section of the >meta file. >>> >>> Always breaking the build is outside of the scope of this BZ and my >>> project >>priorities. I agree it should be done, but it's out of my scope. >>> >>> I am specifically targeting the hashing feature, which relies on UEFI >>> Spec >>requirements. >> >>I think breaking the build immediately (and unconditionally) could >>catch platforms by surprise. Can we make this a warning vs. an error? >>And, I'm totally OK if it's available only with --hash, for now. >> >>BTW -- I'm not sure why the UEFI spec is relevant here. >> >>Thanks >>Laszlo >> >>>> -----Original Message----- >>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>>> Of Felix Polyudov >>>> Sent: Friday, May 10, 2019 12:32 PM >>>> To: Rodriguez, Christian <christian.rodrig...@intel.com>; >>>> devel@edk2.groups.io; 'ler...@redhat.com' <ler...@redhat.com> >>>> Cc: Feng, Bob C <bob.c.f...@intel.com>; Gao, Liming >>>> <liming....@intel.com>; Zhu, Yonghong <yonghong....@intel.com> >>>> Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not >>>> mentioned in inf are not hashed >>>> >>>> My suggestion would be to always break a build (no matter what the >>>> hashing settings are). >>>> Hashing is just an optimization technique, usage of which should not >>>> be changing source file formatting requirements. >>>> >>>>> -----Original Message----- >>>>> From: Rodriguez, Christian [mailto:christian.rodrig...@intel.com] >>>>> Sent: Friday, May 10, 2019 3:14 PM >>>>> To: devel@edk2.groups.io; Felix Polyudov; 'ler...@redhat.com' >>>>> Cc: Feng, Bob C; Gao, Liming; Zhu, Yonghong >>>>> Subject: RE: [edk2-devel] [PATCH] BaseTools: Include headers not >>>>> mentioned in inf are not hashed >>>>> >>>>> After talking to my colleagues about this, the direction seems to >>>>> be to fundamentally change this BZ. Instead of building this sort >>>>> of workaround feature, we should use the information gathered from >>>>> this >>>> feature to cause the build to break when the hash feature is enabled. >>>> This would force users of the hash feature to be UEFI spec complaint. >>>>> >>>>> What do you guys think; Laszlo and Felix? >>>>> >>>>> I'll update the BZ when I get your input. >>>>> >>>>> Thanks, >>>>> Christian Rodriguez >>>>> >>>>>> -----Original Message----- >>>>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>>>>> Of Felix Polyudov >>>>>> Sent: Friday, May 10, 2019 6:41 AM >>>>>> To: devel@edk2.groups.io; 'ler...@redhat.com' ><ler...@redhat.com>; >>>>>> Rodriguez, Christian <christian.rodrig...@intel.com> >>>>>> Cc: Feng, Bob C <bob.c.f...@intel.com>; Gao, Liming >>>>>> <liming....@intel.com>; Zhu, Yonghong <yonghong....@intel.com> >>>>>> Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not >>>>>> mentioned in inf are not hashed >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On >>>>>>> Behalf Of Laszlo Ersek >>>>>>> Sent: Thursday, May 09, 2019 7:53 PM >>>>>>> >>>>>>> Hello Christian, >>>>>>> >>>>>>> On 05/09/19 23:27, Christian Rodriguez wrote: >>>>>>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1787 >>>>>>>> >>>>>>>> Get a list of local header files that are not present in the >>>>>>>> MetaFile for this module. Add those local header files into the >>>>>>>> hashing algorithm for a module. If a local header file is not >>>>>>>> present in the MetaFile, the module will still build correctly >>>>>>>> though the hashing system didn't know about it before. >>>>>>>> >>>>>>>> Signed-off-by: Christian Rodriguez >>>>>>>> <christian.rodrig...@intel.com> >>>>>>>> Cc: Bob Feng <bob.c.f...@intel.com> >>>>>>>> Cc: Liming Gao <liming....@intel.com> >>>>>>>> Cc: Yonghong Zhu <yonghong....@intel.com> >>>>>>>> --- >>>>>>>> BaseTools/Source/Python/AutoGen/AutoGen.py | 24 >>>>>>>> ++++++++++++++++++++++++ >>>>>>>> 1 file changed, 24 insertions(+) >>>>>>> >>>>>>> I saw the BZ soon after it was reported. I almost commented, but >>>>>>> ultimately I couldn't decide what the real use case was. >>>>>>> >>>>>>> With this particular use case (i.e. INF file is missing some >>>>>>> module-specific header files that it could easily list), I think >>>>>>> I disagree, mildly (not too strongly). E.g., we fixed such >>>>>>> omissions in a bunch of INF files, last March, in the series >>>>>> >>>>>> I agree with Lazlo. >>>>>> According to section 3.9 of the INF specification (https://edk2- >>>>>> docs.gitbooks.io/edk-ii-inf- >>>>>> specification/3_edk_ii_inf_file_format/39_[sources]_sections.html) >>>>>> , all source files (including module header files) must be listed >>>>>> in the [Sources] section. >>>>>> Here is the quote: >>>>>> "All HII Unicode format files must be listed in this section as >>>>>> well as any other "source" type file, such as local module header >>>>>> files, Vfr files, >>>> etc. " >>>>>> >>>>>> So, if file X is used by module Y, but is not listed in Y.inf, >>>>>> it's a violation of the INF spec., which makes it a bug that has to be >fixed. >>>>>> >>>>>> >>>>>> Please consider the environment before printing this email. >>>>>> >>>>>> The information contained in this message may be confidential and >>>>>> proprietary to American Megatrends, Inc. This communication is >>>>>> intended to be read only by the individual or entity to whom it is >>>>>> addressed or by their designee. If the reader of this message is >>>>>> not the intended recipient, you are on notice that any >>>>>> distribution of this message, in any form, is strictly prohibited. >>>>>> Please promptly notify the sender by reply e-mail or by telephone >>>>>> at 770-246-8600, and >>>> then delete or destroy all copies of the transmission. >>>>>> l K q y e ,j a + U ?E e w Ӎ i vM *? ^ >>>>>> ,j N6 ˭y8b :) m >? >>>>>> 躛" }y M5 { j躓 z 'z h+ l ' r zm y 6 . Ȩ z 칷! >>>>>> +- 糊{^ & >>>> >>>> Please consider the environment before printing this email. >>>> >>>> The information contained in this message may be confidential and >>>> proprietary to American Megatrends, Inc. This communication is >>>> intended to be read only by the individual or entity to whom it is >>>> addressed or by their designee. If the reader of this message is not >>>> the intended recipient, you are on notice that any distribution of >>>> this message, in any form, is strictly prohibited. Please promptly >>>> notify the sender by reply e-mail or by telephone at 770-246-8600, >>>> and >>then delete or destroy all copies of the transmission. >>>> l K q y e ,j a + U ?E e w ӎ{ i vM *? ^ ,j >>>> N9 ˭y8b :) m ? >>>> 躛" }y M5 { j躓 z 'z h+ l ' r zm y 6 . Ȩ z 칷! +- >>>> 糊{^ & > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#40535): https://edk2.groups.io/g/devel/message/40535 Mute This Topic: https://groups.io/mt/31570019/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-