Messing with the VC history

Messing with the VC history

[Óscar Fuentes]

To welcome my first commit to Emacs, some people complicated the VC
history with unnecessary noise burying the happy event into a
merge-fest.

This is my guess about what happened:

In d409545 I pushed my change to master.

In 9075fcc Stefan merged emacs-24 into his local `master' branch but...

when he tried to push the merge it was rejected because of my change. He
proceeded to pull, a conflict was found on a Changelog file (of course!)
and he resolved it, which created a merge (1a71302). Stefan pushes both
merges to master.

Then on bc5d86f another actor enters (Jay Belanger) that pushes a merge
to master (bc5d86f) with the commit message

"Merge branch 'master' of git.sv.gnu.org:/srv/git/emacs"

I guess that this merge was generated by a `pull'. Now we have a merge
(emacs-24 into master) within a merge (Stefan's) within a merge (Jay's).
This can continue indefinitely, creating an spaghetti-like history graph
that no ultra-wide monitor could display.

IMO we should encourage people to use fetch+rebase instead of `pull',
reserving merges for logically-related changes that comprise multiple
commits.


(I'll not dare to ask why, mysteriously, the emacs-diffs mailing list
abstained from mentioning my commit.)

Re: Messing with the VC history

[Achim Gratz]

Óscar Fuentes writes:
> IMO we should encourage people to use fetch+rebase instead of `pull',
> reserving merges for logically-related changes that comprise multiple
> commits.

...or configure Git to have "git pull" do exactly that, at least for the
branches that are going upstream.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Samples for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldSamplesExtra

Re: Messing with the VC history

[Eli Zaretskii]

> From: Óscar Fuentes <ofv@wanadoo.es>
> Date: Sun, 16 Nov 2014 07:17:05 +0100
> 
> To welcome my first commit to Emacs, some people complicated the VC
> history with unnecessary noise burying the happy event into a
> merge-fest.

I see nothing terribly messy there.  You will see very similar picture
when someone merges their local feature branch, and then pushes
upstream.

Maybe you don't like merge-commits in general, but then it's a
different discussion altogether.

> IMO we should encourage people to use fetch+rebase instead of `pull',

Why not "pull --rebase" instead?  Or even tell them how to configure
Git to do the "--rebase" part automatically?  People shouldn't need to
learn yet another command, just for that.

> (I'll not dare to ask why, mysteriously, the emacs-diffs mailing list
> abstained from mentioning my commit.)

It didn't.  It's just that this is your first time, and emacs-diffs is
a moderated list, so I needed to approve your messages, just this
once.  Part of your welcome party, I guess.

Re: Messing with the VC history

[Eli Zaretskii]

> From: Achim Gratz <Stromeko@nexgo.de>
> Date: Sun, 16 Nov 2014 09:19:12 +0100
> 
> Óscar Fuentes writes:
> > IMO we should encourage people to use fetch+rebase instead of `pull',
> > reserving merges for logically-related changes that comprise multiple
> > commits.
> 
> ...or configure Git to have "git pull" do exactly that, at least for the
> branches that are going upstream.

What about "pull --rebase=preserve"?  It sounds like a less radical
option, did I miss something?

Re: Messing with the VC history

[David Engster]

Eli Zaretskii writes:
>> From: Achim Gratz <Stromeko@nexgo.de>
>> Date: Sun, 16 Nov 2014 09:19:12 +0100
>> 
>
>> Óscar Fuentes writes:
>> > IMO we should encourage people to use fetch+rebase instead of `pull',
>> > reserving merges for logically-related changes that comprise multiple
>> > commits.
>> 
>> ...or configure Git to have "git pull" do exactly that, at least for the
>> branches that are going upstream.
>
> What about "pull --rebase=preserve"?  It sounds like a less radical
> option, did I miss something?

That option only exists since Git 1.8.5 or so. But I agree that it
should be used if available, as to not accidentally flatten local merges
you want to keep (you can make it default by setting the pull.rebase
option to 'preserve' - again something that really should be the
default).

-David

Re: Messing with the VC history

[Óscar Fuentes]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Óscar Fuentes <ofv@wanadoo.es>
>> Date: Sun, 16 Nov 2014 07:17:05 +0100
>> 
>> To welcome my first commit to Emacs, some people complicated the VC
>> history with unnecessary noise burying the happy event into a
>> merge-fest.
>
> I see nothing terribly messy there.  You will see very similar picture
> when someone merges their local feature branch, and then pushes
> upstream.

That's not the same at all. Feature branches and incorporating changes
from other public development branches *shall* be merged. What I'm
describing is an scenario where merges are created just because someone
pushed changes since your last `pull'. Apart from the noise on the VC
history, this procedure has a recursive nature: A tries to push but he
can't because there are new commits on the remote branch; then he pulls
(+merges) and pushes. Meanwhile, B was hacking and now tries to push,
but he can't because the changes pushed by A, so he pulls (+merges).
Then C (or A again) tries to push...

A single hacker can create multiple merges just to commit one change: it
pulls and finds conflicts; he resolves them (a merge is created) and
tries to push, but meanwhile others pushed their changes, so he pulls
again. He now has a merge within a merge just to push one patch.

The result is an entangled DAG that makes very difficult and unpleasant
to read the VC history.

> Maybe you don't like merge-commits in general, but then it's a
> different discussion altogether.

I don't like merge commits that add nothing but noise and confussion to
the VC history.

>> IMO we should encourage people to use fetch+rebase instead of `pull',
>
> Why not "pull --rebase" instead?

Because people here work with at least two branches. "pull --rebase"
pulls from all remote branches, but only rebases the branch you are
working on (let's say `master'). So, when you switch to other branch
(`emacs-24') you must remember that there are changes not yet
incorporated into your emacs-24 checkout. Not a big deal, but IMO it is
a good thing to have an idea of what you are going to incorporate into
your local branch.

With the right tool (Magit, for instance) fetch+rebase is faster than
the command line (just type ffR) and you see at all times your changes,
the fetched commits, the rebase status, etc.

> Or even tell them how to configure
> Git to do the "--rebase" part automatically?  People shouldn't need to
> learn yet another command, just for that.
>
>> (I'll not dare to ask why, mysteriously, the emacs-diffs mailing list
>> abstained from mentioning my commit.)
>
> It didn't.  It's just that this is your first time, and emacs-diffs is
> a moderated list, so I needed to approve your messages, just this
> once.  Part of your welcome party, I guess.

I feel so especial today.

Re: Messing with the VC history

[Eli Zaretskii]

> From: Óscar Fuentes <ofv@wanadoo.es>
> Date: Sun, 16 Nov 2014 17:05:28 +0100
> 
> >> To welcome my first commit to Emacs, some people complicated the VC
> >> history with unnecessary noise burying the happy event into a
> >> merge-fest.
> >
> > I see nothing terribly messy there.  You will see very similar picture
> > when someone merges their local feature branch, and then pushes
> > upstream.
> 
> That's not the same at all. Feature branches and incorporating changes
> from other public development branches *shall* be merged. What I'm
> describing is an scenario where merges are created just because someone
> pushed changes since your last `pull'.

But that's Git's default behavior: "merge" commits.  So when Stefan
merged from the branch, Git made a merge-commit, producing the same
effect as if Stefan were to merge his local feature branch.  What can
we do if the tool behaves like that?

> Apart from the noise on the VC history, this procedure has a
> recursive nature: A tries to push but he can't because there are new
> commits on the remote branch; then he pulls (+merges) and
> pushes. Meanwhile, B was hacking and now tries to push, but he can't
> because the changes pushed by A, so he pulls (+merges).  Then C (or
> A again) tries to push...

Yes, but (a) this danger existed with bzr as well, and (b) the race
you describe is in practice very rare, so the problem normally doesn't
happen, and having it here and there is not different from local
merges.

> > Maybe you don't like merge-commits in general, but then it's a
> > different discussion altogether.
> 
> I don't like merge commits that add nothing but noise and confussion to
> the VC history.

I didn't become confused when I looked at that part of the graph.

> >> IMO we should encourage people to use fetch+rebase instead of `pull',
> >
> > Why not "pull --rebase" instead?
> 
> Because people here work with at least two branches.

Who does?  The recommendation is not to do that, because master and
the release branch diverge very quickly, so switching branches will
almost always require a full bootstrap.

You are, of course, free to do that if you like, but then you probably
know what you are doing anyway.  In contrast, forcing the (at least
somewhat) confused majority to have yet another command in their
everyday workflow means more pain for them.

> With the right tool (Magit, for instance) fetch+rebase is faster than
> the command line (just type ffR) and you see at all times your changes,
> the fetched commits, the rebase status, etc.

We are talking about CLI commands here.  Emacs front ends are a
separate matter, they can simply automate everything, so people will
not have to remember the Git commands that run under the hood.  That's
not the issue at hand.

> >> (I'll not dare to ask why, mysteriously, the emacs-diffs mailing list
> >> abstained from mentioning my commit.)
> >
> > It didn't.  It's just that this is your first time, and emacs-diffs is
> > a moderated list, so I needed to approve your messages, just this
> > once.  Part of your welcome party, I guess.
> 
> I feel so especial today.

You are.  Welcome on board.

Re: Messing with the VC history

[Achim Gratz]

Eli Zaretskii writes:
>> ...or configure Git to have "git pull" do exactly that, at least for the
>> branches that are going upstream.
>
> What about "pull --rebase=preserve"?  It sounds like a less radical
> option, did I miss something?

Yes, you can do that if you use an up-to-date Git version.  Not everyone
involved here is using a new enough version to use the "preserve" option
(and yes, you can configure that on a global, local or branch-by-branch
basis too) I have gathered from the discussions so far.  For the usual
"let's fix this thing and get it upstream" type of work it shouldn't
matter.  Otherwise you'll have to remember to configure your feature
branches differently if they contain merges (local merges that is,
because merges with upstream wouldn't enter into the branch if you
rebase by default).  For longer term work that's kept in feature
branches I prefer to rebase on top of upstream rather than merge and
usually do a full rewrite before pushing it upstream as well.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf microQ V2.22R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: Messing with the VC history

[Eli Zaretskii]

> From: Achim Gratz <Stromeko@nexgo.de>
> Date: Sun, 16 Nov 2014 17:33:32 +0100
> 
> > What about "pull --rebase=preserve"?  It sounds like a less radical
> > option, did I miss something?
> 
> For the usual "let's fix this thing and get it upstream" type of
> work it shouldn't matter.  Otherwise you'll have to remember to
> configure your feature branches differently if they contain merges
> (local merges that is, because merges with upstream wouldn't enter
> into the branch if you rebase by default).

Sorry, I'm not sure I follow: the pull.rebase = preserve setting is
only for the "pull" command, right?  It doesn't affect "merge" and
commands that invoke "merge", like "cherry-pick", right?

If so, why would I need to configure my feature branches differently,
if I don't intend to pull into them from upstream?

My workflow with feature branches is create the branch; do the work;
when it's almost done, merge from master and test; then merge back
into master and push.  There's no pulling here except from upstream to
master.

> For longer term work that's kept in feature branches I prefer to
> rebase on top of upstream rather than merge and usually do a full
> rewrite before pushing it upstream as well.

Well, I prefer merging instead, see above.

Re: Messing with the VC history

[Stephen J. Turnbull]

Óscar Fuentes writes:

 > What I'm describing is an scenario where merges are created just
 > because someone pushed changes since your last `pull'. Apart from
 > the noise on the VC history, this procedure has a recursive nature:

You can't win this one, Óscar.  Many Emacs developers, including
several frequent committers, are uninterested in learning enough about
VCS to deal with these issues.  Some dislike rebasing in principle or
because doing it properly (as they understand it) involves running
tests on all rebased commits.

Re: Messing with the VC history

[John Yates]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

On Sun, Nov 16, 2014 at 6:33 PM, Stephen J. Turnbull <stephen@xemacs.org>
wrote:

> Many Emacs developers, including
> several frequent committers, are uninterested in learning enough about
> VCS to deal with these issues.  Some dislike rebasing in principle or
> because doing it properly (as they understand it) involves running
> tests on all rebased commits.
>

Are they under the impression that by contrast a merge
absolves them of any need to run tests?

/john

[-- Attachment #2: Type: text/html, Size: 831 bytes --]

Re: Messing with the VC history

[Stephen J. Turnbull]

John Yates writes:
 > On Sun, Nov 16, 2014 at 6:33 PM, Stephen J. Turnbull <stephen@xemacs.org>
 > wrote:

 > > Some dislike rebasing in principle or because doing it properly
 > > (as they understand it) involves running tests on all rebased
 > > commits.
 > 
 > Are they under the impression that by contrast a merge
 > absolves them of any need to run tests?

Of course not!  They know for a fact that they already ran them on the
revisions in their feature branch and on the merged code, and that the
rebased revisions will be *different* from the revisions they ran tests
on.  They object to running the tests *twice* when once should do.

Re: Messing with the VC history

[Eli Zaretskii]

> From: "Stephen J. Turnbull" <stephen@xemacs.org>
> Date: Mon, 17 Nov 2014 08:33:14 +0900
> Cc: emacs-devel@gnu.org
> 
> You can't win this one, Óscar.  Many Emacs developers, including
> several frequent committers, are uninterested in learning enough about
> VCS to deal with these issues.  Some dislike rebasing in principle or
> because doing it properly (as they understand it) involves running
> tests on all rebased commits.

I didn't want to start a "to rebase or not to rebase" argument.  Taken
at face value, neither was Óscar's original gripe.  So I pointed out
that a situation similar to what he complained about happens from time
to time in Emacs, and is IMO no catastrophe.

As for the rebase issue, the pros and cons are well known, and
interested parties can find good writeups on the Internet with sound
arguments either way.  We won't come up with anything new about that.
People take sides according to their personal preferences and work
habits, and that is okay as long as Emacs, unlike some other projects,
allows each one to do what they like in this department.  I see no
reason to argue about personal preferences, let alone accuse the
opponents of being unwilling to learn.

Re: Messing with the VC history

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: Achim Gratz <Stromeko@nexgo.de>,  emacs-devel@gnu.org
> Date: Sun, 16 Nov 2014 17:00:20 +0100
> 
> > What about "pull --rebase=preserve"?  It sounds like a less radical
> > option, did I miss something?
> 
> That option only exists since Git 1.8.5 or so. But I agree that it
> should be used if available, as to not accidentally flatten local merges
> you want to keep (you can make it default by setting the pull.rebase
> option to 'preserve' - again something that really should be the
> default).

Btw, those who configure Git to rebase on pull might also wish to set
rebase.stat = true, so the diffstat output is still shown when they
pull.

Re: Messing with the VC history

[Lars Brinkhoff]

Stephen J. Turnbull wrote:
> John Yates wrote:
>  > Stephen J. Turnbull wrote:
>  > > Some dislike rebasing in principle or because doing it properly
>  > > (as they understand it) involves running tests on all rebased
>  > > commits.
>  > 
>  > Are they under the impression that by contrast a merge absolves
>  > them of any need to run tests?
>
> Of course not!  They know for a fact that they already ran them on
> the revisions in their feature branch and on the merged code, and
> that the rebased revisions will be *different* from the revisions
> they ran tests on.  They object to running the tests *twice* when
> once should do.

I've met some of those.  Is there a counterargument?

Re: Messing with the VC history

[David Kastrup]

Lars Brinkhoff <lars@nocrew.org> writes:

> Stephen J. Turnbull wrote:
>> John Yates wrote:
>>  > Stephen J. Turnbull wrote:
>>  > > Some dislike rebasing in principle or because doing it properly
>>  > > (as they understand it) involves running tests on all rebased
>>  > > commits.
>>  > 
>>  > Are they under the impression that by contrast a merge absolves
>>  > them of any need to run tests?
>>
>> Of course not!  They know for a fact that they already ran them on
>> the revisions in their feature branch and on the merged code, and
>> that the rebased revisions will be *different* from the revisions
>> they ran tests on.  They object to running the tests *twice* when
>> once should do.
>
> I've met some of those.  Is there a counterargument?

After having repeated moments of tension with a non-workable master,
LilyPond moved to a different setup.  "Check that everything builds,
including the documentation" takes more than an hour on my system, a
moderately up-to-date dual processor machine.  We have contributors with
considerably less machine power.  We have a few volunteers with
considerably more.

Prospective patches are sent to a tracker.  Volunteers regularly pick
them up with an automated procedure, run all regtests and view the
flagged visual differences in the regression tests and logs and
(weighted and thresholded) memory usage statistics.  The last step of
viewing the differences is manual and making a comment/judgment is
manual, the rest automatic.  So all patches have seen testing against
_some_ version of LilyPond.

When committing any commit, it is committed to a branch called
"staging".  NO HUMAN COMMITS TO MASTER.  staging is picked up regularly
by an automated process, compiled, regtest-checked (without visual
comparison), documentation and translations built (includes all the web
sites).  If compilation completes successfully, the result is pushed to
master.

This automated process can fail for a number of reasons.  Mails get sent
out without stopping the scheduled attempts.

Those reasons are: failure (of course).
master cannot be fast-forwarded to staging (some human pushed to master,
or a staging test run on another machine with a conflicting staging
branch completed first): may need manual fixing
the tested staging is not strictly behind the upstream staging at the
time testing completes: that means that somebody cleared staging and
replaced it by something else while testing progressed: an emergency
stop if you find you pushed something bad and noticed immediately: in
that case resetting staging is enough to stop the automated run from
pushing the no-longer-accepted version of staging even if it tests fine.

The "who messed up master?" panics have gone.  Actually, the frequency
of messups overall has not been a lot recently, but "staging is messed
up" is a comparatively harmless problem.  It halts the queue.  It's only
messy when you have a lot of different contributions queued in staging
and one is bad.  However, the frequency of the automated runs make this
unlikely, and you can always wait for the queue to clear a bit if you
are not really sure your change is good and want to save others from
having to recommit and/or substitute a rebased version of staging with
your bad commits removed.

-- 
David Kastrup

Re: Messing with the VC history

[Stephen J. Turnbull]

Lars Brinkhoff writes:
 > Stephen J. Turnbull wrote:
 > > John Yates wrote:

 > >  > Are they under the impression that by contrast a merge absolves
 > >  > them of any need to run tests?
 > >
 > > Of course not!  They know for a fact that they already ran them on
 > > the revisions in their feature branch and on the merged code, and
 > > that the rebased revisions will be *different* from the revisions
 > > they ran tests on.  They object to running the tests *twice* when
 > > once should do.
 > 
 > I've met some of those.  Is there a counterargument?

(I'm not sure what you're asking.  HTH)  It depends on the purpose of
the tests.

- If the purpose of the tests is to ensure that *released* code is
  "clean", then you only need to test the merge version.  No problem,
  rebase and test when you feel like it. :-)

- If the purpose of the tests is to ensure that each revision in the
  DAG is "clean" (eg, in order to make bisect as accurate as
  possible), then it substantially reduces time and annoyance for the
  the developer to test each change in the feature branch then *merge*
  and test the merge commit.  This case basically comes down to a
  vote between the "pretty DAG" folks and the TDD crowd.[1]

- If the purpose of the tests is to ensure that each commit in the DAG
  is "clean" in the context of the latest revision before yours, then
  it makes sense to rebase and test each revision in the rebased
  branch.  You only do as much testing on the feature branch as gives
  you confidence you won't have to do much debugging and repair for
  rebased commits.  In this case there is no conflict between rebasing
  and thorough testing.

Personally, I'm kinda OCD and tend toward Type C most of the time.
(I've also read and understand the git documentation.  You see my
problem, I think. :-)  I find bisect very useful on occasion, so would
rule out Type A if at all possible.  But I find Type B plausible, and
think it's a matter of taste.

Footnotes: 
[1]  However, some people think a mainline with only merge commits on
it is pretty, and they just oppose rebasing on principle.

Re: Messing with the VC history

[David Kastrup]

"Stephen J. Turnbull" <stephen@xemacs.org> writes:

> [1]  However, some people think a mainline with only merge commits on
> it is pretty, and they just oppose rebasing on principle.

"Beauty" as an underlying concept of "pretty" usually involves
perfection, and perfection in technical settings implies everything
having a purpose.  A merge commit is a branch that starts off somewhere
and ends somewhere.  If there is no point to where the branch starts
off, the branch start is ugly, never mind whether you consider a merge
in itself pretty.

An example for a merge commit in a pretty-based development environment
would be
<URL:https://code.google.com/p/lilypond/issues/detail?id=4097#c14>.  A
merge commit is used because not all intermediate states in the
logically separate commits may be compilable (either they are known to
be uncompilable, or the jury is out on them).

Even through the commit sequence itself has been "prettified" by
structuring it into separate commits doing one logical task, a merge
commit is warranted in order not to cause bisection problems.  The
starting point of the branching point is not arbitrary: it is the parent
of the merge commit.  Such an incestuous merge has to be ordered
explicitly using the --no-ff flag to merge.

-- 
David Kastrup

Re: Messing with the VC history

[Barry Warsaw]

[-- Attachment #1: Type: text/plain, Size: 666 bytes --]

On Nov 18, 2014, at 04:53 PM, Stephen J. Turnbull wrote:

>I find bisect very useful on occasion, so would rule out Type A if at all
>possible.  But I find Type B plausible, and think it's a matter of taste.

bisect would be more useful, IMHO, if it followed first-parents by default.
IOW, when merging a feature branch into "mainline", I don't care whether
intermediate commits on the feature branch don't pass all their tests.  I *do*
care that every commit on "mainline" passes all their tests.

>[1]  However, some people think a mainline with only merge commits on
>it is pretty, and they just oppose rebasing on principle.

\o

Cheers,
-Barry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Messing with the VC history

[Andreas Schwab]

Barry Warsaw <barry@python.org> writes:

> bisect would be more useful, IMHO, if it followed first-parents by default.

Bisecting follows whatever you tell it about the good/bad state of
branches.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: Messing with the VC history

[Stephen J. Turnbull]

Andreas Schwab writes:
 > Barry Warsaw <barry@python.org> writes:
 > 
 > > bisect would be more useful, IMHO, if it followed first-parents by default.
 > 
 > Bisecting follows whatever you tell it about the good/bad state of
 > branches.

No, bisecting has a choice of which of several paths to follow through
a DAG between a branch point and a merge.  Telling it "bisect between
node and merge" doesn't help it make that choice.

I don't know how git chooses, but Barry is saying that in a workflow
where all mainline commits are either merge commits (for multiple-
commit "complex feature" branches) or one-off (for "simple bugfix"
commits), the bisect algorithm should follow the mainline (ie, first
parents) and completely ignore off-mainline "component of feature"
commits.  I suspect that is in fact what git does, but don't have time
to check.

Re: Messing with the VC history

[Stefan Monnier]

> I don't know how git chooses,

I'd expect it follows (one of) the "shortest path".


        Stefan

Re: Messing with the VC history

[Steinar Bang]

>>>>> Eli Zaretskii <eliz@gnu.org>:

> My workflow with feature branches is create the branch; do the work;
> when it's almost done, merge from master and test; then merge back
> into master and push.  There's no pulling here except from upstream to
> master.

Note: This is a very good work pattern in git, and people should
consider this rather than working directly on the tracking branch and
rebasing on pull.