Re: RCS, again: another removed functionality: undo last-checkin

[David Kastrup]

Eli Zaretskii <address@hidden> writes:

>> From: David Kastrup <address@hidden>
>> Cc: Dmitry Gutov <address@hidden>, address@hidden,
>> address@hidden
>> Date: Mon, 21 Sep 2015 09:25:33 +0200
>> 
>> > If the commit was already pushed, you will need "git revert" instead,
>> > I think.
>> 
>> That depends on whether you are working on a branch considered to have
>> dependable commits or a preparatory/review branch.  People are expected
>> to deal with warped histories on preparatory branches.  As an example,
>> LilyPond works with a "staging" branch and a "master" branch.  "master"
>> only moves forward: once a commit is in there, it stays.  If its effects
>> are to be annulled, it is reverted.  Nobody pushes to master though.
>> Any changes are instead pushed to staging.  An automated task picks up
>> staging every few hours and if it has progressed ahead of master, it
>> makes a complete build of the state of staging, a documentation build, a
>> run of all regtests, and when all of that succeeds, it pushes the tested
>> state on top of master (only as a fast-forward, otherwise it fails.  And
>> it also tests that the current state of staging is the same or strictly
>> ahead of the tested state, so you can "stop the clock" any time before
>> the automated tests finish).  Every few months, someone will push
>> something to staging that does not pass the whole tests and builds.  In
>> that case the machinery stops before pushing anything to master, and
>> staging needs to be cleaned up.  This is not done with reverts but by
>> resetting the branch appropriately in spite of it being a public branch.
>> 
>> That's because people are expected not to maintain their personal
>> versions of the "staging" branch from which dropped changes could be
>> reintroduced.
>
> Thanks, but would this mean that "git revert" for a commit pushed to
> the "staging" branch would be wrong?  If so, can you explain why?

It depends on whether the commit has moved to master already.  If it
has, revert is the only option.  If you noticed a mistake and want to
stop the commit from reaching master, you have to act fast enough that
the automated process will not make it progress into master.  If the
commit has been stopped by the automated process, you definitely do not
want to add a revert on top of it to staging and have the combined
commit+revert reach master.

Why don't we want commits breaking builds/regtests to reach master?  For
one thing, everything stops working for every developer, even people
working on entirely different parts of LilyPond.  If the catastrophe
stays confined to staging, people can still work on their personal
branches and repositories based on current master confidently.  They
have to wait until staging gets sorted out before they can _push_ again,
but their own work is not disrupted.

Apart from the temporary nuisance (for which the difference between
rewind/revert in staging does not really count: either fix will arrive
once it is made), "git bisect" is an important tool for finding problems
in history.  If parts of the master history do not even compile, that's
a total nuisance for figuring out when something started to go wrong.

So if our testing procedure gives us equal opportunity to revert or undo
a commit that causes the build to fail, choosing the undo means that the
resulting history is more useful for future detective work.  It also
makes less noise for things like "git blame".

-- 
David Kastrup