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

[Mark Janes]

"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.

>>  - 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
>> 
>>