[PATCH 1/4] Move redo merge code in a function

[PATCH 1/4] Move redo merge code in a function

[Jörg Sommer]

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 8aa7371..1b2381e 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -113,6 +113,22 @@ has_action () {
 	grep '^[^#]' "$1" >/dev/null
 }
 
+redo_merge() {
+	author_script=$(get_author_ident_from_commit $sha1)
+	eval "$author_script"
+	msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
+	if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
+		GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
+		GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
+		output git merge $STRATEGY -m "$msg" \
+			$new_parents
+	then
+		git rerere
+		printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG
+		die Error redoing merge $sha1
+	fi
+}
+
 pick_one () {
 	no_ff=
 	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
@@ -180,22 +196,9 @@ pick_one_preserving_merges () {
 		echo $sha1 > "$DOTEST"/current-commit
 		case "$new_parents" in
 		' '*' '*)
-			# redo merge
-			author_script=$(get_author_ident_from_commit $sha1)
-			eval "$author_script"
-			msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
 			# No point in merging the first parent, that's HEAD
 			new_parents=${new_parents# $first_parent}
-			if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
-				GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
-				GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
-				output git merge $STRATEGY -m "$msg" \
-					$new_parents
-			then
-				git rerere
-				printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG
-				die Error redoing merge $sha1
-			fi
+			redo_merge
 			;;
 		*)
 			output git cherry-pick "$@" ||
-- 
1.5.4.4

[PATCH 2/4] Rework redo_merge

[Jörg Sommer]

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 1b2381e..ffd4823 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -114,14 +114,16 @@ has_action () {
 }
 
 redo_merge() {
-	author_script=$(get_author_ident_from_commit $sha1)
-	eval "$author_script"
+	sha1=$1
+	shift
+
+	eval "$(get_author_ident_from_commit $sha1)"
 	msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
+
 	if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
 		GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
 		GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
-		output git merge $STRATEGY -m "$msg" \
-			$new_parents
+		output git merge $STRATEGY -m "$msg" "$@"
 	then
 		git rerere
 		printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG
@@ -197,8 +199,7 @@ pick_one_preserving_merges () {
 		case "$new_parents" in
 		' '*' '*)
 			# No point in merging the first parent, that's HEAD
-			new_parents=${new_parents# $first_parent}
-			redo_merge
+			redo_merge $sha1 ${new_parents# $first_parent}
 			;;
 		*)
 			output git cherry-pick "$@" ||
-- 
1.5.4.4

[PATCH 3/4] Add a function for get the parents of a commit

[Jörg Sommer]

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ffd4823..94c6827 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -131,6 +131,10 @@ redo_merge() {
 	fi
 }
 
+parents_of_commit() {
+	git rev-list --parents -1 "$1" | cut -d' ' -f2-
+}
+
 pick_one () {
 	no_ff=
 	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
@@ -166,7 +170,7 @@ pick_one_preserving_merges () {
 	fast_forward=t
 	preserve=t
 	new_parents=
-	for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -f2-)
+	for p in $(parents_of_commit $sha1)
 	do
 		if test -f "$REWRITTEN"/$p
 		then
-- 
1.5.4.4

[PATCH 4/4] git-rebase -i: New option to support rebase with merges

[Jörg Sommer]

The option --preserve-merges does not allow to change the order of
commits or squash them. The new option --linear-history does support
this, but doing so it can only look at the commits reachable with through
the first parent of each merge.

Joining merge commits with other commits leads to problems, because git
merge fails with a dirty index (the case “COMMIT squash MERGE”) and
squashing a merge leads to the lost of the parents (case “MERGE squash
COMMIT”). Therefore, I've prohibited these cases.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 Documentation/git-rebase.txt  |    8 ++++
 git-rebase--interactive.sh    |   27 +++++++++++++++-
 t/t3404-rebase-interactive.sh |   72 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+), 1 deletions(-)

I had no better idea for a name of this new option. Propositions are
welcome.

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e0412e0..354b6f0 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -11,6 +11,7 @@ SYNOPSIS
 'git-rebase' [-i | --interactive] [-v | --verbose] [-m | --merge]
 	[-s <strategy> | --strategy=<strategy>]
 	[-C<n>] [ --whitespace=<option>] [-p | --preserve-merges]
+	[-l | --linear-history]
 	[--onto <newbase>] <upstream> [<branch>]
 'git-rebase' --continue | --skip | --abort
 
@@ -247,6 +248,13 @@ OPTIONS
 	Instead of ignoring merges, try to recreate them.  This option
 	only works in interactive mode.
 
+-l, \--linear-history::
+	Use only commits of the branch they are not merged in, i.e.
+	follow only the first parent of a merge. Merges are part of this
+	list and they will be redone. It's possible to move merges in the
+	history forward and backward, but they can't take part on a join
+	(squash). This option only works in interactive mode.
+
 include::merge-strategies.txt[]
 
 NOTES
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 94c6827..a2a61f8 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -26,9 +26,11 @@ REWRITTEN="$DOTEST"/rewritten
 PRESERVE_MERGES=
 STRATEGY=
 VERBOSE=
+LINEAR_HISTORY=
 test -d "$REWRITTEN" && PRESERVE_MERGES=t
 test -f "$DOTEST"/strategy && STRATEGY="$(cat "$DOTEST"/strategy)"
 test -f "$DOTEST"/verbose && VERBOSE=t
+test -f "$DOTEST"/linear_history && LINEAR_HISTORY=t
 
 GIT_CHERRY_PICK_HELP="  After resolving the conflicts,
 mark the corrected paths with 'git add <paths>', and
@@ -150,7 +152,18 @@ pick_one () {
 		sha1=$(git rev-parse --short $sha1)
 		output warn Fast forward to $sha1
 	else
-		output git cherry-pick "$@"
+		if test t = "$LINEAR_HISTORY" &&
+			other_parents="$(parents_of_commit $sha1 | cut -s -d' ' -f2-)" &&
+			test -n "$other_parents"
+		then
+			if test a"$1" = a-n
+			then
+				merge_opt=--no-commit
+			fi
+			redo_merge $sha1 $no_commit $other_parents
+		else
+			output git cherry-pick "$@"
+		fi
 	fi
 }
 
@@ -288,6 +301,11 @@ do_next () {
 		has_action "$DONE" ||
 			die "Cannot 'squash' without a previous commit"
 
+		test t = "$LINEAR_HISTORY" &&
+			( test "$(parents_of_commit HEAD |wc -w)" -gt 1 ||
+				 test "$(parents_of_commit $sha1 |wc -w)" -gt 1) &&
+			die "Joining a merge with a commit is not supported"
+
 		mark_action_done
 		make_squash_message $sha1 > "$MSG"
 		case "$(peek_next_command)" in
@@ -459,6 +477,9 @@ do
 	-i|--interactive)
 		# yeah, we know
 		;;
+	-l|--linear-history)
+		LINEAR_HISTORY=t
+		;;
 	''|-h)
 		usage
 		;;
@@ -522,6 +543,10 @@ do
 					die "Could not init rewritten commits"
 			done
 			MERGES_OPTION=
+		elif test t = "$LINEAR_HISTORY"
+		then
+			: > "$DOTEST"/linear_history
+			MERGES_OPTION=--first-parent
 		else
 			MERGES_OPTION=--no-merges
 		fi
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9cf873f..0476f6a 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -361,4 +361,76 @@ test_expect_success 'rebase with a file named HEAD in worktree' '
 
 '
 
+test_expect_success 'rebase linear history preserves merges' '
+	git tag linear-history-base to-be-preserved~2
+	git checkout -b linear-history linear-history-base &&
+	for i in 1 2 3
+	do
+		test $? -eq 0 &&
+		echo linear history test > lin-h-$i &&
+		git add lin-h-$i &&
+		test_tick &&
+	       	git commit -m "rebase linear history commit $i"
+	done
+	test_tick &&
+	git merge to-be-preserved &&
+	old_head=$(git rev-parse HEAD) &&
+	test_tick &&
+	EXPECT_COUNT=4 FAKE_LINES="2 4 edit 1 3" \
+		git rebase -v -i -l linear-history-base &&
+	EXPECT_COUNT=invalid git rebase --continue &&
+	test "$(git rev-list --parents -1 HEAD~2 | cut -d" " -f3-)" = \
+		"$(git rev-parse to-be-preserved)" &&
+	test "$(git show HEAD~2: | grep ^lin-h- | cut -c7- | tr -d \\012)" = 2 &&
+	test "$(git show HEAD~1: | grep ^lin-h- | cut -c7- | tr -d \\012)" = 12 &&
+	test "$(git cat-file commit HEAD | sed -n "/^tree/{p;q;}")" = \
+		"$(git cat-file commit $old_head | sed -n "/^tree/{p;q;}")"
+'
+
+test_expect_success 'rebase linear history is noop, if base = base' '
+	old_head=$(git rev-parse HEAD) &&
+	test_tick &&
+	EXPECT_COUNT=4 git rebase -v -i -l linear-history-base &&
+	test "$(git rev-parse HEAD)" = $old_head
+'
+
+test_expect_success 'ensure rebase linear history persits across edits' '
+	old_head=$(git rev-parse HEAD) &&
+	test_tick &&
+	EXPECT_COUNT=4 FAKE_LINES="edit 1 2 3 4" \
+		git rebase -v -i -l linear-history-base &&
+	EXPECT_COUNT=invalid git rebase --continue &&
+	test "$(git rev-parse HEAD)" = $old_head
+'
+
+test_str='test_tick &&
+	(
+		outp=$(test_must_fail git rebase -v -i -l HEAD~3 2>&1)
+		rc=$?
+		echo "$outp"
+        	echo "$outp" | grep "^Joining .* not supported\$" >/dev/null &&
+		return $rc
+	) &&
+	git rebase --abort'
+
+test_expect_success 'rebase linear with squashing a merge fails' "
+	export EXPECT_COUNT=3 FAKE_LINES='1 squash 2 3' &&
+	$test_str &&
+	FAKE_LINES='2 squash 1 3' &&
+	$test_str
+"
+
+test_expect_success 'rebase linear history does a fast forward' '
+	old_head=$(git rev-parse HEAD) &&
+	test_tick &&
+	EXPECT_COUNT=4 FAKE_LINES="2 3 4 1" \
+		git rebase -v -i -l linear-history-base &&
+	test "$(git rev-parse HEAD~3)" = "$(git rev-parse to-be-preserved)" &&
+	test "$(git show HEAD~2: | grep ^lin-h- | cut -c7- | tr -d \\012)" = 1 &&
+	test "$(git show HEAD~1: | grep ^lin-h- | cut -c7- | tr -d \\012)" = 13 &&
+	test -z "$(git rev-list --parents -1 HEAD~3 | cut -d" " -f3-)" &&
+	test "$(git cat-file commit HEAD | sed -n "/^tree/{p;q;}")" = \
+		"$(git cat-file commit $old_head | sed -n "/^tree/{p;q;}")"
+'
+
 test_done
-- 
1.5.4.4

Re: [PATCH 4/4] git-rebase -i: New option to support rebase with merges

[Johannes Schindelin]

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

Hi,

On Sun, 23 Mar 2008, Jörg Sommer wrote:

> The option --preserve-merges does not allow to change the order of 
> commits or squash them. The new option --linear-history does support 
> this, but doing so it can only look at the commits reachable with 
> through the first parent of each merge.

Why do you call it "linear-history"?  That name is pretty ambiguous.  Why 
not calling it "--first-parents"?

> Joining merge commits with other commits leads to problems, because git
> merge fails with a dirty index (the case “COMMIT squash MERGE”) and
> squashing a merge leads to the lost of the parents (case “MERGE squash
> COMMIT”).

Please use the term "to squash" not "to join".  And say "to the loss" 
instead of "to the lost".

And I still think that it would be better to fix the bug that squashing 
merges fails.

> @@ -247,6 +248,13 @@ OPTIONS
>  	Instead of ignoring merges, try to recreate them.  This option
>  	only works in interactive mode.
>  
> +-l, \--linear-history::
> +	Use only commits of the branch they are not merged in, i.e.

s/they are/that are/

> +	follow only the first parent of a merge. Merges are part of this

s/first parent of a merge/first parents of the encountered merge commits/

> @@ -150,7 +152,18 @@ pick_one () {
>  		sha1=$(git rev-parse --short $sha1)
>  		output warn Fast forward to $sha1
>  	else
> -		output git cherry-pick "$@"
> +		if test t = "$LINEAR_HISTORY" &&
> +			other_parents="$(parents_of_commit $sha1 | cut -s -d' ' -f2-)" &&
> +			test -n "$other_parents"
> +		then
> +			if test a"$1" = a-n
> +			then
> +				merge_opt=--no-commit
> +			fi
> +			redo_merge $sha1 $no_commit $other_parents
> +		else
> +			output git cherry-pick "$@"
> +		fi

Now, that is funny.  In case of --preserve-merges, I would have expected 
you to touch pick_one_preserving_merges(), not pick_one().

I would find it highly illogical to try to redo merges _without_ -p.

And again, I have to stress that fixing -p for the cases you mentioned 
should be a higher priority than to introduce new options to work around 
the bugs.  Seems like I am repeating myself, but hopefully I don't have to 
do that many more times.

Ciao,
Dscho

Re: [PATCH 4/4] git-rebase -i: New option to support rebase with merges

[Jörg Sommer]

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

Hi Johannes,

Johannes Schindelin schrieb am Sun 23. Mar, 23:41 (+0100):
> On Sun, 23 Mar 2008, Jörg Sommer wrote:
> 
> > @@ -150,7 +152,18 @@ pick_one () {
> >  		sha1=$(git rev-parse --short $sha1)
> >  		output warn Fast forward to $sha1
> >  	else
> > -		output git cherry-pick "$@"
> > +		if test t = "$LINEAR_HISTORY" &&
> > +			other_parents="$(parents_of_commit $sha1 | cut -s -d' ' -f2-)" &&
> > +			test -n "$other_parents"
> > +		then
> > +			if test a"$1" = a-n
> > +			then
> > +				merge_opt=--no-commit
> > +			fi
> > +			redo_merge $sha1 $no_commit $other_parents
> > +		else
> > +			output git cherry-pick "$@"
> > +		fi
> 
> Now, that is funny.  In case of --preserve-merges, I would have expected 
> you to touch pick_one_preserving_merges(), not pick_one().
> 
> I would find it highly illogical to try to redo merges _without_ -p.

Me too, but I think it's not possible to do what I want with -p. -p
misses a definition of the (new) parent of a commit. It tries to preserve
all commits from all branches. But going through the _list_ of commands
couldn't preserve this structure.

o--A--B
 \     \
  C--D--M--E

How should the graph look like after these commands:

pick A
pick C
squash E
# pick D
pick B
pick M

Should

pick A
pick B
pick C
pick D
pick M
pick E

give a same graph like

pick C
pick A
pick D
pick B
pick M
pick E

Bye, Jörg.
-- 
< Mr X.> jo: contact an admin to mount it for you
< jo> The admin is not, well how should I say it, he isn't very familiar with
      the system. What should I tell my admin, what he should do?
< Mr X.> taking a sun solaris administration course.

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 4/4] git-rebase -i: New option to support rebase with merges

[Johannes Schindelin]

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

Hi,

On Mon, 24 Mar 2008, Jörg Sommer wrote:

> Johannes Schindelin schrieb am Sun 23. Mar, 23:41 (+0100):
> > On Sun, 23 Mar 2008, Jörg Sommer wrote:
> > 
> > > @@ -150,7 +152,18 @@ pick_one () {
> > >  		sha1=$(git rev-parse --short $sha1)
> > >  		output warn Fast forward to $sha1
> > >  	else
> > > -		output git cherry-pick "$@"
> > > +		if test t = "$LINEAR_HISTORY" &&
> > > +			other_parents="$(parents_of_commit $sha1 | cut -s -d' ' -f2-)" &&
> > > +			test -n "$other_parents"
> > > +		then
> > > +			if test a"$1" = a-n
> > > +			then
> > > +				merge_opt=--no-commit
> > > +			fi
> > > +			redo_merge $sha1 $no_commit $other_parents
> > > +		else
> > > +			output git cherry-pick "$@"
> > > +		fi
> > 
> > Now, that is funny.  In case of --preserve-merges, I would have 
> > expected you to touch pick_one_preserving_merges(), not pick_one().
> > 
> > I would find it highly illogical to try to redo merges _without_ -p.
> 
> Me too, but I think it's not possible to do what I want with -p.

But that is not a reason to mess up the source code.  If you do a thing as 
--linear-history (or --first-parents, as I would prefer it), that should 
be in the code path of --preserve-merges (and actually _imply_ that 
option).

--linear-history makes no sense at all without --preserve-merges.

Ciao,
Dscho

Re: [PATCH 4/4] git-rebase -i: New option to support rebase with merges

[Jörg Sommer]

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

Hallo Johannes,

Johannes Schindelin schrieb am Mon 24. Mar, 14:08 (+0100):
> On Mon, 24 Mar 2008, Jörg Sommer wrote:
> 
> > Johannes Schindelin schrieb am Sun 23. Mar, 23:41 (+0100):
> > > Now, that is funny.  In case of --preserve-merges, I would have 
> > > expected you to touch pick_one_preserving_merges(), not pick_one().
> > > 
> > > I would find it highly illogical to try to redo merges _without_ -p.
> > 
> > Me too, but I think it's not possible to do what I want with -p.
> 
> But that is not a reason to mess up the source code.  If you do a thing as 
> --linear-history (or --first-parents, as I would prefer it), that should 
> be in the code path of --preserve-merges (and actually _imply_ that 
> option).

Thanks for answering my question. You are really cooperatively.

Bye, Jörg.
-- 
IRC: Der [Prof. Andreas Pfitzmann, TU Dresden] hat gerade vorgeschlagen, sie
  sollen doch statt Trojanern die elektromagnetische Abstrahlung nutzen. Das
  sei nicht massenfähig, ginge ohne Eingriff ins System, sei zielgerichtet,
  und, der Hammer, das funktioniere ja bei Wahlcomputern schon sehr gut.

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 4/4] git-rebase -i: New option to support rebase with merges

[Junio C Hamano]

Jörg Sommer <joerg@alea.gnuu.de> writes:

> Me too, but I think it's not possible to do what I want with -p. -p
> misses a definition of the (new) parent of a commit. It tries to preserve
> all commits from all branches. But going through the _list_ of commands
> couldn't preserve this structure.
>
> o--A--B
>  \     \
>   C--D--M--E
>
> How should the graph look like after these commands:
>
> pick A
> pick C
> squash E
> # pick D
> pick B
> pick M

I am beginning to suspect that the root cause of this is that the todo
language is not expressive enough to reproduce a merge _and_ allow end
user editing.

Let's step back a bit.

If you have this history:

      o---o---o---o---o---Z
     /
    X---Y---A---B
         \       \
          C---D---M---E

and you want to transplant the history  between X..E on top of Z, from the
command line you would say:

	$ git rebase --interactive -p --onto Z X E

First let's think what you would do if you want to do this by hand.  The
sequence would be:

	$ git checkout Z^0 ;# detach at Z

        $ git cherry-pick Y
        $ git tag new-Y ;# remember it
        $ git cherry-pick A
        $ git cherry-pick B
        $ git tag new-B ;# remember it
        $ git checkout new-Y
        $ git cherry-pick C
        $ git cherry-pick D
        $ git merge new-B ;# this reproduces M
        $ git cherry-pick E

	$ git branch -f $the_branch && git checkout $the_branch


Now how does the todo file before you edit look like?

	pick Y
        pick A
        pick B
        pick C
        pick D
        pick M
        pick E

The todo file expects the initial detaching and the final switching back
outside of its control, so it is Ok that the first "checkout Z^0" and the
last "branch && checkout" do not appear, but it should be able to express
the remainder and let you tweak.  Is it expressive enough to do so?  

Most of the "pick" from the above list literally translate to the
"cherry-pick", and if you change any of them to "edit", that is
"cherry-pick --no-edit" followed by a return of control to you with insn
to "rebase --continue" to resume.  There appears nothing magical.

Not really.  There already are two gotchas even without talking about
end-user editing.

First, "pick M" is not "cherry-pick M".  You do not want to end up with
merging an old parent before rewriting.  It has to be something like
"merge rewritten-Y".

Second, before you start picking C, if you want to preserve merges, you
have to switch to rewritten Y.  The original sequence left in todo does
not have that information to begin with.  We need, before the "pick C", to
say the equivalent of "git checkout new-Y" in the manual sequence
illustrated above.  The lack of "checkout new-Y" is perfectly fine if
rebase is meant to linearlize the history, but if you want to preserve the
shape of the history, you would need to give a clue that the sequence that
begins with the "pick C" starts from somewhere else.

You also need to make sure that "pick M" moved elsewhere still merges the
tips of two forked histories.  Moving "pick M" before "pick C" or "pick A"
would not make much sense.  So you would need some kind of "barrier" that
says "do not move this 'pick M' beyond this point".

Perhaps we can make it clearer by introducing a few more primitives to the
todo language: mark, reset and merge.  The above illustrated history would
then become:

	pick Y
        mark #0
	pick A
        pick B
        mark #1
        reset #0
        pick C
        pick D
	mark #2
        merge #1 #2
        pick E

You can change any of the "pick" to "edit, or drop it, and you can reorder
"pick" in a sequence of "pick", but you cannot change "mark", "reset",
"merge", or move "pick" across insn that was not originally "pick".

Re: [PATCH 4/4] git-rebase -i: New option to support rebase with merges

[Junio C Hamano]

I'll extend this topic a bit for the last time, but first a word of
caution.  What I am going to draw is probably not what the current -p
implementation does.  They illustrate what I think should happen.

Again, starting from this topology:

       o---o---o---o---o---Z
      /
     X---Y---A---B
          \       \
           C---D---M---E

and the goal is to rebase your development leading to E on top of the
updated mainline Z.

The earlier example was when you want to end up with this topology:

       o---o---o---o---o---Z---Y'--A'--B'      
      /                         \       \      
     X---Y---A---B               C'--D'--M'--E'
          \       \
           C---D---M---E

In this case, "pick M" cannot be "merge B after pick D".  It needs to
merge in the rewritten B (which is B').

But if you want to end up with this topology:

       o---o---o---o---o---Z---Y'--A'--B'
      /                                 \     
     X---Y---A---B                       M'--E'
          \       \                     /
           C---D---M---E               /
                \                     /
                 `-------------------'

redoing the merge from D when reproducing M' is the right thing to do.

Unfortunately, you cannot express that you would want to rewrite only the
Y--A--B--M--E ancestry from the command line.  We would need a syntax to
do this cleanly first if we want to pursue this.

The "first parent" hack can be used in this case (--first-parent X..E),
but it will probably meet with the same resistance at the philosophical
level (i.e. "merge parents are equal") as the --first-parent option was
criticised for.  But other than that, a sequence to pick Y A B M E in this
order can be presented in the todo list to be edited, and swapping A and M
(for example) should result in this:

       o---o---o---o---o---Z---Y'
      /                         \
     X---Y---A---B               M'--A'--B'
          \       \             /
           C---D---M---E       /
                \             /
                 `-----------'

The existing revision range arithmetic can only come close with "E ^Z ^D"
(or "^D Z..E"), but that would exclude Y as "uninteresting" (because Y is
reachable from D).  You would end up with A--B--M--E.  So even though I am
sympathetic to poeple who complained that the command line parameter to
rebase is different from the log family, using revision range syntax is
not a solution to this.

Just for completeness of the illustration, linealizing form aims to
produce the history like this:

       o---o---o---o---o---Z---Y'--A'--B'--C'--D'--E'
      /
     X---Y---A---B
          \       \
           C---D---M---E

You can freely to reorder anything in the "todo" list without additional
constraints in this form (sequence of "pick" Y, A, B, C, D and E).

Re: [PATCH 4/4] git-rebase -i: New option to support rebase with merges

[Jörg Sommer]

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

Hi Junio,

Junio C Hamano schrieb am Mon 24. Mar, 11:35 (-0700):
> If you have this history:
>
>       o---o---o---o---o---Z
>      /
>     X---Y---A---B
>          \       \
>           C---D---M---E


I would like to extend this example:

      o---o---o---o---o---Z
     /
    X---Y---A-------B
         \           \
          C---D---N---M---E
                 /
                V

> and you want to transplant the history  between X..E on top of Z, from the
> command line you would say:
> 
> 	$ git rebase --interactive -p --onto Z X E
> 
> First let's think what you would do if you want to do this by hand.  The
> sequence would be:
> 
> 	$ git checkout Z^0 ;# detach at Z
> 
>         $ git cherry-pick Y
>         $ git tag new-Y ;# remember it
>         $ git cherry-pick A
>         $ git cherry-pick B
>         $ git tag new-B ;# remember it
>         $ git checkout new-Y
>         $ git cherry-pick C
>         $ git cherry-pick D
% git merge V
>         $ git merge new-B ;# this reproduces M
>         $ git cherry-pick E
> 
> 	$ git branch -f $the_branch && git checkout $the_branch


> Perhaps we can make it clearer by introducing a few more primitives to the
> todo language: mark, reset and merge.  The above illustrated history would
> then become:
> 
> 	pick Y
>         mark #0
> 	pick A
>         pick B
>         mark #1
>         reset #0
>         pick C
>         pick D
merge V

V is not a mark.

> 	mark #2
>         merge #1 #2
>         pick E
> 
> You can change any of the "pick" to "edit, or drop it, and you can reorder
> "pick" in a sequence of "pick", but you cannot change "mark", "reset",
> "merge", or move "pick" across insn that was not originally "pick".

This way we can also merge more than two branches. Your idea sounds good.

Bye, Jörg.
-- 
Nichts ist so langweilig, wie die Wiederholung seinerselbst.
                                        (Marcel Reich‐Ranicki)

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH 4/4] git-rebase -i: New option to support rebase with merges

[Junio C Hamano]

Jörg Sommer <joerg@alea.gnuu.de> writes:

> I would like to extend this example:
>
>       o---o---o---o---o---Z
>      /
>     X---Y---A-------B
>          \           \
>           C---D---N---M---E
>                  /
>                 V
> ...
> merge V
>
> V is not a mark.

That's essentially the same as what I drew in a follow-up to the message
you are responding to, using --first-parents to mark D as "not subject to
rewrite but still interesting".

As I explained there, a bigger issue is how you would express the set of
commits that you would want to rewrite and use intact.  In your
illustration, you would want to rewrite N but you want to reuse V.
"rebase Z E" or "rebase --onto Z X E" would include V (and all of its
ancestors that cannot be reached from Z or X that you did not draw) in the
set to be rewritten.  Extending the input to the rebase command to use
revision range syntax and saying it as Z..E (or X..E) would not help
either.

Teach rebase interactive more commands to do better preserve merges

[Jörg Sommer]

Junio proposed to add the commands mark, merge and reset to rebase
interactive for a better support of rebase with preserve merges. The
current format can only cope with flat lineare lists of commits and not
with the non‐linear structure of branches with merges.

This patch series is not intended for inclusion. It's a RFC and meant for
gathering ideas.

Currently it misses:
· commit messages
· documentation of the new commands
· update of the documentation of preserve merges
· tests for the merge command

I've included the patch to change the behaviour of git-merge from Junio,
because I don't know how to use the second syntax of git merge and pass
the merge strategies -s to git merge. Without this patch, a new merge
message gets appended to the commit message of a rebased merge commit.
This patch should not be part of the final commit series.

Open questions:
· Is it possible to get a list of all commits iterating the parents from
  the last to the first and going to the child of a node before the
  neighbors?

· How to get a symbolic name (branch or tag) for a commit? Not something
  like git describe. I want to know if 0123 refers to the tip of a
  branch.

Jörg.

[PATCH/RFC 01/10] Teach rebase interactive the mark command

[Jörg Sommer]

This new command can be used to set symbolic marks for an commit while
doing a rebase. This symbolic name can later be used for merges or
resets.
---
 git-rebase--interactive.sh    |   36 ++++++++++++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh |   21 +++++++++++++++++++++
 2 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 8aa7371..b001ddf 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -23,6 +23,7 @@ DONE="$DOTEST"/done
 MSG="$DOTEST"/message
 SQUASH_MSG="$DOTEST"/message-squash
 REWRITTEN="$DOTEST"/rewritten
+MARKS="$DOTEST"/marks
 PRESERVE_MERGES=
 STRATEGY=
 VERBOSE=
@@ -240,6 +241,23 @@ peek_next_command () {
 	sed -n "1s/ .*$//p" < "$TODO"
 }
 
+parse_mark() {
+	local tmp
+	tmp="$*"
+
+	case "$tmp" in
+	'#'[0-9]*)
+		tmp="${tmp#\#}"
+		if test "$tmp" = $(printf %d "$tmp")
+		then
+			echo $tmp
+			return 0
+		fi
+		;;
+	esac
+	return 1
+}
+
 do_next () {
 	rm -f "$DOTEST"/message "$DOTEST"/author-script \
 		"$DOTEST"/amend || exit
@@ -317,6 +335,23 @@ do_next () {
 			die_with_patch $sha1 ""
 		fi
 		;;
+	mark|a)
+		if ! mark=$(parse_mark $sha1)
+		then
+			warn "Invalid mark given: $command $sha1 $rest"
+			die_with_patch $sha1 \
+				"Please fix this in the file $TODO."
+		fi
+		mark_action_done
+
+		test -d "$MARKS" || mkdir "$MARKS"
+
+		test -e "$MARKS"/$mark && \
+			warn "mark $mark already exist; overwriting it"
+
+		git rev-parse --verify HEAD > "$MARKS"/$mark || \
+			die "HEAD is invalid"
+		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
 		die_with_patch $sha1 "Please fix this in the file $TODO."
@@ -533,6 +568,7 @@ do
 #  pick = use commit
 #  edit = use commit, but stop for amending
 #  squash = use commit, but meld into previous commit
+#  mark #NUM = mark the current HEAD for later reference
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9cf873f..4461674 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -82,6 +82,9 @@ for line in $FAKE_LINES; do
 	case $line in
 	squash|edit)
 		action="$line";;
+	mark*)
+		echo "mark ${line#mark}"
+		echo "mark ${line#mark}" >> "$1";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
 		sed -n "${line}p" < "$1".tmp
@@ -189,6 +192,24 @@ test_expect_success '-p handles "no changes" gracefully' '
 	test $HEAD = $(git rev-parse HEAD)
 '
 
+test_expect_success 'setting an invalid mark fails' '
+	export FAKE_LINES="mark12 1" && \
+	test_must_fail git rebase -i HEAD~1 &&
+	unset FAKE_LINES &&
+	git rebase --abort
+'
+
+test_expect_success 'setting marks works' '
+	git checkout master &&
+	FAKE_LINES="mark#0 2 1 mark#42 3 edit 4" git rebase -i HEAD~4 &&
+	marks_dir=.git/.dotest-merge/marks
+	test -d $marks_dir &&
+	test $(ls $marks_dir | wc -l) -eq 2 &&
+	test "$(git rev-parse HEAD~4)" = "$(cat $marks_dir/0)" &&
+	test "$(git rev-parse HEAD~2)" = "$(cat $marks_dir/42)" &&
+	git rebase --abort
+'
+
 test_expect_success 'preserve merges with -p' '
 	git checkout -b to-be-preserved master^ &&
 	: > unrelated-file &&
-- 
1.5.4.5

[PATCH/RFC 02/10] Teach rebase interactive the reset command

[Jörg Sommer]

---
 git-rebase--interactive.sh    |   20 ++++++++++++++++++++
 t/t3404-rebase-interactive.sh |   10 ++++++++++
 2 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index b001ddf..7dac51b 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -258,6 +258,16 @@ parse_mark() {
 	return 1
 }
 
+mark_to_sha1() {
+	local mark
+	if mark=$(parse_mark $1)
+	then
+		cat "$MARKS"/$mark
+	else
+		return 1
+	fi
+}
+
 do_next () {
 	rm -f "$DOTEST"/message "$DOTEST"/author-script \
 		"$DOTEST"/amend || exit
@@ -352,6 +362,15 @@ do_next () {
 		git rev-parse --verify HEAD > "$MARKS"/$mark || \
 			die "HEAD is invalid"
 		;;
+	reset|r)
+		comment_for_reflog reset
+
+		mark_action_done
+		tmp=$(mark_to_sha1 $sha1) || \
+			tmp=$(git rev-parse --verify $sha1) ||
+			die "Invalid parent '$sha1' in $command $sha1 $rest"
+		output git reset --hard $tmp
+		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
 		die_with_patch $sha1 "Please fix this in the file $TODO."
@@ -569,6 +588,7 @@ do
 #  edit = use commit, but stop for amending
 #  squash = use commit, but meld into previous commit
 #  mark #NUM = mark the current HEAD for later reference
+#  reset #NUM|commit = reset HEAD to a previous set mark or a commit
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 4461674..521206f 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -85,6 +85,9 @@ for line in $FAKE_LINES; do
 	mark*)
 		echo "mark ${line#mark}"
 		echo "mark ${line#mark}" >> "$1";;
+	reset*)
+		echo "reset ${line#reset}"
+		echo "reset ${line#reset}" >> "$1";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
 		sed -n "${line}p" < "$1".tmp
@@ -210,6 +213,13 @@ test_expect_success 'setting marks works' '
 	git rebase --abort
 '
 
+test_expect_success 'reset with nonexistent mark fails' '
+	export FAKE_LINES="reset#0 1" &&
+	test_must_fail git rebase -i HEAD~1 &&
+	unset FAKE_LINES &&
+	git rebase --abort
+'
+
 test_expect_success 'preserve merges with -p' '
 	git checkout -b to-be-preserved master^ &&
 	: > unrelated-file &&
-- 
1.5.4.5

[PATCH/RFC 03/10] Teach rebase interactive the merge command

[Jörg Sommer]

This command redoes merges. It's useful if you rebase a branch that
contains merges and you want to preserve these merges. You can also use
it to add new merges.
---
 git-rebase--interactive.sh    |   25 +++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh |    3 +++
 2 files changed, 28 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 7dac51b..18cdf3d 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -362,6 +362,29 @@ do_next () {
 		git rev-parse --verify HEAD > "$MARKS"/$mark || \
 			die "HEAD is invalid"
 		;;
+	merge|m)
+		comment_for_reflog merge
+
+		if ! git rev-parse --verify $sha1
+		then
+			die "Invalid reference merge '$sha1' in $command $sha1 $rest"
+		fi
+
+		new_parents=
+		for p in $rest
+		do
+			tmp=$(mark_to_sha1 $p) || \
+				tmp=$(git rev-parse --verify $p) ||
+				die "Invalid parent '$sha1' in $command $sha1 $rest"
+			new_parents="$new_parents $tmp"
+		done
+		new_parents="${new_parents# }"
+		test -n "$new_parents" || \
+			die "You forgot to give the parents for the merge"
+
+		mark_action_done
+		redo_merge $sha1 $new_parents
+		;;
 	reset|r)
 		comment_for_reflog reset
 
@@ -589,6 +612,8 @@ do
 #  squash = use commit, but meld into previous commit
 #  mark #NUM = mark the current HEAD for later reference
 #  reset #NUM|commit = reset HEAD to a previous set mark or a commit
+#  merge commit-M #NUM|commit-P ... = redo merge commit-M with the
+#         current HEAD and the parents marked with #NUM or the commit-P
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 521206f..892fd5d 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -88,6 +88,9 @@ for line in $FAKE_LINES; do
 	reset*)
 		echo "reset ${line#reset}"
 		echo "reset ${line#reset}" >> "$1";;
+	merge*)
+		echo "merge ${line#merge}" | tr / ' '
+		echo "merge ${line#merge}" | tr / ' ' >> "$1";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
 		sed -n "${line}p" < "$1".tmp
-- 
1.5.4.5

[PATCH/RFC 04/10] Move redo merge code in a function

[Jörg Sommer]

---
 git-rebase--interactive.sh |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 18cdf3d..973770e 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -114,6 +114,22 @@ has_action () {
 	grep '^[^#]' "$1" >/dev/null
 }
 
+redo_merge() {
+	author_script=$(get_author_ident_from_commit $sha1)
+	eval "$author_script"
+	msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
+	if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
+		GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
+		GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
+		output git merge $STRATEGY -m "$msg" \
+			$new_parents
+	then
+		git rerere
+		printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG
+		die Error redoing merge $sha1
+	fi
+}
+
 pick_one () {
 	no_ff=
 	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
@@ -181,22 +197,9 @@ pick_one_preserving_merges () {
 		echo $sha1 > "$DOTEST"/current-commit
 		case "$new_parents" in
 		' '*' '*)
-			# redo merge
-			author_script=$(get_author_ident_from_commit $sha1)
-			eval "$author_script"
-			msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
 			# No point in merging the first parent, that's HEAD
 			new_parents=${new_parents# $first_parent}
-			if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
-				GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
-				GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
-				output git merge $STRATEGY -m "$msg" \
-					$new_parents
-			then
-				git rerere
-				printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG
-				die Error redoing merge $sha1
-			fi
+			redo_merge
 			;;
 		*)
 			output git cherry-pick "$@" ||
-- 
1.5.4.5

[PATCH/RFC 05/10] Rework redo_merge

[Jörg Sommer]

---
 git-rebase--interactive.sh |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 973770e..1698c3e 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -115,14 +115,17 @@ has_action () {
 }
 
 redo_merge() {
-	author_script=$(get_author_ident_from_commit $sha1)
-	eval "$author_script"
+	local sha1
+	sha1=$1
+	shift
+
+	eval "$(get_author_ident_from_commit $sha1)"
 	msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
+
 	if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
 		GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
 		GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
-		output git merge $STRATEGY -m "$msg" \
-			$new_parents
+		output git merge $STRATEGY -m "$msg" "$@"
 	then
 		git rerere
 		printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG
@@ -198,8 +201,7 @@ pick_one_preserving_merges () {
 		case "$new_parents" in
 		' '*' '*)
 			# No point in merging the first parent, that's HEAD
-			new_parents=${new_parents# $first_parent}
-			redo_merge
+			redo_merge $sha1 ${new_parents# $first_parent}
 			;;
 		*)
 			output git cherry-pick "$@" ||
-- 
1.5.4.5

[PATCH/RFC 06/10] Unify the lenght of $SHORT* and the commits in the TODO list

[Jörg Sommer]

---
 git-rebase--interactive.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 1698c3e..060b40f 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -600,9 +600,9 @@ do
 			MERGES_OPTION=--no-merges
 		fi
 
-		SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
-		SHORTHEAD=$(git rev-parse --short $HEAD)
-		SHORTONTO=$(git rev-parse --short $ONTO)
+		SHORTUPSTREAM=$(git rev-parse --short=7 $UPSTREAM)
+		SHORTHEAD=$(git rev-parse --short=7 $HEAD)
+		SHORTONTO=$(git rev-parse --short=7 $ONTO)
 		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
 			--abbrev=7 --reverse --left-right --cherry-pick \
 			$UPSTREAM...$HEAD | \
-- 
1.5.4.5

[PATCH/RFC 07/10] fake-editor: output TODO list if unchanged

[Jörg Sommer]

It's much helpful to see the TODO list generated by rebase in the verbose
output. This makes it easier to check, if the list was not broken by
design.
---
 t/t3404-rebase-interactive.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 892fd5d..aa4bb8d 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -73,7 +73,7 @@ esac
 test -z "$EXPECT_COUNT" ||
 	test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) ||
 	exit
-test -z "$FAKE_LINES" && exit
+test -z "$FAKE_LINES" && { grep -v '^#' "$1"; exit; }
 grep -v '^#' < "$1" > "$1".tmp
 rm -f "$1"
 cat "$1".tmp
-- 
1.5.4.5

[PATCH/RFC 08/10] Don't append default merge message to -m message

[Jörg Sommer]

Picked from <7vabko3dm2.fsf@gitster.siamese.dyndns.org>
From: gitster@pobox.com (Junio C Hamano)
Date: Sun, 23 Mar 2008 22:17:09 -0700
---
 git-merge.sh |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/git-merge.sh b/git-merge.sh
index 7dbbb1d..bd9699d 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -250,17 +250,19 @@ else
 	# We are invoked directly as the first-class UI.
 	head_arg=HEAD
 
-	# All the rest are the commits being merged; prepare
-	# the standard merge summary message to be appended to
-	# the given message.  If remote is invalid we will die
-	# later in the common codepath so we discard the error
-	# in this loop.
-	merge_name=$(for remote
-		do
-			merge_name "$remote"
-		done | git fmt-merge-msg
-	)
-	merge_msg="${merge_msg:+$merge_msg$LF$LF}$merge_name"
+	if test -z "$merge_msg"
+	then
+		# All the rest are the commits being merged; prepare
+		# the standard merge summary message to be appended to
+		# the given message.  If remote is invalid we will die
+		# later in the common codepath so we discard the error
+		# in this loop.
+		merge_msg=$(for remote
+			do
+				merge_name "$remote"
+			done | git fmt-merge-msg
+		)
+	fi
 fi
 head=$(git rev-parse --verify "$head_arg"^0) || usage
 
-- 
1.5.4.5

[PATCH/RFC 09/10] Select all lines with fake-editor

[Jörg Sommer]

The old fake-editor selects only lines they start with pick, if you give
the number of a line. With the new commands mark, merge and reset it was
not possible to select such lines for the new TODO list. The new
fake-editor selects all kinds of lines, but replaces only the command
“pick” with a different action.
---
 t/t3404-rebase-interactive.sh |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index aa4bb8d..be26a78 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -92,9 +92,8 @@ for line in $FAKE_LINES; do
 		echo "merge ${line#merge}" | tr / ' '
 		echo "merge ${line#merge}" | tr / ' ' >> "$1";;
 	*)
-		echo sed -n "${line}s/^pick/$action/p"
-		sed -n "${line}p" < "$1".tmp
-		sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
+		sed -n "${line}{s/^pick/$action/; p;}" < "$1".tmp
+		sed -n "${line}{s/^pick/$action/; p;}" < "$1".tmp >> "$1"
 		action=pick;;
 	esac
 done
-- 
1.5.4.5

[PATCH/RFC 10/10] Do rebase with preserve merges with advanced TODO list

[Jörg Sommer]

---
 git-rebase--interactive.sh    |  248 ++++++++++++++++++++++++-----------------
 t/t3404-rebase-interactive.sh |   34 ++++++
 2 files changed, 181 insertions(+), 101 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 060b40f..27bd87e 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -22,12 +22,10 @@ TODO="$DOTEST"/git-rebase-todo
 DONE="$DOTEST"/done
 MSG="$DOTEST"/message
 SQUASH_MSG="$DOTEST"/message-squash
-REWRITTEN="$DOTEST"/rewritten
 MARKS="$DOTEST"/marks
 PRESERVE_MERGES=
 STRATEGY=
 VERBOSE=
-test -d "$REWRITTEN" && PRESERVE_MERGES=t
 test -f "$DOTEST"/strategy && STRATEGY="$(cat "$DOTEST"/strategy)"
 test -f "$DOTEST"/verbose && VERBOSE=t
 
@@ -137,8 +135,6 @@ pick_one () {
 	no_ff=
 	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
 	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
-	test -d "$REWRITTEN" &&
-		pick_one_preserving_merges "$@" && return
 	parent_sha1=$(git rev-parse --verify $sha1^) ||
 		die "Could not get the parent of $sha1"
 	current_sha1=$(git rev-parse --verify HEAD)
@@ -152,66 +148,6 @@ pick_one () {
 	fi
 }
 
-pick_one_preserving_merges () {
-	case "$1" in -n) sha1=$2 ;; *) sha1=$1 ;; esac
-	sha1=$(git rev-parse $sha1)
-
-	if test -f "$DOTEST"/current-commit
-	then
-		current_commit=$(cat "$DOTEST"/current-commit) &&
-		git rev-parse HEAD > "$REWRITTEN"/$current_commit &&
-		rm "$DOTEST"/current-commit ||
-		die "Cannot write current commit's replacement sha1"
-	fi
-
-	# rewrite parents; if none were rewritten, we can fast-forward.
-	fast_forward=t
-	preserve=t
-	new_parents=
-	for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -f2-)
-	do
-		if test -f "$REWRITTEN"/$p
-		then
-			preserve=f
-			new_p=$(cat "$REWRITTEN"/$p)
-			test $p != $new_p && fast_forward=f
-			case "$new_parents" in
-			*$new_p*)
-				;; # do nothing; that parent is already there
-			*)
-				new_parents="$new_parents $new_p"
-				;;
-			esac
-		fi
-	done
-	case $fast_forward in
-	t)
-		output warn "Fast forward to $sha1"
-		test $preserve = f || echo $sha1 > "$REWRITTEN"/$sha1
-		;;
-	f)
-		test "a$1" = a-n && die "Refusing to squash a merge: $sha1"
-
-		first_parent=$(expr "$new_parents" : ' \([^ ]*\)')
-		# detach HEAD to current parent
-		output git checkout $first_parent 2> /dev/null ||
-			die "Cannot move HEAD to $first_parent"
-
-		echo $sha1 > "$DOTEST"/current-commit
-		case "$new_parents" in
-		' '*' '*)
-			# No point in merging the first parent, that's HEAD
-			redo_merge $sha1 ${new_parents# $first_parent}
-			;;
-		*)
-			output git cherry-pick "$@" ||
-				die_with_patch $sha1 "Could not pick $sha1"
-			;;
-		esac
-		;;
-	esac
-}
-
 nth_string () {
 	case "$1" in
 	*1[0-9]|*[04-9]) echo "$1"th;;
@@ -410,20 +346,7 @@ do_next () {
 	HEADNAME=$(cat "$DOTEST"/head-name) &&
 	OLDHEAD=$(cat "$DOTEST"/head) &&
 	SHORTONTO=$(git rev-parse --short $(cat "$DOTEST"/onto)) &&
-	if test -d "$REWRITTEN"
-	then
-		test -f "$DOTEST"/current-commit &&
-			current_commit=$(cat "$DOTEST"/current-commit) &&
-			git rev-parse HEAD > "$REWRITTEN"/$current_commit
-		if test -f "$REWRITTEN"/$OLDHEAD
-		then
-			NEWHEAD=$(cat "$REWRITTEN"/$OLDHEAD)
-		else
-			NEWHEAD=$OLDHEAD
-		fi
-	else
-		NEWHEAD=$(git rev-parse HEAD)
-	fi &&
+	NEWHEAD=$(git rev-parse HEAD) &&
 	case $HEADNAME in
 	refs/*)
 		message="$GIT_REFLOG_ACTION: $HEADNAME onto $SHORTONTO)" &&
@@ -448,6 +371,138 @@ do_rest () {
 	done
 }
 
+sha1_to_mark() {
+	# args: "sha1" " sha1#mark sha1#mark"
+	local tmp
+	case "$2" in
+	*" $1#"*)
+		tmp="${2#* $1#}"
+		echo "${tmp%% *}"
+		;;
+	*)
+		return 1
+		;;
+	esac
+}
+
+insert_sha1_with_mark_in_list() {
+	# args: "sha1" "mark" " sha1#mark sha1#mark"
+	case "$3" in
+	*" $1#"*)
+		echo "$3"
+		return 1
+		;;
+	*)
+		echo "$3 $1#$2"
+		;;
+	esac
+}
+
+create_extended_todo_list() {
+	(
+	while IFS=_ read commit parents subject
+	do
+		if test "${last_parent:-$commit}" != "$commit"
+		then
+			if test t = "${delayed_mark:-f}"
+			then
+				case "${marked_commits:-} " in
+				*" $last_parent ")
+					;;
+				*)
+					marked_commits="${marked_commits:-} $last_parent"
+					;;
+				esac
+				delayed_mark=f
+			fi
+			test "$last_parent" = $SHORTUPSTREAM && \
+				last_parent=$SHORTONTO
+			echo "reset $last_parent"
+		fi
+		last_parent="${parents%% *}"
+
+		case "${marked_commits:-} " in
+		*" $commit "*)
+			echo mark
+			;;
+		esac
+
+		case "$parents" in
+		*' '*)
+			delayed_mark=t
+			new_parents=
+			for p in ${parents#* }
+			do
+				case "${marked_commits:-} " in
+				*" $p ")
+					;;
+				*)
+					marked_commits="${marked_commits:-} $p"
+					;;
+				esac
+				if test "$p" = $SHORTUPSTREAM
+				then
+					new_parents="$new_parents $SHORTONTO"
+				else
+					new_parents="$new_parents $p"
+				fi
+			done
+			unset p
+			echo merge $commit $new_parents
+			unset new_parents
+			;;
+		*)
+			echo "pick $commit $subject"
+			;;
+		esac
+	done
+	test -n "${last_parent:-}" -a "${last_parent:-}" != $SHORTUPSTREAM && \
+		echo reset $last_parent
+	) | \
+	tac | \
+	while read cmd args
+	do
+		: ${commit_mark_list:=} ${last_commit:=000}
+		case "$cmd" in
+		pick)
+			last_commit="${args%% *}"
+			;;
+		mark)
+			: ${next_mark:=0}
+			if commit_mark_list=$(insert_sha1_with_mark_in_list \
+				$last_commit $next_mark "$commit_mark_list")
+			then
+				args="#$next_mark"
+				next_mark=$(($next_mark + 1))
+			else
+				die "Internal error: two marks for the same commit"
+			fi
+			;;
+		reset)
+			if tmp=$(sha1_to_mark $args "$commit_mark_list")
+			then
+				args="#$tmp"
+			fi
+			;;
+		merge)
+			new_args=
+			for i in ${args#* }
+			do
+				if tmp=$(sha1_to_mark $i "$commit_mark_list")
+				then
+					new_args="$new_args #$tmp"
+				else
+					new_args="$new_args $i"
+				fi
+			done
+			last_commit="${args%% *}"
+			args="$last_commit ${new_args# }"
+			;;
+		esac
+		echo "$cmd $args"
+	done
+}
+
 while test $# != 0
 do
 	case "$1" in
@@ -580,33 +635,24 @@ do
 		echo $ONTO > "$DOTEST"/onto
 		test -z "$STRATEGY" || echo "$STRATEGY" > "$DOTEST"/strategy
 		test t = "$VERBOSE" && : > "$DOTEST"/verbose
-		if test t = "$PRESERVE_MERGES"
-		then
-			# $REWRITTEN contains files for each commit that is
-			# reachable by at least one merge base of $HEAD and
-			# $UPSTREAM. They are not necessarily rewritten, but
-			# their children might be.
-			# This ensures that commits on merged, but otherwise
-			# unrelated side branches are left alone. (Think "X"
-			# in the man page's example.)
-			mkdir "$REWRITTEN" &&
-			for c in $(git merge-base --all $HEAD $UPSTREAM)
-			do
-				echo $ONTO > "$REWRITTEN"/$c ||
-					die "Could not init rewritten commits"
-			done
-			MERGES_OPTION=
-		else
-			MERGES_OPTION=--no-merges
-		fi
 
 		SHORTUPSTREAM=$(git rev-parse --short=7 $UPSTREAM)
 		SHORTHEAD=$(git rev-parse --short=7 $HEAD)
 		SHORTONTO=$(git rev-parse --short=7 $ONTO)
-		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
-			--abbrev=7 --reverse --left-right --cherry-pick \
-			$UPSTREAM...$HEAD | \
-			sed -n "s/^>/pick /p" > "$TODO"
+		common_rev_parse_opts="--abbrev-commit
+			--abbrev=7 --left-right --cherry-pick
+			$UPSTREAM...$HEAD"
+		if test t = "$PRESERVE_MERGES"
+		then
+			git rev-list --pretty='format:%h_%p_%s' \
+				--topo-order $common_rev_parse_opts | \
+				grep -v ^commit | \
+				create_extended_todo_list
+		else
+			git rev-list --no-merges --reverse --pretty=oneline \
+				$common_rev_parse_opts | sed -n "s/^>/pick /p"
+		fi > "$TODO"
+
 		cat >> "$TODO" << EOF
 
 # Rebase $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index be26a78..af7d818 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -245,7 +245,41 @@ test_expect_success 'preserve merges with -p' '
 	test $(git show HEAD~2:file1) = B
 '
 
+test_expect_success 'rebase with preserve merge forth and back is a noop' '
+	git checkout -b big-branch-1 master &&
+	: > bb1a &&
+	git add bb1a &&
+	git commit -m "big branch commit 1" &&
+	: > bb1b &&
+	git add bb1b &&
+	git commit -m "big branch commit 2" &&
+	: > bb1c &&
+	git add bb1c &&
+	git commit -m "big branch commit 3" &&
+	git checkout -b big-branch-2 master &&
+	: > bb2a &&
+	git add bb2a &&
+	git commit -m "big branch commit 4" &&
+	: > bb2b &&
+	git add bb2b &&
+	git commit -m "big branch commit 5" &&
+	git merge big-branch-1~1 &&
+	git merge to-be-preserved &&
+	tbp_merge=$(git rev-parse HEAD) &&
+	: > bb2c &&
+	git add bb2c &&
+	git commit -m "big branch commit 6" &&
+	git merge big-branch-1 &&
+	head=$(git rev-parse HEAD) &&
+	FAKE_LINES="16 6 19 20 4 1 2 5 22" \
+		git rebase -i -p --onto dead-end master &&
+	FAKE_LINES="3 7 mark#10 8 9 5 1 2 merge$tbp_merge~1/#10 merge$tbp_merge/to-be-preserved 6 11" \
+	git rebase -i -p --onto master dead-end &&
+	test "$head" = "$(git rev-parse HEAD)"
+'
+
 test_expect_success '--continue tries to commit' '
+	git checkout to-be-rebased &&
 	test_tick &&
 	! git rebase -i --onto new-branch1 HEAD^ &&
 	echo resolved > file1 &&
-- 
1.5.4.5

Re: [PATCH/RFC 06/10] Unify the lenght of $SHORT* and the commits in the TODO list

[Junio C Hamano]

Jörg Sommer <joerg@alea.gnuu.de> writes:

> ---
>  git-rebase--interactive.sh |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 1698c3e..060b40f 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -600,9 +600,9 @@ do
>  			MERGES_OPTION=--no-merges
>  		fi
>  
> -		SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
> -		SHORTHEAD=$(git rev-parse --short $HEAD)
> -		SHORTONTO=$(git rev-parse --short $ONTO)
> +		SHORTUPSTREAM=$(git rev-parse --short=7 $UPSTREAM)
> +		SHORTHEAD=$(git rev-parse --short=7 $HEAD)
> +		SHORTONTO=$(git rev-parse --short=7 $ONTO)

Lacking is a better justification as to why this is a good change and 7 is
a good number.

Re: [PATCH/RFC 06/10] Unify the lenght of $SHORT* and the commits in the TODO list

[Jörg Sommer]

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

Hi Junio,

Junio C Hamano schrieb am Fri 11. Apr, 17:00 (-0700):
> Jörg Sommer <joerg@alea.gnuu.de> writes:
> 
> > ---
> >  git-rebase--interactive.sh |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > index 1698c3e..060b40f 100755
> > --- a/git-rebase--interactive.sh
> > +++ b/git-rebase--interactive.sh
> > @@ -600,9 +600,9 @@ do
> >  			MERGES_OPTION=--no-merges
> >  		fi
> >  
> > -		SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
> > -		SHORTHEAD=$(git rev-parse --short $HEAD)
> > -		SHORTONTO=$(git rev-parse --short $ONTO)
> > +		SHORTUPSTREAM=$(git rev-parse --short=7 $UPSTREAM)
> > +		SHORTHEAD=$(git rev-parse --short=7 $HEAD)
> > +		SHORTONTO=$(git rev-parse --short=7 $ONTO)
> 
> Lacking is a better justification as to why this is a good change

“This makes it easier to test for equality of a commit in the TODO list
and one of SHORTUPSTREAM, SHORTHEAD or SHORTONTO.”

> and 7 is a good number.

I don't know why 7 is a good number. It was there from the beginning:

% git show 1b1dce4b:git-rebase--interactive.sh G -C1 \=7
                git rev-list --no-merges --pretty=oneline --abbrev-commit \
                        --abbrev=7 --reverse $UPSTREAM..$HEAD | \
                        sed "s/^/pick /" >> "$TODO"

Bye, Jörg.
-- 
Damit das Mögliche entsteht, muß immer wieder das Unmögliche versucht
werden.                                       (Hermann Hesse)

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH/RFC 06/10] Unify the lenght of $SHORT* and the commits in the TODO list

[Junio C Hamano]

Jörg Sommer <joerg@alea.gnuu.de> writes:

> “This makes it easier to test for equality of a commit in the TODO list
> and one of SHORTUPSTREAM, SHORTHEAD or SHORTONTO.”

"Equality testing?" --- that makes me worried.  short=7 does not chomp
them at 7 but only tells rev-parse to use at least 7.  You may get 8 or
more if there are other objects that share the same prefix when you get
them.

Perhaps by forcing "at least 7" everywhere you are getting consistent
result that makes them easier to compare.

But considering that this is a candidate for a general mechanism to
eventual grow into the git-sequencer, and that we expect to have richer,
smarter, and/or more complex set of tools that feeds you the TODO list,
I'd feel safer if the internal comparison used to determine which one
commit the user meant in his TODO file is robust and does not rely on
where the abbreviated object name was chomped at.

Some of the tools may even do not show the raw TODO list in vi to the user
but instead present the list in a GUI, and you may get fed the full
40-hexdigit object name in the underlying TODO list they generate.

Re: [PATCH/RFC 06/10] Unify the lenght of $SHORT* and the commits in the TODO list

[Jörg Sommer]

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

Hallo,

Junio C Hamano schrieb am Sat 12. Apr, 23:20 (-0700):
> Jörg Sommer <joerg@alea.gnuu.de> writes:
> 
> > “This makes it easier to test for equality of a commit in the TODO list
> > and one of SHORTUPSTREAM, SHORTHEAD or SHORTONTO.”
> 
> "Equality testing?" --- that makes me worried.  short=7 does not chomp
> them at 7 but only tells rev-parse to use at least 7.  You may get 8 or
> more if there are other objects that share the same prefix when you get
> them.
> 
> Perhaps by forcing "at least 7" everywhere you are getting consistent
> result that makes them easier to compare.

That's what I want.

> But considering that this is a candidate for a general mechanism to
> eventual grow into the git-sequencer, and that we expect to have richer,
> smarter, and/or more complex set of tools that feeds you the TODO list,
> I'd feel safer if the internal comparison used to determine which one
> commit the user meant in his TODO file is robust and does not rely on
> where the abbreviated object name was chomped at.

I use it one time while building the TODO list given to the editor. I've
to replace $UPSTREAM with $ONTO everywhere $UPSTREAM occures.

Do you still have a bad feeling? I can replace all tests by

  test "$(git rev-parse $a)" = "$(git rev-parse $b)"

Bye, Jörg.
-- 
Was man mühelos erreichen kann, ist gewöhnlich nicht der Mühe wert,
erreicht zu werden.

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH/RFC 06/10] Unify the lenght of $SHORT* and the commits in the TODO list

[Tarmigan]

On Sat, Apr 12, 2008 at 11:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
>  "Equality testing?" --- that makes me worried.  short=7 does not chomp
>  them at 7 but only tells rev-parse to use at least 7.  You may get 8 or
>  more if there are other objects that share the same prefix when you get
>  them.
>
>  Perhaps by forcing "at least 7" everywhere you are getting consistent
>  result that makes them easier to compare.
>
>  But considering that this is a candidate for a general mechanism to
>  eventual grow into the git-sequencer, and that we expect to have richer,
>  smarter, and/or more complex set of tools that feeds you the TODO list,
>  I'd feel safer if the internal comparison used to determine which one
>  commit the user meant in his TODO file is robust and does not rely on
>  where the abbreviated object name was chomped at.

Slightly offtopic, but has there ever been any discussion about the
scenario that the during a rebase operation, a new object might be
created that has the same first 7 abbreviation as another "pick" that
comes later?  It's unlikely, but it might get more likely as rebasing
grows in complexity and number of new objects created.

Thanks,
Tarmigan

Re: [PATCH/RFC 02/10] Teach rebase interactive the reset command

[Junio C Hamano]

Jörg Sommer <joerg@alea.gnuu.de> writes:

> ---
>  git-rebase--interactive.sh    |   20 ++++++++++++++++++++
>  t/t3404-rebase-interactive.sh |   10 ++++++++++
>  2 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index b001ddf..7dac51b 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -258,6 +258,16 @@ parse_mark() {
>  	return 1
>  }
>  
> +mark_to_sha1() {
> +	local mark

Bashism.

> +	if mark=$(parse_mark $1)
> +	then
> +		cat "$MARKS"/$mark

This cat may become "rev-parse --verify" if we decide to move $MARKS
underneath refs/ namespace somewhere.

> +	reset|r)
> +		comment_for_reflog reset
> +
> +		mark_action_done
> +		tmp=$(mark_to_sha1 $sha1) || \
> +			tmp=$(git rev-parse --verify $sha1) ||

and you then would not need to do the verify twice.

> +			die "Invalid parent '$sha1' in $command $sha1 $rest"
> +		output git reset --hard $tmp

Could this step fail, and if it does what should happen?

> @@ -569,6 +588,7 @@ do
>  #  edit = use commit, but stop for amending
>  #  squash = use commit, but meld into previous commit
>  #  mark #NUM = mark the current HEAD for later reference
> +#  reset #NUM|commit = reset HEAD to a previous set mark or a commit

"to a previously set mark".  But I would say upfront "in the todo insn
whenever you need to refer to a commit, in addition to
the usual commit object name, you can use '#num' syntax to refer to a
commit previously marked with the 'mark' insn." and make this line just
read:

    #  reset commit = reset HEAD to the commit

Re: [PATCH/RFC 02/10] Teach rebase interactive the reset command

[Jörg Sommer]

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

Hi Junio,

Junio C Hamano schrieb am Fri 11. Apr, 16:56 (-0700):
> Jörg Sommer <joerg@alea.gnuu.de> writes:
> 
> > ---
> >  git-rebase--interactive.sh    |   20 ++++++++++++++++++++
> >  t/t3404-rebase-interactive.sh |   10 ++++++++++
> >  2 files changed, 30 insertions(+), 0 deletions(-)
> >
> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > index b001ddf..7dac51b 100755
> > --- a/git-rebase--interactive.sh
> > +++ b/git-rebase--interactive.sh

> > +			die "Invalid parent '$sha1' in $command $sha1 $rest"
> > +		output git reset --hard $tmp
> 
> Could this step fail, and if it does what should happen?

I don't expect it. tmp is a valid sha1 and reset may fail if the working
directory is dirty, but then the previous command should have failed,
too. Do you think different?

> > @@ -569,6 +588,7 @@ do
> >  #  edit = use commit, but stop for amending
> >  #  squash = use commit, but meld into previous commit
> >  #  mark #NUM = mark the current HEAD for later reference
> > +#  reset #NUM|commit = reset HEAD to a previous set mark or a commit
> 
> "to a previously set mark".  But I would say upfront "in the todo insn
> whenever you need to refer to a commit, in addition to
> the usual commit object name, you can use '#num' syntax to refer to a
> commit previously marked with the 'mark' insn."

Does this mean pick, edit and squash should understand marks, too? But
how useful is this? You can only set a mark if you've picked a commit and
using this commit again, e.g. pick it twice, doesn't sound useful.

-- 
$ cat /dev/random
#!/usr/bin/perl -WT
print "hello world\n";

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Mike Ralphson]

On 10/04/2008, Jörg Sommer <joerg@alea.gnuu.de> wrote:
> This new command can be used to set symbolic marks for an commit while
>  doing a rebase. This symbolic name can later be used for merges or
>  resets.

What would be wrong with using the existing tag machinery for this instead?

Mike

PS: Apologies to anyone who got an html version of this mail.

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Jörg Sommer]

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

Hallo Mike,

Mike Ralphson schrieb am Thu 10. Apr, 10:33 (+0100):
> On 10/04/2008, Jörg Sommer <joerg@alea.gnuu.de> wrote:
> > This new command can be used to set symbolic marks for an commit while
> >  doing a rebase. This symbolic name can later be used for merges or
> >  resets.
> 
> What would be wrong with using the existing tag machinery for this instead?

You may have to deal with conflicts if users named tags 03 or 10. But
Junio suggested to use a ref, too. I think refs/rebase-marks/ is a good
prefix.

Bye, Jörg.
-- 
Der Hase läuft schneller als der Fuchs,
denn der Hase läuft um sein Leben.

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Junio C Hamano]

Jörg Sommer <joerg@alea.gnuu.de> writes:

> This new command can be used to set symbolic marks for an commit while
> doing a rebase. This symbolic name can later be used for merges or
> resets.
> ---

Comments as requested...

> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 8aa7371..b001ddf 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -23,6 +23,7 @@ DONE="$DOTEST"/done
>  MSG="$DOTEST"/message
>  SQUASH_MSG="$DOTEST"/message-squash
>  REWRITTEN="$DOTEST"/rewritten
> +MARKS="$DOTEST"/marks

I wonder if this should live somewhere inside $GIT_DIR/refs namespace, so
that if any object pruning is triggered ever while rebasing interactively
the objects that marks require will be protected.

> @@ -240,6 +241,23 @@ peek_next_command () {
>  	sed -n "1s/ .*$//p" < "$TODO"
>  }
>  
> +parse_mark() {
> +	local tmp

Bashism is not appreciated here.

> +	case "$tmp" in
> +	'#'[0-9]*)
> +		tmp="${tmp#\#}"
> +		if test "$tmp" = $(printf %d "$tmp")
> +		then
> +			echo $tmp
> +			return 0
> +		fi
> +		;;
> +	esac

Wouldn't

	pick 5cc8f37 (init: show "Reinit" message even in ...)
	mark 1
	pick 18d077c (quiltimport: fix misquoting of parse...)
	mark 2
	reset 1
        merge #2

be easier for people?

When you reference a commit with mark name, it is reasonable to require it
to be prefixed with '#', which is a character that cannot be either in
refname nor hex representation of a commit object.  I think it is Ok to
accept an optional prefix when defining one, e.g. "mark #47", but I do not
think requiring '#' prefix when defining one is needed.

> @@ -317,6 +335,23 @@ do_next () {
>  			die_with_patch $sha1 ""
>  		fi
>  		;;
> +	mark|a)

I understand that the reason why you did not pick a more obvious 'm' is
because you would want to add 'merge' later and give it 'm', but we do not
have to give a single-letter equivalent to all commands especially when
there is not an appropriate one.

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Jörg Sommer]

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

Hi Junio,

Junio C Hamano schrieb am Fri 11. Apr, 16:48 (-0700):
> Jörg Sommer <joerg@alea.gnuu.de> writes:
> 
> > This new command can be used to set symbolic marks for an commit while
> > doing a rebase. This symbolic name can later be used for merges or
> > resets.
> > ---
> 
> Comments as requested...

Thanks.

> > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> > index 8aa7371..b001ddf 100755
> > --- a/git-rebase--interactive.sh
> > +++ b/git-rebase--interactive.sh
> > @@ -23,6 +23,7 @@ DONE="$DOTEST"/done
> >  MSG="$DOTEST"/message
> >  SQUASH_MSG="$DOTEST"/message-squash
> >  REWRITTEN="$DOTEST"/rewritten
> > +MARKS="$DOTEST"/marks
> 
> I wonder if this should live somewhere inside $GIT_DIR/refs namespace, so
> that if any object pruning is triggered ever while rebasing interactively
> the objects that marks require will be protected.

Why wasn't the rewritten directory underneath refs/?

> > @@ -240,6 +241,23 @@ peek_next_command () {
> >  	sed -n "1s/ .*$//p" < "$TODO"
> >  }
> >  
> > +parse_mark() {
> > +	local tmp
> 
> Bashism is not appreciated here.

“IEEE P1003.2 Draft 11.2 - September 1991”, page 277:

 Local variables within a function were considered and included in Draft
 9 (controlled by the special built-in local), but were removed because
 they do not fit the simple model developed for the scoping of functions
 and there was some opposition to adding yet another new special built-in
 from outside existing practice.  Implementations should reserve the
 identifier local (as well as typeset, as used in the KornShell) in case
 this local variable mechanism is adopted in a future version of POSIX.2.

… but I didn't found it in “IEEE Std 1003.1, 2004 Edition”.

> > +	case "$tmp" in
> > +	'#'[0-9]*)
> > +		tmp="${tmp#\#}"
> > +		if test "$tmp" = $(printf %d "$tmp")
> > +		then
> > +			echo $tmp
> > +			return 0
> > +		fi
> > +		;;
> > +	esac
> 
> Wouldn't
> 
> 	pick 5cc8f37 (init: show "Reinit" message even in ...)
> 	mark 1
> 	pick 18d077c (quiltimport: fix misquoting of parse...)
> 	mark 2
> 	reset 1

“reset 18d077c~2” or “reset some-tag” or “reset my-branch~12”

>         merge #2
> 
> be easier for people?

I don't know. Using the special sign everywhere a mark is used looks more
consistent to me. The only case where it might be omitted is the mark
command, because it only uses marks.

> When you reference a commit with mark name, it is reasonable to require it
> to be prefixed with '#', which is a character that cannot be either in
> refname nor hex representation of a commit object.  I think it is Ok to
> accept an optional prefix when defining one, e.g. "mark #47", but I do not
> think requiring '#' prefix when defining one is needed.

That sounds useful.

BTW: Should the mark be a number or a string, e.g. 001 == 1 or 001 != 1?

> > @@ -317,6 +335,23 @@ do_next () {
> >  			die_with_patch $sha1 ""
> >  		fi
> >  		;;
> > +	mark|a)
> 
> I understand that the reason why you did not pick a more obvious 'm' is
> because you would want to add 'merge' later and give it 'm', but we do not
> have to give a single-letter equivalent to all commands especially when
> there is not an appropriate one.

That's fine. I thought it's a requirement.

Bye, Jörg.
-- 
Was der Bauer nicht kennt, das frisst er nicht. Würde der Städter kennen,
was er frisst, er würde umgehend Bauer werden.
                                                       Oliver Hassencamp

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Shawn O. Pearce]

Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > Wouldn't
> > 
> > 	pick 5cc8f37 (init: show "Reinit" message even in ...)
> > 	mark 1
> > 	pick 18d077c (quiltimport: fix misquoting of parse...)
> > 	mark 2
> > 	reset 1
> 
> “reset 18d077c~2” or “reset some-tag” or “reset my-branch~12”
> 
> >         merge #2
> > 
> > be easier for people?
> 
> I don't know. Using the special sign everywhere a mark is used looks more
> consistent to me. The only case where it might be omitted is the mark
> command, because it only uses marks.

Why not use the mark syntax that fast-import uses?  In fast-import
we use ":n" anytime we need to refer to a mark, e.g. ":1" or ":5".
Its the same idea.  We already have a language for it.  Heck, the
commands above are bordering on a language not too far from the
one that fast-import accepts.  :-)

-- 
Shawn.

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Jörg Sommer]

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

Hi Shawn,

Shawn O. Pearce schrieb am Sat 12. Apr, 23:56 (-0400):
> Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > > Wouldn't
> > > 
> > > 	pick 5cc8f37 (init: show "Reinit" message even in ...)
> > > 	mark 1
> > > 	pick 18d077c (quiltimport: fix misquoting of parse...)
> > > 	mark 2
> > > 	reset 1
> > 
> > “reset 18d077c~2” or “reset some-tag” or “reset my-branch~12”
> > 
> > >         merge #2
> > > 
> > > be easier for people?
> > 
> > I don't know. Using the special sign everywhere a mark is used looks more
> > consistent to me. The only case where it might be omitted is the mark
> > command, because it only uses marks.
> 
> Why not use the mark syntax that fast-import uses?

I didn't know it.

> In fast-import we use ":n" anytime we need to refer to a mark, e.g.
> ":1" or ":5".

Currently, I don't restrict the mark to be a number. It can anything that
is a valid ref. Should I restrict it?

And how do you handle the :/ syntax? “reset :/Bla” is than not valid.
Mmm, I'll add an exception for :/.

Except of this, I prefer to use the colon to be much closer to the syntax
of fast-import.

Bye, Jörg.
-- 
Der Wunsch, klug zu erscheinen, verhindert oft, es zu werden.
    	    	    			      (Francois de la Rochefoucauld)

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Shawn O. Pearce]

Jrg Sommer <joerg@alea.gnuu.de> wrote:
> Shawn O. Pearce schrieb am Sat 12. Apr, 23:56 (-0400):
> > 
> > Why not use the mark syntax that fast-import uses?
> 
> I didn't know it.
> 
> > In fast-import we use ":n" anytime we need to refer to a mark, e.g.
> > ":1" or ":5".
> 
> Currently, I don't restrict the mark to be a number. It can anything that
> is a valid ref. Should I restrict it?

In fast-import a mark can *only* be a number.  It cannot be a ref
string or anything complex like that.  This reduces the memory load
of fast-import, but does cause a burden on the import frontend.
 
> And how do you handle the :/ syntax? “reset :/Bla” is than not valid.
> Mmm, I'll add an exception for :/.

I think the ':/' syntax came along after fast-import had already started
to use ':' as the mark syntax.  I forgot to object to this bastard form
of looking up a commit when it was introduced by Dscho and now we have
a SHA-1 expression syntax that fast-import will confuse with a mark.  I
originally had chosen to start a mark off with ':' because it is not an
allowed character in a ref, due to its use to split src:dst in a fetch
or push refspec.
 
> Except of this, I prefer to use the colon to be much closer to the syntax
> of fast-import.

Me too, but it looks like in a human edited "TODO" script we may want
to be more friendly and allow named marks.  Though I'm not sure that is
really all that useful.  If you are merging something because it used to
be merged before the rebase I doubt we'd generate a meaningful mark name
when the TODO script is initially formatted.

-- 
Shawn.

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Junio C Hamano]

"Shawn O. Pearce" <spearce@spearce.org> writes:

>> Except of this, I prefer to use the colon to be much closer to the syntax
>> of fast-import.
>
> Me too, but it looks like in a human edited "TODO" script we may want
> to be more friendly and allow named marks.  Though I'm not sure that is
> really all that useful.  If you are merging something because it used to
> be merged before the rebase I doubt we'd generate a meaningful mark name
> when the TODO script is initially formatted.

I'd say a small integer is the only thing we would need.  The TODO insn
sequence would be machine generated then manually tweaked, not the other
way around.

Regarding colon vs pound, I would say that the 'mark' insn can just say
"2" to set the second mark, and then store it in refs/marks/2; the
commands that refer to the commit later can use the usual "refs/marks/2"
or "marks/2" syntax without colon nor pound nor any other special syntax
that way.

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Jörg Sommer]

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

Hi Shawn,

Shawn O. Pearce schrieb am Mon 14. Apr, 02:24 (-0400):
> Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > Shawn O. Pearce schrieb am Sat 12. Apr, 23:56 (-0400):
> > > 
> > > Why not use the mark syntax that fast-import uses?
> > 
> > I didn't know it.
> > 
> > > In fast-import we use ":n" anytime we need to refer to a mark, e.g.
> > > ":1" or ":5".
> > 
> > Currently, I don't restrict the mark to be a number. It can anything that
> > is a valid ref. Should I restrict it?
> 
> In fast-import a mark can *only* be a number.

Only for the record: fast-import uses numbers and not strings of
digits, i.e. 001 == 1, and it ignores stuff following digits, i.e.
"12a" == 12 and "abc" == 0.

> > Except of this, I prefer to use the colon to be much closer to the syntax
> > of fast-import.
> 
> Me too, but it looks like in a human edited "TODO" script we may want
> to be more friendly and allow named marks.

I don't think so. There shouldn't be so much marks that a user can't
remember them.

Thanks for your comments.

Jörg.
-- 
The UNIX Guru's View of Sex:
# unzip ; strip ; touch ; finger ; mount ; fsck ; more ; yes ; umount ; sleep

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[PATCH v2 01/13] fake-editor: output TODO list if unchanged

[Jörg Sommer]

It's much helpful to see the TODO list generated by rebase in the verbose
output of the test. This makes it easier to check, if the list was not
broken from the beginning.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 t/t3404-rebase-interactive.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9cf873f..8d29878 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -73,7 +73,7 @@ esac
 test -z "$EXPECT_COUNT" ||
 	test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) ||
 	exit
-test -z "$FAKE_LINES" && exit
+test -z "$FAKE_LINES" && { grep -v '^#' "$1"; exit; }
 grep -v '^#' < "$1" > "$1".tmp
 rm -f "$1"
 cat "$1".tmp
-- 
1.5.5

[PATCH v2 02/13] Don't append default merge message to -m message

[Jörg Sommer]

Picked from <7vabko3dm2.fsf@gitster.siamese.dyndns.org>
From: gitster@pobox.com (Junio C Hamano)
Date: Sun, 23 Mar 2008 22:17:09 -0700

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-merge.sh |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/git-merge.sh b/git-merge.sh
index 7dbbb1d..bd9699d 100755
--- a/git-merge.sh
+++ b/git-merge.sh
@@ -250,17 +250,19 @@ else
 	# We are invoked directly as the first-class UI.
 	head_arg=HEAD
 
-	# All the rest are the commits being merged; prepare
-	# the standard merge summary message to be appended to
-	# the given message.  If remote is invalid we will die
-	# later in the common codepath so we discard the error
-	# in this loop.
-	merge_name=$(for remote
-		do
-			merge_name "$remote"
-		done | git fmt-merge-msg
-	)
-	merge_msg="${merge_msg:+$merge_msg$LF$LF}$merge_name"
+	if test -z "$merge_msg"
+	then
+		# All the rest are the commits being merged; prepare
+		# the standard merge summary message to be appended to
+		# the given message.  If remote is invalid we will die
+		# later in the common codepath so we discard the error
+		# in this loop.
+		merge_msg=$(for remote
+			do
+				merge_name "$remote"
+			done | git fmt-merge-msg
+		)
+	fi
 fi
 head=$(git rev-parse --verify "$head_arg"^0) || usage
 
-- 
1.5.5

[PATCH v2 03/13] Move cleanup code into it's own function

[Jörg Sommer]

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 8aa7371..531ee94 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -104,8 +104,12 @@ die_with_patch () {
 	die "$2"
 }
 
-die_abort () {
+cleanup_before_quit () {
 	rm -rf "$DOTEST"
+}
+
+die_abort () {
+	cleanup_before_quit
 	die "$1"
 }
 
@@ -352,7 +356,7 @@ do_next () {
 		test ! -f "$DOTEST"/verbose ||
 			git diff-tree --stat $(cat "$DOTEST"/head)..HEAD
 	} &&
-	rm -rf "$DOTEST" &&
+	cleanup_before_quit &&
 	git gc --auto &&
 	warn "Successfully rebased and updated $HEADNAME."
 
@@ -414,7 +418,7 @@ do
 			;;
 		esac &&
 		output git reset --hard $HEAD &&
-		rm -rf "$DOTEST"
+		cleanup_before_quit
 		exit
 		;;
 	--skip)
-- 
1.5.5

[PATCH v2 04/13] Teach rebase interactive the mark command

[Jörg Sommer]

This new command can be used to set symbolic marks for an commit while
doing a rebase. This symbolic name can later be used for merges or
resets.

The decision to use references for the marks and not files like done with
the rewritten commits for preserve merges was made to ensure no commit
objects get lost if prune is started while (a long term) rebase is
running. This also unifies the checking of the validity of marks and
references by using rev-parse for it.

The usage of : as the sign for marks conforms with the tag sign of
fast-export and fast-import.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh    |   37 ++++++++++++++++++++++++++++++++++++-
 t/t3404-rebase-interactive.sh |   17 +++++++++++++++++
 2 files changed, 53 insertions(+), 1 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 531ee94..6ac316a 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -35,6 +35,8 @@ mark the corrected paths with 'git add <paths>', and
 run 'git rebase --continue'"
 export GIT_CHERRY_PICK_HELP
 
+mark_prefix=refs/rebase-marks/
+
 warn () {
 	echo "$*" >&2
 }
@@ -105,7 +107,13 @@ die_with_patch () {
 }
 
 cleanup_before_quit () {
-	rm -rf "$DOTEST"
+	rm -rf "$DOTEST" &&
+	for ref in "$GIT_DIR/$mark_prefix"*
+	do
+		test "$ref" = "$GIT_DIR/$mark_prefix*" && continue
+		git update-ref -d "${ref#$GIT_DIR/}" "${ref#$GIT_DIR/}" || \
+			return 1
+	done
 }
 
 die_abort () {
@@ -244,6 +252,19 @@ peek_next_command () {
 	sed -n "1s/ .*$//p" < "$TODO"
 }
 
+mark_to_ref () {
+	case "$1" in
+	:[!/]*)
+		# :/SOMETHING is a reference for the last commit whose
+                # message starts with SOMETHING
+		echo "$mark_prefix${1#:}"
+		;;
+	*)
+		echo "$1"
+		;;
+	esac
+}
+
 do_next () {
 	rm -f "$DOTEST"/message "$DOTEST"/author-script \
 		"$DOTEST"/amend || exit
@@ -321,6 +342,15 @@ do_next () {
 			die_with_patch $sha1 ""
 		fi
 		;;
+	mark)
+		mark_action_done
+
+		mark=$(mark_to_ref :${sha1#:})
+		git rev-parse --verify "$mark" > /dev/null 2>&1 && \
+			warn "mark $sha1 already exist; overwriting it"
+
+		git update-ref "$mark" HEAD || die "update-ref failed"
+		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
 		die_with_patch $sha1 "Please fix this in the file $TODO."
@@ -533,10 +563,15 @@ do
 
 # Rebase $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO
 #
+# In the todo insn whenever you need to refer to a commit, in addition
+# to the usual commit object name, you can use ':mark' syntax to refer
+# to a commit previously marked with the 'mark' insn.
+#
 # Commands:
 #  pick = use commit
 #  edit = use commit, but stop for amending
 #  squash = use commit, but meld into previous commit
+#  mark :mark = mark the current HEAD for later reference
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8d29878..fa3560e 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -82,6 +82,9 @@ for line in $FAKE_LINES; do
 	case $line in
 	squash|edit)
 		action="$line";;
+	mark*)
+		echo "mark ${line#mark}"
+		echo "mark ${line#mark}" >> "$1";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
 		sed -n "${line}p" < "$1".tmp
@@ -189,6 +192,20 @@ test_expect_success '-p handles "no changes" gracefully' '
 	test $HEAD = $(git rev-parse HEAD)
 '
 
+test_expect_success 'setting marks works' '
+	git checkout master &&
+	FAKE_LINES="mark:0 2 1 mark:42 3 edit 4" git rebase -i HEAD~4 &&
+	marks_dir=.git/refs/rebase-marks &&
+	test -d $marks_dir &&
+	test $(ls $marks_dir | wc -l) -eq 2 &&
+	test "$(git rev-parse HEAD~4)" = \
+		"$(git rev-parse refs/rebase-marks/0)" &&
+	test "$(git rev-parse HEAD~2)" = \
+		"$(git rev-parse refs/rebase-marks/42)" &&
+	git rebase --abort &&
+	ls $marks_dir | wc -l | grep -Fx 0
+'
+
 test_expect_success 'preserve merges with -p' '
 	git checkout -b to-be-preserved master^ &&
 	: > unrelated-file &&
-- 
1.5.5

[PATCH v2 05/13] Teach rebase interactive the reset command

[Jörg Sommer]

This command does a hard reset of the HEAD, i.e. the next operation used
this commit as parent. This is necessary to redo the commits of different
branches they become merged later.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh    |   10 ++++++++++
 t/t3404-rebase-interactive.sh |   17 +++++++++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 6ac316a..a4b7aad 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -351,6 +351,15 @@ do_next () {
 
 		git update-ref "$mark" HEAD || die "update-ref failed"
 		;;
+	reset|r)
+		comment_for_reflog reset
+
+		tmp=$(git rev-parse --verify "$(mark_to_ref $sha1)") ||
+			die "Invalid parent '$sha1' in $command $sha1 $rest"
+
+		mark_action_done
+		output git reset --hard $tmp
+		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
 		die_with_patch $sha1 "Please fix this in the file $TODO."
@@ -572,6 +581,7 @@ do
 #  edit = use commit, but stop for amending
 #  squash = use commit, but meld into previous commit
 #  mark :mark = mark the current HEAD for later reference
+#  reset commit = reset HEAD to the commit
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index fa3560e..78673a6 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -85,6 +85,9 @@ for line in $FAKE_LINES; do
 	mark*)
 		echo "mark ${line#mark}"
 		echo "mark ${line#mark}" >> "$1";;
+	reset*)
+		echo "reset ${line#reset}"
+		echo "reset ${line#reset}" >> "$1";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
 		sed -n "${line}p" < "$1".tmp
@@ -206,6 +209,20 @@ test_expect_success 'setting marks works' '
 	ls $marks_dir | wc -l | grep -Fx 0
 '
 
+test_expect_success 'reset with nonexistent mark fails' '
+	export FAKE_LINES="reset:0 1" &&
+	test_must_fail git rebase -i HEAD~1 &&
+	unset FAKE_LINES &&
+	git rebase --abort
+'
+
+test_expect_success 'reset to HEAD is a nop' '
+	test_tick &&
+	head=$(git rev-parse --short HEAD) &&
+	FAKE_LINES="reset$head" git rebase -i HEAD~4 &&
+	test "$(git rev-parse --short HEAD)" = "$head"
+'
+
 test_expect_success 'preserve merges with -p' '
 	git checkout -b to-be-preserved master^ &&
 	: > unrelated-file &&
-- 
1.5.5

[PATCH v2 06/13] Move redo merge code in a function

[Jörg Sommer]

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh |   35 ++++++++++++++++++++---------------
 1 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index a4b7aad..19145b1 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -125,6 +125,25 @@ has_action () {
 	grep '^[^#]' "$1" >/dev/null
 }
 
+redo_merge () {
+	rm_sha1=$1
+	shift
+
+	eval "$(get_author_ident_from_commit $rm_sha1)"
+	msg="$(git cat-file commit $rm_sha1 | sed -e '1,/^$/d')"
+
+	if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
+		GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
+		GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
+		output git merge $STRATEGY -m "$msg" "$@"
+	then
+		git rerere
+		printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG
+		die Error redoing merge $rm_sha1
+	fi
+	unset rm_sha1
+}
+
 pick_one () {
 	no_ff=
 	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
@@ -192,22 +211,8 @@ pick_one_preserving_merges () {
 		echo $sha1 > "$DOTEST"/current-commit
 		case "$new_parents" in
 		' '*' '*)
-			# redo merge
-			author_script=$(get_author_ident_from_commit $sha1)
-			eval "$author_script"
-			msg="$(git cat-file commit $sha1 | sed -e '1,/^$/d')"
 			# No point in merging the first parent, that's HEAD
-			new_parents=${new_parents# $first_parent}
-			if ! GIT_AUTHOR_NAME="$GIT_AUTHOR_NAME" \
-				GIT_AUTHOR_EMAIL="$GIT_AUTHOR_EMAIL" \
-				GIT_AUTHOR_DATE="$GIT_AUTHOR_DATE" \
-				output git merge $STRATEGY -m "$msg" \
-					$new_parents
-			then
-				git rerere
-				printf "%s\n" "$msg" > "$GIT_DIR"/MERGE_MSG
-				die Error redoing merge $sha1
-			fi
+			redo_merge $sha1 ${new_parents# $first_parent}
 			;;
 		*)
 			output git cherry-pick "$@" ||
-- 
1.5.5

[PATCH v2 07/13] Teach rebase interactive the merge command

[Jörg Sommer]

This command redoes merges. It's useful if you rebase a branch that
contains merges and you want to preserve these merges. You can also use
it to add new merges.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh    |   24 ++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh |   13 +++++++++++++
 2 files changed, 37 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 19145b1..fd41ca0 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -356,6 +356,28 @@ do_next () {
 
 		git update-ref "$mark" HEAD || die "update-ref failed"
 		;;
+	merge|m)
+		comment_for_reflog merge
+
+		if ! git rev-parse --verify $sha1 > /dev/null
+		then
+			die "Invalid reference merge '$sha1' in"
+					"$command $sha1 $rest"
+		fi
+
+		new_parents=
+		for p in $rest
+		do
+			new_parents="$new_parents $(mark_to_ref $p)"
+		done
+		new_parents="${new_parents# }"
+		test -n "$new_parents" || \
+			die "You forgot to give the parents for the" \
+				"merge $sha1. Please fix it in $TODO"
+
+		mark_action_done
+		redo_merge $sha1 $new_parents
+		;;
 	reset|r)
 		comment_for_reflog reset
 
@@ -587,6 +609,8 @@ do
 #  squash = use commit, but meld into previous commit
 #  mark :mark = mark the current HEAD for later reference
 #  reset commit = reset HEAD to the commit
+#  merge commit-M commit-P ... = redo merge commit-M with the
+#         current HEAD and the parents commit-P
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 78673a6..ceb9d74 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -88,6 +88,9 @@ for line in $FAKE_LINES; do
 	reset*)
 		echo "reset ${line#reset}"
 		echo "reset ${line#reset}" >> "$1";;
+	merge*)
+		echo "merge ${line#merge}" | tr / ' '
+		echo "merge ${line#merge}" | tr / ' ' >> "$1";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
 		sed -n "${line}p" < "$1".tmp
@@ -223,6 +226,16 @@ test_expect_success 'reset to HEAD is a nop' '
 	test "$(git rev-parse --short HEAD)" = "$head"
 '
 
+test_expect_success 'merge redoes merges' '
+	test_tick &&
+	git merge dead-end &&
+	merge=$(git rev-parse HEAD) &&
+	git reset --hard HEAD~1 &&
+	FAKE_LINES="1 merge$merge/dead-end" git rebase -i HEAD~1 &&
+	test $merge = "$(git rev-parse HEAD)" &&
+	git reset --hard HEAD~1
+'
+
 test_expect_success 'preserve merges with -p' '
 	git checkout -b to-be-preserved master^ &&
 	: > unrelated-file &&
-- 
1.5.5

[PATCH v2 08/13] Unify the lenght of $SHORT* and the commits in the TODO list

[Jörg Sommer]

This makes it easier to test for equality of a commit in the TODO list
and one of SHORTUPSTREAM, SHORTHEAD or SHORTONTO.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index fd41ca0..d0a7e5c 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -588,9 +588,9 @@ do
 			MERGES_OPTION=--no-merges
 		fi
 
-		SHORTUPSTREAM=$(git rev-parse --short $UPSTREAM)
-		SHORTHEAD=$(git rev-parse --short $HEAD)
-		SHORTONTO=$(git rev-parse --short $ONTO)
+		SHORTUPSTREAM=$(git rev-parse --short=7 $UPSTREAM)
+		SHORTHEAD=$(git rev-parse --short=7 $HEAD)
+		SHORTONTO=$(git rev-parse --short=7 $ONTO)
 		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
 			--abbrev=7 --reverse --left-right --cherry-pick \
 			$UPSTREAM...$HEAD | \
-- 
1.5.5

[PATCH v2 09/13] Select all lines with fake-editor

[Jörg Sommer]

The old fake-editor selects only lines they start with pick, if you give
the number of a line. With the new commands mark, merge and reset it was
not possible to select such lines for the new TODO list. The new
fake-editor selects all kinds of lines, but replaces only the command
“pick” with a different action.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 t/t3404-rebase-interactive.sh |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index ceb9d74..0a8d065 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -92,9 +92,8 @@ for line in $FAKE_LINES; do
 		echo "merge ${line#merge}" | tr / ' '
 		echo "merge ${line#merge}" | tr / ' ' >> "$1";;
 	*)
-		echo sed -n "${line}s/^pick/$action/p"
-		sed -n "${line}p" < "$1".tmp
-		sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
+		sed -n "${line}{s/^pick/$action/; p;}" < "$1".tmp
+		sed -n "${line}{s/^pick/$action/; p;}" < "$1".tmp >> "$1"
 		action=pick;;
 	esac
 done
-- 
1.5.5

[PATCH v2 10/13] Do rebase with preserve merges with advanced TODO list

[Jörg Sommer]

The current algorithmus used to rebase a branch with merges on top of
another has some drawbacks: it's not possible to squash commits, it's not
possible to change the order of commits, particularly the tip of the
branch can't change.

This new algorithmus uses the idea from Junio to create a TODO list with
the commands mark, merge and reset to represent the nonlinear structure
of merges.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh    |  239 ++++++++++++++++++++++++-----------------
 t/t3404-rebase-interactive.sh |   37 +++++++
 2 files changed, 175 insertions(+), 101 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d0a7e5c..d3327a8 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -22,11 +22,9 @@ TODO="$DOTEST"/git-rebase-todo
 DONE="$DOTEST"/done
 MSG="$DOTEST"/message
 SQUASH_MSG="$DOTEST"/message-squash
-REWRITTEN="$DOTEST"/rewritten
 PRESERVE_MERGES=
 STRATEGY=
 VERBOSE=
-test -d "$REWRITTEN" && PRESERVE_MERGES=t
 test -f "$DOTEST"/strategy && STRATEGY="$(cat "$DOTEST"/strategy)"
 test -f "$DOTEST"/verbose && VERBOSE=t
 
@@ -148,8 +146,6 @@ pick_one () {
 	no_ff=
 	case "$1" in -n) sha1=$2; no_ff=t ;; *) sha1=$1 ;; esac
 	output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
-	test -d "$REWRITTEN" &&
-		pick_one_preserving_merges "$@" && return
 	parent_sha1=$(git rev-parse --verify $sha1^) ||
 		die "Could not get the parent of $sha1"
 	current_sha1=$(git rev-parse --verify HEAD)
@@ -163,66 +159,6 @@ pick_one () {
 	fi
 }
 
-pick_one_preserving_merges () {
-	case "$1" in -n) sha1=$2 ;; *) sha1=$1 ;; esac
-	sha1=$(git rev-parse $sha1)
-
-	if test -f "$DOTEST"/current-commit
-	then
-		current_commit=$(cat "$DOTEST"/current-commit) &&
-		git rev-parse HEAD > "$REWRITTEN"/$current_commit &&
-		rm "$DOTEST"/current-commit ||
-		die "Cannot write current commit's replacement sha1"
-	fi
-
-	# rewrite parents; if none were rewritten, we can fast-forward.
-	fast_forward=t
-	preserve=t
-	new_parents=
-	for p in $(git rev-list --parents -1 $sha1 | cut -d' ' -f2-)
-	do
-		if test -f "$REWRITTEN"/$p
-		then
-			preserve=f
-			new_p=$(cat "$REWRITTEN"/$p)
-			test $p != $new_p && fast_forward=f
-			case "$new_parents" in
-			*$new_p*)
-				;; # do nothing; that parent is already there
-			*)
-				new_parents="$new_parents $new_p"
-				;;
-			esac
-		fi
-	done
-	case $fast_forward in
-	t)
-		output warn "Fast forward to $sha1"
-		test $preserve = f || echo $sha1 > "$REWRITTEN"/$sha1
-		;;
-	f)
-		test "a$1" = a-n && die "Refusing to squash a merge: $sha1"
-
-		first_parent=$(expr "$new_parents" : ' \([^ ]*\)')
-		# detach HEAD to current parent
-		output git checkout $first_parent 2> /dev/null ||
-			die "Cannot move HEAD to $first_parent"
-
-		echo $sha1 > "$DOTEST"/current-commit
-		case "$new_parents" in
-		' '*' '*)
-			# No point in merging the first parent, that's HEAD
-			redo_merge $sha1 ${new_parents# $first_parent}
-			;;
-		*)
-			output git cherry-pick "$@" ||
-				die_with_patch $sha1 "Could not pick $sha1"
-			;;
-		esac
-		;;
-	esac
-}
-
 nth_string () {
 	case "$1" in
 	*1[0-9]|*[04-9]) echo "$1"th;;
@@ -398,20 +334,7 @@ do_next () {
 	HEADNAME=$(cat "$DOTEST"/head-name) &&
 	OLDHEAD=$(cat "$DOTEST"/head) &&
 	SHORTONTO=$(git rev-parse --short $(cat "$DOTEST"/onto)) &&
-	if test -d "$REWRITTEN"
-	then
-		test -f "$DOTEST"/current-commit &&
-			current_commit=$(cat "$DOTEST"/current-commit) &&
-			git rev-parse HEAD > "$REWRITTEN"/$current_commit
-		if test -f "$REWRITTEN"/$OLDHEAD
-		then
-			NEWHEAD=$(cat "$REWRITTEN"/$OLDHEAD)
-		else
-			NEWHEAD=$OLDHEAD
-		fi
-	else
-		NEWHEAD=$(git rev-parse HEAD)
-	fi &&
+	NEWHEAD=$(git rev-parse HEAD) &&
 	case $HEADNAME in
 	refs/*)
 		message="$GIT_REFLOG_ACTION: $HEADNAME onto $SHORTONTO)" &&
@@ -436,6 +359,130 @@ do_rest () {
 	done
 }
 
+get_value_from_list () {
+	# args: "key" " key1#value1 key2#value2"
+	case "$2" in
+	*" $1#"*)
+		stm_tmp="${2#* $1#}"
+		echo "${stm_tmp%% *}"
+		unset stm_tmp
+		;;
+	*)
+		return 1
+		;;
+	esac
+}
+
+insert_value_at_key_into_list () {
+	# args: "value" "key" " key1#value1 key2#value2"
+	case "$3 " in
+	*" $2#$1 "*)
+		echo "$3"
+		;;
+	*" $2#"*)
+		echo "$3"
+		return 1
+		;;
+	*)
+		echo "$3 $2#$1"
+		;;
+	esac
+}
+
+create_extended_todo_list () {
+	(
+	while IFS=_ read commit parents subject
+	do
+		if test "${last_parent:-$commit}" != "$commit"
+		then
+			if test t = "${delayed_mark:-f}"
+			then
+				marked_commits=$(insert_value_at_key_into_list \
+					dummy $last_parent "${marked_commits:-}")
+				delayed_mark=f
+			fi
+			test "$last_parent" = $SHORTUPSTREAM && \
+				last_parent=$SHORTONTO
+			echo "reset $last_parent"
+		fi
+		last_parent="${parents%% *}"
+
+		get_value_from_list $commit "${marked_commits:-}" \
+			>/dev/null && echo mark
+
+		case "$parents" in
+		*' '*)
+			delayed_mark=t
+			new_parents=
+			for p in ${parents#* }
+			do
+				marked_commits=$(insert_value_at_key_into_list \
+					dummy "$p" "${marked_commits:-}")
+				if test "$p" = $SHORTUPSTREAM
+				then
+					new_parents="$new_parents $SHORTONTO"
+				else
+					new_parents="$new_parents $p"
+				fi
+			done
+			unset p
+			echo merge $commit $new_parents
+			unset new_parents
+			;;
+		*)
+			echo "pick $commit $subject"
+			;;
+		esac
+	done
+	test -n "${last_parent:-}" -a "${last_parent:-}" != $SHORTUPSTREAM && \
+		echo reset $last_parent
+	) | \
+	tac | \
+	while read cmd args
+	do
+		: ${commit_mark_list:=} ${last_commit:=000}
+		case "$cmd" in
+		pick)
+			last_commit="${args%% *}"
+			;;
+		mark)
+			: ${next_mark:=0}
+			if commit_mark_list=$(insert_value_at_key_into_list \
+				$next_mark $last_commit "$commit_mark_list")
+			then
+				args=":$next_mark"
+				next_mark=$(($next_mark + 1))
+			else
+				die "Internal error: two marks for" \
+					"the same commit"
+			fi
+			;;
+		reset)
+			if tmp=$(get_value_from_list $args "$commit_mark_list")
+			then
+				args=":$tmp"
+			fi
+			;;
+		merge)
+			new_args=
+			for i in ${args#* }
+			do
+				if tmp=$(get_value_from_list $i \
+					"$commit_mark_list")
+				then
+					new_args="$new_args :$tmp"
+				else
+					new_args="$new_args $i"
+				fi
+			done
+			last_commit="${args%% *}"
+			args="$last_commit ${new_args# }"
+			;;
+		esac
+		echo "$cmd $args"
+	done
+}
+
 while test $# != 0
 do
 	case "$1" in
@@ -568,33 +615,23 @@ do
 		echo $ONTO > "$DOTEST"/onto
 		test -z "$STRATEGY" || echo "$STRATEGY" > "$DOTEST"/strategy
 		test t = "$VERBOSE" && : > "$DOTEST"/verbose
-		if test t = "$PRESERVE_MERGES"
-		then
-			# $REWRITTEN contains files for each commit that is
-			# reachable by at least one merge base of $HEAD and
-			# $UPSTREAM. They are not necessarily rewritten, but
-			# their children might be.
-			# This ensures that commits on merged, but otherwise
-			# unrelated side branches are left alone. (Think "X"
-			# in the man page's example.)
-			mkdir "$REWRITTEN" &&
-			for c in $(git merge-base --all $HEAD $UPSTREAM)
-			do
-				echo $ONTO > "$REWRITTEN"/$c ||
-					die "Could not init rewritten commits"
-			done
-			MERGES_OPTION=
-		else
-			MERGES_OPTION=--no-merges
-		fi
 
 		SHORTUPSTREAM=$(git rev-parse --short=7 $UPSTREAM)
 		SHORTHEAD=$(git rev-parse --short=7 $HEAD)
 		SHORTONTO=$(git rev-parse --short=7 $ONTO)
-		git rev-list $MERGES_OPTION --pretty=oneline --abbrev-commit \
-			--abbrev=7 --reverse --left-right --cherry-pick \
-			$UPSTREAM...$HEAD | \
-			sed -n "s/^>/pick /p" > "$TODO"
+		common_rev_list_opts="--abbrev-commit --abbrev=7
+			--left-right --cherry-pick $UPSTREAM...$HEAD"
+		if test t = "$PRESERVE_MERGES"
+		then
+			git rev-list --pretty='format:%h_%p_%s' --topo-order \
+				$common_rev_list_opts | \
+				grep -v ^commit | \
+				create_extended_todo_list
+		else
+			git rev-list --no-merges --reverse --pretty=oneline \
+				 $common_rev_list_opts | sed -n "s/^>/pick /p"
+		fi > "$TODO"
+
 		cat >> "$TODO" << EOF
 
 # Rebase $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 0a8d065..f919aaf 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -258,7 +258,44 @@ test_expect_success 'preserve merges with -p' '
 	test $(git show HEAD~2:file1) = B
 '
 
+test_expect_success 'rebase with preserve merge forth and back is a noop' '
+	git checkout -b big-branch-1 master &&
+	test_tick &&
+	: > bb1a &&
+	git add bb1a &&
+	git commit -m "big branch commit 1" &&
+	: > bb1b &&
+	git add bb1b &&
+	git commit -m "big branch commit 2" &&
+	: > bb1c &&
+	git add bb1c &&
+	git commit -m "big branch commit 3" &&
+	git checkout -b big-branch-2 master &&
+	: > bb2a &&
+	git add bb2a &&
+	git commit -m "big branch commit 4" &&
+	: > bb2b &&
+	git add bb2b &&
+	git commit -m "big branch commit 5" &&
+	git merge big-branch-1~1 &&
+	git merge to-be-preserved &&
+	tbp_merge=$(git rev-parse HEAD) &&
+	: > bb2c &&
+	git add bb2c &&
+	git commit -m "big branch commit 6" &&
+	git merge big-branch-1 &&
+	head=$(git rev-parse HEAD) &&
+	FAKE_LINES="16 6 19 20 4 1 2 5 22" \
+		git rebase -i -p --onto dead-end master &&
+	test "$head" != "$(git rev-parse HEAD)" &&
+	FAKE_LINES="3 7 mark:10 8 9 5 1 2 merge$tbp_merge~1/:10 \
+		merge$tbp_merge/to-be-preserved 6 11" \
+		git rebase -i -p --onto master dead-end &&
+	test "$head" = "$(git rev-parse HEAD)"
+'
+
 test_expect_success '--continue tries to commit' '
+	git checkout to-be-rebased &&
 	test_tick &&
 	! git rebase -i --onto new-branch1 HEAD^ &&
 	echo resolved > file1 &&
-- 
1.5.5

[PATCH v2 11/13] Add option --first-parent

[Jörg Sommer]

With this new option it's possible to narrow the list of commits in the
TODO list to only those commits you get following the first parent of
each merge, i.e. not those from the merged branches.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 Documentation/git-rebase.txt  |    7 ++++++-
 git-rebase--interactive.sh    |   15 +++++++++++----
 t/t3404-rebase-interactive.sh |   12 ++++++++++++
 3 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index e0412e0..9ebbb90 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git-rebase' [-i | --interactive] [-v | --verbose] [-m | --merge]
 	[-s <strategy> | --strategy=<strategy>]
 	[-C<n>] [ --whitespace=<option>] [-p | --preserve-merges]
-	[--onto <newbase>] <upstream> [<branch>]
+	[-f | --first-parent] [--onto <newbase>] <upstream> [<branch>]
 'git-rebase' --continue | --skip | --abort
 
 DESCRIPTION
@@ -247,6 +247,11 @@ OPTIONS
 	Instead of ignoring merges, try to recreate them.  This option
 	only works in interactive mode.
 
+-f, \--first-parent::
+	This option implies the option --preserve-merges, but instead of
+	showing all commits from the merged branches show only the
+	commits and merges following the first parent of each commit.
+
 include::merge-strategies.txt[]
 
 NOTES
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index d3327a8..ea67942 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -10,8 +10,8 @@
 # The original idea comes from Eric W. Biederman, in
 # http://article.gmane.org/gmane.comp.version-control.git/22407
 
-USAGE='(--continue | --abort | --skip | [--preserve-merges] [--verbose]
-	[--onto <branch>] <upstream> [<branch>])'
+USAGE='(--continue | --abort | --skip | [--preserve-merges] [--first-parent]
+	[--verbose] [--onto <branch>] <upstream> [<branch>])'
 
 OPTIONS_SPEC=
 . git-sh-setup
@@ -565,6 +565,10 @@ do
 	-p|--preserve-merges)
 		PRESERVE_MERGES=t
 		;;
+	-f|--first-parent)
+		FIRST_PARENT=t
+		PRESERVE_MERGES=t
+		;;
 	-i|--interactive)
 		# yeah, we know
 		;;
@@ -621,10 +625,13 @@ do
 		SHORTONTO=$(git rev-parse --short=7 $ONTO)
 		common_rev_list_opts="--abbrev-commit --abbrev=7
 			--left-right --cherry-pick $UPSTREAM...$HEAD"
-		if test t = "$PRESERVE_MERGES"
+		if test t = "$PRESERVE_MERGES" -o t = "${FIRST_PARENT:-f}"
 		then
+			opts=
+			test t = "${FIRST_PARENT:-f}" && \
+				opts="$opts --first-parent"
 			git rev-list --pretty='format:%h_%p_%s' --topo-order \
-				$common_rev_list_opts | \
+				$opts $common_rev_list_opts | \
 				grep -v ^commit | \
 				create_extended_todo_list
 		else
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index f919aaf..8da7829 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -294,6 +294,18 @@ test_expect_success 'rebase with preserve merge forth and back is a noop' '
 	test "$head" = "$(git rev-parse HEAD)"
 '
 
+test_expect_success 'interactive --first-parent gives a linear list' '
+	head=$(git rev-parse HEAD) &&
+	EXPECT_COUNT=6 FAKE_LINES="2 1 4 3 6 5" \
+		git rebase -i -f --onto dead-end master &&
+	test "$head" != "$(git rev-parse HEAD)" &&
+	git rev-parse HEAD^^2 &&
+	test "$(git rev-parse HEAD~6)" = "$(git rev-parse dead-end)" &&
+	EXPECT_COUNT=6 FAKE_LINES="2 1 4 3 6 5" \
+		git rebase -i -f --onto master dead-end &&
+	test "$head" = "$(git rev-parse HEAD)"
+'
+
 test_expect_success '--continue tries to commit' '
 	git checkout to-be-rebased &&
 	test_tick &&
-- 
1.5.5

[PATCH v2 12/13] Teach rebase interactive the tag command

[Jörg Sommer]

The intent of the tag command is to (re)set tags for commits in the TODO
list. This way it's possible to rebase a commit together with its tag.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh    |    7 +++++++
 t/t3404-rebase-interactive.sh |   13 +++++++++++++
 2 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index ea67942..c601655 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -323,6 +323,12 @@ do_next () {
 		mark_action_done
 		output git reset --hard $tmp
 		;;
+	tag|t)
+		comment_for_reflog tag
+
+		mark_action_done
+		output git tag -f "$sha1"
+		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
 		die_with_patch $sha1 "Please fix this in the file $TODO."
@@ -655,6 +661,7 @@ do
 #  reset commit = reset HEAD to the commit
 #  merge commit-M commit-P ... = redo merge commit-M with the
 #         current HEAD and the parents commit-P
+#  tag = reset tag to the current HEAD
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8da7829..9901555 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -91,6 +91,9 @@ for line in $FAKE_LINES; do
 	merge*)
 		echo "merge ${line#merge}" | tr / ' '
 		echo "merge ${line#merge}" | tr / ' ' >> "$1";;
+	tag*)
+		echo "tag ${line#tag}"
+		echo "tag ${line#tag}" >> "$1";;
 	*)
 		sed -n "${line}{s/^pick/$action/; p;}" < "$1".tmp
 		sed -n "${line}{s/^pick/$action/; p;}" < "$1".tmp >> "$1"
@@ -306,6 +309,16 @@ test_expect_success 'interactive --first-parent gives a linear list' '
 	test "$head" = "$(git rev-parse HEAD)"
 '
 
+test_expect_success 'tag sets tags' '
+	head=$(git rev-parse HEAD) &&
+	FAKE_LINES="1 2 3 4 5 tagbb-tag1 6 7 8 9 10 11 12 13 14 15 \
+		tagbb-tag2 16 tagbb-tag3a tagbb-tag3b 17 18 19 20 21 22" \
+		EXPECT_COUNT=22 git rebase -i -p master &&
+	test "$head" = "$(git rev-parse HEAD)" &&
+	test "$(git rev-parse bb-tag1 bb-tag2 bb-tag3a bb-tag3b)" = \
+		"$(git rev-parse HEAD^2~2 HEAD~2 HEAD~1 HEAD~1)"
+'
+
 test_expect_success '--continue tries to commit' '
 	git checkout to-be-rebased &&
 	test_tick &&
-- 
1.5.5

[PATCH v2 13/13] Add option --preserve-tags

[Jörg Sommer]

With this new option tags set on commits, they are part of a rebase, are
reset to the rebased commits. This way the tags on a branch are kept
across rebases.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 Documentation/git-rebase.txt  |    6 ++++-
 git-rebase--interactive.sh    |   42 ++++++++++++++++++++++++++++++++++++++--
 t/t3404-rebase-interactive.sh |   10 +++++++++
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 9ebbb90..cc4e94f 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -11,7 +11,8 @@ SYNOPSIS
 'git-rebase' [-i | --interactive] [-v | --verbose] [-m | --merge]
 	[-s <strategy> | --strategy=<strategy>]
 	[-C<n>] [ --whitespace=<option>] [-p | --preserve-merges]
-	[-f | --first-parent] [--onto <newbase>] <upstream> [<branch>]
+	[-f | --first-parent] [-t | --preserve-tags]
+	[--onto <newbase>] <upstream> [<branch>]
 'git-rebase' --continue | --skip | --abort
 
 DESCRIPTION
@@ -252,6 +253,9 @@ OPTIONS
 	showing all commits from the merged branches show only the
 	commits and merges following the first parent of each commit.
 
+-t, \--preserve-tags::
+	If one of the commits has a tag, reset it to the new commit object.
+
 include::merge-strategies.txt[]
 
 NOTES
diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index c601655..e874c31 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -11,7 +11,7 @@
 # http://article.gmane.org/gmane.comp.version-control.git/22407
 
 USAGE='(--continue | --abort | --skip | [--preserve-merges] [--first-parent]
-	[--verbose] [--onto <branch>] <upstream> [<branch>])'
+	[--preserve-tags] [--verbose] [--onto <branch>] <upstream> [<branch>])'
 
 OPTIONS_SPEC=
 . git-sh-setup
@@ -397,9 +397,31 @@ insert_value_at_key_into_list () {
 
 create_extended_todo_list () {
 	(
+	if test t = "${PRESERVE_TAGS:-}"
+	then
+		tag_list=$(git show-ref --abbrev=7 --tags | \
+			(
+			while read sha1 tag
+			do
+				tag=${tag#refs/tags/}
+				if test ${last_sha1:-0000} = $sha1
+				then
+					saved_tags="$saved_tags:$tag"
+				else
+					printf "%s" "${last_sha1:+ $last_sha1#$saved_tags}"
+					last_sha1=$sha1
+					saved_tags=$tag
+				fi
+			done
+			echo "${last_sha1:+ $last_sha1:$saved_tags}"
+			) )
+	else
+		tag_list=
+	fi
 	while IFS=_ read commit parents subject
 	do
-		if test "${last_parent:-$commit}" != "$commit"
+		if test t = "$PRESERVE_MERGES" -a \
+			"${last_parent:-$commit}" != "$commit"
 		then
 			if test t = "${delayed_mark:-f}"
 			then
@@ -416,6 +438,14 @@ create_extended_todo_list () {
 		get_value_from_list $commit "${marked_commits:-}" \
 			>/dev/null && echo mark
 
+		if tmp=$(get_value_from_list $commit "$tag_list")
+		then
+			for t in $(echo $tmp | tr : ' ')
+			do
+				echo tag $t
+			done
+		fi
+
 		case "$parents" in
 		*' '*)
 			delayed_mark=t
@@ -575,6 +605,9 @@ do
 		FIRST_PARENT=t
 		PRESERVE_MERGES=t
 		;;
+	-t|--preserve-tags)
+		PRESERVE_TAGS=t
+		;;
 	-i|--interactive)
 		# yeah, we know
 		;;
@@ -631,11 +664,14 @@ do
 		SHORTONTO=$(git rev-parse --short=7 $ONTO)
 		common_rev_list_opts="--abbrev-commit --abbrev=7
 			--left-right --cherry-pick $UPSTREAM...$HEAD"
-		if test t = "$PRESERVE_MERGES" -o t = "${FIRST_PARENT:-f}"
+		if test t = "$PRESERVE_MERGES" -o t = "${FIRST_PARENT:-f}" \
+			-o t = "${PRESERVE_TAGS:-}"
 		then
 			opts=
 			test t = "${FIRST_PARENT:-f}" && \
 				opts="$opts --first-parent"
+			test t != "$PRESERVE_MERGES" && \
+				opts="$opts --no-merges"
 			git rev-list --pretty='format:%h_%p_%s' --topo-order \
 				$opts $common_rev_list_opts | \
 				grep -v ^commit | \
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 9901555..d20ed4f 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -319,6 +319,16 @@ test_expect_success 'tag sets tags' '
 		"$(git rev-parse HEAD^2~2 HEAD~2 HEAD~1 HEAD~1)"
 '
 
+test_expect_success 'interactive -t preserves tags' '
+	git rebase -i -p -t --onto dead-end master &&
+	test "$(git rev-parse bb-tag1 bb-tag2 bb-tag3a bb-tag3b)" = \
+		"$(git rev-parse HEAD^2~2 HEAD~2 HEAD~1 HEAD~1)" &&
+	head=$(git rev-parse HEAD) &&
+	git rebase -i -t dead-end &&
+	test "$(git rev-parse bb-tag1 bb-tag2 bb-tag3a bb-tag3b)" = \
+		"$(git rev-parse HEAD~7 $head~2 HEAD~1 HEAD~1)"
+'
+
 test_expect_success '--continue tries to commit' '
 	git checkout to-be-rebased &&
 	test_tick &&
-- 
1.5.5

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Jörg Sommer <joerg@alea.gnuu.de> writes:

> This new command can be used to set symbolic marks for an commit while
> doing a rebase. This symbolic name can later be used for merges or
> resets.
>
> The decision to use references for the marks and not files like done with
> the rewritten commits for preserve merges was made to ensure no commit
> objects get lost if prune is started while (a long term) rebase is
> running. This also unifies the checking of the validity of marks and
> references by using rev-parse for it.
>
> The usage of : as the sign for marks conforms with the tag sign of
> fast-export and fast-import.
>
> Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
> ---
>  git-rebase--interactive.sh    |   37 ++++++++++++++++++++++++++++++++++++-
>  t/t3404-rebase-interactive.sh |   17 +++++++++++++++++
>  2 files changed, 53 insertions(+), 1 deletions(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 531ee94..6ac316a 100755
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -35,6 +35,8 @@ mark the corrected paths with 'git add <paths>', and
>  run 'git rebase --continue'"
>  export GIT_CHERRY_PICK_HELP
>  
> +mark_prefix=refs/rebase-marks/
> +
>  warn () {
>  	echo "$*" >&2
>  }
> @@ -105,7 +107,13 @@ die_with_patch () {
>  }
>  
>  cleanup_before_quit () {
> -	rm -rf "$DOTEST"
> +	rm -rf "$DOTEST" &&
> +	for ref in "$GIT_DIR/$mark_prefix"*
> +	do
> +		test "$ref" = "$GIT_DIR/$mark_prefix*" && continue
> +		git update-ref -d "${ref#$GIT_DIR/}" "${ref#$GIT_DIR/}" || \
> +			return 1
> +	done

In practice nobody would "run" pack-refs during the rebase session, but I
have to wonder if it can be triggered to run as part of automated gc or
something, in which case this loop does not work as intended. It needs to
be rewritten using for-each-ref.

> @@ -244,6 +252,19 @@ peek_next_command () {
>  	sed -n "1s/ .*$//p" < "$TODO"
>  }
>  
> +mark_to_ref () {
> +	case "$1" in
> +	:[!/]*)
> +		# :/SOMETHING is a reference for the last commit whose
> +                # message starts with SOMETHING
> +		echo "$mark_prefix${1#:}"
> +		;;

What was the conclusion of the mark-syntax discussion?

While I know the bang in ":[!negated]" is POSIX, I wonder if everybody's
shell we care about groks it.

Could people run this with the shell they care about being supported
(Solaris /bin/sh does not count) try this and yell loudly if you get
"matches" please?  I know bash and dash are Ok, but I do not have easy
access to various flabours of BSDs (OSX included).

	case ":/foo" in
        :[!/]*)	echo matches ;;
	*)	echo does not ;;
	esac

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Junio C Hamano <junio@pobox.com> writes:

>> +mark_prefix=refs/rebase-marks/
>> +
>>  warn () {
>>  	echo "$*" >&2
>>  }
>> @@ -105,7 +107,13 @@ die_with_patch () {
>>  }
>>  
>>  cleanup_before_quit () {
>> -	rm -rf "$DOTEST"
>> +	rm -rf "$DOTEST" &&
>> +	for ref in "$GIT_DIR/$mark_prefix"*
>> +	do
>> +		test "$ref" = "$GIT_DIR/$mark_prefix*" && continue
>> +		git update-ref -d "${ref#$GIT_DIR/}" "${ref#$GIT_DIR/}" || \
>> +			return 1
>> +	done
>
> In practice nobody would "run" pack-refs during the rebase session, but I
> have to wonder if it can be triggered to run as part of automated gc or
> something, in which case this loop does not work as intended. It needs to
> be rewritten using for-each-ref.
>
>> @@ -244,6 +252,19 @@ peek_next_command () {
>>  	sed -n "1s/ .*$//p" < "$TODO"
>>  }
>>  
>> +mark_to_ref () {
>> +	case "$1" in
>> +	:[!/]*)
>> +		# :/SOMETHING is a reference for the last commit whose
>> +                # message starts with SOMETHING
>> +		echo "$mark_prefix${1#:}"
>> +		;;
>
> What was the conclusion of the mark-syntax discussion?

Eh, sorry, I was commenting on a stale one.  Disregard this part please.

But the "$GIT_DIR/$mark_prefix/*" comment still stands.  I've applied the
series as is to 'next' so let's fix them up in-tree as needed.

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

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

Hi,

On Mon, 21 Apr 2008, Junio C Hamano wrote:

> Jörg Sommer <joerg@alea.gnuu.de> writes:
> 
> > @@ -244,6 +252,19 @@ peek_next_command () {
> >  	sed -n "1s/ .*$//p" < "$TODO"
> >  }
> >  
> > +mark_to_ref () {
> > +	case "$1" in
> > +	:[!/]*)
> > +		# :/SOMETHING is a reference for the last commit whose
> > +                # message starts with SOMETHING
> > +		echo "$mark_prefix${1#:}"
> > +		;;
> 
> What was the conclusion of the mark-syntax discussion?

Well, I will probably work on something that is not as intrusive and 
syntax-changing this week.

> While I know the bang in ":[!negated]" is POSIX, I wonder if everybody's
> shell we care about groks it.

The common way to do that in the git sources is

	switch "$x" in
	/*)
		# do nothing
		;;
	*)
		<bla>
		;;
	esac

and frankly, I do not see a reason to move away from that practice.  
Especially since consistency in source code is better than inconsistency.

Ciao,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Jörg Sommer]

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

Hi Junio,

Junio C Hamano schrieb am Mon 21. Apr, 22:32 (-0700):
> Jörg Sommer <joerg@alea.gnuu.de> writes:
> 
> >  cleanup_before_quit () {
> > -	rm -rf "$DOTEST"
> > +	rm -rf "$DOTEST" &&
> > +	for ref in "$GIT_DIR/$mark_prefix"*
> > +	do
> > +		test "$ref" = "$GIT_DIR/$mark_prefix*" && continue
> > +		git update-ref -d "${ref#$GIT_DIR/}" "${ref#$GIT_DIR/}" || \
> > +			return 1
> > +	done
> 
> In practice nobody would "run" pack-refs during the rebase session, but I
> have to wonder if it can be triggered to run as part of automated gc or
> something, in which case this loop does not work as intended. It needs to
> be rewritten using for-each-ref.

What do you think about this version:

cleanup_before_quit () {
	rm -rf "$DOTEST" &&
	for ref in $(git for-each-ref --format='%(refname)' ${mark_prefix%/})
	do
		git update-ref -d "$ref" "$ref" || return 1
	done
}

> > @@ -244,6 +252,19 @@ peek_next_command () {
> >  	sed -n "1s/ .*$//p" < "$TODO"
> >  }
> >  
> > +mark_to_ref () {
> > +	case "$1" in
> > +	:[!/]*)
> > +		# :/SOMETHING is a reference for the last commit whose
> > +                # message starts with SOMETHING
> > +		echo "$mark_prefix${1#:}"
> > +		;;
> 
> What was the conclusion of the mark-syntax discussion?

Use the same as fast-import and fix fast-import. :)

I've posted a new version
<1208169584-15931-1-git-send-email-joerg@alea.gnuu.de>

Bye, Jörg.
-- 
Nutze die Talente, die du hast. Die Wälder wären sehr still,
wenn nur die begabtesten Vögel sängen.                (Henry van Dyke)

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

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

Hi,

On Tue, 22 Apr 2008, Jörg Sommer wrote:

> Junio C Hamano schrieb am Mon 21. Apr, 22:32 (-0700):
>
> > What was the conclusion of the mark-syntax discussion?
> 
> Use the same as fast-import and fix fast-import. :)

I strongly disagree.

Also with the conclusion that this was the conclusion.

fast-import is a fundamentally different thing compared to rebase -i.  The 
former should be very easy to write importers for, and therefore have a 
very easy syntax from the _technical_ view, the latter should be usable by 
people, and therefore have a very easy syntax from the _usability_ point 
of view.

So I really hate the idea of introducing yet other marks when we already 
have unique identifiers: the (abbreviated) commit names.

I guess the only way to prove that I am not wrong is to do it myself. 
Sigh.

Ciao,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> So I really hate the idea of introducing yet other marks when we already 
> have unique identifiers: the (abbreviated) commit names.

Didn't I give you an example why commit object names are _not_ unique
identifiers already?

I also do not understand why you think 'mark' is ugly.  I _suspect_ that
the machinery to read the TODO insns would need to keep a copy of what it
gave to the end user, compare it with what user edited to make sure the
user did not make nonsense insn sequence out of it, which Jörg's code
doesn't do (yet), and I suspect a simple rule like "you cannot move insns
across 'mark' boundary" would be sufficient for that check.

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

Hi,

On Tue, 22 Apr 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > So I really hate the idea of introducing yet other marks when we already 
> > have unique identifiers: the (abbreviated) commit names.
> 
> Didn't I give you an example why commit object names are _not_ unique
> identifiers already?

By that reasoning, rebase -i cannot work anyway: it relies on the 
abbreviated identifiers, not on anything else, for the "pick" command.

> I also do not understand why you think 'mark' is ugly.

Is that not obvious?  You _already_ have identifiers.  And there you add 
other ones.  Only because the original idea of the -p implementation was 
ignored.

Ciao,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Tue, 22 Apr 2008, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > So I really hate the idea of introducing yet other marks when we already 
>> > have unique identifiers: the (abbreviated) commit names.
>> 
>> Didn't I give you an example why commit object names are _not_ unique
>> identifiers already?
>
> By that reasoning, rebase -i cannot work anyway: it relies on the 
> abbreviated identifiers, not on anything else, for the "pick" command.

No, read the message again and think for 5 minutes.

Picking the same commit twice does not make any sense, neither does
picking the resulting commit from an earlier operation in the same
sequencer run.  Which means that the commit object name for 'pick' can
mean _only_ the pre-rewritten commit object, not 'the result of an earlier
operation that used that commit'.  And you always pick on top of the
current (detached) HEAD.

Reset is different.  You can reset either to the named commit to start
building from a known state that existed before the sequencer run started,
or reset to the result of pick (or merge) of the named commit, and your
proposal breaks down here, because you cannot tell between the two.

To rebuild this history on top of a commit O' elsewhere:

        O---A---B
             \   \
              D---E---F---G
                         / 
                        X

you would need to:

	pick A
        pick B
        reset <<to the state after "pick A">>
        pick D
        merge <<the state after "pick B">>
        pick F
        merge X (taken from somebody else)

and the syntax proposed to express <<the above part>> can either be your
"the result of the last operation that used the named commit", which is
simple in some cases, or "named commit, be it with mark or standard sha-1
expression".

Introducing a 'mark' insn to mark the previous result you may want to go
back to is one way to solve this without ambiguity.  Then abbreviated
object name won't have to be mapped as in your proposal.

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

Hi,

On Mon, 28 Apr 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Tue, 22 Apr 2008, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >> 
> >> > So I really hate the idea of introducing yet other marks when we already 
> >> > have unique identifiers: the (abbreviated) commit names.
> >> 
> >> Didn't I give you an example why commit object names are _not_ unique 
> >> identifiers already?
> >
> > By that reasoning, rebase -i cannot work anyway: it relies on the 
> > abbreviated identifiers, not on anything else, for the "pick" command.
> 
> No, read the message again and think for 5 minutes.

pick abcdefg
pick pqrstuv

Now imagine that pqrstuv is a unique commit name _before_ cherry-picking 
abcdefg, but not _after_ it.  Unlikely?  Yes.  Impossible?  No.

Ciao,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> No, read the message again and think for 5 minutes.
>
> pick abcdefg
> pick pqrstuv
>
> Now imagine that pqrstuv is a unique commit name _before_ cherry-picking 
> abcdefg, but not _after_ it.  Unlikely?  Yes.  Impossible?  No.

No, I am still being misunderstood.  That is not the uniqueness issue I
was talking about.

As I tried to describe in a message that used a phrase "the beauty of your
approach" (which I suspect you stopped reading after that phrase), your
rule "an object name names the commit after the last operation that used
the commit, unless it is the first appearance in the sequence, in which
case it names the original commit" works most of the time, but becomes
cumbersome and unwieldy when the history you try to reproduce becomes more
elaborate.  And the reason I suggested 'mark' was because I was seeing
this in the context of the sequencer, not just "rebase -i".

This is just a minor syntax issue and I am not sure why we got into this
misunderstanding, but let's try again.  Suppose you want to recreate this
history on top of a different O'.  For merges, upper parents are earlier
ones:

     A         reset O'
    / \        pick  B
   /   X       reset O'
  /   / \      pick A
 O---B   Z     merge B -- recreate X
  \   \ /      reset O'
   \   Y       pick C
    \ /        merge B? -- recreate Y
     C         reset B -- go back to recreated X
               merge B? -- recreate Z

The above sequence does not work.  As you already used B to create X',
when you want to recreate Y, the last operation used B is now X' and you
cannot name B' to merge that to C' to produce Y' (and the above sequence
is wrong in that it attempts to recreate Y' in a wrong order --- we should
reset to the recreated B and merge recreated C to produce Y', but for that
you need to be able to say "reset to B'", but you've already used B to
recreate X' so you cannot do that).

You could (because you are intelligent human) reorder things to take
advantage of the fact that pick and merge uses the "current detached HEAD"
to avoid refering to B in this particular case, like this:

        reset O'
        pick A
        reset O'
        pick B	
        reset O'
        pick C
        reset B	-- go back to recreated B
        merge C -- recreate Y
        reset A -- go back to recreated A
        merge B -- recreate X
        merge C -- recreate Z

But I think the only reason you can do this in this case is because B is
used only twice.  If you have a history where rewritten B itself (not the
result of applying/merging the rewritten B) needs to be used more than
twice as the second or later merge parent, I do not think you would have a
way to name that rewritten B for the later uses.

I may be mistaken and you may even be able to prove that there is always a
way to recreate any shape of history by reordering insns and still using
your rule, but the point is that the insn sequence needs to be prepared by
tools (i.e. frontends for the sequencer), and I do not want to make the
life of people who are writing the frontends any harder than necessary.

The syntax and semantics that uses 'mark' avoids this unwieldiness by
making the rule much simpler.

 * Operation operates on the current detached HEAD (as in your rule)

 * A commit object name always names _that_ commit object.

 * If you need to name an intermediate result, you 'mark' it.

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Sixt]

Junio C Hamano schrieb:
> This is just a minor syntax issue and I am not sure why we got into this
> misunderstanding, but let's try again.  Suppose you want to recreate this
> history on top of a different O'.  For merges, upper parents are earlier
> ones:
> 
>      A         reset O'
>     / \        pick  B
>    /   X       reset O'
>   /   / \      pick A
>  O---B   Z     merge B -- recreate X
>   \   \ /      reset O'
>    \   Y       pick C
>     \ /        merge B? -- recreate Y
>      C         reset B -- go back to recreated X
>                merge B? -- recreate Z
> 
> The above sequence does not work.

Because it is hand-crafted. I'd expect rebase to suggest a series that
works as long as the user doesn't modify it. Like this:

	reset O'
	pick C
	reset O'
	pick B
	merge C -- recreate Y
	reset O'
	pick A
	merge B -- recreate X
	merge Y -- recreate Z

Here all commit names are clearly the original in the first insn that
references it, and the rewritten version in later references. No marks needed.

If the user modifies the insns, he better knows what he's doing, in
particular, when it's necessary to rebuild such complex histories.

-- Hannes

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

Hi,

On Tue, 29 Apr 2008, Johannes Sixt wrote:

> Junio C Hamano schrieb:
> > This is just a minor syntax issue and I am not sure why we got into 
> > this misunderstanding, but let's try again.  Suppose you want to 
> > recreate this history on top of a different O'.  For merges, upper 
> > parents are earlier ones:
> > 
> >      A         reset O'
> >     / \        pick  B
> >    /   X       reset O'
> >   /   / \      pick A
> >  O---B   Z     merge B -- recreate X
> >   \   \ /      reset O'
> >    \   Y       pick C
> >     \ /        merge B? -- recreate Y
> >      C         reset B -- go back to recreated X
> >                merge B? -- recreate Z
> > 
> > The above sequence does not work.
> 
> Because it is hand-crafted. I'd expect rebase to suggest a series that 
> works as long as the user doesn't modify it. Like this:
> 
> 	reset O'
> 	pick C
> 	reset O'
> 	pick B
> 	merge C -- recreate Y
> 	reset O'
> 	pick A
> 	merge B -- recreate X
> 	merge Y -- recreate Z
> 
> Here all commit names are clearly the original in the first insn that 
> references it, and the rewritten version in later references. No marks 
> needed.
> 
> If the user modifies the insns, he better knows what he's doing, in 
> particular, when it's necessary to rebuild such complex histories.

I fully agree.  rebase -i is _not_ about the same goal as git-sequencer.  
rebase -i is about user interaction.  sequencer is about having a common 
plumbing for the different porcelains.

And of course, if you want to play games with rebase -i, you can _always_ 
use the "edit" command (even if you do not plan to edit) to get the commit 
name of the new commit.

And you can _always_ use the _full_ commit name to reference the original 
commit (at least that is how I planned it: the original _short_ name would 
be replaced by the rewritten commit name, but not the _long_ name).

Ciao,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Tue, 29 Apr 2008, Johannes Sixt wrote:
>
>> Junio C Hamano schrieb:
>> > This is just a minor syntax issue and I am not sure why we got into 
>> > this misunderstanding, but let's try again.  Suppose you want to 
>> > recreate this history on top of a different O'.  For merges, upper 
>> > parents are earlier ones:
>> > 
>> >      A         reset O'
>> >     / \        pick  B
>> >    /   X       reset O'
>> >   /   / \      pick A
>> >  O---B   Z     merge B -- recreate X
>> >   \   \ /      reset O'
>> >    \   Y       pick C
>> >     \ /        merge B? -- recreate Y
>> >      C         reset B -- go back to recreated X
>> >                merge B? -- recreate Z
>> > 
>> > The above sequence does not work.
>> 
>> Because it is hand-crafted. I'd expect rebase to suggest a series that 
>> works as long as the user doesn't modify it. Like this:
>> 
>> 	reset O'
>> 	pick C
>> 	reset O'
>> 	pick B
>> 	merge C -- recreate Y
>> 	reset O'
>> 	pick A
>> 	merge B -- recreate X
>> 	merge Y -- recreate Z
>> 
>> Here all commit names are clearly the original in the first insn that 
>> references it, and the rewritten version in later references. No marks 
>> needed.
>> 
>> If the user modifies the insns, he better knows what he's doing, in 
>> particular, when it's necessary to rebuild such complex histories.
>
> I fully agree.  rebase -i is _not_ about the same goal as git-sequencer.  
> rebase -i is about user interaction.  sequencer is about having a common 
> plumbing for the different porcelains.

The problem is that both of you stopped reading after the part you quoted.

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

Hi,

On Tue, 29 Apr 2008, Junio C Hamano wrote:

> The problem is that both of you stopped reading after the part you 
> quoted.

I did not.  But I assumed that Hannes' example showed that it is always 
possible to reorder the commands such that you there is no problem with 
interpreting a short commit name as the original commit _until_ that 
commit is rewritten, and _then_ as the rewritten commit.

It is a simple matter of the word "acyclic" in the term "DAG".  It means 
that whenever you need to refer to a commit, it either comes before or 
after the commit you need it for, not both directions.

And I tried to make clear that I thought deeply about the issue by 
mentioning that you can always use "edit" to stop somewhere and mark (even 
by a lightweight tag), should you need to split a commit such that you 
need to reference both the original and the rewritten commit.

I think the balance to cut here is that of usability.  You can cater for 
the obscure cases, but that just does not make sense.  With some recipe -- 
as illustrated by Hannes -- it is very easy to see what it does, and as 
easy to modify it, should that be necessary.

The alternative is obviously easier for the cases that next to nobody will 
need.

So no, your argument does not convince me, and I still think that I 
understood it correctly from the start.

Ciao,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> It is a simple matter of the word "acyclic" in the term "DAG".  It means 
> that whenever you need to refer to a commit, it either comes before or 
> after the commit you need it for, not both directions.

I fell in the same "acyclic" fallacy before I realized it was a mistake,
especially after thought about the "rewritten B needs to be used more than
twice as a merge source" issue.  That's why I earlier said the beauty of
your approach is attractive but it "unfortunately" breaks down.

For "rebase -i", the tool needs to spit out insns (and again I'd prefer
not to require the tools to be clever to be able to write them out), and
the generated sequence needs to be easily understood by the end user who
needs to be able to edit (e.g. drop lines, reorder them, s/pick/edit/) and
easily visualize what the resulting shape of the history would be.  If we
limit ourselves to the context of a non-merge-preserving "rebase -i", the
insns will not need 'mark' (nor 'merge') and the resulting todo file would
look identical in both approaches.

But we also would want to have a sequencer generic enough to be capable of
faithfully reproducing a history with merges.

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

hi,

On Tue, 29 Apr 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > It is a simple matter of the word "acyclic" in the term "DAG".  It means 
> > that whenever you need to refer to a commit, it either comes before or 
> > after the commit you need it for, not both directions.
> 
> I fell in the same "acyclic" fallacy before I realized it was a mistake, 
> especially after thought about the "rewritten B needs to be used more 
> than twice as a merge source" issue.  That's why I earlier said the 
> beauty of your approach is attractive but it "unfortunately" breaks 
> down.

I do not understand.  The topological order assures that you have 
rewritten every commit that needs to be rewritten before rewriting the 
current commit.

Puzzled,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Tue, 29 Apr 2008, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > It is a simple matter of the word "acyclic" in the term "DAG".  It means 
>> > that whenever you need to refer to a commit, it either comes before or 
>> > after the commit you need it for, not both directions.
>> 
>> I fell in the same "acyclic" fallacy before I realized it was a mistake, 
>> especially after thought about the "rewritten B needs to be used more 
>> than twice as a merge source" issue.  That's why I earlier said the 
>> beauty of your approach is attractive but it "unfortunately" breaks 
>> down.
>
> I do not understand.  The topological order assures that you have 
> rewritten every commit that needs to be rewritten before rewriting the 
> current commit.

Perhaps it would help to go back to the message J6t incompletely quoted,
and try the example with the parent order of Y swapped (i.e. B == Y^2, C
== Y^1)

Recreating X and Y both need to refer to the rewritten B as the parameter
to "merge" insn.  You create X first then you cannot refer to B anymore to
recreate Y.  The other way around you cannot name B to recreate X.

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

Hi,

On Tue, 29 Apr 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Tue, 29 Apr 2008, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >> 
> >> > It is a simple matter of the word "acyclic" in the term "DAG".  It 
> >> > means that whenever you need to refer to a commit, it either comes 
> >> > before or after the commit you need it for, not both directions.
> >> 
> >> I fell in the same "acyclic" fallacy before I realized it was a 
> >> mistake, especially after thought about the "rewritten B needs to be 
> >> used more than twice as a merge source" issue.  That's why I earlier 
> >> said the beauty of your approach is attractive but it "unfortunately" 
> >> breaks down.
> >
> > I do not understand.  The topological order assures that you have 
> > rewritten every commit that needs to be rewritten before rewriting the 
> > current commit.
> 
> Perhaps it would help to go back to the message J6t incompletely quoted, 
> and try the example with the parent order of Y swapped (i.e. B == Y^2, C 
> == Y^1)
> 
> Recreating X and Y both need to refer to the rewritten B as the 
> parameter to "merge" insn.  You create X first then you cannot refer to 
> B anymore to recreate Y.  The other way around you cannot name B to 
> recreate X.

If you refer to "B" as the "short name of the original commit which refers 
to the rewritten commit as soon as B was rewritten", then I really do not 
see the problem.

Every commit has 0..n parents.  These are properly identified before 
rebasing.  Some of them have to be rewritten, because they are rebased.

So if you order the commits topologically, so that ancestors come first, 
you will have to jump around a bit with the "reset" command, but you can 
basically make sure that all parents that needed rewriting were 
rewritten already before rewriting that commit.

Now, if you want to split a commit, you may want to refer to the original 
commit instead of an already rewritten commit, but I think that this 
occasion is rare enough, that we can ask the user to tag that commit, and 
refer to that commit by its tag in the todo list.

Or you write down the original's long name and use that one.

But if you use the _default_ todo list, i.e. you want to rebase preserving 
merges without interfering manually with the process, what I said about 
the topological ordering still holds true.

At no point will you need to refer _both_ to the original _and_ to the 
rewritten commit name.

Come to think of it, I cannot think of a (default) case where the 
_original_ name of a to-be-rewritten commit has to be referred to, except 
for the "pick" command.

Ciao,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Tue, 29 Apr 2008, Junio C Hamano wrote:
>> Perhaps it would help to go back to the message J6t incompletely quoted, 
>> and try the example with the parent order of Y swapped (i.e. B == Y^2, C 
>> == Y^1)
>> 
>> Recreating X and Y both need to refer to the rewritten B as the 
>> parameter to "merge" insn.  You create X first then you cannot refer to 
>> B anymore to recreate Y.  The other way around you cannot name B to 
>> recreate X.
>
> If you refer to "B" as the "short name of the original commit which refers 
> to the rewritten commit as soon as B was rewritten", then I really do not 
> see the problem.

Hmmm.  Perhaps you are thinking about using not just A, B, C but also
names like X, Y, and Z in the insn sequence?  I was operating under the
impression that you used only single parent commits to name things, and a
name will stand for the result of the last operation that used the name
(e.g. after "pick B", B names the result of cherry-picking the original B
to detached HEAD).


                 A
                / \
               /   X
              /   / \
             O---B   Z
              \   \ /
               \   Y
                \ /
                 C

            X = checkout A, merge B
            Y = checkout C, merge B
            Z = checkout X, merge Y

I start from Q, create A', B' and C' with:

	reset Q
	pick A
        reset Q
        pick B
        reset Q
        pick C

Then I can recreate X by doing

	reset A
        merge B

The problem I had was to figure out the way to go back to "rewritten X".
I assumed you would say "B" because that is the last insn in the sequence
that used that name.

But instead you are thinking of letting me just say "X", and somehow make
the machinery guess by noticing "Ah, original X is a merge between
original A and B, and we have a merge between rewritten A and rewritten B,
so we will treat that merge as rewritten "X"?

I actually was hoping we could avoid that, which feels messy.

But now I may be misunderstanding what you meant to say.

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Sixt]

Junio C Hamano schrieb:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>> On Tue, 29 Apr 2008, Junio C Hamano wrote:
>>> Perhaps it would help to go back to the message J6t incompletely quoted, 
>>> and try the example with the parent order of Y swapped (i.e. B == Y^2, C 
>>> == Y^1)
>>>
>>> Recreating X and Y both need to refer to the rewritten B as the 
>>> parameter to "merge" insn.  You create X first then you cannot refer to 
>>> B anymore to recreate Y.  The other way around you cannot name B to 
>>> recreate X.
>> If you refer to "B" as the "short name of the original commit which refers 
>> to the rewritten commit as soon as B was rewritten", then I really do not 
>> see the problem.
> 
> Hmmm.  Perhaps you are thinking about using not just A, B, C but also
> names like X, Y, and Z in the insn sequence?  I was operating under the
> impression that you used only single parent commits to name things, and a
> name will stand for the result of the last operation that used the name
> (e.g. after "pick B", B names the result of cherry-picking the original B
> to detached HEAD).
> 
> 
>                  A
>                 / \
>                /   X
>               /   / \
>              O---B   Z
>               \   \ /
>                \   Y
>                 \ /
>                  C
> 
>             X = checkout A, merge B
>             Y = checkout C, merge B
>             Z = checkout X, merge Y
> 
> I start from Q, create A', B' and C' with:
> 
> 	reset Q
> 	pick A
>         reset Q
>         pick B
>         reset Q
>         pick C
> 
> Then I can recreate X by doing
> 
> 	reset A
>         merge B
> 
> The problem I had was to figure out the way to go back to "rewritten X".
> I assumed you would say "B" because that is the last insn in the sequence
> that used that name.
> 
> But instead you are thinking of letting me just say "X", and somehow make
> the machinery guess by noticing "Ah, original X is a merge between
> original A and B, and we have a merge between rewritten A and rewritten B,
> so we will treat that merge as rewritten "X"?

You had used this notion in your post:

	merge B -- recreate X

Did you mean the '-- recreate X' part as just a comment? I understood it
as part of the instruction, namely to say that the result of the merge is
the rewritten X. In this case you can refer to X in subsequent insns
unambiguously (keep in mind that it is actually the abbreviated SHA1 of
the original merge commit).

-- Hannes

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

>> The problem I had was to figure out the way to go back to "rewritten X".
>> I assumed you would say "B" because that is the last insn in the sequence
>> that used that name.
>> 
>> But instead you are thinking of letting me just say "X", and somehow make
>> the machinery guess by noticing "Ah, original X is a merge between
>> original A and B, and we have a merge between rewritten A and rewritten B,
>> so we will treat that merge as rewritten "X"?
>
> You had used this notion in your post:
>
> 	merge B -- recreate X
>
> Did you mean the '-- recreate X' part as just a comment?

Purely as a comment to let the readers know what I was describing.

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

Hi,

On Tue, 29 Apr 2008, Junio C Hamano wrote:

> 
>                  A
>                 / \
>                /   X
>               /   / \
>              O---B   Z
>               \   \ /
>                \   Y
>                 \ /
>                  C
> 
>             X = checkout A, merge B
>             Y = checkout C, merge B
>             Z = checkout X, merge Y
> 
> I start from Q, create A', B' and C' with:
> 
> 	reset Q
> 	pick A
>         reset Q
>         pick B
>         reset Q
>         pick C
> 
> Then I can recreate X by doing
> 
> 	reset A
>         merge B
> 
> The problem I had was to figure out the way to go back to "rewritten X". 
> I assumed you would say "B" because that is the last insn in the 
> sequence that used that name.
> 
> But instead you are thinking of letting me just say "X", and somehow 
> make the machinery guess by noticing "Ah, original X is a merge between 
> original A and B, and we have a merge between rewritten A and rewritten 
> B, so we will treat that merge as rewritten "X"?
> 
> I actually was hoping we could avoid that, which feels messy.

I cannot bring myself to feel that this is messy.  The more I think about 
it, the clearer it becomes for me that the pick call should use the 
original commit, whereas the merge call should use the rewritten commit 
(and should therefore only be called when all ancestors of that merge 
which need rebasing were rebased already).

BTW I think that I made a stupid mistake in one of my previous mails: when 
I wrote an example for the "merge" command (as I would like it), I did 
_not_ list the original commit name of that merge.  I.e.

	merge <parent2> <parent3>... <message>

I completely forgot that for the $DOTEST/rewritten/ to work, the original 
commit name of that merge has to be listed.

But this got me thinking, and I think that to leave out the first parent 
was another mistake I made, so I really would like to have this syntax:

	merge <orig-commit> <parent1> <parent2>... <message>

This would allow to change the parents in the interactive rebase, and if 
<parent1> is not the current commit at that point, it would implicitly 
call "reset".

What appeals to me is the simplicity of this approach: you refer to the 
commits by calling them by their (original) name.

In the (probably really rare) occasion that you really need to refer to an 
original _and_ a rewritten commit, you can always use _any_ commit-ish as 
argument to the command.

Ciao,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> But instead you are thinking of letting me just say "X", and somehow 
>> make the machinery guess by noticing "Ah, original X is a merge between 
>> original A and B, and we have a merge between rewritten A and rewritten 
>> B, so we will treat that merge as rewritten "X"?
>> 
>> I actually was hoping we could avoid that, which feels messy.
> ...
> But this got me thinking, and I think that to leave out the first parent 
> was another mistake I made, so I really would like to have this syntax:
>
> 	merge <orig-commit> <parent1> <parent2>... <message>
>
> This would allow to change the parents in the interactive rebase, and if 
> <parent1> is not the current commit at that point, it would implicitly 
> call "reset".
>
> What appeals to me is the simplicity of this approach: you refer to the 
> commits by calling them by their (original) name.

Ok, that clears my confusion, but it raises another issue.

In the context of "rebase -i", this may not be a problem, but by forcing
us to name commits always with original commits, we cannot build (instead
of rebuild) a history that does not yet exist using the sequencer
machinery, can we?

If we want to transplant "^C ^N O" in this history elsewhere:

      --o---C---N
               / 
          B---M
         /   /
        O---A

while inserting a new fix-up commit F on top of B before we merge that
side branch to rewritten A:

              --o---C---N'
                       / 
              B'--X---M'
             /       /
        O---Q-------A'

Would/Should the machinery somehow figure out that the merge between the
rewritten A (which is A') and an inserted commit X (which is made on top
of the rewritten B) corresponds to M in the original history?

This is not a made up example, but something I have to do once (on my
non-git days) or many more times (on my git days) every day when
rebuilding 'pu' on top of updated 'next' using updated tips of topic
branches.  I was hoping that the sequencer mechanism can help me
automating the process a bit more.

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Sixt]

Junio C Hamano schrieb:
> In the context of "rebase -i", this may not be a problem, but by forcing
> us to name commits always with original commits, we cannot build (instead
> of rebuild) a history that does not yet exist using the sequencer
> machinery, can we?
> 
> If we want to transplant "^C ^N O" in this history elsewhere:
> 
>       --o---C---N
>                / 
>           B---M
>          /   /
>         O---A
> 
> while inserting a new fix-up commit F on top of B before we merge that
> side branch to rewritten A:
> 
>               --o---C---N'
>                        / 
>               B'--X---M'
>              /       /
>         O---Q-------A'
> 
> Would/Should the machinery somehow figure out that the merge between the
> rewritten A (which is A') and an inserted commit X (which is made on top
> of the rewritten B) corresponds to M in the original history?

No. The 'merge' insn tells the original merge commit. So, rebase would
suggest this todo list:

	pick B
	reset Q
	pick A
	merge M A B
	merge N M C

and you would have to change it to

	pick B
	pick X       <--
	reset Q
	pick A
	merge M A X  <--
	merge N M C

But you could just as well do this:

	edit B       <--
	reset Q
	pick A
	merge M A B
	merge N M C

When the 'edit B' stops, HEAD is at B'. Now you git-am X, then 'rebase
--continue'. Since now HEAD is at X, X is recorded as the rewritten
version of B, and the 'merge M A B' would pick X as the second parent.

-- Hannes

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

Hi,

On Wed, 30 Apr 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> But instead you are thinking of letting me just say "X", and somehow 
> >> make the machinery guess by noticing "Ah, original X is a merge between 
> >> original A and B, and we have a merge between rewritten A and rewritten 
> >> B, so we will treat that merge as rewritten "X"?
> >> 
> >> I actually was hoping we could avoid that, which feels messy.
> > ...
> > But this got me thinking, and I think that to leave out the first parent 
> > was another mistake I made, so I really would like to have this syntax:
> >
> > 	merge <orig-commit> <parent1> <parent2>... <message>
> >
> > This would allow to change the parents in the interactive rebase, and if 
> > <parent1> is not the current commit at that point, it would implicitly 
> > call "reset".
> >
> > What appeals to me is the simplicity of this approach: you refer to the 
> > commits by calling them by their (original) name.
> 
> Ok, that clears my confusion, but it raises another issue.
> 
> In the context of "rebase -i", this may not be a problem, but by forcing
> us to name commits always with original commits, we cannot build (instead
> of rebuild) a history that does not yet exist using the sequencer
> machinery, can we?

The idea I hinted at was to refer to them by another name than the short 
name.  Then we can use the sequencer machinery.

I still maintain that it is such a rare need (even if you are a power user 
of it) that it makes sense to cater for other, simpler uses.

Ciao,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> The idea I hinted at was to refer to them by another name than the short 
> name.  Then we can use the sequencer machinery.
>
> I still maintain that it is such a rare need (even if you are a power user 
> of it) that it makes sense to cater for other, simpler uses.

As usual, I am greedy and I would want to have both supported in such a
way that (1) simple things are simple and (2) the language is expressive
enough that complex things are possible.  And I try to stress that while
we are still in the drawing board phase, because it would be painful to
change once we start with a language without enough expressiveness.

And that was where my "Can the approach to use the original commit ID to
stand for rewritten one express everything we would want to do in the
future, not just limited to 'rebase -i -p'" series of questions came
from.

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

Hi,

On Thu, 1 May 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > The idea I hinted at was to refer to them by another name than the short 
> > name.  Then we can use the sequencer machinery.
> >
> > I still maintain that it is such a rare need (even if you are a power user 
> > of it) that it makes sense to cater for other, simpler uses.
> 
> As usual, I am greedy and I would want to have both supported in such a
> way that (1) simple things are simple and (2) the language is expressive
> enough that complex things are possible.

Agreed.

> And I try to stress that while we are still in the drawing board phase, 
> because it would be painful to change once we start with a language 
> without enough expressiveness.

Unfortunately, we are no longer in the drawing board phase, because the 
offending code is already in 'next'.

> And that was where my "Can the approach to use the original commit ID to 
> stand for rewritten one express everything we would want to do in the 
> future, not just limited to 'rebase -i -p'" series of questions came 
> from.

I maintain that it can.  Because you can _still_ refer to the original 
commit name quite easily: just take one more letter.

In the meantime I thought about the "<commit name>'" approach (note the 
single apostrophe at the end), though, and it seems that this would not 
be too involved.

But hey, the code is so ugly and complicated by now, that I avert my eyes 
to other tasks.

Ciao,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

>> And I try to stress that while we are still in the drawing board phase, 
>> because it would be painful to change once we start with a language 
>> without enough expressiveness.
>
> Unfortunately, we are no longer in the drawing board phase, because the 
> offending code is already in 'next'.

What does that mean?  "Now we are committed to it, so I will stop
complaining and work within the overall design in a more constructive
way"?

We have precedence of even reverting the whole series from 'next'. When
there is a better design available, I think it is perfectly fine to
improve what is in 'next' in a way that is not backward compatible with
stuff that has not hit 'master'.  That is what 'next' is for.

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

Hi,

On Sat, 3 May 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> >> And I try to stress that while we are still in the drawing board 
> >> phase, because it would be painful to change once we start with a 
> >> language without enough expressiveness.
> >
> > Unfortunately, we are no longer in the drawing board phase, because 
> > the offending code is already in 'next'.
> 
> What does that mean?  "Now we are committed to it, so I will stop 
> complaining and work within the overall design in a more constructive 
> way"?

Fine.

I think that the "mark" mechanism is a fine thing for scripts that import 
into Git.  There, the limitation to integers in a certain range does not 
hurt.

The "mark" mechanism could even be used to implement a user-friendly 
rebase -i -p, but I think that _exposing_ it is a mistake.

Sure, I could imagine that _editing_ a list of commits, you want to mark a 
commit (why not "tag", which would be much more consistent with the rest 
of Git?), but name it something human-readable, such as

	pick 1234567 Clean up rebase -i -p
	tag cleanup
	...
	merge 2345678 cleanup master

Yes, you read that correctly, I think that allowing plain ref names is 
very valuable.  AFAICT my original implementation allows that (dunno about 
the current code).

I actually grow fonder and fonder of the ' idea (rewritten commits can be 
referenced by their short commit name with a single apostroph appended, 
and if that commit was not rewritten at all, it falls back to the original 
commit).

That should please you, and the other guy commenting on the "magic".

In the meantime I am also convinced that it could be implemented in an 
elegant way.

Ciao,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Jörg Sommer]

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

Hi,

Johannes Schindelin schrieb am Sun 04. May, 10:38 (+0100):
> 	pick 1234567 Clean up rebase -i -p
> 	tag cleanup
> 	...
> 	merge 2345678 cleanup master
> 
> Yes, you read that correctly, I think that allowing plain ref names is 
> very valuable.  AFAICT my original implementation allows that (dunno about 
> the current code).

Your code is still used and you can do things like:

merge abc --strategy=YOUR_STRATEGY master 022ef38 my_tag

And I think it's really easy to do a git-describe for each ref that is not
marked of reset and merge commands

From fec670a4dc6fdf484a197687db5fbd0a4f668449 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?J=C3=B6rg=20Sommer?= <joerg@alea.gnuu.de>
Date: Sun, 4 May 2008 14:52:01 +0200
Subject: [PATCH] Use names instead of sha1 ID for reset and merge if possible
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit

Sometimes it's much more helpful to see the name of a branch or a tag as
the argument of a merge or reset command.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 2a01182..2bd858c 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -493,6 +493,10 @@ create_extended_todo_list () {
 			if tmp=$(get_value_from_list $args "$commit_mark_list")
 			then
 				args=":$tmp"
+			elif tmp=$(git describe --all --exact-match $args \
+				2>/dev/null)
+			then
+				args="$tmp"
 			fi
 			;;
 		merge)
@@ -503,6 +507,10 @@ create_extended_todo_list () {
 					"$commit_mark_list")
 				then
 					new_args="$new_args :$tmp"
+				elif tmp=$(git describe --all --exact-match \
+					$i 2>/dev/null)
+				then
+					new_args="$new_args $tmp"
 				else
 					new_args="$new_args $i"
 				fi
-- 
1.5.5.1

Bye, Jörg.
-- 
Diskutiere nie mit einem Idioten:
Sie ziehen Dich auf ihr Niveau herab und schlagen Dich dann mit
Erfahrung.

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Dmitry Potapov]

On Wed, Apr 30, 2008 at 09:47:02AM +0100, Johannes Schindelin wrote:
> 
> I cannot bring myself to feel that this is messy.  The more I think about 
> it, the clearer it becomes for me that the pick call should use the 
> original commit, whereas the merge call should use the rewritten commit 
> (and should therefore only be called when all ancestors of that merge 
> which need rebasing were rebased already).
>

Maybe, it would be better if re-written commits were marked a bit
differently, so there will be no confusion about whether an original
or re-written commit is referred. For instance, re-written commits can
be marked by adding apostrophe at the end, so if the original commit
was "abcdef" then the re-written should be called as "abcdef'". At
least, it will make plain clear for anyone where in merge rewritten
commits are mentioned. Otherwise, it looks too magical to me.

Dmitry

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Johannes Schindelin]

Hi,

On Wed, 30 Apr 2008, Dmitry Potapov wrote:

> On Wed, Apr 30, 2008 at 09:47:02AM +0100, Johannes Schindelin wrote:
> > 
> > I cannot bring myself to feel that this is messy.  The more I think about 
> > it, the clearer it becomes for me that the pick call should use the 
> > original commit, whereas the merge call should use the rewritten commit 
> > (and should therefore only be called when all ancestors of that merge 
> > which need rebasing were rebased already).
> >
> 
> Maybe, it would be better if re-written commits were marked a bit 
> differently, so there will be no confusion about whether an original or 
> re-written commit is referred. For instance, re-written commits can be 
> marked by adding apostrophe at the end, so if the original commit was 
> "abcdef" then the re-written should be called as "abcdef'". At least, it 
> will make plain clear for anyone where in merge rewritten commits are 
> mentioned. Otherwise, it looks too magical to me.

Fair enough.  (For the "too magical" part.)

But it would break down if you picked one commit twice, in order to split 
it.  OTOH, this is a rare thing, and you really only need to refer to 
rewritten commits in the "reset" and "merge" commands.

But there is a bigger problem with what you suggest: When merging a commit 
that is _not_ in the rewritten part of the history, adding an apostrophe 
is actively wrong.

And I still believe strongly that a regular "rebase -i -p" user will not 
want to refer to original commits, except for the "pick" command.

Ciao,
Dscho

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Junio C Hamano]

Jörg Sommer <joerg@alea.gnuu.de> writes:

> What do you think about this version:
>
> cleanup_before_quit () {
> 	rm -rf "$DOTEST" &&
> 	for ref in $(git for-each-ref --format='%(refname)' ${mark_prefix%/})
> 	do
> 		git update-ref -d "$ref" "$ref" || return 1
> 	done
> }

Yeah, except you would want to dqquote "${mark_prefix%/}" part.

Also this being a "clean-up" phase, I wonder if we want to stop at the
first error (e.g. should unremovable "$DOTEST" leave marks behind?  should
unremovable one mark leave other marks that happen to sort after it
behind?).

Re: [PATCH v2 04/13] Teach rebase interactive the mark command

[Jörg Sommer]

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

Hi Junio,

Junio C Hamano schrieb am Tue 22. Apr, 11:04 (-0700):
> Jörg Sommer <joerg@alea.gnuu.de> writes:
> 
> > What do you think about this version:
> >
> > cleanup_before_quit () {
> > 	rm -rf "$DOTEST" &&
> > 	for ref in $(git for-each-ref --format='%(refname)' ${mark_prefix%/})
> > 	do
> > 		git update-ref -d "$ref" "$ref" || return 1
> > 	done
> > }
> 
> Yeah, except you would want to dqquote "${mark_prefix%/}" part.

Oh, yes.

> Also this being a "clean-up" phase, I wonder if we want to stop at the
> first error (e.g. should unremovable "$DOTEST" leave marks behind?

I think it should be the other way: unremovable marks should leave the
DOTEST behind. This way a rebase should refuse to start a new session and
stumble accross the old marks and it's possible to run git rebase --abort
after manually removing the marks.

Bye, Jörg.
-- 
Damit das Mögliche entsteht, muß immer wieder das Unmögliche versucht
werden.                                       (Hermann Hesse)

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[PATCH v2.2] Teach rebase interactive the mark command

[Jörg Sommer]

This new command can be used to set symbolic marks for an commit while
doing a rebase. This symbolic name can later be used for merges or
resets.

The decision to use references for the marks and not files like done with
the rewritten commits for preserve merges was made to ensure no commit
objects get lost if prune is started while (a long term) rebase is
running. This also unifies the checking of the validity of marks and
references by using rev-parse for it.

The format of the marks is as close as possible to the format of the
marks used by fast-export and fast-import, i.e. :001 == :1 and “:12a” is
an invalid mark. It differs from the format of fast-import in that point
that the colon is not required after the mark command, i.e. “mark 12” is
the same as “mark :12”. This should ease the writing of commads.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh    |   31 +++++++++++++++++++++++++++++++
 t/t3404-rebase-interactive.sh |   17 +++++++++++++++++
 2 files changed, 48 insertions(+), 0 deletions(-)

That's the diff to the last version:
>diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>index eaf5563..1751b08 100755
>--- a/git-rebase--interactive.sh
>+++ b/git-rebase--interactive.sh
>@@ -105,13 +105,11 @@ die_with_patch () {
> }
> 
> cleanup_before_quit () {
>-	rm -rf "$DOTEST" &&
>-	for ref in "$GIT_DIR/$mark_prefix"*
>+	for ref in $(git for-each-ref --format='%(refname)' "${mark_prefix%/}")
> 	do
>-		test "$ref" = "$GIT_DIR/$mark_prefix*" && continue
>-		git update-ref -d "${ref#$GIT_DIR/}" "${ref#$GIT_DIR/}" || \
>-			return 1
>+		git update-ref -d "$ref" "$ref" || return 1
> 	done
>+	rm -rf "$DOTEST"
> }
> 
> die_abort () {
>@@ -194,14 +192,12 @@ peek_next_command () {
> }
> 
> mark_to_ref () {
>-	case "$1" in
>-	:[0-9]*)
>-		echo "$mark_prefix$(printf %d ${1#:} 2>/dev/null)"
>-		;;
>-	*)
>+	if expr match "$1" "^:[0-9][0-9]*$" >/dev/null
>+	then
>+		echo "$mark_prefix$(printf %d ${1#:})"
>+	else
> 		echo "$1"
>-		;;
>-	esac
>+	fi
> }
> 
> do_next () {
>@@ -285,6 +281,8 @@ do_next () {
> 		mark_action_done
> 
> 		mark=$(mark_to_ref :${sha1#:})
>+		test :${sha1#:} = "$mark" && die "Invalid mark '$sha1'"
>+
> 		git rev-parse --verify "$mark" > /dev/null 2>&1 && \
> 			warn "mark $sha1 already exist; overwriting it"
> 


diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 531ee94..c0abc01 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -35,6 +35,8 @@ mark the corrected paths with 'git add <paths>', and
 run 'git rebase --continue'"
 export GIT_CHERRY_PICK_HELP
 
+mark_prefix=refs/rebase-marks/
+
 warn () {
 	echo "$*" >&2
 }
@@ -105,6 +107,10 @@ die_with_patch () {
 }
 
 cleanup_before_quit () {
+	for ref in $(git for-each-ref --format='%(refname)' "${mark_prefix%/}")
+	do
+		git update-ref -d "$ref" "$ref" || return 1
+	done
 	rm -rf "$DOTEST"
 }
 
@@ -244,6 +250,15 @@ peek_next_command () {
 	sed -n "1s/ .*$//p" < "$TODO"
 }
 
+mark_to_ref () {
+	if expr match "$1" "^:[0-9][0-9]*$" >/dev/null
+	then
+		echo "$mark_prefix$(printf %d ${1#:})"
+	else
+		echo "$1"
+	fi
+}
+
 do_next () {
 	rm -f "$DOTEST"/message "$DOTEST"/author-script \
 		"$DOTEST"/amend || exit
@@ -321,6 +336,17 @@ do_next () {
 			die_with_patch $sha1 ""
 		fi
 		;;
+	mark)
+		mark_action_done
+
+		mark=$(mark_to_ref :${sha1#:})
+		test :${sha1#:} = "$mark" && die "Invalid mark '$sha1'"
+
+		git rev-parse --verify "$mark" > /dev/null 2>&1 && \
+			warn "mark $sha1 already exist; overwriting it"
+
+		git update-ref "$mark" HEAD || die "update-ref failed"
+		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
 		die_with_patch $sha1 "Please fix this in the file $TODO."
@@ -533,10 +559,15 @@ do
 
 # Rebase $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO
 #
+# In the todo insn whenever you need to refer to a commit, in addition
+# to the usual commit object name, you can use ':mark' syntax to refer
+# to a commit previously marked with the 'mark' insn.
+#
 # Commands:
 #  pick = use commit
 #  edit = use commit, but stop for amending
 #  squash = use commit, but meld into previous commit
+#  mark :mark = mark the current HEAD for later reference
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8d29878..fa3560e 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -82,6 +82,9 @@ for line in $FAKE_LINES; do
 	case $line in
 	squash|edit)
 		action="$line";;
+	mark*)
+		echo "mark ${line#mark}"
+		echo "mark ${line#mark}" >> "$1";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
 		sed -n "${line}p" < "$1".tmp
@@ -189,6 +192,20 @@ test_expect_success '-p handles "no changes" gracefully' '
 	test $HEAD = $(git rev-parse HEAD)
 '
 
+test_expect_success 'setting marks works' '
+	git checkout master &&
+	FAKE_LINES="mark:0 2 1 mark:42 3 edit 4" git rebase -i HEAD~4 &&
+	marks_dir=.git/refs/rebase-marks &&
+	test -d $marks_dir &&
+	test $(ls $marks_dir | wc -l) -eq 2 &&
+	test "$(git rev-parse HEAD~4)" = \
+		"$(git rev-parse refs/rebase-marks/0)" &&
+	test "$(git rev-parse HEAD~2)" = \
+		"$(git rev-parse refs/rebase-marks/42)" &&
+	git rebase --abort &&
+	ls $marks_dir | wc -l | grep -Fx 0
+'
+
 test_expect_success 'preserve merges with -p' '
 	git checkout -b to-be-preserved master^ &&
 	: > unrelated-file &&
-- 
1.5.5.1

Re: [PATCH v2.2] Teach rebase interactive the mark command

[Junio C Hamano]

Thanks; as I already queued the series to 'next', I took the liberty of
applying the interdiff as a separate fix-up/improvement commit to the tip
of the topic.

Re: [PATCH v2.2] Teach rebase interactive the mark command

[Jörg Sommer]

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

Hallo Junio,

Junio C Hamano schrieb am Sat 26. Apr, 23:13 (-0700):
> Thanks; as I already queued the series to 'next', I took the liberty of
> applying the interdiff as a separate fix-up/improvement commit to the tip
> of the topic.

That's alright.

Bye, Jörg.
-- 
Je planmäßiger ein Mensch vorgeht,
desto stärker mag ihn der Zufall treffen.
		    Erich Krunau ‚Die Physiker‘

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[PATCH v2.1] Teach rebase interactive the mark command

[Jörg Sommer]

This new command can be used to set symbolic marks for an commit while
doing a rebase. This symbolic name can later be used for merges or
resets.

The decision to use references for the marks and not files like done with
the rewritten commits for preserve merges was made to ensure no commit
objects get lost if prune is started while (a long term) rebase is
running. This also unifies the checking of the validity of marks and
references by using rev-parse for it.

The format of the marks is as close as possible to the format of the
marks used by fast-export and fast-import, i.e. :001 == :1 and
“:12a” == :12. It differs from the format of fast-import in that point
that it requires a digit after the colon, i.e. “:abc” != :0 and “:-12”
and “:+12” aren't allowed.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 git-rebase--interactive.sh    |   35 ++++++++++++++++++++++++++++++++++-
 t/t3404-rebase-interactive.sh |   17 +++++++++++++++++
 2 files changed, 51 insertions(+), 1 deletions(-)

The difference to the v2 patch is the definition of the mark as
discussed with "Shawn O. Pearce"

>diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
>index 6ac316a..05d04da 100755
>--- a/git-rebase--interactive.sh
>+++ b/git-rebase--interactive.sh
>@@ -254,10 +254,8 @@ peek_next_command () {
> 
> mark_to_ref () {
>        case "$1" in
>-       :[!/]*)
>-               # :/SOMETHING is a reference for the last commit whose
>-                # message starts with SOMETHING
>-               echo "$mark_prefix${1#:}"
>+       :[0-9]*)
>+               echo "$mark_prefix$(printf %d ${1#:} 2>/dev/null)"
>                ;;
>        *)
>                echo "$1"
>

diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
index 531ee94..05d04da 100755
--- a/git-rebase--interactive.sh
+++ b/git-rebase--interactive.sh
@@ -35,6 +35,8 @@ mark the corrected paths with 'git add <paths>', and
 run 'git rebase --continue'"
 export GIT_CHERRY_PICK_HELP
 
+mark_prefix=refs/rebase-marks/
+
 warn () {
 	echo "$*" >&2
 }
@@ -105,7 +107,13 @@ die_with_patch () {
 }
 
 cleanup_before_quit () {
-	rm -rf "$DOTEST"
+	rm -rf "$DOTEST" &&
+	for ref in "$GIT_DIR/$mark_prefix"*
+	do
+		test "$ref" = "$GIT_DIR/$mark_prefix*" && continue
+		git update-ref -d "${ref#$GIT_DIR/}" "${ref#$GIT_DIR/}" || \
+			return 1
+	done
 }
 
 die_abort () {
@@ -244,6 +252,17 @@ peek_next_command () {
 	sed -n "1s/ .*$//p" < "$TODO"
 }
 
+mark_to_ref () {
+	case "$1" in
+	:[0-9]*)
+		echo "$mark_prefix$(printf %d ${1#:} 2>/dev/null)"
+		;;
+	*)
+		echo "$1"
+		;;
+	esac
+}
+
 do_next () {
 	rm -f "$DOTEST"/message "$DOTEST"/author-script \
 		"$DOTEST"/amend || exit
@@ -321,6 +340,15 @@ do_next () {
 			die_with_patch $sha1 ""
 		fi
 		;;
+	mark)
+		mark_action_done
+
+		mark=$(mark_to_ref :${sha1#:})
+		git rev-parse --verify "$mark" > /dev/null 2>&1 && \
+			warn "mark $sha1 already exist; overwriting it"
+
+		git update-ref "$mark" HEAD || die "update-ref failed"
+		;;
 	*)
 		warn "Unknown command: $command $sha1 $rest"
 		die_with_patch $sha1 "Please fix this in the file $TODO."
@@ -533,10 +561,15 @@ do
 
 # Rebase $SHORTUPSTREAM..$SHORTHEAD onto $SHORTONTO
 #
+# In the todo insn whenever you need to refer to a commit, in addition
+# to the usual commit object name, you can use ':mark' syntax to refer
+# to a commit previously marked with the 'mark' insn.
+#
 # Commands:
 #  pick = use commit
 #  edit = use commit, but stop for amending
 #  squash = use commit, but meld into previous commit
+#  mark :mark = mark the current HEAD for later reference
 #
 # If you remove a line here THAT COMMIT WILL BE LOST.
 # However, if you remove everything, the rebase will be aborted.
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 8d29878..fa3560e 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -82,6 +82,9 @@ for line in $FAKE_LINES; do
 	case $line in
 	squash|edit)
 		action="$line";;
+	mark*)
+		echo "mark ${line#mark}"
+		echo "mark ${line#mark}" >> "$1";;
 	*)
 		echo sed -n "${line}s/^pick/$action/p"
 		sed -n "${line}p" < "$1".tmp
@@ -189,6 +192,20 @@ test_expect_success '-p handles "no changes" gracefully' '
 	test $HEAD = $(git rev-parse HEAD)
 '
 
+test_expect_success 'setting marks works' '
+	git checkout master &&
+	FAKE_LINES="mark:0 2 1 mark:42 3 edit 4" git rebase -i HEAD~4 &&
+	marks_dir=.git/refs/rebase-marks &&
+	test -d $marks_dir &&
+	test $(ls $marks_dir | wc -l) -eq 2 &&
+	test "$(git rev-parse HEAD~4)" = \
+		"$(git rev-parse refs/rebase-marks/0)" &&
+	test "$(git rev-parse HEAD~2)" = \
+		"$(git rev-parse refs/rebase-marks/42)" &&
+	git rebase --abort &&
+	ls $marks_dir | wc -l | grep -Fx 0
+'
+
 test_expect_success 'preserve merges with -p' '
 	git checkout -b to-be-preserved master^ &&
 	: > unrelated-file &&
-- 
1.5.5

Re: [PATCH v2.1] Teach rebase interactive the mark command

[Shawn O. Pearce]

Jrg Sommer <joerg@alea.gnuu.de> wrote:
> The format of the marks is as close as possible to the format of the
> marks used by fast-export and fast-import,

Yay.

> i.e. :001 == :1 and
> “:12a” == :12. It differs from the format of fast-import in that point
> that it requires a digit after the colon, i.e. “:abc” != :0 and “:-12”
> and “:+12” aren't allowed.

Uh, that's a bug in fast-import.  ":4abc" is _not_ a mark if you
read the language specification.  Only ":4" is a mark.  So we are
accepting crap and reading it in odd ways.  Not good.

-- 
Shawn.

mark parsing in fast-import

[Jörg Sommer]

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

Hallo Shawn,

Shawn O. Pearce schrieb am Mon 14. Apr, 19:29 (-0400):
> Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > The format of the marks is as close as possible to the format of the
> > marks used by fast-export and fast-import,
> 
> Yay.
> 
> > i.e. :001 == :1 and
> > “:12a” == :12. It differs from the format of fast-import in that point
> > that it requires a digit after the colon, i.e. “:abc” != :0 and “:-12”
> > and “:+12” aren't allowed.
> 
> Uh, that's a bug in fast-import.  ":4abc" is _not_ a mark if you
> read the language specification.  Only ":4" is a mark.  So we are
> accepting crap and reading it in odd ways.  Not good.

What about this:

diff --git a/fast-import.c b/fast-import.c
index 73e5439..f60e4ab 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1690,10 +1690,31 @@ static void skip_optional_lf(void)
 		ungetc(term_char, stdin);
 }
 
+static inline int parse_mark(const const char *str, uintmax_t* mark,
+	char **after_mark)
+{
+	if (!str || str[0] != ':' || !isdigit(str[1]))
+		return 1;
+
+	char *am;
+	const uintmax_t m = strtoumax(&str[1], &am, 10);
+	if (errno == 0) {
+		*mark = m;
+		*after_mark = am;
+		return 0;
+	}
+	return 1;
+}
+
 static void cmd_mark(void)
 {
-	if (!prefixcmp(command_buf.buf, "mark :")) {
-		next_mark = strtoumax(command_buf.buf + 6, NULL, 10);
+	uintmax_t mark = 0;
+	char *after_mark = NULL;
+
+	if (!prefixcmp(command_buf.buf, "mark ") &&
+		parse_mark(&command_buf.buf[5], &mark, &after_mark) &&
+		*after_mark == '\0') {
+		next_mark = mark;
 		read_next_command();
 	}
 	else
@@ -1878,7 +1899,10 @@ static void file_change_m(struct branch *b)
 
 	if (*p == ':') {
 		char *x;
-		oe = find_mark(strtoumax(p + 1, &x, 10));
+		uintmax_t m;
+		if (parse_mark(p, &m, &x))
+			die("Invalid mark: %s", p);
+		oe = find_mark(m);
 		hashcpy(sha1, oe->sha1);
 		p = x;
 	} else if (!prefixcmp(p, "inline")) {
@@ -2045,7 +2069,11 @@ static int cmd_from(struct branch *b)
 		hashcpy(b->branch_tree.versions[0].sha1, t);
 		hashcpy(b->branch_tree.versions[1].sha1, t);
 	} else if (*from == ':') {
-		uintmax_t idnum = strtoumax(from + 1, NULL, 10);
+		char *after_mark;
+		uintmax_t idnum;
+		if (parse_mark(from, &idnum, &after_mark) ||
+			*after_mark != '\0')
+			die("Not a valid mark: %s", from);
 		struct object_entry *oe = find_mark(idnum);
 		if (oe->type != OBJ_COMMIT)
 			die("Mark :%" PRIuMAX " not a commit", idnum);
@@ -2080,7 +2108,11 @@ static struct hash_list *cmd_merge(unsigned int *count)
 		if (s)
 			hashcpy(n->sha1, s->sha1);
 		else if (*from == ':') {
-			uintmax_t idnum = strtoumax(from + 1, NULL, 10);
+			char *after_mark;
+			uintmax_t idnum;
+			if (parse_mark(from, &idnum, &after_mark) ||
+				*after_mark != '\0')
+				die("Not a valid mark: %s", from);
 			struct object_entry *oe = find_mark(idnum);
 			if (oe->type != OBJ_COMMIT)
 				die("Mark :%" PRIuMAX " not a commit", idnum);
@@ -2228,7 +2260,10 @@ static void cmd_new_tag(void)
 		hashcpy(sha1, s->sha1);
 	} else if (*from == ':') {
 		struct object_entry *oe;
-		from_mark = strtoumax(from + 1, NULL, 10);
+		char *after_mark;
+		if (parse_mark(from, &from_mark, &after_mark) ||
+			*after_mark != '\0')
+			die("Not a valid mark: %s", from);
 		oe = find_mark(from_mark);
 		if (oe->type != OBJ_COMMIT)
 			die("Mark :%" PRIuMAX " not a commit", from_mark);
@@ -2333,9 +2368,8 @@ static void import_marks(const char *input_file)
 		if (line[0] != ':' || !end)
 			die("corrupt mark line: %s", line);
 		*end = 0;
-		mark = strtoumax(line + 1, &end, 10);
-		if (!mark || end == line + 1
-			|| *end != ' ' || get_sha1(end + 1, sha1))
+		if (parse_mark(line, &mark, &end) || !mark ||
+			*end != ' ' || get_sha1(end + 1, sha1))
 			die("corrupt mark line: %s", line);
 		e = find_object(sha1);
 		if (!e) {

Bye, Jörg.
-- 
Wer eher stirbt ist länger tot.
    	 	    	       			(Un B. Kant)

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: mark parsing in fast-import

[Shawn O. Pearce]

Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > 
> > Uh, that's a bug in fast-import.  ":4abc" is _not_ a mark if you
> > read the language specification.  Only ":4" is a mark.  So we are
> > accepting crap and reading it in odd ways.  Not good.
> 
> What about this:
> 
> diff --git a/fast-import.c b/fast-import.c
> index 73e5439..f60e4ab 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1690,10 +1690,31 @@ static void skip_optional_lf(void)
>  		ungetc(term_char, stdin);
>  }
>  
> +static inline int parse_mark(const const char *str, uintmax_t* mark,
> +	char **after_mark)
> +{
> +	if (!str || str[0] != ':' || !isdigit(str[1]))
> +		return 1;
> +
> +	char *am;

Although we conform to mostly C99 style, variables should be
declared at the top of the scope and not after a statement.

> +	const uintmax_t m = strtoumax(&str[1], &am, 10);
> +	if (errno == 0) {
> +		*mark = m;
> +		*after_mark = am;
> +		return 0;
> +	}
> +	return 1;
> +}
> +
>  static void cmd_mark(void)
>  {
> -	if (!prefixcmp(command_buf.buf, "mark :")) {
> -		next_mark = strtoumax(command_buf.buf + 6, NULL, 10);
> +	uintmax_t mark = 0;
> +	char *after_mark = NULL;
> +
> +	if (!prefixcmp(command_buf.buf, "mark ") &&
> +		parse_mark(&command_buf.buf[5], &mark, &after_mark) &&

Hmm.  Shouldn't this be ! parse_mark given that it returns 0
on success and 1 on failure?


-- 
Shawn.

Re: mark parsing in fast-import

[Jörg Sommer]

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

Hi Shawn,

Shawn O. Pearce schrieb am Sun 20. Apr, 20:26 (-0400):
> Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > +static inline int parse_mark(const const char *str, uintmax_t* mark,

Is inline okay?

> > +	char **after_mark)
> > +{
> > +	if (!str || str[0] != ':' || !isdigit(str[1]))
> > +		return 1;
> > +
> > +	char *am;
> 
> Although we conform to mostly C99 style, variables should be
> declared at the top of the scope and not after a statement.

Changed.

> >  static void cmd_mark(void)
> >  {
> > -	if (!prefixcmp(command_buf.buf, "mark :")) {
> > -		next_mark = strtoumax(command_buf.buf + 6, NULL, 10);
> > +	uintmax_t mark = 0;
> > +	char *after_mark = NULL;
> > +
> > +	if (!prefixcmp(command_buf.buf, "mark ") &&
> > +		parse_mark(&command_buf.buf[5], &mark, &after_mark) &&
> 
> Hmm.  Shouldn't this be ! parse_mark given that it returns 0
> on success and 1 on failure?

Yes, you're right. I've checked some other functions and found this
behaviour. Can I use a different behabiour, i.e. return 0 on failure and
!0 on success?

Bye, Jörg.
-- 
„Wer im Usenet gelesen werden will, sollte leserorientiert schreiben. Wer nur
 für sich schreiben will, dem ist mit einem Tagebuch vielleicht besser
 geholfen. Gelesen zu werden ist kein Recht, sondern ein Privileg.“
     Thore Tams in <90tfv8$49b$1@keks.kruemel.dyndns.org>

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: mark parsing in fast-import

[Shawn O. Pearce]

Jrg Sommer <joerg@alea.gnuu.de> wrote:
> Shawn O. Pearce schrieb am Sun 20. Apr, 20:26 (-0400):
> > Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > > +static inline int parse_mark(const const char *str, uintmax_t* mark,
> 
> Is inline okay?

Yea, inline is fine.  We use "static inline" often in Git when it
is a good idea.
 
> > >  static void cmd_mark(void)
> > >  {
> > > -	if (!prefixcmp(command_buf.buf, "mark :")) {
> > > -		next_mark = strtoumax(command_buf.buf + 6, NULL, 10);
> > > +	uintmax_t mark = 0;
> > > +	char *after_mark = NULL;
> > > +
> > > +	if (!prefixcmp(command_buf.buf, "mark ") &&
> > > +		parse_mark(&command_buf.buf[5], &mark, &after_mark) &&
> > 
> > Hmm.  Shouldn't this be ! parse_mark given that it returns 0
> > on success and 1 on failure?
> 
> Yes, you're right. I've checked some other functions and found this
> behaviour. Can I use a different behabiour, i.e. return 0 on failure and
> !0 on success?

I wasn't objected to the return values as written, but more to the
fact that it seemed like a logic error to me.  We use both patterns
in Git.  Perhaps the best example to follow is get_sha1_hex();
it returns -1 on error and 0 on success.  So a common pattern is
"!get_sha1_hex()" to ensure a successful conversion of a hex string
to an unsigned char array.

-- 
Shawn.

Re: mark parsing in fast-import

[Jörg Sommer]

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

Hallo Shawn,

Shawn O. Pearce schrieb am Mon 21. Apr, 19:59 (-0400):
> Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > Yes, you're right. I've checked some other functions and found this
> > behaviour. Can I use a different behabiour, i.e. return 0 on failure and
> > !0 on success?
> 
> I wasn't objected to the return values as written, but more to the
> fact that it seemed like a logic error to me.  We use both patterns
> in Git.  Perhaps the best example to follow is get_sha1_hex();
> it returns -1 on error and 0 on success.  So a common pattern is
> "!get_sha1_hex()" to ensure a successful conversion of a hex string
> to an unsigned char array.

Thanks for this explanation. This was what I was looking for.

Another question: Is :0 a valid mark? In import_marks() is a check for
!mark, but I haven't seen it anywhere else.

Bye, Jörg.
-- 
Du hast keine Chance – also nutze sie.

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: mark parsing in fast-import

[Shawn O. Pearce]

Jrg Sommer <joerg@alea.gnuu.de> wrote:
> 
> Another question: Is :0 a valid mark? In import_marks() is a check for
> !mark, but I haven't seen it anywhere else.

No, in fast-import ":0" is _not_ a valid mark.  We burn the first
entry in the marks table (always leaving it empty) as then we can
use the idiom "!mark" to say "no mark was requested/given" and
"mark" to say "mark was requested/given".  Hence we do not need an
extra flag to tell us either way.

Given that a mark is just a pointer, and that extra flag would
likely have been a global "static int have_mark" or some such it
works out to be about the same amount of memory - 4 or 8 bytes.
No big deal, and the code is probably easier to follow as a result.

-- 
Shawn.

[PATCH v2] Make mark parsing much more restrictive

[Jörg Sommer]

The current implementation of mark parsing doesn't care for trailing
garbage like in :12a and doesn't check for unsigned numbers, i.e. it
accepts :-12 as a valid mark.

This patch enforces a number follows the colon and there comes nothing
after the bignum.

Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
---
 fast-import.c |   49 ++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 38 insertions(+), 11 deletions(-)

"Shawn O. Pearce" <spearce@spearce.org> wrote:
> Jrg Sommer <joerg@alea.gnuu.de> wrote:
> >
> > Another question: Is :0 a valid mark? In import_marks() is a check for
> > !mark, but I haven't seen it anywhere else.
> 
> No, in fast-import ":0" is _not_ a valid mark.

Then I propose the following patch.

diff --git a/fast-import.c b/fast-import.c
index 73e5439..0c71da8 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1690,12 +1690,31 @@ static void skip_optional_lf(void)
 		ungetc(term_char, stdin);
 }
 
+static inline uintmax_t parse_mark(const const char *str, char **after_mark)
+{
+	char *am;
+	uintmax_t m;
+
+	if (!str || str[0] != ':' || !isdigit(str[1]))
+		return 0;
+
+	m = strtoumax(&str[1], &am, 10);
+	if (m != UINTMAX_MAX || errno == 0) {
+		*after_mark = am;
+		return m;
+	}
+	return 0;
+}
+
 static void cmd_mark(void)
 {
-	if (!prefixcmp(command_buf.buf, "mark :")) {
-		next_mark = strtoumax(command_buf.buf + 6, NULL, 10);
+	uintmax_t mark;
+	char *after_mark = NULL;
+
+	if (!prefixcmp(command_buf.buf, "mark ") &&
+		(next_mark = parse_mark(&command_buf.buf[5], &after_mark)) &&
+		*after_mark == '\0')
 		read_next_command();
-	}
 	else
 		next_mark = 0;
 }
@@ -1877,8 +1896,8 @@ static void file_change_m(struct branch *b)
 	}
 
 	if (*p == ':') {
-		char *x;
-		oe = find_mark(strtoumax(p + 1, &x, 10));
+		char *x = NULL;
+		oe = find_mark(parse_mark(p, &x));
 		hashcpy(sha1, oe->sha1);
 		p = x;
 	} else if (!prefixcmp(p, "inline")) {
@@ -2045,7 +2064,10 @@ static int cmd_from(struct branch *b)
 		hashcpy(b->branch_tree.versions[0].sha1, t);
 		hashcpy(b->branch_tree.versions[1].sha1, t);
 	} else if (*from == ':') {
-		uintmax_t idnum = strtoumax(from + 1, NULL, 10);
+		char *after_mark;
+		uintmax_t idnum = parse_mark(from, &after_mark);
+		if (!idnum || *after_mark != '\0')
+			die("Not a valid mark: %s", from);
 		struct object_entry *oe = find_mark(idnum);
 		if (oe->type != OBJ_COMMIT)
 			die("Mark :%" PRIuMAX " not a commit", idnum);
@@ -2080,7 +2102,10 @@ static struct hash_list *cmd_merge(unsigned int *count)
 		if (s)
 			hashcpy(n->sha1, s->sha1);
 		else if (*from == ':') {
-			uintmax_t idnum = strtoumax(from + 1, NULL, 10);
+			char *after_mark;
+			uintmax_t idnum = parse_mark(from, &after_mark);
+			if (!idnum || *after_mark != '\0')
+				die("Not a valid mark: %s", from);
 			struct object_entry *oe = find_mark(idnum);
 			if (oe->type != OBJ_COMMIT)
 				die("Mark :%" PRIuMAX " not a commit", idnum);
@@ -2228,7 +2253,10 @@ static void cmd_new_tag(void)
 		hashcpy(sha1, s->sha1);
 	} else if (*from == ':') {
 		struct object_entry *oe;
-		from_mark = strtoumax(from + 1, NULL, 10);
+		char *after_mark;
+		from_mark = parse_mark(from, &after_mark);
+		if (!from_mark || *after_mark != '\0')
+			die("Not a valid mark: %s", from);
 		oe = find_mark(from_mark);
 		if (oe->type != OBJ_COMMIT)
 			die("Mark :%" PRIuMAX " not a commit", from_mark);
@@ -2333,9 +2361,8 @@ static void import_marks(const char *input_file)
 		if (line[0] != ':' || !end)
 			die("corrupt mark line: %s", line);
 		*end = 0;
-		mark = strtoumax(line + 1, &end, 10);
-		if (!mark || end == line + 1
-			|| *end != ' ' || get_sha1(end + 1, sha1))
+		mark = parse_mark(line, &end);
+		if (!mark || *end != ' ' || get_sha1(end + 1, sha1))
 			die("corrupt mark line: %s", line);
 		e = find_object(sha1);
 		if (!e) {
-- 
1.5.5.1

Re: [PATCH v2 02/13] Don't append default merge message to -m message

[Junio C Hamano]

Jörg Sommer <joerg@alea.gnuu.de> writes:

> From: gitster@pobox.com (Junio C Hamano)
> Date: Sun, 23 Mar 2008 22:17:09 -0700
>
> Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>

I'd admit that this was taken from my "You could do this" patch, and I am
inclined to think that the users would probably want this behaviour of
dropping the default merge summary when giving their own message with -m,
but I am not absolutely convinced that doing this unconditionally is the
right thing to do (iow, some people might have relied on the current
behaviour).

List, any objections?

> ---
>  git-merge.sh |   24 +++++++++++++-----------
>  1 files changed, 13 insertions(+), 11 deletions(-)
>
> diff --git a/git-merge.sh b/git-merge.sh
> index 7dbbb1d..bd9699d 100755
> --- a/git-merge.sh
> +++ b/git-merge.sh
> @@ -250,17 +250,19 @@ else
>  	# We are invoked directly as the first-class UI.
>  	head_arg=HEAD
>  
> -	# All the rest are the commits being merged; prepare
> -	# the standard merge summary message to be appended to
> -	# the given message.  If remote is invalid we will die
> -	# later in the common codepath so we discard the error
> -	# in this loop.
> -	merge_name=$(for remote
> -		do
> -			merge_name "$remote"
> -		done | git fmt-merge-msg
> -	)
> -	merge_msg="${merge_msg:+$merge_msg$LF$LF}$merge_name"
> +	if test -z "$merge_msg"
> +	then
> +		# All the rest are the commits being merged; prepare
> +		# the standard merge summary message to be appended to
> +		# the given message.  If remote is invalid we will die
> +		# later in the common codepath so we discard the error
> +		# in this loop.
> +		merge_msg=$(for remote
> +			do
> +				merge_name "$remote"
> +			done | git fmt-merge-msg
> +		)
> +	fi
>  fi
>  head=$(git rev-parse --verify "$head_arg"^0) || usage
>  
> -- 
> 1.5.5

Re: [PATCH v2 02/13] Don't append default merge message to -m message

[Jörg Sommer]

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

Hello Junio,

Junio C Hamano schrieb am Sun 20. Apr, 09:52 (-0700):
> Jörg Sommer <joerg@alea.gnuu.de> writes:
> 
> > From: gitster@pobox.com (Junio C Hamano)
> > Date: Sun, 23 Mar 2008 22:17:09 -0700
> >
> > Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>
> 
> I'd admit that this was taken from my "You could do this" patch, and I am
> inclined to think that the users would probably want this behaviour of
> dropping the default merge summary when giving their own message with -m,
> but I am not absolutely convinced that doing this unconditionally is the
> right thing to do (iow, some people might have relied on the current
> behaviour).
> 
> List, any objections?

What about a new option -M? But I doubt someone expects this behaviour
because the manpage says:

  The second syntax (<msg> HEAD <remote>) is supported for historical
  reasons. Do not use it from the command line or in new scripts. It is
  the same as git merge -m <msg> <remote>.
      ^^^^              ^^

Currently, it's not the same, but someone might expect it.

Bye, Jörg.
-- 
Die Erde ist das einzigste Irrenhaus, das von seinen eigenen Insassen
verwaltet wird.
                                                (U. Schmidt)

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH v2 02/13] Don't append default merge message to -m message

[Junio C Hamano]

Jörg Sommer <joerg@alea.gnuu.de> writes:

> ... But I doubt someone expects this behaviour
> because the manpage says:
>
>   The second syntax (<msg> HEAD <remote>) is supported for historical
>   reasons. Do not use it from the command line or in new scripts. It is
>   the same as git merge -m <msg> <remote>.
>       ^^^^              ^^
>
> Currently, it's not the same, but someone might expect it.

Yeah, the manpage is loosely written and does not reflect the reality.  I
am _VERY_ tempted to apply your change as-is and see if anybody screams.
Because git is designed to be very scriptable, people who want the current
behaviour can still call fmt-merge-msg themselves.

Re: [PATCH 3/4] Add a function for get the parents of a commit

[Johannes Schindelin]

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

Hi,

On Sun, 23 Mar 2008, Jörg Sommer wrote:

> 
> Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>

"for get"?  You mean "to get".

> +parents_of_commit() {
> +	git rev-list --parents -1 "$1" | cut -d' ' -f2-
> +}

>From the rest of Git's source code, I would have expected this to be 
called "get_parents", and to have a space before the parens.  In general, 
it is always good to imitate the style around the code you are writing.

Ciao,
Dscho

Re: [PATCH 2/4] Rework redo_merge

[Johannes Schindelin]

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

Hi,

On Sun, 23 Mar 2008, Jörg Sommer wrote:

> 
> Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>

"Rework" is not really informative.

> -		output git merge $STRATEGY -m "$msg" \
> -			$new_parents
> +		output git merge $STRATEGY -m "$msg" "$@"

This should rather go into 1/4.

> @@ -197,8 +199,7 @@ pick_one_preserving_merges () {
>  		case "$new_parents" in
>  		' '*' '*)
>  			# No point in merging the first parent, that's HEAD
> -			new_parents=${new_parents# $first_parent}
> -			redo_merge
> +			redo_merge $sha1 ${new_parents# $first_parent}

Likewise.

Sidenote: it is a bit of cheating to set sha1=$1 in redo_merge(), since it 
is _not_ a local variable there, but the code _relies_ on sha1 being the 
same after calling redo_merge().

Ciao,
Dscho

Re: [PATCH 1/4] Move redo merge code in a function

[Johannes Schindelin]

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

Hi,

On Sun, 23 Mar 2008, Jörg Sommer wrote:

> 
> Signed-off-by: Jörg Sommer <joerg@alea.gnuu.de>

I like the patch, but the commit message is not really meaningful if you 
do not read the patch.  Maybe you want to prefix it with "rebase -i:"?  
And _just_ maybe you want to give an explanation what you want to do with 
it?

(For reference, I think the recent patch series by Linus is a _wonderful_ 
example how to do it: it has a very informative cover letter, and each 
patch tells you more about what and why, leaving the how mostly to the 
patch.  Basically, it is a pleasure to read (and understand):

http://article.gmane.org/gmane.comp.version-control.git/77827)

Ciao,
Dscho

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Paul Fredrickson]

> Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > > Wouldn't
> > >
> > > pick 5cc8f37 (init: show "Reinit" message even in ...)
> > > mark 1
> > > pick 18d077c (quiltimport: fix misquoting of parse...)
> > > mark 2
> > > reset 1
> >
> > "reset 18d077c~2" or "reset some-tag" or "reset my-branch~12"
> >
> >         merge #2
> > >
> > > be easier for people?
> >
> > I don't know. Using the special sign everywhere a mark is used looks more
> > consistent to me. The only case where it might be omitted is the mark
> > command, because it only uses marks.
>
> Why not use the mark syntax that fast-import uses?  In fast-import
> we use ":n" anytime we need to refer to a mark, e.g. ":1" or ":5".
> Its the same idea.  We already have a language for it.  Heck, the
> commands above are bordering on a language not too far from the
> one that fast-import accepts.  :-)

I like the idea of adding marks to an interactive rebase in general, but instead
of adding a separate command, what if rebase *automatically* marked all the
commits in the session:

    1: pick 5cc8f37 (init: show "Reinit" message even in ...)
    2: pick 18d007c (quiltimport: fix misquoting of parse ...)
    reset 1
    merge 2

or "reset :1" and "merge :2".  Neither notation bothers me for marks.

--Paul

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Jörg Sommer]

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

Hello Paul,

Paul Fredrickson schrieb am Sun 13. Apr, 13:51 (-0700):
> > Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > > > Wouldn't
> > > >
> > > > pick 5cc8f37 (init: show "Reinit" message even in ...)
> > > > mark 1
> > > > pick 18d077c (quiltimport: fix misquoting of parse...)
> > > > mark 2
> > > > reset 1
> 
> I like the idea of adding marks to an interactive rebase in general, but instead
> of adding a separate command, what if rebase *automatically* marked all the
> commits in the session:
> 
>     1: pick 5cc8f37 (init: show "Reinit" message even in ...)
>     2: pick 18d007c (quiltimport: fix misquoting of parse ...)
>     reset 1
>     merge 2

This format would be incompatible with the current format and it makes
the parsing a little bit more difficult; the first column contains a mark
or a command. No, I think that's not a good idea.

Have a nice day, Jörg.
-- 
Der Hase läuft schneller als der Fuchs,
denn der Hase läuft um sein Leben.

[-- Attachment #2: Digital signature http://en.wikipedia.org/wiki/OpenPGP --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Johannes Schindelin]

Hi,

On Sun, 13 Apr 2008, Paul Fredrickson wrote:

> > Jrg Sommer <joerg@alea.gnuu.de> wrote:
> > > > Wouldn't
> > > >
> > > > pick 5cc8f37 (init: show "Reinit" message even in ...)
> > > > mark 1
> > > > pick 18d077c (quiltimport: fix misquoting of parse...)
> > > > mark 2
> > > > reset 1
> > >
> > > "reset 18d077c~2" or "reset some-tag" or "reset my-branch~12"
> > >
> > >         merge #2
> > > >
> > > > be easier for people?

Actually, I think that this whole "mark" stuff is way too complicated, as 
can be seen by the amount of patches needed to get it somewhere usable.

I would like it much better, if there was something like

pick 5cc8f37 (init: show "Reinit" message even in ...)
pick 18d077c (quiltimport: fix misquoting of parse...)
merge 9876543:5cc8f37,18d077c (Merge blub)
reset 5cc8f37
...

I.e. like with filter-branch, and like with rebase -i -p in its current 
form, we take the _original_ names as keys as to which commits to merge, 
or where to reset to.

That would be relatively easy to implement, since the whole infrastructure 
for it is already there: whenever a commit was rewritten, the new commit 
name is saved in $DOTEST/rewritten/<original-commit-name>.

I really do not like complicating things unnecessarily.

Ciao,
Dscho

Re: [PATCH/RFC 01/10] Teach rebase interactive the mark command

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> I would like it much better, if there was something like
>
> pick 5cc8f37 (init: show "Reinit" message even in ...)
> pick 18d077c (quiltimport: fix misquoting of parse...)
> merge 9876543:5cc8f37,18d077c (Merge blub)
> reset 5cc8f37
> ...
>
> I.e. like with filter-branch, and like with rebase -i -p in its current 
> form, we take the _original_ names as keys as to which commits to merge, 
> or where to reset to.

While the need probably would not be felt strongly if we design this only
for rebase -i, I suspect that you would want to have two kinds of reset if
you go that route.  There might be some other insn that may have similar
issues.

For example, imagine a case where you want to create a merge with a
recontructed side branch.  First you grow the branch you would merge into,
with a sequence:

	pick A
        pick B
        pick C

Then in order to reconstruct a side branch that begins from a known point,
say the tip of "master", you would want to reset to a commit that is
outside of the scope of this rewriting.  And then you rebuild that side
branch:

	reset master
        pick D
        pick E

And finally (and this step shows the beauty of your approach), come back
to the other tip and make the merge:

	reset C
        merge E

Two resets above would have different semantics.  The former resets to
unwritten, and the latter rewritten.