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.web10.14065.1591191830527760961 for ; Wed, 03 Jun 2020 06:43:50 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=RhPRsr8/; 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=1591191829; 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=KZDDfgJqR4BiA5B9EOEwP9gM19MLSgZEBUqgZRiL1Vs=; b=RhPRsr8/u0uMG3TxlAGDkTpsyBBGrpxEt7VUtdBZtVw6+NNmTNy5TJ3IFttdP8mogbj/HM 2wHZzIFGzBTxxLJte3h40UAjbgIzP38eMjsm/iVDKc8T+gSIFjgAeKDCGNHt3/Q68xUp6l b+AZo2lFjgzUayfgUOSCBR2mj4/H7YI= 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-43-IuAehsb1OZOC1fUZxjVvKg-1; Wed, 03 Jun 2020 09:43:43 -0400 X-MC-Unique: IuAehsb1OZOC1fUZxjVvKg-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B0BCD107ACF3; Wed, 3 Jun 2020 13:43:41 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-115-50.ams2.redhat.com [10.36.115.50]) by smtp.corp.redhat.com (Postfix) with ESMTP id 57D2C78F12; Wed, 3 Jun 2020 13:43:40 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH edk2-InfSpecification] Drop statement on package ordering To: Leif Lindholm , devel@edk2.groups.io Cc: michael.d.kinney@intel.com, Andrew Fish , Pankaj Bansal References: <20200529140251.23933-1-leif@nuviainc.com> <20200531224339.GA28566@vanye> <20200602142010.GL28566@vanye> <20200603114404.GQ28566@vanye> From: "Laszlo Ersek" Message-ID: Date: Wed, 3 Jun 2020 15:43:39 +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: <20200603114404.GQ28566@vanye> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 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/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 >> , 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