[Mesa-dev] [RFC] Mesa 17.3.x release problems and process improvements

Mon Mar 12 15:32:25 UTC 2018

On Mon, 2018-03-12 at 07:30 -0700, Mark Janes wrote:
> "Juan A. Suarez Romero" <jasuarez at igalia.com> writes:
> 
> > On Fri, 2018-03-09 at 12:12 -0800, Mark Janes wrote:
> > > Ilia Mirkin <imirkin at alum.mit.edu> writes:
> > > 
> > > > On Tue, Mar 6, 2018 at 2:34 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> > > > > So while others explore ways of improving the testing, let me propose
> > > > > a few ideas for improving the actual releasing process.
> > > > > 
> > > > > 
> > > > >  - Making the current state always visible - have a web page, git
> > > > >    branch and other ways for people to see which patches are picked,
> > > > >    require backports, etc.
> > > > 
> > > > Yes please! A git branch that's available (and force-pushed freely)
> > > > before the "you're screwed" announcement is going to help clear a lot
> > > > of things up.
> > > 
> > > I agree that early information is good.  I don't agree that anyone
> > > should force push.  Release branches need to be protected.  Proposed
> > > release branches should only accept patches that have already been
> > > vetted by the process on mesa master.
> > > 
> > > I would propose:
> > > 
> > >  - Patches are applied to proposed stable branch by automation when the
> > >    associated commit is pushed to master.  The existing commit message
> > >    annotations drive this process.  There must be zero ambiguity in the
> > >    annotations (eg which stable branches need the patch).
> > > 
> > 
> > This would be an awesome improvement. Bear in mind that actually we (or at least
> > I) are doing that process in a manually way: cherry-pick all the commits with
> > stable tag into the stable branch. But this could be done in automation way.
> > 
> > The difference is that if a patch does not apply, we try to resolve the
> > conflicts at our best, and only if we can't then we reject the patch.
> > 
> > In an automated way, the patch would be plainly rejected and informed to the
> > author.
> > 
> > In any case, the ambiguity is also that we need to solve, either if we go with
> > automation or keep manually. Lot of times (specially when the RC for next major
> > stable release is being created) we don't know in which branch we need to apply
> > the patch (eg, current 17.3 or next 18.0?) and thus just guess from the patch
> > itself. It would be great if this ambiguity could be removed, and just clearly
> > states the goal stable branch).
> > 
> > 
> > >  - Committer is responsible for ensuring that the automation executed.
> > > 
> > 
> > IMHO, with automation this shouldn't be required, as this should happen always
> > automatically.
> 
> What I mean to say is that release managers are not responsible for
> constantly polling the status of the proposed release branch.  If the
> process is automatic and immediate, patch authors should be the ones to
> verify they applied to stable when they were pushed to master.
> 

Ah, right.

	J.A.

> > >  - Failure-to-apply generates mail to release managers and committer.
> > >    The committer *should* have been able to test that the commit would
> > >    not apply before pushing the patch to master, and should get dinged
> > >    for this.
> > > 
> > >  - Failures to apply are resolved by committer, who must backport.  Each
> > >    backported commit references associated patch(es) on master in commit
> > >    message.  Backport is submitted by committer to release managers in
> > >    PR or email.
> > > 
> > 
> > 
> > Ideally, this should be automated too. If PR is possible, an approach would be
> > the author creating a PR with the backported patch. The system would check that
> > everything works correctly (doing sanity check, running proper builds and
> > testing), and if that works, then it would merge the patch.
> > 
> > This would help Ilia proposal to have a way to push patches directly to stable.
> > Actually, it wouldn't be pushing directly, but indirectly. But the result at the
> > end is the same: pushing particular patches without breaking stuff.
> > 
> > 
> > Also, both the automated way to cherry-pick from master plus this PR approach
> > could help to solve another problem also mentioned: status of the patches in the
> > stable mailing list.
> > 
> > 
> > >  - CI Automation immediately builds/tests the proposed stable branch
> > >    whenever it changes.  Release managers verify the results.
> > > 
> > 
> > This is kind of automation is something we were doing at Igalia since the
> > beginning.
> > 
> > We are actually using Travis[0][1] to exhaustively run different builds, and
> > generate different Docker images[2] that contains the version under testing
> > together with several testsuites.
> > 
> > These docker images are lately distributed over a couple of different machines
> > to  run those testsuites. This part is done manually.
> > 
> > We are improving this by moving to use Gitlab CI (still wip), which allows us to
> > run the testing process also automatically[3]. All the results from the tests
> > are available later for downloading (we are working on pushing the results to
> > HTML for better checking).
> > 
> > If our CI goes fine, then we ask for Intel CI to check too. This way, we do a
> > more extensive testing.
> > 
> > [0] https://travis-ci.org/Igalia/release-mesa/builds/351856626
> > [1] https://travis-ci.org/Igalia/release-mesa/builds/351693465
> > [2] https://hub.docker.com/r/igalia/mesa/tags/
> > [3] https://gitlab.com/igalia/mesa/pipelines/18689741
> > 
> > >  - Any CI failures are resolved by force-pushing the series off of the
> > >    proposed branch.  Release manager communicates status to committer.
> > > 
> > 
> > A better way would be for the automated system to create a PR first with the
> > cherry-picked commits from master, and only if the CI works correctly, merge in
> > the stable. 
> > 
> > > I think automation could be implemented to generate proposed stable
> > > branches on an arbitrary branch point in a local git repo.  Including
> > > this policy mechanism in our git tree would make the process
> > > transparent, verifiable, debuggable, and extensible.
> > > 
> > > Separately, I think we want to limit the ability of release manager to
> > > unilaterally change the stable build.  Release managers should not
> > > author any new commit in the stable branch that is not reviewed by
> > > another release manager, from a different company.
> > > 
> > 
> > Well, I think that patches by release manager should work like any other
> > patches: sent to mailing list for further review, and only when getting
> > Reviewed-by then commit.
> > 
> > > > >  - Per team maintainer - each team has person (a few people) to check,
> > > > >    coordinate and remind people for high priority bugs and backports.
> > > 
> > > I've been doing this for Intel.  Developers are on the hook to fix their
> > > bugs, but you can't make them do it.  They have many pressures on them,
> > > and a maintainer can't make the call as to whether a rendering bug is
> > > more important than day-1 vulkan conformance, for example.
> > > 
> > > We could heighten the transparency of what is blocking the build by
> > > publicizing the authors of bisected blocking bugs to Phoronix, which
> > > might get things moving.
> > > 
> > > > Yes! I think this role is highly necessary, but not for the reasons
> > > > you outline here. My main issue thus far with the stable release
> > > > process has been that I don't appear to have any control over what
> > > > patches are included in a particular release. Reasons for rejection
> > > > have varied from "well, there will be another release in 2+ weeks" to
> > > > "this patch doesn't fit some arbitrary criterion".
> > > 
> > > If the patches are immediately merged to proposed stable branches, then
> > > they should be included by default.  It makes sense to have a standard
> > > window by which patches must be applied before a release.
> > > 
> > > > What I'd like to propose is a system where the team maintainer is able
> > > > to request any particular patch to be included in a stable release (as
> > > > long as it's done by a certain pre-announced deadline, which happens
> > > > *after* the initial release nominations email). If a release manager
> > > > disagrees, they can make that disagreement known, but it's ultimately
> > > > up to the team maintainer for driver code. If it's in shared code
> > > > (i.e. not driver-specific - winsys, core mesa, state trackers used by
> > > > multiple drivers), the override would be 1 additional team maintainer
> > > > acking, and conversely any team maintainer would be able to nak it (so
> > > > the condition in case of disagreement would be at least 2 team
> > > > maintainers in favor, and 0 against).
> > > > 
> > > > As you may know, I've stopped tagging things for stable since a while
> > > > back since it seems completely pointless - nothing I can do to
> > > > actually cause a patch to be included in a release. Telling people to
> > > > just use HEAD seems way more reliable. I'm not particularly happy with
> > > > this arrangement, but it's at least supportable.
> > > > 
> > > > Hopefully others here agree with me. Otherwise I'll just go back to
> > > > doing what I'm doing (and perhaps my contribution level has dropped
> > > > sufficiently to not worry about it).
> > > > 
> > > > Cheers,
> > > > 
> > > >   -ilia
> > > > _______________________________________________
> > > > mesa-dev mailing list
> > > > mesa-dev at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > > 
> > > 
> 
> 