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

["Christian Rodriguez" <christian.rodriguez@intel.com>]

Oh sorry about that, I misspoke. I meant to say Edk2 INF spec.

Thanks,
Christian

>-----Original Message-----
>From: Laszlo Ersek [mailto:lersek@redhat.com]
>Sent: Monday, May 13, 2019 1:20 PM
>To: devel@edk2.groups.io; Rodriguez, Christian
><christian.rodriguez@intel.com>; felixp@ami.com
>Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
><liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
>Subject: Re: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned
>in inf are not hashed
>
>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:lersek@redhat.com]
>>> Sent: Monday, May 13, 2019 4:39 AM
>>> To: Rodriguez, Christian <christian.rodriguez@intel.com>;
>>> devel@edk2.groups.io; felixp@ami.com
>>> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
>>> <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@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.rodriguez@intel.com>;
>>>>> devel@edk2.groups.io; 'lersek@redhat.com' <lersek@redhat.com>
>>>>> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
>>>>> <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@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@intel.com]
>>>>>> Sent: Friday, May 10, 2019 3:14 PM
>>>>>> To: devel@edk2.groups.io; Felix Polyudov; 'lersek@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; 'lersek@redhat.com'
>>>>>>> <lersek@redhat.com>; Rodriguez, Christian
>>>>>>> <christian.rodriguez@intel.com>
>>>>>>> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
>>>>>>> <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@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.rodriguez@intel.com>
>>>>>>>>> Cc: Bob Feng <bob.c.feng@intel.com>
>>>>>>>>> Cc: Liming Gao <liming.gao@intel.com>
>>>>>>>>> Cc: Yonghong Zhu <yonghong.zhu@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.
>>>>>>> \x01   l   K\x18   q y e  ,j      a  +  U  ?E e   w Ӎ   i   vM     *?  ^  \x7f  ,j   N6 ˭y8b :)  m
>?
>>>>>>> 躛"    }y M5  { ޷ j躓    z    'z   h+  l  '   r  z^[m   y 6  . Ȩ \x0f 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.
>>>>> \x01   l   K\x18   q y e  ,j      a  +  U  ?E e   w ӎ{  i   vM     *?  ^  \x7f  ,j   N9 ˭y8b :)  m  ?
>>>>> 躛"    }y M5  { ޷ j躓    z    'z   h+  l  '   r  z^[m   y 6  . Ȩ \x0f z    칷! +-     糊{^  &
>>
>>
>> 
>>