patchAdding rebase merge strategy

In addition to adding git-merge-rebase.sh, git-merge.sh is modified to handle the rebase strategy specially and avoids running update-ref as rebase won't generate a merge commit.

Signed-off-by: Tom Clarke <tom@u2i.com>
---

Re-submitting this patch as we were in a pre-release freeze last time I submitted

 .gitignore                         |    1 +
 Documentation/merge-strategies.txt |    6 ++++++
 Makefile                           |    2 +-
 git-merge-rebase.sh                |   34 ++++++++++++++++++++++++++++++++++
 git-merge.sh                       |   14 +++++++++++---
 t/t3031-merge-rebase.sh            |   34 ++++++++++++++++++++++++++++++++++
 6 files changed, 87 insertions(+), 4 deletions(-)
 create mode 100755 git-merge-rebase.sh
 create mode 100755 t/t3031-merge-rebase.sh

Hi,

On Fri, 28 Sep 2007, Tom Clarke wrote:

Really?

You can check that much earlier, no?  IOW something like

	while test $# != 0
	do
		case "$1" in --) break ;; esac
		shift
	done

	test $# = 3 || die "merge stragey rebase needs exactly one ref"

	git rebase "$3"

(It's not like you need the variables...) Hmm?

You may want to warn earlier, if a message was supplied with -s rebase, or even error out.

I like to have something like this in a "test_expect_success setup" part, and then have the meat of the test in its own test case.

But hey, you're the author, it's for you to decide.

Thanks, Dscho

On 9/28/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Well 90% of the file was copy and pasted from one of the other merge strategies which was indeed written by Linus. So it seemed appropriate. If that's not appropriate attribution I can remove :-).

-Tom

In addition to adding git-merge-rebase.sh, git-merge.sh is modified to handle the rebase strategy specially and avoids running update-ref as rebase won't generate a merge commit. It also adds a warning if a message is supplied when the rebase isn't used as this will be ignored.

Signed-off-by: Tom Clarke <tom@u2i.com>
---

Incorporated comments from Johannes Schindlen.

 .gitignore                         |    1 +
 Documentation/merge-strategies.txt |    6 +++++
 Makefile                           |    2 +-
 git-merge-rebase.sh                |   17 ++++++++++++++
 git-merge.sh                       |   24 +++++++++++++++++--
 t/t3031-merge-rebase.sh            |   44 ++++++++++++++++++++++++++++++++++++
 6 files changed, 90 insertions(+), 4 deletions(-)
 create mode 100755 git-merge-rebase.sh
 create mode 100755 t/t3031-merge-rebase.sh

Hi,

On Mon, 1 Oct 2007, Tom Clarke wrote:

Thanks.

I think the user wants to know in this case, too.  How about

test $# = 1 || {
	echo "Cannot handle octopus." >&2
	exit 2
}

Please lose the parentheses here.

Why not do it as is done elsewhere in the test suit: use a "cat << EOF" before "test_expect_success"?

Same here.

Other than that, I like it.

Ciao, Dscho

Tom Clarke <tom@u2i.com> writes:

This would give a handier shortcut iff the rebase goes well, but the workflow after stopping would be entirely different from the normal "merge". I am a bit worried about it giving confusion to the end users.

This code makes rebase strategy to signal the caller that rebase it punted by exit status 2 when it spots conflicting changes.

At that point, what is the state of the branch tip, the index and the work tree?

When a merge strategy exits with 1 ("I cannot handle this fully but here is a partial attempt"), it is expected to leave something that can be resolved in the working tree to be committed (iow, do some edit, update-index and the a single "commit" will conclude the whole "git merge" business).

When the strategy exits with 2 ("I cannot deal with this at all"), git-merge will rewind the branch, index and the work tree by calling restorestate to the pristine state (in order to clear whatever mess the strategy may have created), say "Merge with strategy rebase failed.", and exits with 2.

And at that point, what can the user do?

The user then can initiate the rebase process again from the command line (perhaps even with "-i"):

	$ git rebase FETCH_HEAD

and deal manually with the incremental conflict resolution. This needs to be documented a bit more clearly, I think, than your change in the Documentation/merge-strategies.txt.

But more importantly, the above recovery is possible only if you abort that failed rebase you did in your strategy module, isn't it?

Although I haven't tried, I suspect that you need to change the final "git rebase $1 || exit 2" in your script with something like:

	git rebase "$1" || {
        	git rebase --abort
                exit 2
	}

Have you tested conflicting cases to see how well it works and what the user experience would look like?

Is this because a successful rebase strategy already updated the ref? I would not object from the end user experience point of view to allow "git merge -s rebase" command, but I suspect the changes to the merge frontend could to be restructured a bit to handle this easier to read.

Probably this is a good readability improvement.

That quoting is backwards, isn't it?

Literal "t" is not empty and you know you do not need to quote; you instead need to quote $have_message, because that could be empty and will not even be given as a separate token to "test" command without quoting when empty.

You have an unnecessary extra indentation here.

I had to spend 3 minutes thinking about this; if you had a comment like "A strategy that updates the ref by itself (iow, bypassing git-merge) does not give git-merge to record the merge commit using the given message." here, I did not have to.

On 10/1/07, Junio C Hamano <gitster@pobox.com> wrote:

Thanks for the ample feedback, you raise a number of interesting issues. I am wondering now if making rebase a merge strategy is really a good idea. Rebasing is not merging, a difference that could perhaps be overlooked in the no-conflict scenario, but as you point out, is glaringly obvious as soon as you have conflicts.

I'm happy to try to address the issues you raised, but I wonder if we would do better to look back at my original proposal which was to add a --rebase option to git-pull. git-pull is the main place there I see need for using a rebase instead of a merge, as anywhere where you might use git-merge directly, if what you really want is a rebase, you can just run git-rebase.

-Tom

On Mon, 1 Oct 2007 23:41:56 +0200, "Tom Clarke" wrote:

What I think I've always wanted is something like the following behavior for "git pull":

  * Fast forward if possible

  * Otherwise, rebase, but only if there are no conflicts at all

  * Otherwise, do the merge as normal, (leave conflict markers in
    place allowing the user to fix them up and then commit).

Would it be straightforward to turn your rebase merge strategy into something like the above? And if so, would that address the primary concerns that Junio raised?

-Carl

On 10/2/07, Carl Worth <cworth@cworth.org> wrote:

Maybe we need a 'pull' strategy' - merge, rebase or <insert name for strategy you describe above>.

-Tom

Tom Clarke <tom@u2i.com> wrote:

`git pull -s <name>` takes any merge strategy that git-merge will accept for its -s option. There is also the config option of pull.twohead that indicates what the default merge/pull strategy should be for a two head merge. Most people don't set this, letting the code default to 'recursive'.

But I have to agree with (was it Junio who said this?) doing a rebase in a merge strategy doesn't make sense when conflicts come into play. It gets confusing fast for the end-user as the conflict resolution process is different than for a merge. A long time ago I wrote a git-merge-rebase strategy and gave up on it for basically the same reason. I posted it to the mailing list and I think Linus said "Why?!". That was the end of that thread as I wound up agreeing with him.

Multiple merge strategies can be given (and attempted). A rebase strategy could be attempted before recursive, and if the rebase fails but the recursive succeeds then you get (roughly) what is being described above by Carl. But that still requires a rebase merge strategy. :-\

On Mon, 1 Oct 2007 18:30:50 -0400, "Shawn O. Pearce" wrote:

Ah, "pull.twohead". I don't think I ever would have guessed that. (And I was just about to ask if there was a nice place to find all these options, but then found it myself on my first guess with "man git-config". Thanks everyone for writing that!).

Sure. Rebase alone isn't useful as a complete merge strategy. But a rebase strategy that simply fails in the face of a conflict, (deferring to a subsequent merge strategy), could be very useful.

Yes, I thought I recalled seeing a rebase strategy go by in the past, but I had never gotten around to trying it out. I'll try to do better on this try.

Yes, this sounds exactly like what I want. So, I put "rebase recursive" in place as the value for the pull.twohead configuration? An then make sure that the rebase strategy aborts as "failed" instead of "conflicted and left for user to resolve"? I saw Junio talking about return values up above in the thread but didn't pay attention to details, (2 vs. 1 or something)?

Has anyone tried this rebase then recursive strategy yet? I'm definitely interested in trying it out, as I think I'd find it quite nice as a default for pull in my usage.

Though actually I'd like it even more if there was some way to mark a commit as having been "published" and the rebase strategy would refuse to rebase published commits. Maybe that's a per-branch "last-published" reference? I think I'd even like git-push to update the last-published reference for each pushed branch by default, but then perhaps have an option to mark a particular remote so that pushing to that remote doesn't count as publishing.

-Carl

Carl Worth <cworth@cworth.org> writes:

That is not "though actually", but an independent topic.

The need is indeed so real that there is a hook git-rebase pays attention to in order to help you with that. I use it to prevent myself from accidentally rebasing the topic branches that I already merged to 'next'.

Unsurprisingly, it is called pre-rebase hook.

Hi,

On Mon, 1 Oct 2007, Shawn O. Pearce wrote:

In contrast, I think that it makes sense, absolutely. If you asked for "rebase", you _have_ to know what is coming.

It's all about convenience: in many repos, I just to "git pull", because there is really only one upstream.

But in one repo, the upstream is svn, and I mistakenly checked in a merge. Not wanting to know svn deeply, I have no nice way (as I would have with git) to cover up my mistake. So in this repo, I would have liked to set branch.master.mergeOptions to '-s rebase'.

Ciao, Dscho

On 10/2/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

That's a good point, in addition to being able to do a git pull that uses rebase, it would be useful make this configurable so you can always safely do 'git pull'.

So it's perhaps the question is whether rebasing should be treated as a kind of merging, or as an alternative to merging when pulling. Incidentally, are there any other cases other than pulling where using rebase as an alternative merge strategy is useful?

-Tom

"Tom Clarke" <tom@u2i.com> writes:

Very well put; I like concise summary of the point in a discussion thread like this.

And the question in your last sentence is not merely incidental, but I think is the most crucial one to decide which way we should proceed.

I do not offhand think of a place other than "git pull" that would make sense to sometimes be able to rebase when you normally use merge, so I am inclined to say it would be easier to teach that "'git pull' is usually a 'git fetch' followed by 'git merge', but in certain workflow it is handier to 'git fetch' and then 'git rebase', and here are the ways to get the rebasing behaviour...".

On 10/2/07, Junio C Hamano <gitster@pobox.com> wrote:

I agree. I'll revisit teaching pull to be able to use rebase.

Thanks,

-Tom

Hi,

On Wed, 3 Oct 2007, Tom Clarke wrote:

In that case, may I request a config variable to set this behaviour automatically when calling "git pull <nick>"?

Had we stayed with the merge strategy approach, that would have come for free with the --no-ff patch series.

Ciao, Dscho

On Mon, Oct 01, 2007 at 03:17:28PM -0700, Carl Worth wrote:

Surely the job of a merge strategy is to take two heads and produce a single merge commit?

If it's worth automating the steps you describe above, I think it'd be better to choose an entirely different name for the command.

--b.

"Tom Clarke" <tom@u2i.com> writes:

Yeah, we have taught "git-pull == git-fetch + git-merge" to our users, and "-s strategy" has been a way to specify _how_ the merge is done, and not about doing something that is not a merge.

As you say, rebase is not doing a merge.  But neither is "squash".

"git-pull --rebase == git-fetch + git-rebase" might be simpler for end users to understand. I dunno.