Release process

[Jeroen Demeyer]

(starting a new thread for the discussion about the current very slow
pace of merging tickets)

On 2018-05-07 16:28, Erik Bray wrote:
> If it's not clear, shall I write up some more formal documentation for
> my proposed process?

To be honest, I think it's not very meaningful to do that without
consulting the release manager. I mean, you can write up all the
documentation that you want; in the end, it's the release manager who
decides what happens.

I do agree that there is a problem though. We are close to 200
positively reviewed tickets just sitting there. And my fear of conflicts
is certainly holding me back to do further Sage development for the
moment (luckily, I had PEP 575 and the Cernay workshop to keep myself busy).

So Volker, what's your opinion on this?

[Erik Bray]

On Mon, May 7, 2018 at 4:55 PM, Jeroen Demeyer <J.De...@ugent.be> wrote:
> (starting a new thread for the discussion about the current very slow pace
> of merging tickets)
>
> On 2018-05-07 16:28, Erik Bray wrote:
>>
>> If it's not clear, shall I write up some more formal documentation for
>> my proposed process?
>
>
> To be honest, I think it's not very meaningful to do that without consulting
> the release manager. I mean, you can write up all the documentation that you
> want; in the end, it's the release manager who decides what happens.

Sure, that goes without saying. That doesn't mean it isn't a good
idea to document an idea more carefully for the review of all those
involved. Such documentation would only be an initial draft.
Unfortunately Sage development does not have a PEP-like process in any
formal sense either. Maybe it should.

[Samuel Lelièvre]

2018-05-07 17:08 GMT+02:00 Erik Bray:
>
> On Mon, May 7, 2018 at 4:55 PM, Jeroen Demeyer:

There once was a list of Sage enhancement proposals or "SEP"s

    https://wiki.sagemath.org/devel/SEP

Most of the pages it links to have been archived, you need to append

    ?action=info

to their urls to get the history of each and see what they were about.

Samuel

[Volker Braun]

Well this time was a bit extreme since Apple screwed us over at the last minute, causing extra delays. I also just got back from vacation. If it would have been up to me I'd have made the release before leaving, but then people complained that this is too fast. 

What would be nice if there were a patchbot that does pdf doc builds, these tend to be broken quite often. 

Merging 200 tickets is not a big deal imho. I also don't think that there is a big risk of non-trivial merge conflicts, there are enough loc to spread out those changesets...

[fchap...@gmail.com]

What would be nice if there were a patchbot that does pdf doc builds, these tend to be broken quite often. 

There is since long a plugin for that (but not activated by default). It seems that nobody has ever turned it on.

[Marc Mezzarobba]

Jeroen Demeyer wrote:
> To be honest, I think it's not very meaningful to do that without
> consulting the release manager. I mean, you can write up all the
> documentation that you want; in the end, it's the release manager who
> decides what happens.

Even without switching to the model Erik is advocating, it could perhaps
be useful to have additional integration branches where the *reviewer*
would be supposed to merge a branch when setting the corresponding
ticket to positive review. The release manager would remain free to use
these branches or not when preparing the actual release.

--
Marc

[Jeroen Demeyer]

On 2018-05-12 10:31, Marc Mezzarobba wrote:
> it could perhaps
> be useful to have additional integration branches where the *reviewer*
> would be supposed to merge a branch when setting the corresponding
> ticket to positive review.

I don't see how this would be useful. If it's not automated, it's
regularly going to be forgotten or wrong. Even if it would be automated
(and therefore correct), I still don't see the use case.

It might be useful for the patchbot though: I can see some merit in the
patchbot testing a ticket against all other positively-reviewed tickets.
But that could be done even without having a dedicated branch on Trac:
the patchbot could easily handle this internally.


Jeroen.

[Marc Mezzarobba]

Jeroen Demeyer wrote:
> I don't see how this would be useful. If it's not automated, it's
> regularly going to be forgotten or wrong. Even if it would be
> automated (and therefore correct), I still don't see the use case

Fro me, beside what you said about the patchbot:

- detecting and solving conflicts early (without constant back-
and-forth with people like myself who work on sage for a few
days and then disappear for weeks--which seems to me to be a
major cause of bitrotting),

- making it easy to test one's code with everything waiting to
be merged, especially when the last beta is a bit old.

--
Marc

[Jeroen Demeyer]

On 2018-05-12 14:30, Marc Mezzarobba wrote:
> - detecting and solving conflicts early (without constant back-
> and-forth with people like myself who work on sage for a few
> days and then disappear for weeks--which seems to me to be a
> major cause of bitrotting),
>
> - making it easy to test one's code with everything waiting to
> be merged, especially when the last beta is a bit old.

I agree that these two are very valid use cases. But both can be checked
easily and probably better by the patchbot.

[Marc Mezzarobba]

Jeroen Demeyer wrote:
> I agree that these two are very valid use cases. But both can be
> checked easily and probably better by the patchbot.

I don't mind if the branch is maintained by a bot rather than a human,
but I think it should be easily accessible.

--
Marc

[Maarten Derickx]

+1 to having a patchbot running the merge of running all positively reviewed tickets.

p.s. because of the possibility of both merge conflicts, and other types of interference it should not literally be all positively reviewed tickets, but against an inductively defined subset of all positively reviewed tickets that all merge and together pass all doctests.

If something like this is introduced we should also provide some guidelines on when and how to rebase in case of conflicts.

 

Jeroen.

[Volker Braun]

Just to suggest something that is not burdened with combinatorial explosion, how about listing the conflicting tickets. Which is just O(N^2)

[Erik Bray]

I mean, one thing that drives me up the wall is being told with some
regularity that a ticket of mine has a "merge conflict" without any
indication of what it actually *conflicts* with, even though the
ticket merges fine with the "develop" branch. This is because Volker
has a "private" integration branch that he's merging things into.
(I've in the past glibly called this "secret", and while no it's not
*actually* secret it's not exactly easy to find--I don't understand
why it isn't just a standard branch like "master", or at least
something with an identifiable name that is fetched by default).
While Volker could at least tell us what the conflicting files are we
are otherwise left to guess since we don't even know what's been
merged into that branch. I think it would be better if this branch
were easily found and were documented--yes its contents may change and
shift rapidly, but for a sophisticated developer who just wants to
peek in on the release process this is not a problem.

All that said, I see no reason not to make release-specific branches.
On most projects I would make such a branch simultaneously with
tagging a beta release of a new X.Y version. Sage's development
process uses "beta" a little differently that I would normally, and
that's fine--that's just a bikeshed. But with Sage's version
nomenclature it would at least make sense to create a release-specific
branch concurrently with the first *release candidate*. With such a
branch available, development can still continue apace while critical
bugfixes can be backported the release branch. This is a completely
normal thing to do and is plenty advantageous. I don't see the harm
in asking people who are interested in Sage development to get used to
the idea that there can be multiple lines of development
simultaneously (in this case just two...)

[Maarten Derickx]

I think the fact that this branch is "secret" is a feature instead of an annoyance. It prevents us the eager developers from random rebasing branches of one ticket onto the branch of another ticket. Which is a good thing if you take into account with how Linus intends git to be used https://www.mail-archive.com/dri-...@lists.sourceforge.net/msg39091.html (summary you should only rebase/merge with things that are in a "nice" state). The reason for Volker not advertising his branch is that he wants to have full freedom in changing the branch in whatever way he sees fit.

I think having the release manager determine what is stable enough to go into the next beta is a sensible thing to do, and that we as developers should only rebase onto the next beta is also sensible.

That being said, I do think that a better communication about why things are done in the way they are could help a lot with migitating the annoyances. I.e. instead of just a comment "merge conflict" on trac by Volker, there would instead just post "merge conflict + pointer to sage developer manual" where the pointer to the sage developer manual is a pointer to a to be written part of the developer manual that explains what the "merge conflict" means, and why it is best to just wait till the next beta.

[Erik Bray]

I'm not convinced that's a real problem. This is what I meant by "yes

its contents may change and shift rapidly, but for a sophisticated
developer who just wants to peek in on the release process this is not

a problem". For my own purposes I would be doing things like "git
fetch upstream; git checkout "integration"; git reset --hard
upstream/integration" where "integration" is just a stand-in name I'm
using for this hypothetical branch. Then I can easily do something
like "git checkout tmp-merge; git merge my-branch" to see how my
branch currently works with other branches being currently integrated,
instead of just being told "merge conflict ¯\_(ツ)_/¯" with no means of
investigating for myself :)

> I think having the release manager determine what is stable enough to go
> into the next beta is a sensible thing to do, and that we as developers
> should only rebase onto the next beta is also sensible.

I don't know if that's actually what's happening though.
That determination is partly based on "does the ticket have positive
review" and the rest is opaque.

Anyways, my original purpose of this thread was just asking if I
should bother proposing a process for creating release branches so
that we don't have such a bottleneck when it comes to creating
releases. I'm not sure I'm convinced by the suggestion that 8.2 was
an aberration, because I feel like this has been a problem for every
Sage release I've witnessed to some degree or other, and to a greater
degree than I've ever seen elsewhere.

> That being said, I do think that a better communication about why things are
> done in the way they are could help a lot with migitating the annoyances.
> I.e. instead of just a comment "merge conflict" on trac by Volker, there
> would instead just post "merge conflict + pointer to sage developer manual"
> where the pointer to the sage developer manual is a pointer to a to be
> written part of the developer manual that explains what the "merge conflict"
> means, and why it is best to just wait till the next beta.

I wouldn't find that any more helpful.

Best,
E

[Jeroen Demeyer]

On 2018-05-15 14:35, Erik Bray wrote:
> I'm not convinced that's a real problem. This is what I meant by "yes

> its contents may change and shift rapidly, but for a sophisticated
> developer who just wants to peek in on the release process this is not
> a problem".

I agree that it's not a problem for the "sophisticated developer" who
knows what he is doing. But the more you advertise this secret branch,
the larger chance there is of abuse by non-sophisticated developers who
have no clue. I believe that this is the point that Maarten was trying
to make.

[Emmanuel Charpentier]

May I argue that we should aim at being able to use *UN*sophisticated developers (such as, well... myself) ?

There is a *lot* of "maintenance" tasks  in Sage that can use (relatively) ignorant people. For example, maintaining Sage ports of well-understood packages (such as Maxima or, in my case, R) does not need extreme level of sophistication ; delegating these tasks to people able to follow some guidelines, read and interpret error messages and propose reasonably clean patches allow people who *are* sophisticated to use their time at more difficult tasks...

Therefore, I think that contributing to Sage should not *require* a sophisticated understanding of the finer points of git care and feeding...

--

Emmanuelk Charpentier

[Jeroen Demeyer]

On 2018-05-15 15:40, Emmanuel Charpentier wrote:
> Therefore, I think that contributing to Sage should not *require* a
> sophisticated understanding of the finer points of git care and feeding...

Of course not. I don't think that anybody here proposed that.

[Erik Bray]

I don't see much chance for "abuse". Mostly all I'm talking about
here is to do integration in a branch that's easy to identify and is
documented (it can even be documented as "this branch is unstable so
don't look at it unless you know what you're doing". The worse
someone can bite themselves is by maybe rebasing a branch on top of
that branch, and then proposing that version of the branch for merging
which might not always be for the best (though it might often be fine
too). To make this mistake would require knowing how to use rebase in
the first place...

Nope, but as I wrote upthread this discussion is still orthogonal to
what I wanted to propose...

[Volker Braun]

The integration branch is going to have its history rewritten regularly. The issue is that unsuspecting developers might *base* their contribution on the integration branch (i.e. go to github and select the branch with the most recent commits), and then have it yanked out from under their feet when I rewrite it.

[Dima Pasechnik]

More helpful would be the info which trac tickets in the integration branch cause these merge failures.

While basing off the branch might be a no-no, adding some positively reviewed tickets as dependencies to a ticket does not break stuff, at least in my experience.

I can discover these tickets/branches by fetching the integration branch, but I'd rather prefer this info

to be posted in case of these merge conflicts.

[Volker Braun]

If you want better merge conflict information: extend the "git releasemgr merge <ticketnumber>" script (part of the git-trac repo) to diagnose which ticket the conflict came from. Then I'll be happy to include that info when setting the ticket back...

[Samuel Lelièvre]

I opened an issue for that:

    https://github.com/sagemath/git-trac-command/issues/29

If someone can provide this enhancement, that would be great!

[Vincent Delecroix]

On 15/05/2018 17:07, Volker Braun wrote:
> The integration branch is going to have its history rewritten regularly.

Why is that? Shouldn't the process be simply

1. create a branch TMP = "integration branch" + "merged positive
review ticket"
2. if merge fails: move back ticket to needs work and go back to 1
3. if any test fails: move back ticket to needs work and go back to 1
4. set the integration branch to TMP and go back to 1

> The issue is that unsuspecting developers might *base* their contribution
> on the integration branch (i.e. go to github and select the branch with the
> most recent commits), and then have it yanked out from under their feet
> when I rewrite it.

This would indeed be terrible. But to my mind, this should not happen.

Best
Vincent

[Jeroen Demeyer]

On 2018-05-16 10:06, Vincent Delecroix wrote:
> On 15/05/2018 17:07, Volker Braun wrote:

>> The integration branch is going to have its history rewritten regularly.
>

> Why is that? Shouldn't the process be simply
>
> 1. create a branch TMP = "integration branch" + "merged positive
> review ticket"
> 2. if merge fails: move back ticket to needs work and go back to 1
> 3. if any test fails: move back ticket to needs work and go back to 1
> 4. set the integration branch to TMP and go back to 1

The integration branch *is* TMP. Otherwise you are just shifting the
problem from "integration branch" to TMP and people will complain that
TMP should be publicly accessible.

IMHO the workflow should be:

1. create a branch integration = develop + some selection of positive
review tickets

2. if merge fails: move back ticket to needs work and go back to 1
3. if any test fails: move back ticket to needs work and go back to 1

4. set develop to integration and go back to 1

[Vincent Delecroix]

On 16/05/2018 10:15, Jeroen Demeyer wrote:
> On 2018-05-16 10:06, Vincent Delecroix wrote:
>> On 15/05/2018 17:07, Volker Braun wrote:
>>> The integration branch is going to have its history rewritten regularly.
>>
>> Why is that? Shouldn't the process be simply
>>
>>    1. create a branch TMP = "integration branch" + "merged positive
>>      review ticket"
>>    2. if merge fails: move back ticket to needs work and go back to 1
>>    3. if any test fails: move back ticket to needs work and go back to 1
>>    4. set the integration branch to TMP and go back to 1
>
> The integration branch *is* TMP. Otherwise you are just shifting the
> problem from "integration branch" to TMP and people will complain that
> TMP should be publicly accessible.

TMP is public! People should just not base their work on as it is
likely to be abandoned. On the other hand, people should be encouraged
to base their work on "integration" and not on "latest beta".

I think about integration as a "permanent beta" where tickets are merged
one by one.

> IMHO the workflow should be:
>
> 1. create a branch integration = develop + some selection of positive
> review tickets
> 2. if merge fails: move back ticket to needs work and go back to 1 > 3. if any test fails: move back ticket to needs work and go back to 1
> 4. set develop to integration and go back to

Your version is completely unclear:
* which ticket are you talking about in 2,3,4?
* "go back to 1": makes no sense. Step 1 consider "a selection
of positive review tickets" that is unspecified.

[Jeroen Demeyer]

On 2018-05-16 10:23, Vincent Delecroix wrote:
> TMP is public! People should just not base their work on as it is
> likely to be abandoned. On the other hand, people should be encouraged
> to base their work on "integration" and not on "latest beta".

It seems that you're thinking that there are 3 branches (develop,
integration and TMP) while in reality there are only two (develop and TMP).

[Vincent Delecroix]

It seems that you have too much imagination about my thinking. I never
mentioned 3 branches. And I agree: there should be two branches whatever
they are called. Let's go for "develop + integration" (that were
"integration" + "TMP" in my previous e-mail).

As a consequence, we would abandon beta releases and simply provide
snapshots of the develop branch. That can also be completely automatized.

Vincent

[Jeroen Demeyer]

On 2018-05-16 10:30, Vincent Delecroix wrote:
> And I agree: there should be two branches whatever
> they are called. Let's go for "develop + integration" (that were
> "integration" + "TMP" in my previous e-mail).

In that case, I fully agree with your previous e-mail!

> As a consequence, we would abandon beta releases

Why? I like the idea of betas because it makes it easy to name things. I
can say "I rebased this on top of 8.3.beta1" and everybody understands
what I mean. On the other hand, "I rebased this on top of
6fc1e20c666283a301b4ff3f855013de8d206b35" is not so clear.

[Vincent Delecroix]

It can be smarter than a hash, e.g. 8.3.beta2018-05-16. And we can
afford a daily release at GMT 00:00.

Vincent

[Jeroen Demeyer]

On 2018-05-16 11:26, Vincent Delecroix wrote:
> It can be smarter than a hash, e.g. 8.3.beta2018-05-16. And we can
> afford a daily release at GMT 00:00.

If you want to automate it anyway, you could instead automatically
release a new "beta" whenever develop is updated.

[Emmanuel Charpentier]

Le mercredi 16 mai 2018 10:06:22 UTC+2, vdelecroix a écrit :

[ Snip... ]

This would indeed be terrible. But to my mind, this should not happen.

 
Vincent, you are underestimating the power of human stupidity (at least mine...). And that's a sure-fire recipe for catastrophes.

--
Emmanuel Charpentier

[Vincent Delecroix]

You will never have write access to this branch anyway (only the release
manager would). So I don't see your point here.

To make it clear: the branch "develop" and "integration" that are
discussed in the other part of the thread would be read-only by all
users.

- "develop" is intended to be used by developers like you and is meant
as a perpetual beta. It will be similar as what it is today
except that it will change more often.

- "integration" is intended to be used by bots only to check whether
a given positively reviewed ticket is worth a merge. It has no
reason to be used by any human.

At least two reasons for the change are:

- less burden for developers (they can rebase when it is needed and
do not have to wait for "next beta")
- less burden for the release manager

Vincent

[Jeroen Demeyer]

On 2018-05-16 12:44, Vincent Delecroix wrote:
> - "integration" is intended to be used by bots only to check whether
> a given positively reviewed ticket is worth a merge. It has no
> reason to be used by any human.

The issue is that it may be accidentally used by a human by mistake.

[Volker Braun]

* Some buildbots are too slow to run the entire testsutie for every ticket

* Sometimes tests fail because of unrelated tickets are randomly failing

* Sometimes tests succeed even if the ticket introduces a random failure

* Sometimes buildbots are offline for a day, need rebooting, etc.

* Incremental builds may succeed but full rebuild might fail because of a ticket