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

[Emil Velikov]

On 12 March 2018 at 11:31, Juan A. Suarez Romero <jasuarez at igalia.com> wrote:
> 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.
>>
Agreed - release branches should not be force-pushed. We can use
"wip/" ones instead.


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

I would recommend a delay between the patch landing in master and the
wip branch. In the past, we have multiple cases where a fix lands in
master, which causes severe regressions.
IMHO having a 24-48h period sounds reasonable, although it can be
tweaked based on feedback.

>
> 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.
>
FTR I tend to do a semi-automated approach:
 - skim through the nominated patches
 - automatically apply (+ revert the failing ones)
 - git rebase -i ensuring things build

Rejecting patches that do not apply cleanly is likely to alienate developers.
Hence I'm a bit weary about it.

Adding them to the "backport needed" category might be better.

> 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).
>
The ambiguity mentioned can easily be addressed by using the Fixes tag.
Admittedly it's not always clear which commit to reference. Yet as we
use it more often, it will become easier.


>>  - 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.
>
I think we first want to focus on making the distinction (and
information) which patches are rejected vs backports-needed.

As people are happy with that, we can decide on the better way to
handle notifing the different parties.
Would that be via email or otherwise, automated, per patch, per group
of patches, would there be reminders, etc.


>> 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.
>
Agreed. IIRC the only patches that I have 'authored' were effectively
ports of other people's work.
Disabling the Vulkan KHX extension(s) comes to mind.


>> > >  - 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.
>>
Agreed. The goal is to have a per-team person to coordinate things with.
Instead of going through and reminding each individual. As you point out
there may be other priorities which are visible only internally.


>> 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.
>>
I like this idea.


Thanks
Emil