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 (#40495): https://edk2.groups.io/g/devel/message/40495 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] -=-=-=-=-=-=-=-=-=-=-=-