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

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

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

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.