Re: [edk2-devel] [PATCH edk2-InfSpecification] Drop statement on package ordering

["Pankaj Bansal" <pankaj.bansal@nxp.com>]

Hi Mike,

But doesn't this violate the edk2 c guidelines ?

https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/5_source_files/53_include_files#5-3-2-every-include-file-shall-have-a-unique-name

The case above explicitly forbids the edk2 port to have the same filename that is used by MdePkg / MdeModulePkg.

Which one to follow ??

Regards,
Pankaj Bansal 

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Monday, June 1, 2020 10:45 AM
> To: Pankaj Bansal (OSS) <pankaj.bansal@oss.nxp.com>; Leif Lindholm
> <leif@nuviainc.com>; devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Andrew Fish <afish@apple.com>; Laszlo Ersek <lersek@redhat.com>
> Subject: RE: [edk2-devel] [PATCH edk2-InfSpecification] Drop statement on
> package ordering
> 
> Pankaj Bansal,
> 
> The override package should be listed before the standard package
> in the [Packages] section of an INF file.
> 
> The internal behavior of the build system is to generate makefiles
> that invoke compilers.  The list of -I include paths passed to
> compilers is generated from [Includes] sections of the package DEC
> files listed in an INF [Packages] section.  The order that the
> -I directives are listed is based is the same order that the
> packages are listed in the INF [Packages] section.
> 
> Mike
> 
> > -----Original Message-----
> > From: Pankaj Bansal (OSS) <pankaj.bansal@oss.nxp.com>
> > Sent: Sunday, May 31, 2020 8:39 PM
> > To: Leif Lindholm <leif@nuviainc.com>;
> > devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Cc: Andrew Fish <afish@apple.com>; Laszlo Ersek
> > <lersek@redhat.com>; Pankaj Bansal (OSS)
> > <pankaj.bansal@oss.nxp.com>
> > Subject: RE: [edk2-devel] [PATCH edk2-InfSpecification]
> > Drop statement on package ordering
> >
> > Hi Mike,
> >
> > This means any port of edk2, should it so wish to
> > override the include file provided by edk2 packages
> > (MdePkg or MdeModulePkg),
> > must be listed after these dec files (MdePkg.dec or
> > MdeModulePkg.dec) in an inf file?
> >
> > Regards,
> > Pankaj Bansal
> >
> > > -----Original Message-----
> > > From: Leif Lindholm <leif@nuviainc.com>
> > > Sent: Monday, June 1, 2020 4:14 AM
> > > To: devel@edk2.groups.io; michael.d.kinney@intel.com
> > > Cc: Andrew Fish <afish@apple.com>; Laszlo Ersek
> > <lersek@redhat.com>; Pankaj
> > > Bansal (OSS) <pankaj.bansal@oss.nxp.com>
> > > Subject: Re: [edk2-devel] [PATCH edk2-
> > InfSpecification] Drop statement on
> > > package ordering
> > >
> > > Hi Mike,
> > >
> > > Ok, I'm happy to hear that.
> > >
> > > I agree that the overriding behaviour is useful, and
> > it would be good
> > > to document it. The problem is that the current
> > wording does not say
> > > that (in a way that is useful to anyone who does not
> > already know what
> > > it means). And the MdePkg/MdeModulePkg example sounds
> > positively
> > > horrific when interpreted in this light.
> > >
> > > Clearly, my proposed modification is not the right
> > thing to do here.
> > >
> > > The problem with the document implying that the order
> > should reflect
> > > some sort of hierarchy *apart from when explicitly
> > overriding* is that
> > > this is asking a human to do the thing that humans
> > are bad at and
> > > computers are good at. It can't scale where humans
> > are reviewing ports
> > > that they understand less well than the people
> > contributing them.
> > >
> > > I think we should find a wording that explains the
> > behaviour instead
> > > of explaining some potential derivative of the
> > behaviour, as well as
> > > providing a realistic example instead of the
> > MdePkg/MdeModulePkg
> > > statament.
> > >
> > > My suggestion is to keep it simple: say something
> > like "where there is
> > > a need to override an include file provided by one
> > package with one
> > > provided by another package, know that the compiler
> > invocation will
> > > list the include directories in the same order as the
> > .dec files are
> > > listed in the .inf".
> > >
> > > Regards,
> > >
> > > Leif
> > >
> > > On Sun, May 31, 2020 at 22:19:24 +0000, Michael D
> > Kinney wrote:
> > > > Hi Leif,
> > > >
> > > > The reason for this statement is not for
> > performance.
> > > >
> > > > It is if the same include file exists in the same
> > path
> > > > in more than one package.  Defining this behavior
> > makes
> > > > the build system deterministic.
> > > >
> > > > There is use case where a platform package can
> > provide
> > > > an include override of a common package and the
> > platform
> > > > modules list the platform package before the common
> > > > package in the [Packages] section.
> > > >
> > > > So deterministic build when there are include file
> > > > name collisions and overrides are 2 reasons to keep
> > > > the currently defined behavior.
> > > >
> > > > With this background, perhaps some clarification or
> > > > rewording of the spec is required?  Do you have
> > suggestions?
> > > >
> > > > Thanks,
> > > >
> > > > Mike
> > > >
> > > >   This is not a common use case,
> > > > but one that does apply is a platform module that
> > wants
> > > > to use an override of a standard include file in a
> > platform
> > > > package.
> > > >
> > > > For the build system autogen stage, this statement
> > > >
> > > > Mike
> > > >
> > > > > -----Original Message-----
> > > > > From: Leif Lindholm <leif@nuviainc.com>
> > > > > Sent: Friday, May 29, 2020 7:03 AM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Kinney, Michael D
> > <michael.d.kinney@intel.com>;
> > > > > Andrew Fish <afish@apple.com>; Laszlo Ersek
> > > > > <lersek@redhat.com>; Pankaj Bansal
> > > > > <pankaj.bansal@oss.nxp.com>
> > > > > Subject: [PATCH edk2-InfSpecification] Drop
> > statement
> > > > > on package ordering
> > > > >
> > > > > The description of [Packages] sections stated
> > that
> > > > > "Packages must be listed in the order that may be
> > > > > required for specifying
> > > > >  include path statements for a compiler. For
> > example,
> > > > > the
> > > > >  MdePkg/MdePkg.dec file must be listed before the
> > > > >  MdeModulePkg/MdeModulePkg.dec file."
> > > > >
> > > > > Drop it.
> > > > >
> > > > > Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> > > > > ---
> > > > >
> > > > > Surely this isn't something we take seriously?
> > > > > If there is a measurable performance impact to
> > the
> > > > > order of -I option
> > > > > on the compiler command line, we should approach
> > this
> > > > > programmatically.
> > > > >
> > > > >
> > 3_edk_ii_inf_file_format/37_[packages]_sections.md | 7
> > > > > ++-----
> > > > >  1 file changed, 2 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git
> > > > >
> > a/3_edk_ii_inf_file_format/37_[packages]_sections.md
> > > > >
> > b/3_edk_ii_inf_file_format/37_[packages]_sections.md
> > > > > index 17a8d91..c09112b 100644
> > > > > ---
> > > > >
> > a/3_edk_ii_inf_file_format/37_[packages]_sections.md
> > > > > +++
> > > > >
> > b/3_edk_ii_inf_file_format/37_[packages]_sections.md
> > > > > @@ -42,11 +42,8 @@ Defines the `[Packages]`
> > section tag
> > > > > that is used in EDK II module INF files.
> > > > >  Each entry in this section contains a directory
> > name,
> > > > > forward slash character
> > > > >  and the name of the DEC file contained in the
> > > > > directory name.
> > > > >
> > > > > -Packages must be listed in the order that may be
> > > > > required for specifying
> > > > > -include path statements for a compiler. For
> > example,
> > > > > the _MdePkg/MdePkg.dec_
> > > > > -file must be listed before the
> > > > > `MdeModulePkg/MdeModulePkg.dec` file. If there
> > > > > -are PCDs listed in the generated "As Built" INF,
> > the
> > > > > packages that declare any
> > > > > -PCDs must be listed in this section.
> > > > > +If there are PCDs listed in the generated "As
> > Built"
> > > > > INF, the packages that
> > > > > +declare any PCDs must be listed in this section.
> > > > >
> > > > >  Each package filename must be listed only once
> > per
> > > > > section. Package filenames
> > > > >  listed in architectural sections are not
> > permitted to
> > > > > be listed in the common
> > > > > --
> > > > > 2.20.1
> > > >
> > > >
> > > > 
> > > >