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 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#60599): https://edk2.groups.io/g/devel/message/60599 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] -=-=-=-=-=-=-=-=-=-=-=-
