public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: "Duran, Leo" <leo.duran@amd.com>,
	"edk2-devel@ml01.01.org" <edk2-devel@ml01.01.org>
Cc: "michael.d.kinney@intel.com" <michael.d.kinney@intel.com>,
	"jeff.fan@intel.com" <jeff.fan@intel.com>,
	"liming.gao@intel.com" <liming.gao@intel.com>
Subject: Re: LocalApicLib: Why two separate directories?
Date: Fri, 28 Oct 2016 21:22:20 +0200	[thread overview]
Message-ID: <ebd20306-c606-0c8b-5494-8fdc781f188c@redhat.com> (raw)
In-Reply-To: <DM5PR12MB124388F52311AD97E78926CFF9AD0@DM5PR12MB1243.namprd12.prod.outlook.com>

On 10/28/16 21:03, Duran, Leo wrote:
> All,
> Just a quick observation to request comments:
> 
> Since a lot of the code in BaseXApicX2ApicLib.c and BaseXApicLib is
> the same, how about we merge the common code and build the libraries
> from the same directory?
> 
> UefiCpuPkg/Library/LocalApilLib/
> - LocalApicLib.c --> common code
> - BaseXApicLib.c --> legacy APIC code
> - BaseXApicX2ApicLib.c --> X2APIC code
> - BaseXApicLib.inf -> builds from LocalApicLib.c + BaseXApicLib.c
> - BaseXApicX2ApicLib.inf -> builds from LocalApicLib.c + BaseXApicX2ApicLib.c
> 
> Of course, doing this would require modification to existing .DSC files, to point to the appropriate .INF under the merged LocalApicLib directory.
> Would that be too disruptive?

In my opinion, if:
- you post all the patches (for all affected platforms) in a series,
- keep the series bisectable (i.e., all the platforms build at any stage
throughout the series),
- CC the relevant maintainers,
- and they accept (R-b) the patches,

then it should be fine.

Conversions like this usually involve creating a copy with the existent
(or to-be-migrated) functionality in the new place, gradually pointing
the platform DSCs to that place, and once the old INF files / library
instances become unreferenced, they are removed in the last patches.

For simpler (Pkg-internal) code movement this is not always necessary
(you can usually solve it all within a single atomic patch, like the one
you just posted). However, when multiple Pkgs (with multiple sets of
maintainers) are affected, we mostly avoid patches that would
simultaneously straddle two or more Pkgs.

It's almost always possible to structure a series like described above
-- modify just one Pkg per step, gradually flipping the pointers from
"old" to "new", and when "old"'s refcount goes to zero, remove "old".

(Clearly the actual answer comes from the UefiCpuPkg maintainers; I'm
just talking about the process.)

Thanks!
Laszlo



  reply	other threads:[~2016-10-28 19:22 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-28 19:03 LocalApicLib: Why two separate directories? Duran, Leo
2016-10-28 19:22 ` Laszlo Ersek [this message]
2016-10-28 19:31   ` Duran, Leo
2016-10-28 19:28 ` Kinney, Michael D
2016-10-28 19:36   ` Duran, Leo
2016-10-28 19:40   ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ebd20306-c606-0c8b-5494-8fdc781f188c@redhat.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox