Forcing --no-ff on pull

Forcing --no-ff on pull

[R. Tyler Ballance]

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

While I'm in the email writing mood tonight, I figured I'd ask this
question.

We've recently moved a giant tree with a number of developers over to
Git from Subversion. One of the biggest stumbling points we have right
now is the concept of a "fast-forward", insofar that it's "screwed" us a
couple times (see: people not RTFM'ing then crying that Git is broken
because they cannot RTFM ;))

The most common use-case involves a user merging a project branch into a
stabilization branch (`git checkout stable && git pull . project`) in
such a way that no merge commit is generated. Of course, without
thinking they'll push these changes up to the centralized repository.
Not 15 minutes later they realize "ruh roh! I didn't want to do that"
and become very frustrated that they have to resort to asking for help
or hand-reverting N number of commits. 

Is there a header macro I can define or a config option I could define
to make --no-ff on `git pull` implicit instead of explicit? Making sure
we are always generating merge commits as a "just-in-case" safe guard
about merge-happy developers who think after hitting enter? :)


Cheers
-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Forcing --no-ff on pull

[Jakub Narebski]

"R. Tyler Ballance" <tyler@slide.com> writes:

> Is there a header macro I can define or a config option I could define
> to make --no-ff on `git pull` implicit instead of explicit? Making sure
> we are always generating merge commits as a "just-in-case" safe guard
> about merge-happy developers who think after hitting enter? :)

branch.<name>.mergeoptions ?
-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: Forcing --no-ff on pull

[Lars Hjemli]

On Tue, Dec 9, 2008 at 10:34, R. Tyler Ballance <tyler@slide.com> wrote:
> Is there a header macro I can define or a config option I could define
> to make --no-ff on `git pull` implicit instead of explicit?

Try this:
$ git config branch.stable.mergeoptions "--no-ff"

--
lh

Re: Forcing --no-ff on pull

[Johannes Sixt]

R. Tyler Ballance schrieb:
> The most common use-case involves a user merging a project branch into a
> stabilization branch (`git checkout stable && git pull . project`) in
> such a way that no merge commit is generated. Of course, without
> thinking they'll push these changes up to the centralized repository.
> Not 15 minutes later they realize "ruh roh! I didn't want to do that"
> and become very frustrated that they have to resort to asking for help
> or hand-reverting N number of commits. 

Is the problem

 * that there is no merge commit, or

 * that you have to undo N commits instead of just one?

The latter is probably helped by

   $ git reset --hard ORIG_HEAD && git push -f origin

-- Hannes

Re: Forcing --no-ff on pull

[R. Tyler Ballance]

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

On Tue, 2008-12-09 at 10:49 +0100, Lars Hjemli wrote:
> On Tue, Dec 9, 2008 at 10:34, R. Tyler Ballance <tyler@slide.com> wrote:
> > Is there a header macro I can define or a config option I could define
> > to make --no-ff on `git pull` implicit instead of explicit?
> 
> Try this:
> $ git config branch.stable.mergeoptions "--no-ff"

I recall stumbling across this a while ago looking at the git-config(1)
man page, but this isn't /quite/ what we need.

I'm talking about forcing for *every* pull, it's a safe assumption to
make that we want a merge commit every time somebody fast-forwards a
branch. 

The only way I could think to make use of branch.<name>.mergeoptions
would be to automagically set it up in a "pre-merge" hook, but alas
post-merge exists but not pre-merge.

I could certainly patch to support a pre-merge, but that seems like the
longest possible route to my desired destination ;)


Cheers
-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Forcing --no-ff on pull

[Nanako Shiraishi]

Quoting "R. Tyler Ballance" <tyler@slide.com>:

> The most common use-case involves a user merging a project branch into a
> stabilization branch (`git checkout stable && git pull . project`) in
> such a way that no merge commit is generated. Of course, without
> thinking they'll push these changes up to the centralized repository.
> Not 15 minutes later they realize "ruh roh! I didn't want to do that"

Why does the user not want to fast-forward, if the merge she wants to do is actually a fast-forward?

If you mean that the user merged branches in a wrong direction, how does it help her avoid such a mistake to unconditionally forbid fast-forward merges?  Doesn't people often do:

 Start on a topic branch, have a potentially bright idea...
 % git checkout -b experiment
 Hack on experiment branch.
 Happy because it indeed was an excellent idea.
 % git checkout topic
 % git pull . experiment
 % git branch -d experiement

If you forbid fast-forward merges, when they merge their successful experiment back to the original topic, it will leave an unwanted merge in the history.

In other words, I do not think --no-ff is a right solution for the problem you are trying to solve.  Perhaps you would need a hook that prevents a merge from certain direction from taking place instead?

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: Forcing --no-ff on pull

[Lars Hjemli]

On Tue, Dec 9, 2008 at 11:12, R. Tyler Ballance <tyler@slide.com> wrote:
> On Tue, 2008-12-09 at 10:49 +0100, Lars Hjemli wrote:
>> On Tue, Dec 9, 2008 at 10:34, R. Tyler Ballance <tyler@slide.com> wrote:
>> > Is there a header macro I can define or a config option I could define
>> > to make --no-ff on `git pull` implicit instead of explicit?
>>
>> Try this:
>> $ git config branch.stable.mergeoptions "--no-ff"
>
> I recall stumbling across this a while ago looking at the git-config(1)
> man page, but this isn't /quite/ what we need.
>
> I'm talking about forcing for *every* pull, it's a safe assumption to
> make that we want a merge commit every time somebody fast-forwards a
> branch.

$ git config alias.xpull "pull --no-ff" ?

But are you sure you never want a fast-forward on _any_ branch? I use
--no-ff unconditionally on the master and stable branches as $dayjob,
to make sure that the merging of feature/bugfix-branches are
explicitly noted in history, but I almost never use it on other
branches.

--
larsh

Re: Forcing --no-ff on pull

[R. Tyler Ballance]

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

On Tue, 2008-12-09 at 19:17 +0900, Nanako Shiraishi wrote:
> Quoting "R. Tyler Ballance" <tyler@slide.com>:
> 
> > The most common use-case involves a user merging a project branch into a
> > stabilization branch (`git checkout stable && git pull . project`) in
> > such a way that no merge commit is generated. Of course, without
> > thinking they'll push these changes up to the centralized repository.
> > Not 15 minutes later they realize "ruh roh! I didn't want to do that"
> 
> Why does the user not want to fast-forward, if the merge she wants to do is actually a fast-forward?

I agree with you, this is more about preventing coworkers who are too
lazy to understand the entirety of what they're doing from hurting the
workflow of "the rest of us". It's a technically solution to a people
problem (I understand technology far more than people ;))

Consider the following scenarion:
  % git checkout -b project
  % <work>
  % git commit -am "A"
  % <work>
  % git commit -am "B"
  % <work>
  % git commit -am "C"
  % <work>
  % git commit -am "D"
  % git checkout stable
  % git pull . project
  % <fast-forward>
  % git push origin stable

At this point, QA is involved and what can happen is that QA realizes
that this code is *not* stable and *never* should have been brought into
the stable branch.

Now we have two options "block" the stable branch until LazyDeveloper
makes the appropriate changes to stabilize the branch again *OR* back
out LazyDeveloper's changes (A, B, C, D) and beat them up in the
alleyway :)

Given the nature of our work, we have a stable branch per-team, and one
funneling stable branch for the entire company (master), that branch
being used to push the live web site with. 

The first option (block) is not feasible as it will block the 40+ other
developers from pushing code until LazyDeveloper sufficiently gets their
crap together.

The second option is why I want to force --no-ff on *all* pulls if
possible. With --no-ff we can simply `git revert -sn <hash> -m 1 && git
commit -a` in order to back out A, B, C, D. With a true fast-forward,
we've had to use git-rev-list(1) trickery and some bash scriptery to
properly revert a series of commits from a given time frame from a given
developer.


> If you forbid fast-forward merges, when they merge their successful
> experiment back to the original topic, it will leave an unwanted merge
> in the history.

I'm less concerned at this point, the company switched entirely to Git
two weeks ago, with the history containing possible unwanted merges. I'm
more concerned however with LazyDeveloper inadvertently polluting stable
branches as LazyDeveloper does not yet fully grasp the concepts that Git
offers

> 
> In other words, I do not think --no-ff is a right solution for the problem you are trying to solve.  Perhaps you would need a hook that prevents a merge from certain direction from taking place instead?

If you do have a better solution to this problem (I dislike git push -f
origin[1]) I'm all ears, I'm more concerned with the end result at this
point ;)

Cheers


[1] We've stressed with our developers as much as possible that the
"origin" repository is to remain" pristine", that every action should be
"auditable" insofar that if you rollback a change, we want to see a
Revert commit, merges should create merge commits to where we can replay
or unwind the revision history correctly at any point in time or slice
of time. I *really* don't want "origin" to "lose commits".
-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Forcing --no-ff on pull

[R. Tyler Ballance]

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

On Tue, 2008-12-09 at 11:31 +0100, Lars Hjemli wrote:
> On Tue, Dec 9, 2008 at 11:12, R. Tyler Ballance <tyler@slide.com> wrote:
> > On Tue, 2008-12-09 at 10:49 +0100, Lars Hjemli wrote:
> >> On Tue, Dec 9, 2008 at 10:34, R. Tyler Ballance <tyler@slide.com> wrote:
> >> > Is there a header macro I can define or a config option I could define
> >> > to make --no-ff on `git pull` implicit instead of explicit?
> >>
> >> Try this:
> >> $ git config branch.stable.mergeoptions "--no-ff"
> >
> > I recall stumbling across this a while ago looking at the git-config(1)
> > man page, but this isn't /quite/ what we need.
> >
> > I'm talking about forcing for *every* pull, it's a safe assumption to
> > make that we want a merge commit every time somebody fast-forwards a
> > branch.
> 
> $ git config alias.xpull "pull --no-ff" ?

Interesting, I might have to try that out (wasn't aware of `git config
alias.<alias>`)

> 
> But are you sure you never want a fast-forward on _any_ branch? I use
> --no-ff unconditionally on the master and stable branches as $dayjob,
> to make sure that the merging of feature/bugfix-branches are
> explicitly noted in history, but I almost never use it on other
> branches.

I understand this, it's a funny situation. When we were evaluating Git
my team *never* had these issues because we all kept our trees in good
condition such that we never accidentally merged down to a stable
branch, but we also almost always generated merge commits because of the
variety of changes that would be going into stable at any given time.

I agree that I wouldn't want/need to use it on WIP branches or purely
local branches for development, so if I were able to restrict --no-ff to
only be forced on tracked branches I would be happy enough :)

Really hate to take this much bandwidth up on the mailing list over such
a silly problem, but after spending a week trying to /talk/ and educate
some folks, I feel drastic measures need to be taken ;)

Cheers 
-- 
-R. Tyler Ballance
Slide, Inc.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Forcing --no-ff on pull

[Jeff King]

On Tue, Dec 09, 2008 at 02:38:07AM -0800, R. Tyler Ballance wrote:

> At this point, QA is involved and what can happen is that QA realizes
> that this code is *not* stable and *never* should have been brought into
> the stable branch.
> 
> Now we have two options "block" the stable branch until LazyDeveloper
> makes the appropriate changes to stabilize the branch again *OR* back
> out LazyDeveloper's changes (A, B, C, D) and beat them up in the
> alleyway :)

It sounds like the problem is that LazyDeveloper has the authority to
push to the stable branch that everyone else pulls from, but can't be
trusted with that authority (because he is pushing bad work).

Maybe you would do better to invert your workflow:

  1. LazyDeveloper does some work on the 'foo' branch locally. Either
     his work repo is accessible to everyone, or he pushes it to a
     personal public repo (or a personal namespace within a shared
     repo).

  2. LazyDeveloper tells QA "check out foo, which should be ready for
     integration."

  3. QA pulls LazyDeveloper's foo. If it is OK, they merge and push to
     the official "stable" branch. If it isn't, they reject and
     LazyDeveloper fixes and goes back to step 2. LazyDeveloper is free
     to reset, rewind, or rebase as appropriate, since nobody but QA has
     ever even looked at this branch (and once they reached the "reject"
     conclusion, they don't care anymore).

So everyone builds off of the official "stable" branch, which by
definition is stuff that has passed through QA.

> Given the nature of our work, we have a stable branch per-team, and one
> funneling stable branch for the entire company (master), that branch
> being used to push the live web site with. 

And you could of course have per-team QA if you wanted to organize it
that way.

> The second option is why I want to force --no-ff on *all* pulls if
> possible. With --no-ff we can simply `git revert -sn <hash> -m 1 && git
> commit -a` in order to back out A, B, C, D. With a true fast-forward,
> we've had to use git-rev-list(1) trickery and some bash scriptery to
> properly revert a series of commits from a given time frame from a given
> developer.

There isn't good support for multiple reverts, but you can do the moral
equivalent with a big patch (note that revert can actually be more
clever about resolving the three way merge, but if you are close to the
tip, you shouldn't find any conflicts):

  git diff HEAD last-good-commit | git apply

If they are the tip commits, then you can always just make a new commit
with the pre-breakage state. This is sort of a mix of "git reset" and
"git revert" in that it throws away changes, but not history.

I don't think there is good porcelain support for this, but you can do:

  GIT_INDEX_FILE=index.tmp; export GIT_INDEX_FILE
  git read-tree last-good-commit
  git commit -m 'revert crappy commits'

-Peff

Re: Forcing --no-ff on pull

[Lars Hjemli]

On Tue, Dec 9, 2008 at 11:45, R. Tyler Ballance <tyler@slide.com> wrote:
> Really hate to take this much bandwidth up on the mailing list over such
> a silly problem, but after spending a week trying to /talk/ and educate
> some folks, I feel drastic measures need to be taken ;)

A possible solution could be the "Integration manager workflow" described here:

  http://whygitisbetterthanx.com/#any-workflow

But it has the potential of confusing your co-devs ;-)

--
larsh

Re: Forcing --no-ff on pull

[Boyd Stephen Smith Jr.]

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

On Tuesday 09 December 2008, "R. Tyler Ballance" <tyler@slide.com> wrote 
about 'Re: Forcing --no-ff on pull':
>On Tue, 2008-12-09 at 19:17 +0900, Nanako Shiraishi wrote:
>> Quoting "R. Tyler Ballance" <tyler@slide.com>:
>> > The most common use-case involves a user merging a project branch
>> > into a stabilization branch (`git checkout stable && git pull .
>> > project`) in such a way that no merge commit is generated. Of course,
>> > without thinking they'll push these changes up to the centralized
>> > repository. Not 15 minutes later they realize "ruh roh! I didn't want
>> > to do that"
>>
>> Why does the user not want to fast-forward, if the merge she wants to
>> do is actually a fast-forward?
>
>I agree with you, this is more about preventing coworkers who are too
>lazy to understand the entirety of what they're doing from hurting the
>workflow of "the rest of us". It's a technically solution to a people
>problem (I understand technology far more than people ;))
>
>Consider the following scenarion:
>  % git checkout -b project
>  % <work>
>  % git commit -am "A"
>  % <work>
>  % git commit -am "B"
>  % <work>
>  % git commit -am "C"
>  % <work>
>  % git commit -am "D"
>  % git checkout stable
>  % git pull . project
>  % <fast-forward>
>  % git push origin stable
>
>At this point, QA is involved and what can happen is that QA realizes
>that this code is *not* stable and *never* should have been brought into
>the stable branch.
>
>Now we have two options "block" the stable branch until LazyDeveloper
>makes the appropriate changes to stabilize the branch again *OR* back
>out LazyDeveloper's changes (A, B, C, D) and beat them up in the
>alleyway :)
>
>Given the nature of our work, we have a stable branch per-team, and one
>funneling stable branch for the entire company (master), that branch
>being used to push the live web site with.

In the words of 4chan: "You're doing it wrong."

If QA decides what is appropriate for the stable branch, only QA should be 
pushing to stable (not just any dev. or team) and this should be enforced.

QA can retrieve commits from individual developers or teams, via email, by 
pulling from their private repositories, or pulling from "private" 
branches in the public repository.  The last seems most appropriate for 
your organization.

I think a better workflow would be for developers to pull from "stable" but 
push to "<username>-tbr" (TBR = to be reviewed).  Team leads would review 
code by pulling from "<developer>-tbr" and if it looked okay would push 
to "<team>-tbt" (TBT = to be tested).  Of course, if they needed to 
originate a change they could pull from "stable" instead of any individual 
developer's branch.  QA would pull from "<team>-tbt", build, deploy, and 
test and if it's good push to "stable".  Some automated process would 
watch "stable" and update production from it.

This way bad commits are generally rejected before they become part of 
history.  Hooks can be used to notify team leads and QA about new commits 
for review or testing.

>[1] We've stressed with our developers as much as possible that the
>"origin" repository is to remain" pristine", that every action should be
>"auditable" insofar that if you rollback a change, we want to see a
>Revert commit, merges should create merge commits to where we can replay
>or unwind the revision history correctly at any point in time or slice
>of time. I *really* don't want "origin" to "lose commits".

To this end, I'd probably forbid non-ff commits to "stable".
-- 
Boyd Stephen Smith Jr.                     ,= ,-_-. =. 
bss03@volumehost.net                      ((_/)o o(\_))
ICQ: 514984 YM/AIM: DaTwinkDaddy           `-'(. .)`-' 
http://iguanasuicide.org/                      \_/     

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: Forcing --no-ff on pull

[Stephen Haberman]

> > $ git config branch.stable.mergeoptions "--no-ff"
> 
> I recall stumbling across this a while ago looking at the git-config(1)
> man page, but this isn't /quite/ what we need.
> 
> I'm talking about forcing for *every* pull, it's a safe assumption to
> make that we want a merge commit every time somebody fast-forwards a
> branch. 
> 
> The only way I could think to make use of branch.<name>.mergeoptions
> would be to automagically set it up in a "pre-merge" hook, but alas
> post-merge exists but not pre-merge.

I had done something like this with a post-checkout hook. After checking
out any branch, the hook sets various branch.<name>.options.

Also, I wrote a hook to enforce "only no-ff commits can move stable" and
other fun stuff. It's out on github, in a semi-documented/unannounced
project with the email/trac/etc. hooks we put in place:

http://github.com/stephenh/gc/tree/master/server/update-stable

- Stephen

Re: Forcing --no-ff on pull

[Daniel Barkalow]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3180 bytes --]

On Tue, 9 Dec 2008, R. Tyler Ballance wrote:

> On Tue, 2008-12-09 at 19:17 +0900, Nanako Shiraishi wrote:
> > Quoting "R. Tyler Ballance" <tyler@slide.com>:
> > 
> > > The most common use-case involves a user merging a project branch into a
> > > stabilization branch (`git checkout stable && git pull . project`) in
> > > such a way that no merge commit is generated. Of course, without
> > > thinking they'll push these changes up to the centralized repository.
> > > Not 15 minutes later they realize "ruh roh! I didn't want to do that"
> > 
> > Why does the user not want to fast-forward, if the merge she wants to do is actually a fast-forward?
> 
> I agree with you, this is more about preventing coworkers who are too
> lazy to understand the entirety of what they're doing from hurting the
> workflow of "the rest of us". It's a technically solution to a people
> problem (I understand technology far more than people ;))
> 
> Consider the following scenarion:
>   % git checkout -b project
>   % <work>
>   % git commit -am "A"
>   % <work>
>   % git commit -am "B"
>   % <work>
>   % git commit -am "C"
>   % <work>
>   % git commit -am "D"
>   % git checkout stable
>   % git pull . project
>   % <fast-forward>
>   % git push origin stable
> 
> At this point, QA is involved and what can happen is that QA realizes
> that this code is *not* stable and *never* should have been brought into
> the stable branch.

How do you prevent the (IMHO more likely) case of:

% git checkout -b project
% git checkout stable
<fix some bug in stable>
% git commit -a
<forget to switch branches back>
<work>
% git commit -am "A"
<work>
% git commit -am "B"
...
% git push origin stable

That is, the developer makes a whole bunch of inappropriate commits on 
their stable branch instead of their project branch and then pushes it out 
(perhaps as part of a push rule, or thinking only the bug fix went there). 
I suspect that "pull" step there isn't the point where things are going 
wrong.

If you've actually got QA in the process, have developers push to a 
per-developer location and send a pull request to QA. QA can reject bad 
changes instead of putting them into the stable branch at all, and then 
they can reply to the pull requests with snide comments instead of having 
to beat up the developers, because the developer doesn't inconvenience 
anybody (except QA, whose job is to be inconvenienced by developers).

> I'm less concerned at this point, the company switched entirely to Git
> two weeks ago, with the history containing possible unwanted merges. I'm
> more concerned however with LazyDeveloper inadvertently polluting stable
> branches as LazyDeveloper does not yet fully grasp the concepts that Git
> offers

I think such developers are more likely to push some bad commits to 
"stable" directly than they are to make their bad commits on a branch, 
merge it (fast-forward or otherwise) and push the result. It's also easy 
to discover:

% git push origin project:stable

And not generate a merge commit simply by virtue of not merging branches.

	-Daniel
*This .sig left intentionally blank*

Re: Forcing --no-ff on pull

[Stephen Haberman]

On Tue, 9 Dec 2008 17:32:36 -0500 (EST)
Daniel Barkalow <barkalow@iabervon.org> wrote:

> > At this point, QA is involved and what can happen is that QA realizes
> > that this code is *not* stable and *never* should have been brought into
> > the stable branch.
>
> How do you prevent the (IMHO more likely) case of:
> 
> % git checkout -b project
> % git checkout stable
> <fix some bug in stable>
> % git commit -a
> <forget to switch branches back>
> <work>
> % git commit -am "A"
> <work>
> % git commit -am "B"
> ...
> % git push origin stable
> 
> That is, the developer makes a whole bunch of inappropriate commits on 
> their stable branch instead of their project branch and then pushes it out 
> (perhaps as part of a push rule, or thinking only the bug fix went there). 
> I suspect that "pull" step there isn't the point where things are going 
> wrong.

Well, two things:

1) The hook script at [1] really would prevent this from getting published.
   Although it only looks for "stable"--if you have per-team stable branches,
   you might need to match on "*-stable" or something like that. But it does
   (copy/paste from [1]):

# * stable must move by only 1 commit-per-push
# * the stable commit must have 2 and only 2 parents
#   * The first parent must be the previous stable commit
#   * The second parent is the tip of the candidate branch being released
# * the stable commit must have the same contents as the candidate tip
#   * Any merge conflicts should have been resolved in the candidate tip
#     by pulling stable into the candidate and having qa/tests done--pulling
#     candidate into stable should then apply cleanly

So, no fast forwards, no direct commits, only "good"/empty merges of
topic branches can move stable. Anything else is rejected and LazyDev
has to try again.

2) As far as "pull" isn't where things are going wrong, that is not
   entirely true, as even with the server-side enforcement like [1],
   I think you'd still like to help LazyDev out and have `git pull`
   "just work" for your given setup. Especially if you don't have full
   management buy-in to git, pacifying LazyDev's can be necessary.

- Stephen

1: http://github.com/stephenh/gc/tree/master/server/update-stable