[PATCH] subtree: add squash handling for split and push

[PATCH] subtree: add squash handling for split and push

[Pierre Penninckx]

>From 2763be1fe68d07af60945762178b8494228eb45f Mon Sep 17 00:00:00 2001
From: Pierre Penninckx <ibizapeanut@gmail.com>
Date: Sat, 23 Nov 2013 20:03:20 +0100
Subject: [PATCH] subtree: add squash handling for split and push

The documentation of subtree says that the --squash option can be used
for add, merge, split and push subtree commands but only add and merge
is implemented.

cmd_push() simply forwards the --squash argument to subtree split. All
the job is done by cmd_split().

cmd_split() first lets split do it's job: finding which commits need to
be extracted. Now we remember which commit is the parent of the first
extracted commit. When this step is done, cmd_split() generates a squash
of the new commits, starting from the aforementioned parent to the last
extracted commit. This new commit's sha1 is then used for the rest of
the script.

Tests verify that `git subtree split/push --squash` makes only one
commit where `git subtree split/push` without squash makes two.
---
 contrib/subtree/git-subtree.sh     | 20 ++++++++++++++++++-
 contrib/subtree/t/t7900-subtree.sh | 40 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7d7af03..76eb136 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -183,6 +183,7 @@ cache_set()
 	newrev="$2"
 	if [ "$oldrev" != "latest_old" \
 	     -a "$oldrev" != "latest_new" \
+	     -a "$oldrev" != "firstnewparents" \
 	     -a -e "$cachedir/$oldrev" ]; then
 		die "cache for $oldrev already exists!"
 	fi
@@ -603,6 +604,10 @@ cmd_split()
 		debug "  parents: $parents"
 		newparents=$(cache_get $parents)
 		debug "  newparents: $newparents"
+		if [ -z "$(cache_get firstnewparents)" ]; then
+			cache_set firstnewparents $newparents
+			debug "  firstnewparents: $(cache_get firstnewparents)"
+		fi

 		tree=$(subtree_for_commit $rev "$dir")
 		debug "  tree is: $tree"
@@ -625,11 +630,18 @@ cmd_split()
 		cache_set latest_new $newrev
 		cache_set latest_old $rev
 	done || exit $?
+
 	latest_new=$(cache_get latest_new)
 	if [ -z "$latest_new" ]; then
 		die "No new revisions were found"
 	fi

+	if [ -n "$squash" ]; then
+		from=$(cache_get firstnewparents)
+		latest_new=$(new_squash_commit "$from" "$from" "$latest_new") || exit $?
+		debug "New squash commit: $latest_new"
+	fi
+
 	if [ -n "$rejoin" ]; then
 		debug "Merging split branch into HEAD..."
 		latest_old=$(cache_get latest_old)
@@ -711,11 +723,17 @@ cmd_push()
 	if [ $# -ne 2 ]; then
 	    die "You must provide <repository> <refspec>"
 	fi
+
+	squash_cmd=
+	if [ -n "$squash" ]; then
+	    squash_cmd="--squash"
+	fi
+
 	if [ -e "$dir" ]; then
 	    repository=$1
 	    refspec=$2
 	    echo "git push using: " $repository $refspec
-	    localrev=$(git subtree split --prefix="$prefix") || die
+	    localrev=$(git subtree split --prefix="$prefix" $squash_cmd) || die
 	    git push $repository $localrev:refs/heads/$refspec
 	else
 	    die "'$dir' must already exist. Try 'git subtree add'."
diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 66ce4b0..04eea94 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -31,6 +31,16 @@ check_equal()
 	fi
 }

+check_not_equal()
+{
+        check_equal "$1" "$2"
+        if [ $? -eq 0 ]; then
+                return 1
+        else
+                return 0
+        fi
+}
+
 fixnl()
 {
 	t=""
@@ -161,6 +171,36 @@ test_expect_success 'fetch new subproj history' '
         git branch sub2 FETCH_HEAD
 '

+test_expect_success 'check that split and split --squash produce different commits' '
+        split=$(git subtree split --prefix=subdir) &&
+        split_squash=$(git subtree split --prefix=subdir --squash) &&
+        check_not_equal "$split" "$split_squash"
+'
+
+test_expect_success 'check that split and split --squash produce same diff' '
+        split=$(git subtree split --prefix=subdir) &&
+        split_squash=$(git subtree split --prefix=subdir --squash) &&
+        split_diff=$(git diff sub1 $split) &&
+        split_squash_diff=$(git diff sub1 $split_squash) &&
+        check_equal "$split_diff" "$split_squash_diff"
+'
+
+test_expect_success 'check that push introduces two commits in subproj' '
+        git subtree push --prefix=subdir subproj mainline &&
+        cd subproj &&
+        check_equal "$(git rev-parse mainline~2)" "$(git rev-parse sub1)" &&
+        git branch -D mainline &&
+        cd ..
+'
+
+test_expect_success 'check that push --squash introduces only one commit in subproj' '
+        git subtree push --prefix=subdir subproj mainline --squash &&
+        cd subproj &&
+        check_equal "$(git rev-parse mainline^)" "$(git rev-parse sub1)" &&
+        git branch -D mainline &&
+        cd ..
+'
+
 test_expect_success 'check if --message works for merge' '
         git subtree merge --prefix=subdir -m "Merged changes from subproject" sub2 &&
         check_equal ''"$(last_commit_message)"'' "Merged changes from subproject" &&

Re: [PATCH] subtree: add squash handling for split and push

[Matthew Ogilvie]

On Sat, Nov 23, 2013 at 09:18:56PM +0100, Pierre Penninckx wrote:
> The documentation of subtree says that the --squash option can be used
> for add, merge, split and push subtree commands but only add and merge
> is implemented.

Clarification: The current documentation (correctly) doesn't
actually claim to support "split --squash", but it does erroneously
claim to support "push --squash".

> cmd_split() first lets split do it's job: finding which commits need to
> be extracted. Now we remember which commit is the parent of the first
> extracted commit. When this step is done, cmd_split() generates a squash
> of the new commits, starting from the aforementioned parent to the last
> extracted commit. This new commit's sha1 is then used for the rest of
> the script.

I've been planning to implement something similar to this patch,
but the semantics I am aiming at are slightly different.

It looks like your patch is basically squashing the new subtree commits
together, throwing out those commits completely, and only keeping
the squashed commit in the split --branch.  

I intend to implement slightly different semantics, where
--squash only affects --rejoin, not the printed commit nor
the split-off --branch.  This is intended to provide a better,
third option for --rejoin'ing a subtree with a lot of history,
while preserving history in the split-off branch:

1. (existing/slow) Don't ever use --rejoin at all?  You can use
   "merge --squash" to merge in unrelated changes to the
   split-off project, but every "split" still gets slower
   and slower as each "split" needs to re-sift-through all
   the same history the previous "split"s have sifted
   through. 
   
2. (existing/huge mass of duplicated history) Use "split --rejoin"
   occasionally.  This pulls in the entire history of the
   subtree branch (since the last --rejoin or non-squash merge,
   or everything if neither has been done), which is difficult
   to ignore when looking at global history of the full project,
   especially if it is many pages of commits.  But subsequent
   splits can stop history traversal at the known-common point,
   and will run MUCH faster.
   
3. (new/better) Use "split --rejoin --squash" (or some other
   invocation to be defined).  The subtree branch is generated
   exactly like normal, including fine-grained history.  But
   instead of merging the subtree branch directly, --rejoin
   will squash all the changes to that branch, and merge in
   just the squash (referencing the unsquashed split
   branch tip in the commit message, but not the
   parent).  Subsequent splits can run very fast, while the
   "--rejoin" only generated two commits instead of the 
   potentially thousands of (mostly) duplicates it would pull
   in without the "--squash".

I have this third option half-coded already, but I still need
to finish it.

I'm fairly sure I can make this work without new adverse effects,
but if someone sees something I'm missing, let me know.

Does anyone have any suggestions about the UI?  Do we need to also
support Pierre Penninckx's "split --squash" semantics somehow?  If
so, what command line options would allow for distinguishing the
two cases?

--
Matthew Ogilvie   [mmogilvi_git@miniinfo.net]

Re: [PATCH] subtree: add squash handling for split and push

[Pierre Penninckx]

Hi Matthew,

> Clarification: The current documentation (correctly) doesn't
> actually claim to support "split --squash", but it does erroneously
> claim to support "push --squash ».

Yes indeed. ;)

> It looks like your patch is basically squashing the new subtree commits
> together, throwing out those commits completely, and only keeping
> the squashed commit in the split —branch.

Exactly.

> 3. (new/better) Use "split --rejoin --squash" (or some other
>   invocation to be defined).  The subtree branch is generated
>   exactly like normal, including fine-grained history.  But
>   instead of merging the subtree branch directly, --rejoin
>   will squash all the changes to that branch, and merge in
>   just the squash (referencing the unsquashed split
>   branch tip in the commit message, but not the
>   parent).  Subsequent splits can run very fast, while the
>   "--rejoin" only generated two commits instead of the 
>   potentially thousands of (mostly) duplicates it would pull
>   in without the "--squash ».

Isn’t this similar to "my" way? I mean I too generate the fine-grained history and make a squash afterwards, no?
I also don’t get why would your solution generate any duplicates. Would mine generate some?
I suppose the two answers are linked.

> I have this third option half-coded already, but I still need
> to finish it.

I’m eager to test it!

> Does anyone have any suggestions about the UI?  Do we need to also
> support Pierre Penninckx's "split --squash" semantics somehow?  If
> so, what command line options would allow for distinguishing the
> two cases?

Maybe `split --rejoin-squash` since it’s really a third way?
I intended to use `push --squash` to send a squash of the commits to hide the actual tinkering. So if your way allows to do it, I vote to stick with yours.

Regards,
Pierre Penninckx

[PATCH 1/4] subtree: support split --rejoin --squash

[Matthew Ogilvie]

Allow using --squash with "git subtree split --rejoin".  It
will still split off (and save to --branch) the complete
subtree history, but the merge done for the "--rejoin" will
be merging a squashed representation of the new subtree
commits, instead of the commits themselves (similar to
how "git subtree merge --squash" works).

Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
---

I can think of a couple of possible objections to this patch.
Are these (or any others) worth fixing?

1. Perhaps someone want the saved subtree (--branch) to have
   a squashed representation as well, as an option?  Maybe we
   need two different --squash options?  Something
   like "--rejoin-squash"?
2. It could definitely use some automated tests.  In fact,
   pre-existing --squash functionality is hardly tested at
   all, either.
      See patch 4 comments for a script I use to help with
   mostly-manual testing.



 contrib/subtree/git-subtree.sh  | 60 +++++++++++++++++++++++++++++++----------
 contrib/subtree/git-subtree.txt | 27 ++++++++++++-------
 2 files changed, 63 insertions(+), 24 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 7d7af03..998a9c5 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -20,14 +20,13 @@ q             quiet
 d             show debug messages
 P,prefix=     the name of the subdir to split out
 m,message=    use the given message as the commit message for the merge commit
+squash        merge subtree changes as a single commit
  options for 'split'
 annotate=     add a prefix to commit message of new commits
 b,branch=     create a new branch from the split subtree
 ignore-joins  ignore prior --rejoin commits
 onto=         try connecting new tree to an existing one
 rejoin        merge the new branch back into HEAD
- options for 'add', 'merge', 'pull' and 'push'
-squash        merge subtree changes as a single commit
 "
 eval "$(echo "$OPTS_SPEC" | git rev-parse --parseopt -- "$@" || echo exit $?)"
 
@@ -229,13 +228,19 @@ find_latest_squash()
 	sq=
 	main=
 	sub=
+	par1=
+	par2=
 	git log --grep="^git-subtree-dir: $dir/*\$" \
-		--pretty=format:'START %H%n%s%n%n%b%nEND%n' HEAD |
-	while read a b junk; do
-		debug "$a $b $junk"
+		--pretty=format:'START %H %P%n%s%n%n%b%nEND%n' HEAD |
+	while read a b c d junk; do
+		debug "$a $b $c $d $junk"
 		debug "{{$sq/$main/$sub}}"
 		case "$a" in
-			START) sq="$b" ;;
+			START)
+				sq="$b"
+				par1="$c"
+				par2="$d"
+				;;
 			git-subtree-mainline:) main="$b" ;;
 			git-subtree-split:) sub="$b" ;;
 			END)
@@ -243,7 +248,8 @@ find_latest_squash()
 					if [ -n "$main" ]; then
 						# a rejoin commit?
 						# Pretend its sub was a squash.
-						sq="$sub"
+						assert [ "$main" = "$par1" ]
+						sq="$par2"
 					fi
 					debug "Squash found: $sq $sub"
 					echo "$sq" "$sub"
@@ -252,6 +258,8 @@ find_latest_squash()
 				sq=
 				main=
 				sub=
+				par1=
+				par2=
 				;;
 		esac
 	done
@@ -565,6 +573,13 @@ cmd_split()
 	debug "Splitting $dir..."
 	cache_setup || exit $?
 	
+	if [ -n "$rejoin" ]; then
+		ensure_clean
+		if [ -n "$squash" ]; then
+			first_split="$(find_latest_squash "$dir")"
+		fi
+	fi
+
 	if [ -n "$onto" ]; then
 		debug "Reading history for --onto=$onto..."
 		git rev-list $onto |
@@ -630,13 +645,6 @@ cmd_split()
 		die "No new revisions were found"
 	fi
 	
-	if [ -n "$rejoin" ]; then
-		debug "Merging split branch into HEAD..."
-		latest_old=$(cache_get latest_old)
-		git merge -s ours \
-			-m "$(rejoin_msg $dir $latest_old $latest_new)" \
-			$latest_new >&2 || exit $?
-	fi
 	if [ -n "$branch" ]; then
 		if rev_exists "refs/heads/$branch"; then
 			if ! rev_is_descendant_of_branch $latest_new $branch; then
@@ -649,6 +657,30 @@ cmd_split()
 		git update-ref -m 'subtree split' "refs/heads/$branch" $latest_new || exit $?
 		say "$action branch '$branch'"
 	fi
+	if [ -n "$rejoin" ]; then
+		debug "Merging split branch into HEAD..."
+		latest_old=$(cache_get latest_old)
+		new=$latest_new
+
+		if [ -n "$squash" ]; then
+			debug "Squashing split branch."
+
+			set $first_split
+			old=$1
+			sub=$2
+			if [ "$sub" = "$latest_new" ]; then
+				say "Subtree is already at commit $latest_new."
+				exit 0
+			fi
+			new=$(new_squash_commit "$old" "$sub" "$latest_new") \
+				|| exit $?
+			debug "New squash commit: $new"
+		fi
+
+		git merge -s ours -m \
+			"$(rejoin_msg $dir $latest_old $latest_new)" \
+			$new >&2 || exit $?
+	fi
 	echo $latest_new
 	exit 0
 }
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index e0957ee..92e7a4d 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -140,18 +140,20 @@ OPTIONS
 	want to manipulate.  This option is mandatory
 	for all commands.
 
+
+OPTIONS FOR add, merge, pull, rejoin
+----------------------------------
 -m <message>::
 --message=<message>::
-	This option is only valid for add, merge and pull (unsure).
-	Specify <message> as the commit message for the merge commit.
+	This option is only valid for add, merge, pull, and
+	split '--rejoin'.
 
+	Specify <message> as the commit message for the merge commit.
 
-OPTIONS FOR add, merge, push, pull
-----------------------------------
 --squash::
-	This option is only valid for add, merge, push and pull
-	commands.
-
+	This option is only valid for add, merge, pull, and
+	split '--rejoin'.
+
 	Instead of merging the entire history from the subtree
 	project, produce only a single commit that contains all
 	the differences you want to merge, and then merge that
@@ -180,6 +182,10 @@ OPTIONS FOR add, merge, push, pull
 	local repository remain intact and can be later split
 	and send upstream to the subproject.
 
+	Using '--squash' with split '--rejoin' only squashes
+	the merge back to the mainline, not the synthetic subtree
+	history.
+
 
 OPTIONS FOR split
 -----------------
@@ -251,9 +257,10 @@ OPTIONS FOR split
 	showing an extra copy of every new commit that was
 	created (the original, and the synthetic one).
 	
-	If you do all your merges with '--squash', don't use
-	'--rejoin' when you split, because you don't want the
-	subproject's history to be part of your project anyway.
+	Fortunately, you can use '--squash' with '--rejoin'
+	to simplify a sequence of synthetic commits as a
+	single squashed commit in the mainline.  The subtree
+	will still have full history.
 
 
 EXAMPLE 1. Add command
-- 
1.8.3.2

[PATCH 2/4] subtree: allow --squash and --message with push

[Matthew Ogilvie]

Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
---
 contrib/subtree/git-subtree.sh  | 8 +++++++-
 contrib/subtree/git-subtree.txt | 9 ---------
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 998a9c5..56d915f 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -743,11 +743,17 @@ cmd_push()
 	if [ $# -ne 2 ]; then
 	    die "You must provide <repository> <refspec>"
 	fi
+
+	opts=
+	if [ -n "$squash" ]; then
+		opts="-squash"
+	fi
+
 	if [ -e "$dir" ]; then
 	    repository=$1
 	    refspec=$2
 	    echo "git push using: " $repository $refspec
-	    localrev=$(git subtree split --prefix="$prefix") || die
+	    localrev=$(git subtree split --prefix="$prefix" $opts --message="$message") || die
 	    git push $repository $localrev:refs/heads/$refspec
 	else
 	    die "'$dir' must already exist. Try 'git subtree add'."
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index 92e7a4d..03092bc 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -140,20 +140,11 @@ OPTIONS
 	want to manipulate.  This option is mandatory
 	for all commands.
 
-
-OPTIONS FOR add, merge, pull, rejoin
-----------------------------------
 -m <message>::
 --message=<message>::
-	This option is only valid for add, merge, pull, and
-	split '--rejoin'.
-
 	Specify <message> as the commit message for the merge commit.
 
 --squash::
-	This option is only valid for add, merge, pull, and
-	split '--rejoin'.
-
 	Instead of merging the entire history from the subtree
 	project, produce only a single commit that contains all
 	the differences you want to merge, and then merge that
-- 
1.8.3.2

[PATCH 3/4] subtree: add --edit option

[Matthew Ogilvie]

Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
---
 contrib/subtree/git-subtree.sh  | 37 +++++++++++++++++++++++++++++--------
 contrib/subtree/git-subtree.txt |  4 ++++
 2 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 56d915f..ac82b4d 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -21,6 +21,7 @@ d             show debug messages
 P,prefix=     the name of the subdir to split out
 m,message=    use the given message as the commit message for the merge commit
 squash        merge subtree changes as a single commit
+edit          allow user to edit squash commit message interactively
  options for 'split'
 annotate=     add a prefix to commit message of new commits
 b,branch=     create a new branch from the split subtree
@@ -45,6 +46,7 @@ ignore_joins=
 annotate=
 squash=
 message=
+edit=
 
 debug()
 {
@@ -91,6 +93,7 @@ while [ $# -gt 0 ]; do
 		--ignore-joins) ignore_joins=1 ;;
 		--no-ignore-joins) ignore_joins= ;;
 		--squash) squash=1 ;;
+		--edit) edit=1 ;;
 		--no-squash) squash= ;;
 		--) break ;;
 		*) die "Unexpected option: $opt" ;;
@@ -434,13 +437,12 @@ new_squash_commit()
 	old="$1"
 	oldsub="$2"
 	newsub="$3"
+	msg_file="$4"
 	tree=$(toptree_for_commit $newsub) || exit $?
 	if [ -n "$old" ]; then
-		squash_msg "$dir" "$oldsub" "$newsub" | 
-			git commit-tree "$tree" -p "$old" || exit $?
+		git commit-tree "$tree" -p "$old" -F "$msg_file" || exit $?
 	else
-		squash_msg "$dir" "" "$newsub" |
-			git commit-tree "$tree" || exit $?
+		git commit-tree "$tree" -F "$msg_file" || exit $?
 	fi
 }
 
@@ -556,7 +558,13 @@ cmd_add_commit()
 	fi
 	
 	if [ -n "$squash" ]; then
-		rev=$(new_squash_commit "" "" "$rev") || exit $?
+		msg_file="$GIT_DIR/COMMIT_EDITMSG"
+		squash_msg "$dir" "" "$rev" >"$msg_file"
+		if [ -n "$edit" ]; then
+			git_editor "$msg_file"
+		fi
+		rev=$(new_squash_commit "" "" "$rev" "$msg_file") || exit $?
+		rm -f "$msg_file"
 		commit=$(add_squashed_msg "$rev" "$dir" |
 			 git commit-tree $tree $headp -p "$rev") || exit $?
 	else
@@ -672,8 +680,14 @@ cmd_split()
 				say "Subtree is already at commit $latest_new."
 				exit 0
 			fi
-			new=$(new_squash_commit "$old" "$sub" "$latest_new") \
-				|| exit $?
+			msg_file="$GIT_DIR/COMMIT_EDITMSG"
+			squash_msg "$dir" "$sub" "$latest_new" >"$msg_file"
+			if [ -n "$edit" ]; then
+				git_editor "$msg_file"
+			fi
+			new=$(new_squash_commit "$old" "$sub" "$latest_new" \
+						"$msg_file") || exit $?
+			rm -f "$msg_file"
 			debug "New squash commit: $new"
 		fi
 
@@ -708,7 +722,13 @@ cmd_merge()
 			say "Subtree is already at commit $rev."
 			exit 0
 		fi
-		new=$(new_squash_commit "$old" "$sub" "$rev") || exit $?
+		msg_file="$GIT_DIR/COMMIT_EDITMSG"
+		squash_msg "$dir" "$sub" "$rev" >"$msg_file"
+		if [ -n "$edit" ]; then
+			git_editor "$msg_file"
+		fi
+		new=$(new_squash_commit "$old" "$sub" "$rev" "$msg_file") || exit $?
+		rm -f "$msg_file"
 		debug "New squash commit: $new"
 		rev="$new"
 	fi
@@ -748,6 +768,7 @@ cmd_push()
 	if [ -n "$squash" ]; then
 		opts="-squash"
 	fi
+	# Can't easily pass on --edit because of stdout capture redirection
 
 	if [ -e "$dir" ]; then
 	    repository=$1
diff --git a/contrib/subtree/git-subtree.txt b/contrib/subtree/git-subtree.txt
index 03092bc..16525d4 100644
--- a/contrib/subtree/git-subtree.txt
+++ b/contrib/subtree/git-subtree.txt
@@ -177,6 +177,10 @@ OPTIONS
 	the merge back to the mainline, not the synthetic subtree
 	history.
 
+--edit::
+	When used with '--squash', bring up an editor on the squash
+	commit message, to allow customizing it.
+
 
 OPTIONS FOR split
 -----------------
-- 
1.8.3.2

[PATCH/BAD 4/4] subtree: poor bugfix for split new commits with parents before previous split

[Matthew Ogilvie]

Bug description: Unless you use --ignore-joins, "git subtree split"'s
optimization to avoid re-scanning all of history can trim too much.
Any new merged branches that have parents before the previous "split"
will not be re-attached properly in the split-off subtree.
In the extreme case (if all the changes occurred on such
branches), I've seen the old subtree head might not be an
ancestor of the new head at all.

This "fix" is only for illustration.  It probably should not be
included as-is; it probably scales worse than using --ignore-joins
for anything much beyond trivial cases.

I'm not sure it is possible to speed it up much (while remaining
correct) without switching to a better language than shell script.

I'm not sure how to explain the constraint clearly (or even
precisely define the "minimal" constraint).  One attempt:
Exclude from "unrevs" any rejoin commit X for which ANY new commit
(not just revs itself) has some other common ancestor (merge base)
besides X.  ("New commit" is one that is not an ancestor of
some previous rejoin commit.)

It would probably be fairly straightforward to adapt
graph traversal algorithms to do this efficiently, but as near
as I can tell, none of the existing options in git-rev-list or
git-merge-base really does anything useful for optimizing this
in shell script...

The simplest traversal technique to fix this might
be if there was a way to make history traversal only stop
at SPECIFIC unrevs, instead of any ancestor of any unrevs.

Other workarounds besides this patch:
   * Use --ignore-joins and wait for the really slow processing...
   * Plan ahead and delay: After using git subtree split --rejoin,
     leave the rejoin merge commit off on a side branch until such time
     that all other pre-split branches have been merged.  Then
     "merge the merge" at that later time.  Only do rejoins fairly
     rarely, based on the schedule of merging other branches.
   * Ignore the problem: Allow the occasional falsely-disconnected
     branch roots to be generated by split.  Of course, this
     means --ignore-joins would no longer generate a consistent
     subtree history, it is hard to examine what actually changed
     in the false roots, etc.
   * Perhaps manually use temporary grafts or similar to hide rejoins
     that would throw it off when doing later splits.

[Intentionally not signed off: Poor performance.]
---

Testing: Below I include a script I've been using to help with
mostly-manual testing.  I regularly modify it based on
what I'm testing at the moment.  It basically
creates and manipulates a subtree out of git's own "contrib"
directory.

It may be convenient to use this on a copy of git's
repository instead of the copy in which you are messing with
git-subtree (avoiding changing code out from under the script).
It may also be convenient to symlink git-subtree to the top-level
directory, rather than copy it, so you don't need to keep
re-copying it.

------CUT------
#!/bin/sh

die()
{ echo "$@" 1>&2
  exit 1
}

GIT=../git-sandbox/bin-wrappers/git
#GIT=git

DBG=
#DBG=-d

EDIT=
#EDIT=--edit

git branch -D br-contrib

git checkout 73bbc0796b4ce65bfb1a12b47a0edc27845ecf50 || die "checkout"

"$GIT" subtree split -P contrib \
         --branch br-contrib --squash --rejoin $EDIT || die "subtree 1"

git tag -f step1

git merge -m 'MY MERGE1' 68a65f5fe54c2b21bfe16ef3a0b48956ecf5658a ||
        die "merge 1"

"$GIT" subtree split -P contrib \
         --branch br-contrib --squash --rejoin || die "subtree 2"

git tag -f step2

git merge -m 'MY MERGE2' 15f7221686eac053902b906c278680b485c865ce || \
        die "merge 2"

"$GIT" subtree split -P contrib \
         --branch br-contrib --squash --rejoin $EDIT || die "subtree 3"

#####
if [ -n "$EDIT" ]; then
    exit 0
fi

git tag -f step3

git merge -m 'MY MERGE3' 26145c9c73ed51bbd8261949d31899d6507519d5 || \
        die "merge 3"

"$GIT" subtree split -P contrib \
         $DBG --branch br-contrib --squash --rejoin || die "subtree 4"

git tag -f step4

git merge -m 'MY MERGE4' 583736c0bcf09adaa5621b142d8e43c22354041b || \
        die "merge 4"

"$GIT" subtree split -P contrib \
         --branch br-contrib --squash --rejoin || die "subtree 5"

git tag -f step5
------CUT------



 contrib/subtree/git-subtree.sh | 61 +++++++++++++++++++++++++++++++++++++-----
 1 file changed, 55 insertions(+), 6 deletions(-)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index ac82b4d..6ff4362 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -36,6 +36,8 @@ PATH=$PATH:$(git --exec-path)
 
 require_work_tree
 
+nl='
+'
 quiet=
 branch=
 debug=
@@ -224,6 +226,13 @@ try_remove_previous()
 	fi
 }
 
+try_remove()
+{
+	if rev_exists "$1"; then
+		echo "$1"
+	fi
+}
+
 find_latest_squash()
 {
 	debug "Looking for latest squash ($dir)..."
@@ -293,8 +302,8 @@ find_existing_splits()
 					debug "  Prior: $main -> $sub"
 					cache_set $main $sub
 					cache_set $sub $sub
-					try_remove_previous "$main"
-					try_remove_previous "$sub"
+					try_remove "$main"
+					try_remove "$sub"
 				fi
 				main=
 				sub=
@@ -303,6 +312,40 @@ find_existing_splits()
 	done
 }
 
+filter_out_needed_existing()
+{
+	revs="$1"
+	existing="$2"
+	base=
+	git rev-list --merges --parents $revs --not $existing | \
+		sed -e 's/^[^ ]* //' -e "y/ /\\$nl/" | sort | uniq | \
+	{
+		debug "filter_out_needed_existing"
+		debug "  revs: $revs"
+		debug "  existing: $existing"
+		while read needed; do
+			debug "loop: $needed"
+			nextexisting=
+			for exist in $existing ; do
+				tmp="$(git merge-base $exist $needed)"
+				if [ x"$tmp" = x"$exist" -o -z "$tmp" ]; then
+					nextexisting="$nextexisting $exist"
+				fi
+			done
+			existing=$nextexisting
+		done
+		echo "$existing"
+	}
+}
+
+existing_to_unrevs()
+{
+	existing="$1"
+	for exist in $existing ; do
+		try_remove_previous $exist
+	done
+}
+
 copy_commit()
 {
 	# We're going to set some environment vars here, so
@@ -599,10 +642,16 @@ cmd_split()
 		done
 	fi
 	
-	if [ -n "$ignore_joins" ]; then
-		unrevs=
-	else
-		unrevs="$(find_existing_splits "$dir" "$revs")"
+	unrevs=
+	if [ -z "$ignore_joins" ]; then
+		existing="$(find_existing_splits "$dir" "$revs")"
+		if [ -n "$existing" ]; then
+			debug "existing: $existing"
+			existing="$(filter_out_needed_existing "$revs" "$existing")"
+			unrevs="$(existing_to_unrevs "$existing")"
+			debug "base: $base"
+			debug "unrevs: $unrevs"
+		fi
 	fi
 	
 	# We can't restrict rev-list to only $dir here, because some of our
-- 
1.8.3.2

Re: [PATCH 1/4] subtree: support split --rejoin --squash

[Junio C Hamano]

Matthew Ogilvie <mmogilvi_git@miniinfo.net> writes:

> Allow using --squash with "git subtree split --rejoin".  It
> will still split off (and save to --branch) the complete
> subtree history, but the merge done for the "--rejoin" will
> be merging a squashed representation of the new subtree
> commits, instead of the commits themselves (similar to
> how "git subtree merge --squash" works).
>
> Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
> ---
>
> I can think of a couple of possible objections to this patch.
> Are these (or any others) worth fixing?
>
> 1. Perhaps someone want the saved subtree (--branch) to have
>    a squashed representation as well, as an option?  Maybe we
>    need two different --squash options?  Something
>    like "--rejoin-squash"?
> 2. It could definitely use some automated tests.  In fact,
>    pre-existing --squash functionality is hardly tested at
>    all, either.
>       See patch 4 comments for a script I use to help with
>    mostly-manual testing.

As I am totally uninterested in "git subtree" (sorry!), I'll queue
these three patches as-is so that others who are interested and
motivated to work on polishing it can take a look at them more
easily.

Thanks.

Re: [PATCH 1/4] subtree: support split --rejoin --squash

[Matthew Ogilvie]

On Wed, Jan 22, 2014 at 03:58:28PM +0100, Pierre Penninckx wrote:
> 2013/12/7 Matthew Ogilvie <mmogilvi_git@miniinfo.net>
> > Subject: [PATCH 1/4] subtree: support split --rejoin --squash
> >
> > Allow using --squash with "git subtree split --rejoin".  It
> > will still split off (and save to --branch) the complete
> > subtree history, but the merge done for the "--rejoin" will
> > be merging a squashed representation of the new subtree
> > commits, instead of the commits themselves (similar to
> > how "git subtree merge --squash" works).
> >
> > Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
> > ---
> >
> > I can think of a couple of possible objections to this patch.
> > Are these (or any others) worth fixing?
> >
> > 1. Perhaps someone want the saved subtree (--branch) to have
> >    a squashed representation as well, as an option?  Maybe we
> >    need two different --squash options?  Something
> >    like "--rejoin-squash"?
> > 2. It could definitely use some automated tests.  In fact,
> >    pre-existing --squash functionality is hardly tested at
> >    all, either.
> >       See patch 4 comments for a script I use to help with
> >    mostly-manual testing.
>
> Sorry to bother you with this again, but I was wondering if those patches
> would be integrated into git anytime soon.
> And if not, if there is something I can do to help.
> 
> I found them by the way, thanks a lot!
> 
> Pierre

I'm not sure when or if the patches will make it in.  Junio's
weekly "What's cooking..." email has asked for "Comments?" about
them for the past several weeks, but I have yet to see
anyone actually comment about them.

Searching throught the last couple of years of mailing list
archives for "subtree" reveals a general lack of a active
maintainer(s) to help review and improve patches for "git
subtree".  Given the general lack of help and feedback, it is
understandable that Junio has largely limited inclusion of
subtree patches to trivially obvious bug fixes.

                        - Matthew Ogilvie

Re: [PATCH 1/4] subtree: support split --rejoin --squash

[Pierre Penninckx]

Hi again,

After using the patched git-subtree (with patches 1 to 3) for a while,
I suspect the added functionality does not do exactly what I wanted.
So yes, now when doing a rejoin, the squash of the split commits is
used. But how can I push this squash instead of the individual
commits? The problem is I don't know how to reference that squashed
commit.

I tried adding the --branch option but it adds the branch to the top
of the individual commits so no luck there.
This is maybe obvious but I'm not at ease with commit references in git.

Pierre

2014/1/23 Matthew Ogilvie <mmogilvi_git@miniinfo.net>:
> On Wed, Jan 22, 2014 at 03:58:28PM +0100, Pierre Penninckx wrote:
>> 2013/12/7 Matthew Ogilvie <mmogilvi_git@miniinfo.net>
>> > Subject: [PATCH 1/4] subtree: support split --rejoin --squash
>> >
>> > Allow using --squash with "git subtree split --rejoin".  It
>> > will still split off (and save to --branch) the complete
>> > subtree history, but the merge done for the "--rejoin" will
>> > be merging a squashed representation of the new subtree
>> > commits, instead of the commits themselves (similar to
>> > how "git subtree merge --squash" works).
>> >
>> > Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
>> > ---
>> >
>> > I can think of a couple of possible objections to this patch.
>> > Are these (or any others) worth fixing?
>> >
>> > 1. Perhaps someone want the saved subtree (--branch) to have
>> >    a squashed representation as well, as an option?  Maybe we
>> >    need two different --squash options?  Something
>> >    like "--rejoin-squash"?
>> > 2. It could definitely use some automated tests.  In fact,
>> >    pre-existing --squash functionality is hardly tested at
>> >    all, either.
>> >       See patch 4 comments for a script I use to help with
>> >    mostly-manual testing.
>>
>> Sorry to bother you with this again, but I was wondering if those patches
>> would be integrated into git anytime soon.
>> And if not, if there is something I can do to help.
>>
>> I found them by the way, thanks a lot!
>>
>> Pierre
>
> I'm not sure when or if the patches will make it in.  Junio's
> weekly "What's cooking..." email has asked for "Comments?" about
> them for the past several weeks, but I have yet to see
> anyone actually comment about them.
>
> Searching throught the last couple of years of mailing list
> archives for "subtree" reveals a general lack of a active
> maintainer(s) to help review and improve patches for "git
> subtree".  Given the general lack of help and feedback, it is
> understandable that Junio has largely limited inclusion of
> subtree patches to trivially obvious bug fixes.
>
>                         - Matthew Ogilvie

Re: [PATCH 1/4] subtree: support split --rejoin --squash

[Matthew Ogilvie]

On Thu, Jan 23, 2014 at 09:51:49AM +0100, Pierre Penninckx wrote:
> Hi again,
> 
> After using the patched git-subtree (with patches 1 to 3) for a while,
> I suspect the added functionality does not do exactly what I wanted.
> So yes, now when doing a rejoin, the squash of the split commits is
> used. But how can I push this squash instead of the individual
> commits? The problem is I don't know how to reference that squashed
> commit.
> 
> I tried adding the --branch option but it adds the branch to the top
> of the individual commits so no luck there.
> This is maybe obvious but I'm not at ease with commit references in git.

Note that there are essentially two trees output by subtree --join.

The first output is the main branch (with --join).  With my
patches and --squash, the main branch merges in a squashed
representation of the subtree changes, so that the main
project history doesn't have two copies of potentially
tons of different commits in it's history (the
original and the subtree, shown merged together).

The second output is the new branch tip of the subtree itself.
My patch always outputs the full history of the subtree, not
a squashed representation.  This is what's different from your
patch, and is what I wanted.  If you want this subtree output
to ALSO be squashed, then it would need another option to
support this.

Note that there is at least one technical reason to prefer my
strategy.  "git subtree" tries to make it so you can
re-run it (potentially from scratch) on the main project at
any point in time, and re-generate exactly the same final
subtree history, regardless of previous runs of "git subtree".
But if some of that history was originally squashed, it currently
has no way of knowing which commits should be squashed together
to properly regenerate exactly the same subtree history.
This is especially true if you use "--ignore-joins", which
is currently the only practical workaround to the bug described
in my patch 4 (about merging in history that originally branched
off before the previous subtree split point).  Perhaps this
issue could be addressed by enhancing subtree to recognize
specially-formatted squash messages, and intentionally
regenerate the squashed based on them?

[Side note: I think the convention on this list is to respond
inline or after the previous message, not at the top, so new
people can more easily pick up the discussion.]

                       - Matthew

> 2014/1/23 Matthew Ogilvie <mmogilvi_git@miniinfo.net>:
> > On Wed, Jan 22, 2014 at 03:58:28PM +0100, Pierre Penninckx wrote:
> >> 2013/12/7 Matthew Ogilvie <mmogilvi_git@miniinfo.net>
> >> > Subject: [PATCH 1/4] subtree: support split --rejoin --squash
> >> >
> >> > Allow using --squash with "git subtree split --rejoin".  It
> >> > will still split off (and save to --branch) the complete
> >> > subtree history, but the merge done for the "--rejoin" will
> >> > be merging a squashed representation of the new subtree
> >> > commits, instead of the commits themselves (similar to
> >> > how "git subtree merge --squash" works).
> >> >
> >> > Signed-off-by: Matthew Ogilvie <mmogilvi_git@miniinfo.net>
> >> > ---
> >> >
> >> > I can think of a couple of possible objections to this patch.
> >> > Are these (or any others) worth fixing?
> >> >
> >> > 1. Perhaps someone want the saved subtree (--branch) to have
> >> >    a squashed representation as well, as an option?  Maybe we
> >> >    need two different --squash options?  Something
> >> >    like "--rejoin-squash"?
> >> > 2. It could definitely use some automated tests.  In fact,
> >> >    pre-existing --squash functionality is hardly tested at
> >> >    all, either.
> >> >       See patch 4 comments for a script I use to help with
> >> >    mostly-manual testing.
> >>
> >> Sorry to bother you with this again, but I was wondering if those patches
> >> would be integrated into git anytime soon.
> >> And if not, if there is something I can do to help.
> >>
> >> I found them by the way, thanks a lot!
> >>
> >> Pierre
> >
> > I'm not sure when or if the patches will make it in.  Junio's
> > weekly "What's cooking..." email has asked for "Comments?" about
> > them for the past several weeks, but I have yet to see
> > anyone actually comment about them.
> >
> > Searching throught the last couple of years of mailing list
> > archives for "subtree" reveals a general lack of a active
> > maintainer(s) to help review and improve patches for "git
> > subtree".  Given the general lack of help and feedback, it is
> > understandable that Junio has largely limited inclusion of
> > subtree patches to trivially obvious bug fixes.
> >
> >                         - Matthew Ogilvie