On 06/03/20 13:44, Leif Lindholm wrote: > On Tue, Jun 02, 2020 at 18:20:26 +0200, Laszlo Ersek wrote: >> 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). > > My wording "edk2-related" was imprecise to the level of being > incorrect, apologies for adding confusion. > > My intended point wast that here is nothing specific about colliding > with headers in the edk2 repository. This aspect relates to *all* > different repos used by a specific platform. (And given how much magic > the EDK2 build system looks to a newcomer anyway...) > >>> 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" :) > > So, umm, given that the entire actual content of the patch would now > be written by you - could you submit possibly your own patch, and I'll > abandon this one? > > I'd be happy with (or frankly, without :) a Suggested-by.
I can add this to my queue, sure. I'll get to it sometime. If it's urgent, anyone please feel free to post the patch with the updated wording. Thanks, Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#60663): https://edk2.groups.io/g/devel/message/60663 Mute This Topic: https://groups.io/mt/74544111/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
