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

["Laszlo Ersek" <lersek@redhat.com>]

On 06/02/20 16:20, Leif Lindholm wrote:
> On Tue, Jun 02, 2020 at 15:29:55 +0200, Laszlo Ersek wrote:
>> I have not been aware of the header name collision scenario (nor that
>> the [Packages] ordering was supposed to work around such issues).
> 
> Nor had I...
> 
>> 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.
>> """
> 
> Could I suggest striking:
> " -- 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"?

I'm sad; that's the part I like the most! ;) That describes the actual
use case (I'm a fan of use case details in commit messages too).

Anyway, I don't insist...

> 
> This document specifies a file format, not automatically edk2-related.

I disagree with this specific statement; the INF spec says "edk2" in the
*name*. It's called "edk2 INF specification".

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Specifications

"This page contains the released versions of the EDK II Specifications
published using Gitbook."

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Specifications#inf

"This document describes the EDK II build information (INF) file format."

The following link doesn't seem to load at the moment:

https://edk2-docs.gitbooks.io/edk-ii-inf-specification/content/v/release/1.27/

but checking the source in the git repo
<https://github.com/tianocore-docs/edk2-InfSpecification>, the actual
text seems to say "EDK II Module Information (INF) File Specification".

The whole feature is related to out-of-tree INF files (where header file
name collisions cannot be easily detected).

> 
> I think we're reaching a point where a major documentation overhaul is
> necessary. I had already been reflecting on how the coding style
> document encompasses more than coding style (at one point it explains
> how while() loops are different from do{}while() loops). And we
> recently had that conversation around struct assignments which some
> maintainers claim are banned, but which is not mentioned in that
> document.
> 
> Not trying to resolve that issue *now*, just reflecting on how some
> things have been added to these documents historically to deal with a
> specific issue, and ended up confusing things as improved development
> practices have made the original problem go away.
> 
> So with the edk2 refences removed, I like your new wording.

OK -- I won't let "perfect" get in the way of "good" :)

Thanks!
Laszlo