From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web11.2266.1591114859604812042 for ; Tue, 02 Jun 2020 09:20:59 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=WRm2AHgC; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1591114858; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ZWGNxUxAOKKGJlaH90ynVKWO71CROSnB47KbHD1/s04=; b=WRm2AHgCzCTffYz6haw19XHnFfutw5lP32xYVgi/Owlx+c2SZ5IznJm7LZSBbqIATmWFLp 9UofjZxn9DTjjRkky2dlKgZG45ywZXZ7VbcOSxSs2Y5UtZwhMijF+n8p9pXBn5K2LVJkZk DBntZ48q0CXjKgygTUjT3MpjSwrinMI= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-463-dq4DW52dN1eyA3tW0Y_zxw-1; Tue, 02 Jun 2020 12:20:30 -0400 X-MC-Unique: dq4DW52dN1eyA3tW0Y_zxw-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 132DDBFC0; Tue, 2 Jun 2020 16:20:29 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-115-66.ams2.redhat.com [10.36.115.66]) by smtp.corp.redhat.com (Postfix) with ESMTP id AEABD60F8D; Tue, 2 Jun 2020 16:20:27 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH edk2-InfSpecification] Drop statement on package ordering To: Leif Lindholm Cc: devel@edk2.groups.io, michael.d.kinney@intel.com, Andrew Fish , Pankaj Bansal References: <20200529140251.23933-1-leif@nuviainc.com> <20200531224339.GA28566@vanye> <20200602142010.GL28566@vanye> From: "Laszlo Ersek" Message-ID: Date: Tue, 2 Jun 2020 18:20:26 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200602142010.GL28566@vanye> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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 , 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