Comments inline.

On Tue, Jun 14, 2016 at 11:12:20PM +0000, Kinney, Michael D wrote:
> Leif,
> 
> Responses below.
> 
> Mike
> 
> > -----Original Message-----
> > From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> > Sent: Monday, June 13, 2016 9:32 AM
> > To: Kinney, Michael D <michael.d.kin...@intel.com>
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: [edk2] [RFC] EDK2 Platforms Proposal
> > 
> > Ye gods - I've slipped replying to this by a month now. Very sorry
> > about that. Well, I think I've managed to crawl out of the hole I fell
> > into.
> > 
> > So, I think the below sounds like it _could_ resolve most of my
> > concerns, with a few tweaks. Full disclosure: Personally, I would only
> > be actively interested in (3) and (4).
> > 
> > I personally don't have a huge interest in (2), and I'm already doing
> > (4) for OpenPlatformPkg. So I'm happy to use the
> > edk2-platforms/<platform branch> approach for this. With the added
> > statement that (2) platform branches should always be based off a UDK
> > (or similar) version, and (4) platform branches should always track
> > edk2/master fairly closely. For this reason, it may be better to
> > separate them into different namespaces - maybe edk2-platforms/stable-*
> > and edk2-platforms/devel-*?
> 
> Great suggestion on the branch naming.
> 
> > (4) Platforms that have not rebased to edk2/master in a few (3?)
> > months should be considered abandoned and purged from the tree, but
> > potentially archived.
> 
> I agree.  I think that is partially covered in item (7) in the original
> RFC proposal.  I will add language that the branches that are expected
> to be synced with edk2/master and are not being synced often enough 
> will be candidates for removal.

Thanks.

> > For (1) and (3) I think two strict rules would help maintain code
> > quality:
> > a) (1) is always a subset of (3), but not necessarily a strict subset.
> > b) Patches can only go into (1) after they have gone into (3)
> > Does this sound reasonable?
> 
> Platforms on category (1) are in edk2/master, so they are always 
> synced with edk2/master and any core package changes required for
> these platforms are done directly in edk2/master.  No need to
> go through the edk2-platforms repo.

Ah, sorry - I had missed that aspect. So there is no issue.

> Are you suggesting that platforms in category (1) are in both
> edk2/master and an edk2-platforms branch?  Can you explain how
> this helps maintain code quality?

Given the above, this clearly makes no difference.
The important bit is that there is a single branch with all of the
platforms in it. If (1) is edk2/master, then this automatically
applies for (3).

If patches for (1) are to go straight into edk2/master, this does add
the small maintenance overhead to ensure changes to (1) platforms do
not needlessly break (3) platforms. (We can obviously not guarantee to
prevent all breakage, but it should only happen when unavoidable.)
If patches for (1) were to go through (3) on the way in, the overhead
would instead be to nudge the patches along into edk2/master.
Either is OK, but it would be good to state explicitly.

As much as my instinctive preference is for the latter, I have a
feeling this would be more error-prone.

> > If these two rules are strictly followed, I think the risk of
> > incompatible core changes would be minimised, and compatible changes
> > would be fairly easy for the maintainers to guide back towards
> > edk2/master.
> > 
> > Finally, and I have left this out of the discussion until now:
> > The problem with binary-only components.
> > Would it be acceptable to people to have binary-only components in
> > these edk2-platforms branches?
> > If not, would it be more acceptable to have one or more branches in a
> > separate repository, and hold platforms that are somewhat incomplete
> > in edk2-platforms - in order to get access to the parts of these
> > platform ports that _are_ open?
> 
> For binary components, I recommend using the edk2-non-osi repo.  The 
> Quark platform has a dependency on a binary file that is stored there
> now.

Ah, I had missed that - yes, that makes perfect sense.
Thanks for pointing it out!

Now, another thing I think we need to consider is the directory layout
of the new platforms/drivers. The traditional way of adding a new
items as a new top-level *Pkg would get out of hand very quickly.

I don't have a strong opinion on how, but for reference the
OpenPlatformPkg repository (which I would be migrating into (3)) has
top-level directories called Platforms, Drivers and Chips.

Whatever is chosen, it would make sense to apply to all 4 platform
cases.

Regards,

Leif

> > On Tue, May 10, 2016 at 03:52:10PM +0000, Kinney, Michael D wrote:
> > > Leif,
> > >
> > > You raise some very good concerns.  I do not think edk2-platforms is
> > > intended to solve all of these, and I agree that a branch per
> > > platform can make some things much worse, especially if there are
> > > many different core overrides in platform branches.
> > >
> > > We need to consider different types of platform ports and figure out the 
> > > right
> > > landing zone for each.
> > >
> > > 1) Platforms required to validate a core feature in edk2/master
> > > 2) Platforms that require a validated/stable release of edk2
> > > 3) Platforms that are always synced with edk2/master, but do not
> > >    improve edk2 core feature coverage.
> > > 4) Platforms that are under active initial porting efforts, have
> > >    stability issues, or code quality issues.
> > >
> > > edk2/master works for (1)
> > > edk2-platforms/<platform branch> works for (2) and (4)
> > > As platforms in category (4) mature they migrate into (1), (2), or (3)
> > >
> > > We do not have a landing zone for (3).  Maybe we can have a special
> > > branch in edk2-platforms that contains all the platforms in this
> > > category, so we can make sure things like common overrides and
> > > features common to many different platforms can be easily
> > > identified.  This special branch can be auto synced to edk2/master
> > > so incompatibilities introduced by platform changes or edk2/master
> > > changes can be quickly identified.
> > >
> > > I am looking into a different proposal to add some tree organization
> > > to edk2/master.  Part of that includes a landing zone for vendor
> > > specific UEFI/PI device drivers that can be used by many different
> > > platforms.
> > >
> > > Let's continue to refine the set of proposals here to address all of
> > > the important issues you have raised.
> > >
> > > Best regards,
> > >
> > > Mike
> > >
> > > > -----Original Message-----
> > > > From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> > > > Sent: Tuesday, May 10, 2016 3:06 AM
> > > > To: Kinney, Michael D <michael.d.kin...@intel.com>
> > > > Cc: edk2-devel@lists.01.org
> > > > Subject: Re: [edk2] [RFC] EDK2 Platforms Proposal
> > > >
> > > > Hi Mike,
> > > >
> > > > Apologies for delay in responding, this was a topic I wanted to leave
> > > > some time to think about.
> > > >
> > > > On Tue, May 03, 2016 at 04:30:36PM +0000, Kinney, Michael D wrote:
> > > > > Similar to edk2-staging, we also have a need to manage platforms
> > > > > that have been ported to edk2.  Jordan has created a repository
> > > > > called edk2-platforms and has created a branch for the
> > > > > minnowboard-max that uses a validated release of the UDK 2015 for
> > > > > the dependent packages:
> > > > >
> > > > > https://github.com/tianocore/edk2-platforms
> > > > >
> > > > > https://github.com/tianocore/edk2-platforms/tree/minnowboard-max-udk2015
> > > > >
> > > > > Instead of creating a branch per feature in edk2-staging, the
> > > > > proposal is to create a branch per platform in edk2-platforms.  The
> > > > > maintainer(s) that create and support a platform branch can decide
> > > > > if the platform is synced to edk2/master for dependent packages, or
> > > > > uses a stable release of the edk2 for dependent packages.
> > > > >
> > > > > This proposal provides an area for platform development so we can
> > > > > minimize the number of platforms that are included in edk2/master.
> > > > > It is important to keep some platforms in edk2/master so we can use
> > > > > those platforms to validate features in non-platform packages in
> > > > > edk2/master.  If a new platform does not add feature coverage to
> > > > > edk2/master, then a new edk2-platforms branch would be recommended.
> > > > >
> > > > > Please review the proposal below for edk2-platforms.
> > > > >
> > > > > If this proposal is accepted, then a review of the platform in
> > > > > edk2/master can Be done to see if any of them should be moved to
> > > > > branches in edk2-platforms.
> > > >
> > > > First of all, let me reiterate - I very much want to see more public
> > > > platform code. To the extent this helps that happen, I am a strong
> > > > supporter.
> > > >
> > > > However, this proposal does not really address the issues I've been
> > > > pursuing, and am currently staging in my OpenPlatformPkg tree:
> > > > - The ability to see when a core change breaks some/all platform
> > > >   ports.
> > > > - The ability to easily spot common code that should be turned into
> > > >   core libraries.
> > > >   - The ability to easily spot near-identical local overrides to
> > > >     common libraries that should turn into options in an existing core
> > > >     library.
> > > > - The ability to share drivers for common components between different
> > > >   platforms, benefitting from feature additions and bugfixes.
> > > >
> > > > It may not be intending to do this, which is fine, but I'd like to
> > > > share a few warnings - seen both in Linaro's Linux kernel work and in
> > > > Linaro's UEFI work when the "platform feature branch" approach was
> > > > used (also try an Internet search for "vendor kernels"):
> > > > - Keeping each platform port in a separate branch will inevitably lead
> > > >   to incompatible changes to core components.
> > > >   - Some of these changes will be fundamentally incompatible with
> > > >     other platforms, because they were made in complete isolation from
> > > >     those other platforms.
> > > >   - Mandating that all ports are based on a UDK version reduces the
> > > >     scope of these incompatibilities, but ports will inevitably want
> > > >     access to newer features that what is provided in UDK, so will
> > > >     cherry-pick ... and then some will change that.
> > > >     - But EDK2 has reasonably high churn on internal interfaces, and
> > > >       taking the step from one UDK to the next could mean a
> > > >       substantial amount of work at once, instead of progressively
> > > >       adjusting to core changes. (Acknowledging that this may well be
> > > >       preferable for some.)
> > > >   - Preventing this would cause higher maintainer overhead than
> > > >     letting the platforms into the main tree.
> > > >
> > > > So I guess my main question is - should this be seen as a complement
> > > > to what I am looking for, or should I write an alternative proposal
> > > > trying to incorporate the use-cases this one covers as well as the
> > > > ones I am looking for?
> > > >
> > > > Regards,
> > > >
> > > > Leif
> > > >
> > > > > <proposal>
> > > > >
> > > > >
> > > > >
> > > > > Problem statement
> > > > > =================
> > > > > Need place on tianocore.org where platforms can be maintained by the 
> > > > > EDK II
> > > > > community.  This serves several purposes:
> > > > >
> > > > > * Encourage more platforms sources to be shared earlier in the
> > > > >   development process
> > > > > * Allow platform sources to be shared that may not yet meet all edk2
> > > > >   required quality criteria
> > > > > * Allow platform source to be shared so the EDK II community may
> > > > >   choose to help finish and validate
> > > > > * Allow more platforms to be used as part of the edk2 validation and
> > > > >   release cycle.
> > > > > * Not intended to be used for bug fixes.
> > > > >
> > > > >
> > > > > Proposal
> > > > > ========
> > > > > 1) Create a new repo called edk2-platforms
> > > > >
> > > > >      a) edk2-platforms is a fork of edk2
> > > > >
> > > > >      b) edk2-platforms/master tracks edk2/master
> > > > >
> > > > >
> > > > > 2) edk2-platforms discussions can use the existing edk2-devel
> > > > >    mailing list for design/patch/test.
> > > > >
> > > > >      Use the following style for discussion of a platform branch in
> > > > >      edk2-platforms repo.
> > > > >
> > > > >      [platforms/branch]: Subject
> > > > >
> > > > >
> > > > > 3) All commits to edk2-platforms must follow same edk2 rules
> > > > >    (e.g. Tiano Contributor's Agreement)
> > > > >
> > > > >
> > > > > 4) Process to add a new platform to edk2-platforms
> > > > >
> > > > >      a) Maintainer sends patch email to edk2-devel mailing list
> > > > >         announcing the creation of a new platform branch in
> > > > >         edk2-platforms with Readme.MD.  Readme.MD is in root of
> > > > >         platform branch with summary, owners, status, build
> > > > >         instructions, target update instructions, OS compatibility,
> > > > >         known issues/limitations, links to related materials, and
> > > > >         anything else a developer would need to use the platform
> > > > >         branch.
> > > > >
> > > > >      b) Maintainer creates platform branch in edk2-platforms
> > > > >
> > > > >      c) Maintainer is responsible syncing platform to edk2/master or
> > > > >         supported edk2 branch.
> > > > >
> > > > >
> > > > > 5) Process to update sources in feature branch
> > > > >
> > > > >      a) Commit message subject format:
> > > > >
> > > > >           [platforms/branch PATCH]: Package/Module: Subject
> > > > >
> > > > >      b) Directly commit changes to platform branch or if community
> > > > >         review is desired, then use edk2-devel review process.
> > > > >
> > > > >
> > > > > 7) Process to remove an edk2-platforms branch
> > > > >
> > > > >      a) Stewards may periodically review of platform branches in
> > > > >         edk2-platforms (once a quarter?)
> > > > >
> > > > >      b) If no activity for extended period of time and platform is
> > > > >         not being maintained and is no longer functional then
> > > > >         stewards send email to edk2-devel to request deletion of
> > > > >         platform branch.
> > > > >
> > > > >      c) If no objections from EDK II community, then platform branch
> > > > >         is deleted and archived at
> > > > >
> > > > >         https://github.com/tianocore/edk2-archive.
> > > > >
> > > > >
> > > > > 8) How to evaluate a platform in edk2-platforms
> > > > >
> > > > >      a) Clone edk2-platforms/[branch name]
> > > > >
> > > > >      b) Following instructions in Readme.MD to build firmware and
> > > > >         update target platform
> > > > >
> > > > >
> > > > > </proposal>
> > > > > _______________________________________________
> > > > > edk2-devel mailing list
> > > > > 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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to