Re: [PATCH 0/3] Reject non-ff pulls by default

From: Jeff King <peff@peff.net>
Date: Sep 9, 2013, 20:44 UTC
Message-ID: <20130909204415.GC14536@sigill.intra.peff.net>
In-Reply-To: <20130909202435.GJ2582@serenity.lan>

On Mon, Sep 09, 2013 at 09:24:35PM +0100, John Keeping wrote:

Show 17 quoted lines

> > I think that would address the concern I raised, because it does not
> > create a roadblock to new users accomplishing their task. They can
> > ignore the warning, or choose "merge" as the default to shut up the
> > warning (and it is easy to choose that if you are confused, because it
> > is what git is doing by default alongside the warning).
> 
> I think we need to make sure that we give instructions for how to go
> back if the default hasn't done what you wanted.  Something like this:
> 
>     Your pull did not fast-forward, so Git has merged '$upstream' into
>     your branch, which may not be correct for your project.  If you
>     would rather rebase your changes, run
> 
>         git rebase
> 
>     See "pull.mode" in git-config(1) to suppress this message in the
>     future.

Yes, that's a good point. I don't know if just "git rebase" is the right advice, though; it would depend on whether we were actually pulling from the upstream or not.

I wonder if we have sufficient information at the time of the warning to print out the actual "git rebase" invocation that would rebase as if they had run "pull --rebase". I think we may have to do a little refactoring around the base selection from the reflog (IIRC, git-pull does not even calculate it at all if you are not using --rebase).

It is also depending on "git rebase" throwing away the merge commit we just created. Which I think should happen always if you have not configured anything (though perhaps we will eventually support a pull mode that does "rebase -p", you would not see this warning with that option anyway). But another option would be to simply tell them:

  git reset --keep HEAD^
  git pull --rebase [X...]

where "[X...]" is the arguments they gave to rebase in the first place. That looks a little less friendly, though.

-Peff