public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: "Ni, Ray" <ray.ni@intel.com>, "Gao, Liming" <liming.gao@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>,
	"Zimmer, Vincent" <vincent.zimmer@intel.com>,
	 "Wolman, Ayellet" <ayellet.wolman@intel.com>,
	 "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Cetola, Stephano" <stephano.cetola@intel.com>,
	 "Kinney, Michael D" <michael.d.kinney@intel.com>,
	Laszlo Ersek <lersek@redhat.com>
Subject: Re: [RFC] Proposal to split Pkgs
Date: Thu, 31 Jan 2019 08:51:41 +0100	[thread overview]
Message-ID: <CAKv+Gu9JvZL5TW49Zzt1AsoV-4N295Ykv_X8kgDctCG4rDfn3Q@mail.gmail.com> (raw)
In-Reply-To: <06bf1381-5f8b-7bda-1d30-df0b03c8f3fd@Intel.com>

On Thu, 31 Jan 2019 at 08:09, Ni, Ray <ray.ni@intel.com> wrote:
>
> On 1/29/2019 10:21 PM, Leif Lindholm wrote:
> > Hi Ray,
> >
> > First of all, thank you for restarting this discussion, and putting
> > the effort in for a POC.
> >
> > On Tue, Jan 29, 2019 at 05:59:35AM +0000, Ni, Ray wrote:
> >> Hello,
> >> I'd like to propose to split today's BIG packages in following ways:
> >>
> >> ==============Overview =================
> >>
> >> 1. Separate Industry standard definitions from UEFI and PI interfaces.
> >> 2. Separate UEFI and PI interfaces from implementations.
> >>      a. Separate UEFI and PI interfaces to different packages
> >>      b. Separate PI PEI, DXE and MM phase interfaces to different packages
> >> 3. Separate different features into feature packages.
> >>      Feature interface may be in the feature package to provide minimal
> >>      common interface packages.
> >>
> >> The POC code is in https://github.com/jyao1/edk2/tree/ReOrg.
> >> It basically split the EDKII common code to three directories:
> >> Core/, Device/, and Feature/.
> >
> > I'm not sure I like the 'Device' name, but that's not that important
> > at this stage - I like that this is split up the way it is.
> >
> > I also think a lot of things currently under Feture/misc could be
> > broken out into additional subdirectories under Feature/.
> >
> >> The code is in very early POC phase and only code in Core/ directory
> >> can pass the build.
> >> I would like to gather feedbacks through this RFC to see whether
> >> this splitting direction makes sense.
> >> Is there any other better ways of splitting?
> >> Or perhaps do not split in such a small granularity?
> >
> > I think this would be my only negative(ish) feedback on the POC.
> >
> > While it has the benefit of modules requiring to specify more
> > precisely which functionality they are pulling in...
> > ...it means that they now need to list *everything*.
> >
> > And with the mechanism by which I have seen Intel engineers make use
> > of PACKAGES_PATH in the past*, we end up with a bazillion packages
> > that need to be added individually to this variable for each build.
>
> No. "PACKAGES_PATH" only contains paths pointing to the *parent*
> directory of *Pkg. In this case, it equals to:
> /work/edk2/Core:/work/edk2/Device:/work/edk2/Feature/security:/work/edk2/Feature/network:/work/edk2/Feature/misc
>

Could we *please* not do this? PACKAGES_PATH is very useful for
combining packages from different repositories, but that does not mean
we should split up the core repository in this way.

What we will end up with is platforms that use

  Core/Package/Foo.inf

and platforms that use

  Package/Foo.inf

which creates an aliasing mess where the same file is reachable in
different ways. Also, it encourages the behavior where you need to
fetch version X of repo A and version Y of repo B, and the whole point
of open sourcing is to have all code co-exist in the same source tree.





>
> >
> > * (arguably how it was designed to be used - just not how I am
> >    interested in using it)
> >
> > It also means that we need *tons* of dummy .dsc files in order to run
> > through simple build tests.
>
> Yes each small package needs its own DSC and DEC.
> DSC to list all components.
> DEC to list all interfaces.
>
> OK maybe the "need" in your wording means the build test script file
> needs to reference all DSC files.
> That maybe can be solved by an advanced script to call "build" for each
> DSC files inside a directory that matches a specific pattern. So the
> build test script to build PiPeiPkg/PiDxePkg/PiMmPkg will be like:
>    build-any-dsc /work/edk2/Core/ --pattern="*Pi*"
>
> >
> > My preference would be to push the packages back up to the top-level.
> > The split still makes sense, and allocating maintainers to sub-parts
> > can happen without a full-out redesign of the Maintainers.txt format.
> > (Which I will try to resurrect with a proposal over the next few weeks
> > anyway.)
>
> Combining all packages inside the Device and Feature directory to still
> one single big package but with another directory layout so that only
> single DSC/DEC is needed?
>
> >
> >> Or perhaps Mike's work to move lib-c content to edk2-libc repo,
> >> to move real platform code to edk2-platform repo is enough for
> >> now?
> >
> > I think we need this further restructuring, and renaming. Not just for
> > splitting up maintainership duties (which as I mentioned in email to
> > Laszlo, we will still need to do with this change). But to make the
> > codebase more approachable.
> >
>
> "more approachable" means more friendly to new comers? Just try to
> understand everyone's real concern to today's codebase.
>
> >> ==============More explanations =================
> >>
> >> ####There are 9 packages inside Core/ directory:
> >> 1. BasePkg
> >> Contains industry standard definitions (exclude UEFI and PI) and base
> >> libraries that non-UEFI and non-PI development can depend on.
> >> UEFI or PI development can also depend on this package.
> >> 2. UefiPkg
> >> Contains UEFI interfaces and libraries that UEFI driver-model driver
> >> development can depend on.
> >> 3. PiPeiPkg
> >> Contains PI interfaces and libraries for PEI phase that PEI module
> >> development can depend on.
> >> 4. PiDxePkg
> >> Contains PI interfaces and libraries for DXE phase that DXE module
> >> development can depend on.
> >
> > Really happy with the above split.
> >
> >> 5. PiMmPkg
> >> Contains PI interfaces and libraries for MM phase that MM/SMM
> >> module development can depend on.
> >
> > How would/should this work relative to StandaloneMmPkg?
> The "Mm" is to reflect to the same word in PI spec.
> But I didn't check whether "StandaloneMmPkg" can work with the this.
>
> >
> >> 6. MdeModulePkg (TianoPkg? Name is still TBD)
> >
> > Yes, this needs to be renamed. I don't object to the Tiano naming.
> >
>
> Thanks.
>
> >> Contains Tiano implementation specific interfaces and libraries.
> >> Developing modules for pure UEFI or PI should not depend on this package.
> >> 7. PeiFoundationPkg
> >> Contains the PEI foundation modules (PeiCore and DxeIpl) and supported
> >> libraries.
> >> 8. DxeFoundationPkg
> >> Contains the DXE foundation modules (DxeCore and RuntimeDxe) and
> >> supported libraries.
> >> 9. SmmFoundationPkg
> >> Contains the SMM foundation modules (SmmCore, SmmIpl and
> >> SmmCommunicationBuffer) and supported libraries.
> >
> > So as a feedback on the whole, this would somewhat help with
> > addressing the confusion caused by PI defining interfaces named EFI_*
> > (which we really ought to address on the spec level at some point).
> >
> >> These packages are positioned in different layers. The package in higher
> >> layer depends on all packages that are in lower layers.
> >> Layer 0: BasePkg.
> >> Layer 1: UefiPkg.
> >
> > This looks a bit weird though - surely PI components should not have
> > dependencies on UEFI components? I presume this is what Laszlo
> > referred to with regards to Base.h and the thread from earlier this
> > month.
>
> I will provide more data on how the dependency is there.
> For now, I know that "EFI_GUID" is defined in UefiPkg. But "EFI_GUID" is
> needed everywhere.
>
> >
> >> Layer 2: PiPeiPkg
> >> Layer 3: PiDxePkg
> >> Layer 4: PiMmPkg
> >> Layer 5: MdeModulePkg (TianoPkg?)
> >>
> >> ####All other modules are put to small packages under Device/ or Feature/.
> >>
> >> ============== Benefit of this proposal =================
> >>
> >> This helps to reduce the size of each package, especially the very BIG
> >> MdeModulePkg which contains almost all edk2 modules (except
> >> CPU, network, etc). So platform can use git sparse checkout feature
> >> to only clone the needed code still in package granularity.
> >> This also helps to separate the code maintenance to more expert
> >> developers. MdeModulePkg is just too huge to be maintained by 2 or 3
> >> developers.
> >
> > So, I technically don't care about any of the above, but I still
> > support this proposal (given some additional tweaks). To me, this is
> > all about reflecting boundaries between specification and
> > implementation, as well as boundaries between specifications.
>
> Good to know what you care.
>
> >
> > Best Regards,
> >
> > Leif
> >
>
>
> --
> Thanks,
> Ray
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


  reply	other threads:[~2019-01-31  7:51 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-29  5:59 [RFC] Proposal to split Pkgs Ni, Ray
2019-01-29 11:11 ` Laszlo Ersek
2019-01-29 11:39   ` Leif Lindholm
2019-01-29 13:13   ` Ni, Ray
2019-01-29 13:28     ` Laszlo Ersek
2019-01-29 14:21 ` Leif Lindholm
2019-01-31  7:12   ` Ni, Ray
2019-01-31  7:51     ` Ard Biesheuvel [this message]
2019-01-31  8:02       ` Ni, Ray
2019-01-31 13:09 ` Andrew Fish

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=CAKv+Gu9JvZL5TW49Zzt1AsoV-4N295Ykv_X8kgDctCG4rDfn3Q@mail.gmail.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