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


Pankaj Bansal
 

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@...>
Sent: Monday, June 1, 2020 10:45 AM
To: Pankaj Bansal (OSS) <pankaj.bansal@...>; Leif Lindholm
<leif@...>; devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@...>
Cc: Andrew Fish <afish@...>; Laszlo Ersek <lersek@...>
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@...>
Sent: Sunday, May 31, 2020 8:39 PM
To: Leif Lindholm <leif@...>;
devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@...>
Cc: Andrew Fish <afish@...>; Laszlo Ersek
<lersek@...>; Pankaj Bansal (OSS)
<pankaj.bansal@...>
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@...>
Sent: Monday, June 1, 2020 4:14 AM
To: devel@edk2.groups.io; michael.d.kinney@...
Cc: Andrew Fish <afish@...>; Laszlo Ersek
<lersek@...>; Pankaj
Bansal (OSS) <pankaj.bansal@...>
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@...>
Sent: Friday, May 29, 2020 7:03 AM
To: devel@edk2.groups.io
Cc: Kinney, Michael D
<michael.d.kinney@...>;
Andrew Fish <afish@...>; Laszlo Ersek
<lersek@...>; Pankaj Bansal
<pankaj.bansal@...>
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@...>
---

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


Join devel@edk2.groups.io to automatically receive all group messages.