public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Pankaj Bansal" <pankaj.bansal@nxp.com>
To: Laszlo Ersek <lersek@redhat.com>,
	Leif Lindholm <leif@nuviainc.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>,
	"michael.d.kinney@intel.com" <michael.d.kinney@intel.com>
Cc: Andrew Fish <afish@apple.com>
Subject: Re: [edk2-devel] [PATCH edk2-InfSpecification] Drop statement on package ordering
Date: Tue, 2 Jun 2020 13:37:30 +0000	[thread overview]
Message-ID: <AM0PR04MB6580B9AC140CDAE146301163B08B0@AM0PR04MB6580.eurprd04.prod.outlook.com> (raw)
In-Reply-To: <afad3e6a-18cd-db46-d9c5-3de87aced9cf@redhat.com>



> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Tuesday, June 2, 2020 7:00 PM
> To: Leif Lindholm <leif@nuviainc.com>; devel@edk2.groups.io;
> michael.d.kinney@intel.com
> Cc: Andrew Fish <afish@apple.com>; Pankaj Bansal (OSS)
> <pankaj.bansal@oss.nxp.com>
> Subject: Re: [edk2-devel] [PATCH edk2-InfSpecification] Drop statement on
> package ordering
> 
> On 06/01/20 00:43, Leif Lindholm wrote:
> > 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".
> 
> (since I've been copied)
> 
> I have not been aware of the header name collision scenario (nor that
> the [Packages] ordering was supposed to work around such issues).
> 
> I work strictly with edk2 proper, where a name collision like this can
> be detected, and so should be prevented. (Insert yet another argument
> why keeping platform code outside of edk2 is a bad idea.) In particular,
> a collision between MdePkg and MdeModulePkg would be super bad.
> 
> Which now seems to turn out consistent with my general review point that
> the [Packages] section, like (almost) all other INF file sections,
> should be sorted lexicographically.
> 
> How about replacing
> 
> """
> 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.
> """
> 
> with
> 
> """
> The order in which packages are listed may be relevant. Said order
> specifies in what order include path statements are generated for a
> compiler. Normally, header file name collisions are not expected between
> packages -- they are forbidden in edk2 proper --, but with a module INF
> consuming both edk2-native and out-of-edk2 packages, header file names
> may collide. For setting specific include path priorities, the packages
> may be listed in matching order in the INF file. Listing a package
> earlier will cause a compiler to consider include paths from that
> package earlier.
> """

Nicely summed up! it is much clearer now for anyone like me who wants to port edk2 for his platform.
one more suggestion. should this be mentioned along with above explaination:
"whenever possible use lexicographically ascending order"

> 
> Thanks
> Laszlo


  reply	other threads:[~2020-06-02 13:37 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-29 14:02 [PATCH edk2-InfSpecification] Drop statement on package ordering Leif Lindholm
2020-05-31 22:19 ` Michael D Kinney
2020-05-31 22:43   ` [edk2-devel] " Leif Lindholm
2020-06-01  3:39     ` Pankaj Bansal
2020-06-01  5:15       ` Michael D Kinney
2020-06-01  7:01         ` Pankaj Bansal
2020-06-01 15:31           ` Michael D Kinney
2020-06-02 13:29     ` Laszlo Ersek
2020-06-02 13:37       ` Pankaj Bansal [this message]
2020-06-02 14:22         ` Leif Lindholm
2020-06-02 16:11         ` Laszlo Ersek
2020-06-03  3:12           ` Pankaj Bansal
2020-06-02 14:20       ` Leif Lindholm
2020-06-02 16:20         ` Laszlo Ersek
2020-06-03 11:44           ` Leif Lindholm
2020-06-03 13:43             ` Laszlo Ersek
2020-06-03  3:33 ` Andrew Fish
     [not found] ` <1614EB3F428C08F5.21938@groups.io>
2020-06-03  3:41   ` [edk2-devel] " Andrew Fish

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AM0PR04MB6580B9AC140CDAE146301163B08B0@AM0PR04MB6580.eurprd04.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox