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

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

[Christian Rodriguez]

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(+)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 31721a6f9f..bd282d3ec1 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -4098,8 +4098,10 @@ class ModuleAutoGen(AutoGen):
         if self.Name in GlobalData.gModuleHash[self.Arch] and GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
             return False
         m = hashlib.md5()
+
         # Add Platform level hash
         m.update(GlobalData.gPlatformHash.encode('utf-8'))
+
         # Add Package level hash
         if self.DependentPackageList:
             for Pkg in sorted(self.DependentPackageList, key=lambda x: x.PackageName):
@@ -4118,14 +4120,36 @@ class ModuleAutoGen(AutoGen):
         Content = f.read()
         f.close()
         m.update(Content)
+
         # Add Module's source files
+        localSourceFileList = set()
         if self.SourceFileList:
             for File in sorted(self.SourceFileList, key=lambda x: str(x)):
+                localSourceFileList.add(str(File))
                 f = open(str(File), 'rb')
                 Content = f.read()
                 f.close()
                 m.update(Content)
 
+        # Get a list of Module's local header files not included in metaFile
+        localHeaderList = set()
+        if self.SourceDir:
+            for root, dirs, files in os.walk(self.SourceDir):
+                for aFile in files:
+                    filePath = os.path.join(self.WorkspaceDir, os.path.join(root, aFile))
+                    if not filePath.endswith(('.h', '.H')):
+                        continue
+                    if filePath not in localSourceFileList:
+                        localHeaderList.add(filePath)
+
+        # Add Module's local header files
+        localHeaderList = list(localHeaderList)
+        for File in sorted(localHeaderList):
+            f = open(str(File), 'rb')
+            Content = f.read()
+            f.close()
+            m.update(Content)
+
         ModuleHashFile = path.join(self.BuildDir, self.Name + ".hash")
         if self.Name not in GlobalData.gModuleHash[self.Arch]:
             GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
-- 
2.19.1.windows.1

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

[Carsey, Jaben]

Some questions inline.

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Christian Rodriguez
> Sent: Thursday, May 09, 2019 2:27 PM
> To: devel@edk2.groups.io
> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
> Subject: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned in
> inf are not hashed
> 
> 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(+)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> b/BaseTools/Source/Python/AutoGen/AutoGen.py
> index 31721a6f9f..bd282d3ec1 100644
> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> @@ -4098,8 +4098,10 @@ class ModuleAutoGen(AutoGen):
>          if self.Name in GlobalData.gModuleHash[self.Arch] and
> GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
>              return False
>          m = hashlib.md5()
> +
>          # Add Platform level hash
>          m.update(GlobalData.gPlatformHash.encode('utf-8'))
> +
>          # Add Package level hash
>          if self.DependentPackageList:
>              for Pkg in sorted(self.DependentPackageList, key=lambda x:
> x.PackageName):
> @@ -4118,14 +4120,36 @@ class ModuleAutoGen(AutoGen):
>          Content = f.read()
>          f.close()
>          m.update(Content)
> +
>          # Add Module's source files
> +        localSourceFileList = set()
>          if self.SourceFileList:
>              for File in sorted(self.SourceFileList, key=lambda x: str(x)):
> +                localSourceFileList.add(str(File))
>                  f = open(str(File), 'rb')
>                  Content = f.read()
>                  f.close()
>                  m.update(Content)
> 
> +        # Get a list of Module's local header files not included in metaFile
> +        localHeaderList = set()
> +        if self.SourceDir:
> +            for root, dirs, files in os.walk(self.SourceDir):
> +                for aFile in files:
> +                    filePath = os.path.join(self.WorkspaceDir, os.path.join(root,
> aFile))
> +                    if not filePath.endswith(('.h', '.H')):
> +                        continue
> +                    if filePath not in localSourceFileList:

Confused about localSourceFileList.
Why is it a set and named list?
Why not just use self.SourceFileList?

> +                        localHeaderList.add(filePath)
> +
> +        # Add Module's local header files
> +        localHeaderList = list(localHeaderList)
> +        for File in sorted(localHeaderList):
> +            f = open(str(File), 'rb')
> +            Content = f.read()

Can you use 'with open(...) as...:' syntax to make sure the file is always closed?

> +            f.close()
> +            m.update(Content)
> +
>          ModuleHashFile = path.join(self.BuildDir, self.Name + ".hash")
>          if self.Name not in GlobalData.gModuleHash[self.Arch]:
>              GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
> --
> 2.19.1.windows.1
> 
> 
> 

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

[Christian Rodriguez]

Replies inline.

>-----Original Message-----
>From: Carsey, Jaben
>Sent: Thursday, May 9, 2019 4:39 PM
>To: devel@edk2.groups.io; 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
>
>Some questions inline.
>
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Christian Rodriguez
>> Sent: Thursday, May 09, 2019 2:27 PM
>> To: devel@edk2.groups.io
>> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
>> <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
>> Subject: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned
>> in inf are not hashed
>>
>> 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(+)
>>
>> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
>> b/BaseTools/Source/Python/AutoGen/AutoGen.py
>> index 31721a6f9f..bd282d3ec1 100644
>> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
>> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
>> @@ -4098,8 +4098,10 @@ class ModuleAutoGen(AutoGen):
>>          if self.Name in GlobalData.gModuleHash[self.Arch] and
>> GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
>>              return False
>>          m = hashlib.md5()
>> +
>>          # Add Platform level hash
>>          m.update(GlobalData.gPlatformHash.encode('utf-8'))
>> +
>>          # Add Package level hash
>>          if self.DependentPackageList:
>>              for Pkg in sorted(self.DependentPackageList, key=lambda x:
>> x.PackageName):
>> @@ -4118,14 +4120,36 @@ class ModuleAutoGen(AutoGen):
>>          Content = f.read()
>>          f.close()
>>          m.update(Content)
>> +
>>          # Add Module's source files
>> +        localSourceFileList = set()
>>          if self.SourceFileList:
>>              for File in sorted(self.SourceFileList, key=lambda x: str(x)):
>> +                localSourceFileList.add(str(File))
>>                  f = open(str(File), 'rb')
>>                  Content = f.read()
>>                  f.close()
>>                  m.update(Content)
>>
>> +        # Get a list of Module's local header files not included in metaFile
>> +        localHeaderList = set()
>> +        if self.SourceDir:
>> +            for root, dirs, files in os.walk(self.SourceDir):
>> +                for aFile in files:
>> +                    filePath = os.path.join(self.WorkspaceDir,
>> + os.path.join(root,
>> aFile))
>> +                    if not filePath.endswith(('.h', '.H')):
>> +                        continue
>> +                    if filePath not in localSourceFileList:
>
>Confused about localSourceFileList.
>Why is it a set and named list?
>Why not just use self.SourceFileList?
>
The naming convention could be better and I can address that in a different patch, if we decide to go forward with this idea overall.
It should probably be named a set.
The reason to using this new set is for a few reasons: 
1. self.SourceFileList contains source file paths of class PathClass and not type str
2. If we want to use self.SourceFileList you must convert PathClass to a str for string comparison
The set just allows for a unique list of objects and theoretically faster to check existence.

>> +                        localHeaderList.add(filePath)
>> +
>> +        # Add Module's local header files
>> +        localHeaderList = list(localHeaderList)
>> +        for File in sorted(localHeaderList):
>> +            f = open(str(File), 'rb')
>> +            Content = f.read()
>
>Can you use 'with open(...) as...:' syntax to make sure the file is always closed?
I just used the same implementation as the above existing code. I can definitely change it to use 'with open(...)'.
Though explicitly calling f.close() as below should be making sure the file is always closed.
>
>> +            f.close()
>> +            m.update(Content)
>> +
>>          ModuleHashFile = path.join(self.BuildDir, self.Name + ".hash")
>>          if self.Name not in GlobalData.gModuleHash[self.Arch]:
>>              GlobalData.gModuleHash[self.Arch][self.Name] =
>> m.hexdigest()
>> --
>> 2.19.1.windows.1
>>
>>
>> 

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

[Carsey, Jaben]

Inline.

tldr: good answers. 

If change list to set in name of set object:
Reviewed-by: jaben carsey <Jaben.carsey@intel.com>

> -----Original Message-----
> From: Rodriguez, Christian
> Sent: Friday, May 10, 2019 8:28 AM
> To: Carsey, Jaben <jaben.carsey@intel.com>; devel@edk2.groups.io
> 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
> Importance: High
> 
> Replies inline.
> 
> >-----Original Message-----
> >From: Carsey, Jaben
> >Sent: Thursday, May 9, 2019 4:39 PM
> >To: devel@edk2.groups.io; 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
> >
> >Some questions inline.
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> >> Christian Rodriguez
> >> Sent: Thursday, May 09, 2019 2:27 PM
> >> To: devel@edk2.groups.io
> >> Cc: Feng, Bob C <bob.c.feng@intel.com>; Gao, Liming
> >> <liming.gao@intel.com>; Zhu, Yonghong <yonghong.zhu@intel.com>
> >> Subject: [edk2-devel] [PATCH] BaseTools: Include headers not mentioned
> >> in inf are not hashed
> >>
> >> 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(+)
> >>
> >> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> >> b/BaseTools/Source/Python/AutoGen/AutoGen.py
> >> index 31721a6f9f..bd282d3ec1 100644
> >> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> >> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> >> @@ -4098,8 +4098,10 @@ class ModuleAutoGen(AutoGen):
> >>          if self.Name in GlobalData.gModuleHash[self.Arch] and
> >> GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
> >>              return False
> >>          m = hashlib.md5()
> >> +
> >>          # Add Platform level hash
> >>          m.update(GlobalData.gPlatformHash.encode('utf-8'))
> >> +
> >>          # Add Package level hash
> >>          if self.DependentPackageList:
> >>              for Pkg in sorted(self.DependentPackageList, key=lambda x:
> >> x.PackageName):
> >> @@ -4118,14 +4120,36 @@ class ModuleAutoGen(AutoGen):
> >>          Content = f.read()
> >>          f.close()
> >>          m.update(Content)
> >> +
> >>          # Add Module's source files
> >> +        localSourceFileList = set()
> >>          if self.SourceFileList:
> >>              for File in sorted(self.SourceFileList, key=lambda x: str(x)):
> >> +                localSourceFileList.add(str(File))
> >>                  f = open(str(File), 'rb')
> >>                  Content = f.read()
> >>                  f.close()
> >>                  m.update(Content)
> >>
> >> +        # Get a list of Module's local header files not included in metaFile
> >> +        localHeaderList = set()
> >> +        if self.SourceDir:
> >> +            for root, dirs, files in os.walk(self.SourceDir):
> >> +                for aFile in files:
> >> +                    filePath = os.path.join(self.WorkspaceDir,
> >> + os.path.join(root,
> >> aFile))
> >> +                    if not filePath.endswith(('.h', '.H')):
> >> +                        continue
> >> +                    if filePath not in localSourceFileList:
> >
> >Confused about localSourceFileList.
> >Why is it a set and named list?
> >Why not just use self.SourceFileList?
> >
> The naming convention could be better and I can address that in a different
> patch, if we decide to go forward with this idea overall.
> It should probably be named a set.
> The reason to using this new set is for a few reasons:
> 1. self.SourceFileList contains source file paths of class PathClass and not type
> str
> 2. If we want to use self.SourceFileList you must convert PathClass to a str for
> string comparison
> The set just allows for a unique list of objects and theoretically faster to
> check existence.

I agree and really prefer the set datatype for this operation, the name is confusing.  I wonder what the ROI is for the custom PathClass sometimes.  Seems confusing often.

> 
> >> +                        localHeaderList.add(filePath)
> >> +
> >> +        # Add Module's local header files
> >> +        localHeaderList = list(localHeaderList)
> >> +        for File in sorted(localHeaderList):
> >> +            f = open(str(File), 'rb')
> >> +            Content = f.read()
> >
> >Can you use 'with open(...) as...:' syntax to make sure the file is always
> closed?
> I just used the same implementation as the above existing code. I can
> definitely change it to use 'with open(...)'.
> Though explicitly calling f.close() as below should be making sure the file is
> always closed.

Agreed, except if something raises an exception.  I think we should change all the code myself.

> >
> >> +            f.close()
> >> +            m.update(Content)
> >> +
> >>          ModuleHashFile = path.join(self.BuildDir, self.Name + ".hash")
> >>          if self.Name not in GlobalData.gModuleHash[self.Arch]:
> >>              GlobalData.gModuleHash[self.Arch][self.Name] =
> >> m.hexdigest()
> >> --
> >> 2.19.1.windows.1
> >>
> >>
> >> 

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

[Laszlo Ersek]

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

  [PATCH 00/45]
  ArmVirtPkg, OvmfPkg: list module-internal headers in INF files

So my thinking is that this change is neither really required (people
can simply fix INF files), nor really sufficient.

Here's why the change is not sufficient (deduced purely from the commit
message -- because the commit message clarifies what the BZ report
didn't). Assume you are developing a module (driver or library) that
consumes a new EDKII extension protocol that your series *also*
introduces, under MdeModulePkg/Include/Protocol, in an earlier,
stand-alone patch. So one of your later patches, in the driver or lib
instance, says:

#include <Protocol/EdkiiAwesomeCoolFeature.h>

As you develop the driver, and perhaps even other code that consumes the
protocol produced by the new driver, you realize you need a new member
function, or one of the member functions needs a new parameter. You
change the protocol header file -- for example, you insert a new
function pointer at the *top* of the protocol structure -- and some
consumer code now breaks in testing. Because, the consumer code's hash
was never changed, and so it was never rebuilt, against the member
function field offsets from the updated protocol structure.

In other words, BaseTools doesn't do recursive dependency tracking (with
or without hashes). It's not a huge deal, it's just why I'm saying this
patch is not enough to address the real problem. And, if we had
recursive dependency tracking (i.e. a change in the hash of the *public*
<EdkiiAwesomeCoolFeature.h>  header triggered a rebuild of all dependent
modules), then we wouldn't have to look at module-internal unlisted
header files specifically.

Now, if the present change is being implemented because it's a small
patch, in comparison to updating hundreds of INF files in out-of-tree
platforms, I'm happy with that; just please state this motivation in the
commit message (and the BZ too, if possible).

One comment below, on the code:

> 
> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py b/BaseTools/Source/Python/AutoGen/AutoGen.py
> index 31721a6f9f..bd282d3ec1 100644
> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> @@ -4098,8 +4098,10 @@ class ModuleAutoGen(AutoGen):
>          if self.Name in GlobalData.gModuleHash[self.Arch] and GlobalData.gBinCacheSource and self.AttemptModuleCacheCopy():
>              return False
>          m = hashlib.md5()
> +
>          # Add Platform level hash
>          m.update(GlobalData.gPlatformHash.encode('utf-8'))
> +
>          # Add Package level hash
>          if self.DependentPackageList:
>              for Pkg in sorted(self.DependentPackageList, key=lambda x: x.PackageName):
> @@ -4118,14 +4120,36 @@ class ModuleAutoGen(AutoGen):
>          Content = f.read()
>          f.close()
>          m.update(Content)
> +
>          # Add Module's source files
> +        localSourceFileList = set()
>          if self.SourceFileList:
>              for File in sorted(self.SourceFileList, key=lambda x: str(x)):
> +                localSourceFileList.add(str(File))
>                  f = open(str(File), 'rb')
>                  Content = f.read()
>                  f.close()
>                  m.update(Content)
>  
> +        # Get a list of Module's local header files not included in metaFile
> +        localHeaderList = set()
> +        if self.SourceDir:
> +            for root, dirs, files in os.walk(self.SourceDir):
> +                for aFile in files:
> +                    filePath = os.path.join(self.WorkspaceDir, os.path.join(root, aFile))
> +                    if not filePath.endswith(('.h', '.H')):

".H" files should not be covered, in my opinion, as they stand for C++
header files under at least some toolchains, and C++ is not a supported
edk2 language.

... But, if there are hundreds of .H files out-of-tree, I guess I can be
convinced about this too. :)

Thanks
Laszlo

> +                        continue
> +                    if filePath not in localSourceFileList:
> +                        localHeaderList.add(filePath)
> +
> +        # Add Module's local header files
> +        localHeaderList = list(localHeaderList)
> +        for File in sorted(localHeaderList):
> +            f = open(str(File), 'rb')
> +            Content = f.read()
> +            f.close()
> +            m.update(Content)
> +
>          ModuleHashFile = path.join(self.BuildDir, self.Name + ".hash")
>          if self.Name not in GlobalData.gModuleHash[self.Arch]:
>              GlobalData.gModuleHash[self.Arch][self.Name] = m.hexdigest()
> 

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

[Felix Polyudov]

> -----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.

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

[Christian Rodriguez]

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    칷! +-     糊{^  &

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

[Felix Polyudov]

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.

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

[Christian Rodriguez]

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.

>-----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    칷! +-     糊{^  &

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

[Laszlo Ersek]

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    칷! +-     糊{^  &

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

[Bob Feng]

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@redhat.com] 
Sent: Monday, May 13, 2019 7:39 PM
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    칷! +-     糊{^  &

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

[Christian Rodriguez]

Yes there could be a performance hit. It might be better to make it a script.
But this is only going to happen when the hash feature is enabled, not ever build. Also we can minimize the performance hit by saving the data from ModuleMakefile.GetFileDependency() to memory and reuse it later in GenMake to save on IO and search.

Thanks,
Christian

>-----Original Message-----
>From: Feng, Bob C
>Sent: Monday, May 13, 2019 5:23 AM
>To: Laszlo Ersek <lersek@redhat.com>; Rodriguez, Christian
><christian.rodriguez@intel.com>; devel@edk2.groups.io; felixp@ami.com
>Cc: 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
>
>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@redhat.com]
>Sent: Monday, May 13, 2019 7:39 PM
>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    칷! +-     糊{^  &

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

[Bob Feng]

I entered a new BZ https://bugzilla.tianocore.org/show_bug.cgi?id=1804 to track the [Sources] section check function.

Agree that "minimize the performance hit by saving the data from ModuleMakefile.GetFileDependency() to memory and reuse it later in GenMake to save on IO and search."

Thanks,
Bob

-----Original Message-----
From: Rodriguez, Christian 
Sent: Tuesday, May 14, 2019 2:41 AM
To: Feng, Bob C <bob.c.feng@intel.com>; Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; felixp@ami.com
Cc: 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

Yes there could be a performance hit. It might be better to make it a script.
But this is only going to happen when the hash feature is enabled, not ever build. Also we can minimize the performance hit by saving the data from ModuleMakefile.GetFileDependency() to memory and reuse it later in GenMake to save on IO and search.

Thanks,
Christian

>-----Original Message-----
>From: Feng, Bob C
>Sent: Monday, May 13, 2019 5:23 AM
>To: Laszlo Ersek <lersek@redhat.com>; Rodriguez, Christian 
><christian.rodriguez@intel.com>; devel@edk2.groups.io; felixp@ami.com
>Cc: 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
>
>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@redhat.com]
>Sent: Monday, May 13, 2019 7:39 PM
>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    칷! +-     糊{^  &

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

[Christian Rodriguez]

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: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    칷! +-     糊{^  &

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

[Laszlo Ersek]

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    칷! +-     糊{^  &
> 
> 
> 
> 

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

[Christian Rodriguez]

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    칷! +-     糊{^  &
>>
>>
>> 
>>

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

[Christian Rodriguez]

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 <lersek@redhat.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
>
>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: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    칷! +-     糊{^  &
>
>
>

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

[Bob Feng]

Hi Christian,

ModuleMakefile.GetFileDependency()  gets the header file recursively for each source file under Inf [Sources] section. You can check if this function can resolve the problem.

Thanks,
Bob

-----Original Message-----
From: Rodriguez, Christian 
Sent: Saturday, May 11, 2019 3:45 AM
To: devel@edk2.groups.io; felixp@ami.com; '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

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.

>-----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    칷! +-     糊{^  &