On 05/13/19 20:53, Christian Rodriguez wrote: > 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,
That could be required by the edk2 INF spec, yes. It's totally irrelevant for the UEFI spec however. (Originally you wrote, "[...] This would force users of the hash feature to be UEFI spec complaint [...]".) Laszlo > 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 (#40536): https://edk2.groups.io/g/devel/message/40536 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] -=-=-=-=-=-=-=-=-=-=-=-
