[edk2-devel] [PATCH] BaseTools: Include headers not mentioned in inf are not hashed

Bob Feng bob.c.feng at intel.com
Mon May 13 12:23:04 UTC 2019


I think checking if [Source] includes all the "source" file under module's folder for each module during build would make build slow down, we have been doing a lot to improve the build performance.
Maybe we could create a new python script under BaseTools/Scripts folder to do this check.

Thanks,
Bob 

-----Original Message-----
From: Laszlo Ersek [mailto:lersek at redhat.com] 
Sent: Monday, May 13, 2019 7:39 PM
To: Rodriguez, Christian <christian.rodriguez at intel.com>; devel at edk2.groups.io; felixp at ami.com
Cc: Feng, Bob C <bob.c.feng at intel.com>; Gao, Liming <liming.gao at intel.com>; Zhu, Yonghong <yonghong.zhu at 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 at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of 
>> Felix Polyudov
>> Sent: Friday, May 10, 2019 12:32 PM
>> To: Rodriguez, Christian <christian.rodriguez at intel.com>; 
>> devel at edk2.groups.io; 'lersek at redhat.com' <lersek at redhat.com>
>> Cc: Feng, Bob C <bob.c.feng at intel.com>; Gao, Liming 
>> <liming.gao at intel.com>; Zhu, Yonghong <yonghong.zhu at 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.rodriguez at intel.com]
>>> Sent: Friday, May 10, 2019 3:14 PM
>>> To: devel at edk2.groups.io; Felix Polyudov; 'lersek at 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 at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf 
>>>> Of Felix Polyudov
>>>> Sent: Friday, May 10, 2019 6:41 AM
>>>> To: devel at edk2.groups.io; 'lersek at redhat.com' <lersek at redhat.com>; 
>>>> Rodriguez, Christian <christian.rodriguez at intel.com>
>>>> Cc: Feng, Bob C <bob.c.feng at intel.com>; Gao, Liming 
>>>> <liming.gao at intel.com>; Zhu, Yonghong <yonghong.zhu at intel.com>
>>>> Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not 
>>>> mentioned in inf are not hashed
>>>>
>>>>> -----Original Message-----
>>>>> From: devel at edk2.groups.io [mailto:devel at 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.rodriguez at intel.com>
>>>>>> Cc: Bob Feng <bob.c.feng at intel.com>
>>>>>> Cc: Liming Gao <liming.gao at intel.com>
>>>>>> Cc: Yonghong Zhu <yonghong.zhu at 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 (#40497): https://edk2.groups.io/g/devel/message/40497
Mute This Topic: https://groups.io/mt/31570019/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list