Re: [RFC] migration of OpenPlatformPkg to tianocore

[Leif Lindholm <leif.lindholm@linaro.org>]

Thanks Mike,

On Thu, Jun 01, 2017 at 04:49:25PM +0000, Kinney, Michael D wrote:
> For directory name convention, I do recommend if a directory
> contains a package  DEC file, then the directory should end in
> "Pkg".

Yes, that's fine. I'll do that.

> I would not recommend a change to the behavior of PACKAGES_PATH at
> this time. Let's attempt to work through the detailed proposal with
> current PACKAGES_PATH behavior and see if additional features or
> tools are really required.

I'm OK with rejigging all of this such that we use PACKAGES_PATH in
its current functionality for now. But this is something that will
become prohibitively annoying at tens at platforms.

> One of the key learnings I had in working in the directory structure
> proposal for the edk2 repo is to minimize the number of directory
> levels added so the PACKAGE_PATH setting does not get too long.

Right, but my concern is less with operating system limits on
environment variables than having yet another place to modify when
adding a new platform, or adding a new module to an existing one.

> For the edk2-platforms layout, we can choose to have the default
> setting for PACKAGES_PATH to contain paths to all packages in
> edk2-platforms repo.  This would allow all platforms to be built
> with a single PACKAGES_PATH setting. This is what we would also do
> if we moved forward with adding directory layout to edk2 repo.  This
> does imply that the PACKAGES_PATH could be fairly long (depending on
> depth of directories added to these repos).
> 
> If a developer wants to pull a subset of the content from the edk2
> related repositories using the git sparse checkout feature to limit
> the content to exactly what they need for a specific platform, then
> it would make sense to reduce the number of paths in PACKAGES_PATH
> to match the spare checkout.
> 
> We may need some tool(s) to help manage multiple repos and
> PACKAGES_PATH. For example, you could have a tool that searches a
> repo for .dec files and update PACKAGES_PATH to make all packages in
> that repo available to the build.
>
> IMHO, adding directory structure to the repos is to help put
> packages into Different categories (e.g. core, silicon, platform,
> vendor specific).  The side effect of adding these categories in the
> directory structure is increased complexity in the build
> configuration and build tools.

I do not dispute this. This is why I think in the end something like
WORKSPACES_PATH is going to be a be a much more scalable solution. But
let's revisit that a few months down the line.

Best Regards,

Leif

> 
> Best regards,
> 
> Mike
> 
> 
> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindholm@linaro.org] 
> Sent: Thursday, June 1, 2017 8:33 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: edk2-devel@lists.01.org; Richardson, Brian <brian.richardson@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Chenhui Sun <chenhui.sun@linaro.org>; Andrew Fish <afish@apple.com>; Alan Ott <alan@softiron.co.uk>; Ryan Harkin <ryan.harkin@linaro.org>; Duran, Leo <leo.duran@amd.com>; haojian.zhuang@linaro.org; Linaro UEFI <linaro-uefi@lists.linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Heyi Guo <heyi.guo@linaro.org>
> Subject: Re: [edk2] [RFC] migration of OpenPlatformPkg to tianocore
> 
> Hi Jiewen,
> 
> Apologies for ridicilously slow response - I caught a bad cold and am
> only now getting back on track with this.
> 
> Many thanks for having a look, and your comments.
> 
> On Fri, May 05, 2017 at 02:03:43PM +0000, Yao, Jiewen wrote:
> > Some comments for the build failure.
> > 
> > I think we might have different understanding on PACKAGES_PATH.
> > 
> > We are using below structure:
> > =========================
> > Platform\
> >         Intel\
> >                 PlatformAPkg
> >                 PlatformBPkg
> > Silicon\
> >         Intel\
> >                 SiliconAPkg
> >                 SiliconBPkg
> > 
> > We set PACKAGES_PATH to "Platform\Intel:Silicon\Intel".
> > As such Build.DSC can include PlatformAPkg/DriverA/DriverA.inf.
> > =========================
> > 
> > 
> > My suggestion for your case is below: We can use
> > =========================
> > Platform\
> >         Arm\
> >                 JunoPkg
> >                 VExpressPkg
> 
> Yes, so this is probably a good point.
> I tried to model this repository on Mike's proposal for updated
> directory structure, and is was not entirely clear to me whether the
> *Pkg naming convention was intended to be retained at lower levels
> when it disappeared at the top level.
> 
> /* BEGIN sidetrack */
> My understanding from before was that the "package" convention stemmed
> from a situation where each top-level directory was treated as an "IP
> silo". Since that aspect disappeared with the proposed layout changes,
> and the edk2-* repositories can now be seen much more (if not
> entirely) as cohesive structures, I had simply assumed the Pkg suffix
> convention would disappear. I do not actually have a strong opinion on
> the matter.
> /* END sidetrack */
> 
> In the specific cases of JunoPkg and VExpressPkg, these are not
> technically "packages" in this tree, since their .dec files are still
> in edk2. However, these should move, and if the Pkg naming convention
> remains, they should indeed be renamed.
> 
> > 
> > We set PACKAGES_PATH to "Platform\Arm".
> > ArmJunoPkg.dsc can just use "VExpressPkg\ArmVExpress.dsc.inc" and "JunoPkg\Library\JunoPciHostBridgeLib\ JunoPciHostBridgeLib.dsc"
> > =========================
> > 
> > The benefit of adding "Pkg" is that people can have a clear picture on where the path starts from. :)
> 
> OK, so I think we have a situation where I have misunderstood the
> intended purpose of the PACKAGES_PATH mechanism. But, I would like to
> discuss the possibility of extending it to cover the use-case that I
> thought it provided - or potentially adding new functionality to
> provide the same.
> 
> Since my goal is to have many platforms (for scope, let's call it
> hundreds) in the edk2-platforms master branch, needing to specify for
> each platform all directories that contains packages used by that
> platform becomes quite tedious. Especially when that stretches across
> multiple packages in multiple repositories.
> 
> As an example, take the SoftIron 1000 platform. If the PACKAGES_PATH
> needs to contain all directories holding packages used outside of
> edk2, I would need to add:
> - edk2-non-osi/Platform/SoftIron/
> - edk2-non-osi/Silicon/AMD/Styx/
> - edk2-platforms/Platform/SoftIron/
> - edk2-platforms/Silicon/AMD/Styx/
> 
> And for each other platform, I would need to specify a similar (unique
> for that platform) PACKAGES_PATH.
> And I can see more complex splits than that appearing.
> 
> Now, if everything I want to build resides across the master branch of
> edk2, the master branch of edk2-platforms and the master branch of
> edk2-non-osi, then I just need a way for the build command to search
> in all three repositories. At that point, the only platform-specific
> information I need in order to build is the path to the .dsc.
> 
> So, what I was hoping for was a way to simply extend the path
> resolution in .dsc/.fdf files to multiple repositories. I had hoped
> PACKAGES_PATH could do this, but since this was never its design goal
> I understand why it does not. Do you think the PACKAGES_PATH
> functionality could/should be extended to support this, or do I need a
> new feature (WORKSPACES_PATH?)?
> 
> Best Regards,
> 
> Leif
> 
> > Thank you
> > Yao Jiewen
> > 
> > 
> > 
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen
> > Sent: Friday, May 5, 2017 9:46 PM
> > To: Leif Lindholm <leif.lindholm@linaro.org>; edk2-devel@lists.01.org
> > Cc: Richardson, Brian <brian.richardson@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Chenhui Sun <chenhui.sun@linaro.org>; Andrew Fish <afish@apple.com>; Alan Ott <alan@softiron.co.uk>; Ryan Harkin <ryan.harkin@linaro.org>; Duran, Leo <leo.duran@amd.com>; haojian.zhuang@linaro.org; Linaro UEFI <linaro-uefi@lists.linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Heyi Guo <heyi.guo@linaro.org>
> > Subject: Re: [edk2] [RFC] migration of OpenPlatformPkg to tianocore
> > 
> > HI Leif
> > It is great that you are adding more platform to edk2-platforms.
> > 
> > We (Intel) also have plan to add more boards there. In general, we are very close on having silicon and platform folder.
> > 
> > I have a quick look. One minor suggestions here:
> > Take Arm folder as example. (I assume Juno is one package, and VExpress is the other package.)
> > Can we name Juno to be JunoPkg, and VExpress to be VExpressPkg ?
> > We do not add "Pkg" to a folder. And we usually add "Pkg" suffix to a package.
> > 
> > In general, I think it is a very good start.
> > I may review the content in more detail and provide more feedback.
> > 
> > 
> > Thank you
> > Yao Jiewen
> > 
> > 
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Leif Lindholm
> > Sent: Thursday, May 4, 2017 6:56 AM
> > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > Cc: Ryan Harkin <ryan.harkin@linaro.org<mailto:ryan.harkin@linaro.org>>; Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>; Chenhui Sun <chenhui.sun@linaro.org<mailto:chenhui.sun@linaro.org>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; Alan Ott <alan@softiron.co.uk<mailto:alan@softiron.co.uk>>; Richardson, Brian <brian.richardson@intel.com<mailto:brian.richardson@intel.com>>; Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; haojian.zhuang@linaro.org<mailto:haojian.zhuang@linaro.org>; Linaro UEFI <linaro-uefi@lists.linaro.org<mailto:linaro-uefi@lists.linaro.org>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Heyi Guo <heyi.guo@linaro.org<mailto:heyi.guo@linaro.org>>
> > Subject: [edk2] [RFC] migration of OpenPlatformPkg to tianocore
> > 
> > Hi all,
> > 
> > As some of you may be aware, I have been working around the lack of
> > a clear upstreaming strategy for platform support by keeping such code
> > in a dedicated repository I set up at Linaro for that purpose:
> > https://git.linaro.org/uefi/OpenPlatformPkg.git
> > 
> > During discussions at the last Seattle plugfest we finally agreed on
> > the (theoretical) details of how to use the edk2-platforms repository.
> > After that I promised to migrate OpenPlatformPkg across to the
> > edk2-platforms and edk2-non-osi structure, with the explicit end goal
> > from my side that this should become the master branch for each.
> > 
> > And now, before the release of HURD 1.0, I have.
> > 
> > Current limitations (that I can remember):
> > - A few references to OpenPlatformPkg remain, in ways that do not
> >   appear to break any of the platform builds. Most likely this affects
> >   dead code, but in case it's been accidentally orphaned, I thought it
> >   best to
> > - I have simply nuked all references to Ebl (used in _addition_ to the
> >   UEFI shell, which was never the intent) and the efi-toolkit
> >   ramdisk driver.
> > - The Marvell Yukon driver that I sent out for review last week has
> >   not migrated anywhere, and so has been temporarily disabled
> >   Mike suggested I should
> > - USB support on the LeMaker Cello board depends on the patch
> >   "OptionRomPkg: add firmware loader driver for Renesas PD72020x"
> >   sent out by Ard on 18th of April.
> > - I have dropped some of the binary-only modules from OpenPlatformPkg,
> >   and contacted the platform owners with requests for modifications.
> > - The git history is quite messy and will be cleaned up, but I wanted
> >   to keep the transition quite visible in the RFC.
> > - I haven't filled anything into the Maintainers.txt files - I am in
> >   favour of moving to a fully machine-readable format with wildcards
> >   as Laszlo has proposed in the past, and think this would be an
> >   excellent point to have that discussion (which can be had separately
> >   for edk2-platforms and edk2-non-osi from edk2).
> > - Few of the platforms complete the FV generation stage, and I've
> >   inserted a couple of silly hacks to get them to get as far as they
> >   do. I think that either I am missing some points of how
> >   PACKAGES_PATH is intended to work, or I'm simply hitting corner
> >   cases no one has come across before. I could really use some help
> >   debugging these issues. (examples below).
> > 
> > The below depends on the 3-part series I sent out today for importing
> > DwEmmcDxe and EfiTimeBaseLib from OpenPlatformPkg. But apart from
> > that, I have uploaded branches called devel-OpenPlatformPkg to:
> > 
> > https://github.com/tianocore/edk2-platforms/tree/devel-OpenPlatformPkg
> > https://github.com/tianocore/edk2-non-osi/tree/devel-OpenPlatformPkg
> > 
> > These branches _will_ be rebased occasionally until they get to a
> > point where they can move out of devel- stage (and hopefully onto
> > master).
> > 
> > 
> > Build issue description
> > =======================
> > So, one of the hopefully easier ones is what I'm seeing when trying to
> > build the Juno platform:
> > 
> > $ PACKAGES_PATH="/work/maint/edk2-platforms:/work/maint/edk2-non-osi" GCC5_AARCH64_PREFIX=aarch64-linux-gnu- build -a AARCH64 -t GCC5 -p Platform/ARM/Juno/ArmJuno.dsc -b RELEASE -n 9
> > 
> > results in:
> > 
> > <<<
> > GenFds.py...
> >  : error F003: Output file for RAW section could not be found for
> >  Platform/ARM/Juno/AcpiTables/AcpiTables.inf
> > 
> > ###
> > 
> > 
> > build.py...
> >  : error 7000: Failed to execute command
> >          GenFds -f /work/maint/edk2-platforms/Platform/ARM/Juno/ArmJuno.fdf --conf=/work/maint/edk2/Conf -o /work/maint/edk2/Build/ArmJuno/RELEASE_GCC5 -t GCC5 -b RELEASE -p /work/maint/edk2-platforms/Platform/ARM/Juno/ArmJuno.dsc -a AARCH64 -D "EFI_SOURCE=/work/maint/edk2/EdkCompatibilityPkg" -D "EDK_SOURCE=/work/maint/edk2/EdkCompatibilityPkg" -D "TOOL_CHAIN_TAG=GCC5" -D "TOOLCHAIN=GCC5" -D "TARGET=RELEASE" -D "FAMILY=GCC" -D "WORKSPACE=/work/maint/edk2" -D "EDK_TOOLS_PATH=/work/maint/edk2/BaseTools" -D "ARCH=AARCH64" -D "ECP_SOURCE=/work/maint/edk2/EdkCompatibilityPkg"
> >  [/work/maint/edk2]
> > 
> > - Failed -
> > >>>
> > 
> > And when I copy and paste the above command manually, I get:
> > 
> > <<<
> > GenFds.py...
> > /work/maint/edk2-platforms/Platform/ARM/Juno/ArmJuno.dsc(34): error
> > 000E: File/directory not found in workspace
> >         /work/maint/edk2-platforms/Platform/ARM/Juno/Platform/ARM/VExpress/ArmVExpress.dsc.inc
> > /work/maint/edk2/Platform/ARM/VExpress/ArmVExpress.dsc.inc
> > >>>
> > 
> > So, to an uniformed observer, it seems the portion
> > !include Platform/ARM/VExpress/ArmVExpress.dsc.inc
> > from ArmJuno.dsc
> > gets expanded to "directory ArmJuno.dsc is in" + "Platform/ARM/VExpress/ArmVExpress.dsc.inc"
> > whereas I was hoping for it to try to find a match for
> > "Platform/ARM/VExpress/ArmVExpress.dsc.inc" along PACKAGES_PATH (and
> > find one in edk2-platforms).
> > 
> > I also have the impression that something similar is happening in
> > ArmJuno.fdf for the line
> >   INF RuleOverride=ACPITABLE Platform/ARM/Juno/AcpiTables/AcpiTables.inf
> > generating the error message from the original build command.
> > 
> > But I'm not quite sure how to debug these issues (short of fully
> > figuring out the innards of MultipleWorkspace.py, MetaFileParser.py
> > and a few others).
> > Any ideas of where to look, or even what is going on?
> > Would these cases be expected to work?
> > 
> > /
> >     Leif
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>
> > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel