[PATCH 0/2] git-subtree: Bail out if we find output from Rust rewrite

[PATCH 0/2] git-subtree: Bail out if we find output from Rust rewrite

[Ian Jackson]

My rewrite of git-subtree is coming along fairly nicely.  I have done
a bunch of data model design work, as well as successfully implemented
the "split" operation.  (It's several orders of magnitude faster than
the shell script implementation, and produces much better output.)

I have concluded that it is going to be too difficult to have
bidirectional interoperability, for a number of reasons.  One reason
is that I want to change the way split commits are constructed and
they should be as stable as we can make them.

Another, bigger, reason is that current git-subtree generates unmarked
subtree merges (ie, without any git-subtree trailers); reliably
figuring out whether something is such a merge requires a complete
history walk, which is very unfortunate.  To avoid having to do this
in perpetuity, I plan to have new git-subtree assume that commits
which show signs of use of new git-subtree are not unmarked subtree
merges generated by old git-subtree.

So mycompatibility plan is:

 - New git-subtree can read old git-subtree data
 - Old git-subtree (this one here) will *not* handle new data

New git-subtree needs an in-downstream-tree config file with at least
an indication of the downstream project name, because otherwise split
commits can be quite inscrutable.  So the presence of that file is a
convenient way to signal the change.


The purpose of the present patch is to help prevent messes, where old
git-subtree is used *afer* new git-subtree, generating output that no
tooling can handle correctly.  We change old git-subtree to spot the
new git-subtree's config file, and bail out.

Right now there is no new data in existence, becauwe new git-subtree
is not useable.  So right now this change has no effect.

But if we ship this change now, users with this change will be
defended from this lossage, as their collaborators start to use new
git-subtree.  I'm hoping to possibly persuade distros etc. to take
this patch as a backport.

I wasn't sure how precisely to word the error message.  I chose to
refer to the new tool, as if it really exists.  By the time anyone
actually seex this message, it will do.


Ian Jackson (2):
  git-subtree: Bail out if we find output from Rust rewrite
  git-subtree: Bail out if we find output from Rust rewrite (test)

 contrib/subtree/git-subtree.sh     | 18 ++++++++++++++++++
 contrib/subtree/t/t7900-subtree.sh | 18 ++++++++++++++++++
 2 files changed, 36 insertions(+)

-- 
2.47.3

[PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite

[Ian Jackson]

This is going to be forward compatible, but not backward compatible:
projects are expected to adopt the new tool, but not go back to this
old one.

CC: Colin Stagner <ask+git@howdoi.land>
CC: Johannes Schindelin <Johannes.Schindelin@gmx.de>
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
 contrib/subtree/git-subtree.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
index 791fd8260c..e9c7ca7cf5 100755
--- a/contrib/subtree/git-subtree.sh
+++ b/contrib/subtree/git-subtree.sh
@@ -278,6 +278,20 @@ main () {
 	"cmd_$arg_command" "$@"
 }
 
+# Usage: reject_if_v2_config REV
+#
+# Bails if we find .git-subtree/config.  This file is used by the RIIR
+# git-subtree, which can read data from this script, but which generates
+# data that this script cannot cope with.  So if we find that the user's
+# project has already been processed with the new tool, we stop, to
+# avoid generating broken output.
+reject_if_v2_config () {
+	local config=.git-subtree/config
+	if git rev-parse --verify -q "$rev:$config"; then
+		die "fatal: tree contains $config: has been processed with new standalone (Rust) git-subtree; use that tool instead of this one.  See https://codeberg.org/diziet/git-subtree https://crates.io/crates/git-subtree"
+	fi
+}
+
 # Usage: cache_setup
 cache_setup () {
 	assert test $# = 0
@@ -846,6 +860,7 @@ process_split_commit () {
 #    Or: cmd_add REPOSITORY REF
 cmd_add () {
 
+	reject_if_v2_config HEAD
 	ensure_clean
 
 	if test $# -eq 1
@@ -934,6 +949,8 @@ cmd_split () {
 		die "fatal: you must provide exactly one revision, and optionally a repository.  Got: '$*'"
 	fi
 
+	reject_if_v2_config "$rev"
+
 	# Now validate prefix against the commit, not the working tree
 	if ! git cat-file -e "$rev:$dir" 2>/dev/null
 	then
@@ -1034,6 +1051,7 @@ cmd_merge () {
 	then
 		repository="$2"
 	fi
+	reject_if_v2_config HEAD
 	ensure_clean
 
 	if test -n "$arg_addmerge_squash"
-- 
2.47.3

[PATCH 2/2] git-subtree: Bail out if we find output from Rust rewrite (test)

[Ian Jackson]

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
 contrib/subtree/t/t7900-subtree.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/contrib/subtree/t/t7900-subtree.sh b/contrib/subtree/t/t7900-subtree.sh
index 4194687cfb..e8fa640166 100755
--- a/contrib/subtree/t/t7900-subtree.sh
+++ b/contrib/subtree/t/t7900-subtree.sh
@@ -439,6 +439,24 @@ test_expect_success 'split sub dir/ with --rejoin' '
 	)
 '
 
+test_expect_success 'split fail on RIIR git subtree data' '
+	subtree_test_create_repo "$test_count" &&
+	subtree_test_create_repo "$test_count/sub proj" &&
+	test_create_commit "$test_count" main1 &&
+	test_create_commit "$test_count/sub proj" sub1 &&
+	(
+		cd "$test_count" &&
+		git fetch ./"sub proj" HEAD &&
+		git subtree add --prefix="sub dir" FETCH_HEAD &&
+		# simulate RIIR git-subtree generated data
+		mkdir .git-subtree &&
+		echo "# sabotage" >.git-subtree/config &&
+		git add .git-subtree/config &&
+		git commit -m sabotage &&
+		test_must_fail git subtree split -P "sub dir" HEAD
+	)
+'
+
 # Tests that commits from other subtrees are not processed as
 # part of a split.
 #
-- 
2.47.3

Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite

[Junio C Hamano]

Ian Jackson <ijackson@chiark.greenend.org.uk> writes:

> +# Usage: reject_if_v2_config REV
> +#
> +# Bails if we find .git-subtree/config.  This file is used by the RIIR
> +# git-subtree, which can read data from this script, but which generates
> +# data that this script cannot cope with.  So if we find that the user's
> +# project has already been processed with the new tool, we stop, to
> +# avoid generating broken output.
> +reject_if_v2_config () {
> +	local config=.git-subtree/config
> +	if git rev-parse --verify -q "$rev:$config"; then
> +		die "fatal: tree contains $config: has been processed with new standalone (Rust) git-subtree; use that tool instead of this one.  See https://codeberg.org/diziet/git-subtree https://crates.io/crates/git-subtree"
> +	fi
> +}

[warning: I have no idea what is going on in the code we see here,
as I do not use subtree script at all]

The above helper may work for one caller that passes "$rev" but not
for the other caller that passes "HEAD", no?


	if git rev-parse --verify -q "$1:$config"
	then
		die "fatal: tree contains $config: has been processed with new standalone (Rust) git-subtree; use that tool instead of this one.  See https://codeberg.org/diziet/git-subtree https://crates.io/crates/git-subtree"
	fi

Overly long output does not look very easy to read, but I kept it
the same as the original.

> @@ -846,6 +860,7 @@ process_split_commit () {
>  #    Or: cmd_add REPOSITORY REF
>  cmd_add () {
>  
> +	reject_if_v2_config HEAD
>  	ensure_clean

If (global) $rev is not set here, we'd check :.git-subtree/config in
the index in order to detect the v2's configuration.  It seems to me
that this code however wants to inspect HEAD's tree.

> @@ -934,6 +949,8 @@ cmd_split () {
>  		die "fatal: you must provide exactly one revision, and optionally a repository.  Got: '$*'"
>  	fi
>  
> +	reject_if_v2_config "$rev"

This would happen to work, as the global "$rev" visible here is the
same one as what the new helper function sees and uses.

>  	# Now validate prefix against the commit, not the working tree
>  	if ! git cat-file -e "$rev:$dir" 2>/dev/null
>  	then
> @@ -1034,6 +1051,7 @@ cmd_merge () {
>  	then
>  		repository="$2"
>  	fi
> +	reject_if_v2_config HEAD
>  	ensure_clean

The same comment as the one for cmd_add's usage.

>  	if test -n "$arg_addmerge_squash"

Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite

[Ian Jackson]

Hi.  Thanks for the quick review.

Junio C Hamano writes ("Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite"):
> If (global) $rev is not set here, we'd check :.git-subtree/config in
> the index in order to detect the v2's configuration.  It seems to me
> that this code however wants to inspect HEAD's tree.

This was a slip.  The code in reject_if_v2_config is supposed to use
its argument (as per the usage comment I added), not a global.  I'll
fix this with a respin.

(I think it may somehow work by accident in my tests.)

> The above helper may work for one caller that passes "$rev" but not
> for the other caller that passes "HEAD", no?

HEAD is a valid revision spec for git-rev-parse, but the
function should use $1 (which in that case would be HEAD), not $rev.

> 	if git rev-parse --verify -q "$1:$config"
> 	then
> 		die "fatal: tree contains $config: has been processed with new standalone (Rust) git-subtree; use that tool instead of this one.  See https://codeberg.org/diziet/git-subtree https://crates.io/crates/git-subtree"
> 	fi
> 
> Overly long output does not look very easy to read, but I kept it
> the same as the original.

I'm not a great fan of the long error message myself, but it seemed to
be what the rest of the script was doing.  I didn't find any
multi-line calls to die, so that's why I did it this way.

I'm happy to reformat this to your taste.

Thanks,
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.  

Pronouns: they/he.  If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.

Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite

[Junio C Hamano]

Ian Jackson <ijackson@chiark.greenend.org.uk> writes:

>> 	if git rev-parse --verify -q "$1:$config"
>> 	then
>> 		die "fatal: tree contains $config: has been processed with new standalone (Rust) git-subtree; use that tool instead of this one.  See https://codeberg.org/diziet/git-subtree https://crates.io/crates/git-subtree"
>> 	fi
>> 
>> Overly long output does not look very easy to read, but I kept it
>> the same as the original.
>
> I'm not a great fan of the long error message myself, but it seemed to
> be what the rest of the script was doing.  I didn't find any
> multi-line calls to die, so that's why I did it this way.
>
> I'm happy to reformat this to your taste.

Nah, it seems your plan is to deprecate this script over time and
move everybody to a newer implementation, so as long as "die" does
its job to stop and prevent breakages from spreading, that would be
fine.

Thanks.

Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite

[Colin Stagner]

On 7/6/26 06:58, Ian Jackson wrote:

> Another, bigger, reason is that current git-subtree generates unmarked
> subtree merges (ie, without any git-subtree trailers)

Subtree merges can be performed without git-subtree, via the `-X 
subtree` merge strategy option. While the design of RIIR git-subtree is 
outside the scope of this patch series, this may be worth thinking about 
in your rewrite.


> --- a/contrib/subtree/git-subtree.sh
> +++ b/contrib/subtree/git-subtree.sh
> @@ -278,6 +278,20 @@ main () {


> +reject_if_v2_config () {
> +	local config=.git-subtree/config

This is a nit, but `local` is not specified by POSIX. I know it is used 
elsewhere within git-subtree, but it is specifically discouraged.

> +	if git rev-parse --verify -q "$rev:$config"; then

For subtree split, should we also test for this file in tree you are 
splitting: i.e., "$dir/$config"? The answer might be no.

I think that subtree merge should only test the top-level project, as 
this patch does now.

Colin

Re: [PATCH 2/2] git-subtree: Bail out if we find output from Rust rewrite (test)

[Colin Stagner]

On 7/6/26 06:58, Ian Jackson wrote:

> --- a/contrib/subtree/t/t7900-subtree.sh
> +++ b/contrib/subtree/t/t7900-subtree.sh
> @@ -439,6 +439,24 @@ test_expect_success 'split sub dir/ with --rejoin' '
>   	)
>   '
>   
> +test_expect_success 'split fail on RIIR git subtree data' '
> +	subtree_test_create_repo "$test_count" &&
> +	subtree_test_create_repo "$test_count/sub proj" &&

It may be slightly faster to create only one repo and just make orphan 
branches, like `test_create_subtree_add()` does.

> +		echo "# sabotage" >.git-subtree/config &&
> +		git add .git-subtree/config &&
> +		git commit -m sabotage &&

`test_commit()` from test-lib-functions.sh may be superior to manually 
writing and committing this file.


Colin

Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite [and 1 more messages]

[Ian Jackson]

Hi.  Thanks for the review.  I'll go through it point by point:

Colin Stagner writes ("Re: [PATCH 2/2] git-subtree: Bail out if we find output from Rust rewrite (test)"):
> It may be slightly faster to create only one repo and just make orphan 
> branches, like `test_create_subtree_add()` does.
...
> `test_commit()` from test-lib-functions.sh may be superior to manually 
> writing and committing this file.

Thanks for the suggestions.  I'll take a look.

TBH I found this test framework quite awkward to work with.  Maybe
folks here have some tips:

One thing I was missing was a primitive for "check this fails *and
produces an error message matching this regexp*".  test_must_fail
makes it easy for a slips in the command (or some kinds of regression)
to go undetected: the test then passes because the command *does* fail
with a usage error or whatever.  And AFAICT there isn't a way to
manually inspect the output when the tests pass?  I resorted to
sabotaging the test by adding `&& false` to the end of the shell
snippet string, and eyeballing t/test-results/t7900-subtree.out.

Colin Stagner writes ("Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite"):
> > +reject_if_v2_config () {
> > +	local config=.git-subtree/config
> 
> This is a nit, but `local` is not specified by POSIX. I know it is used 
> elsewhere within git-subtree, but it is specifically discouraged.

There are 7 existing uses of `local`.  I think I prefer to use it here
too.  In practice I think there are no shells we might want to use
that don't have local.  The alternative is to change all the variable
names to be obviously globally unique, which is clumsy and also seems
to me to put us at greater risk of bugs.

> > +	if git rev-parse --verify -q "$rev:$config"; then
> 
> For subtree split, should we also test for this file in tree you are 
> splitting: i.e., "$dir/$config"? The answer might be no.

You're right that we should consider this question.  The answer is:
no, we should not.  Briefly, whether to use the new or old algorithms
depends on whether the downstream has adopted the new git-subtree, not
on whether the upstream has added some optional config.

https://codeberg.org/diziet/git-subtree/src/branch/main/DATA-MODEL.md#control-of-unmarked-subtree-merges-guessing-config

> I think that subtree merge should only test the top-level project, as 
> this patch does now.

By "top-level" I think you mean what I've taken to calling the
"downstream": the project where the subtree is in a subdir, and whose
top-level has other stuff.  In which case I agree.

> On 7/6/26 06:58, Ian Jackson wrote:
> > Another, bigger, reason is that current git-subtree generates unmarked
> > subtree merges (ie, without any git-subtree trailers)
> 
> Subtree merges can be performed without git-subtree, via the `-X 
> subtree` merge strategy option. While the design of RIIR git-subtree is 
> outside the scope of this patch series, this may be worth thinking about 
> in your rewrite.

This is what I'm calling an "unmarked subtree merge".  My rewrite is
not going to support this user behaviour.  The problem is that it is
not possible to reliably determine whetheer something is an unmarked
subtree merge.

It is possible to guess based on tree similarity, but that's a
heuristic.  It's also possible to guess based on root commits.
Both of these approaches can go wrong in some cases.  I prefer to
write reliable software, which doesn't guess.

I'll advise against this practice in the documentation, but I'm
reasonably confident that if a user does this anyway the results won't
be terrible.  The upstream input to an unmarked subtree merge in a
downstream that has already used my rewrite, will be treated as if it
were a downstream branch that predates the subtree addition.  The
effect on split (in most cases) is a missing parent relationship,
which is undesirable but not catastrophic.I've made a note to add a
test case for this scenario.

Combining manual -X subtree merges with git-subtree --squash merges
could easily produce quite weird and wrong results in the tree (even
before anyone tries split, or something).  I don't think I can even
reliably detect this situation after the user has done it, and of
course since that user is using plain git, I certainly can't prevent
it.  This is another reason why manual use of -X subtree should be
discouraged.

Regards,
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.  

Pronouns: they/he.  If I emailed you from @fyvzl.net or @evade.org.uk,
that is a private address which bypasses my fierce spamfilter.

Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite [and 1 more messages]

[Phillip Wood]

Hi Ian

On 09/07/2026 10:36, Ian Jackson wrote:
> 
> Colin Stagner writes ("Re: [PATCH 2/2] git-subtree: Bail out if we find output from Rust rewrite (test)"):
>> It may be slightly faster to create only one repo and just make orphan
>> branches, like `test_create_subtree_add()` does.
> ...
>> `test_commit()` from test-lib-functions.sh may be superior to manually
>> writing and committing this file.
> 
> Thanks for the suggestions.  I'll take a look.

I think

     test_commit --no-tag sabotage .git-subtree/config "# sabotage"

is the equivalent of what you have in the test at the moment

> TBH I found this test framework quite awkward to work with.  Maybe
> folks here have some tips:
> 
> One thing I was missing was a primitive for "check this fails *and
> produces an error message matching this regexp*".  test_must_fail
> makes it easy for a slips in the command (or some kinds of regression)
> to go undetected: the test then passes because the command *does* fail
> with a usage error or whatever.  And AFAICT there isn't a way to
> manually inspect the output when the tests pass?  I resorted to
> sabotaging the test by adding `&& false` to the end of the shell
> snippet string, and eyeballing t/test-results/t7900-subtree.out.

The usual approach to checking that a command fails for the expected 
reason is

     test_must_fail git ... 2>err &&
     test_grep regexp err

which prints the contents of err if it does not match regexp. To see the 
output of the tests run them with "-v". I frequently use "-v -i -x" to 
debug test failures. "-i" stops the test run at the first failure so you 
can inspect the test repository and "-x" turns on tracing so you can see 
which command failed which is useful when I test has not been written 
with debugging in mind.

Thanks

Phillip
	
> Colin Stagner writes ("Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite"):
>>> +reject_if_v2_config () {
>>> +	local config=.git-subtree/config
>>
>> This is a nit, but `local` is not specified by POSIX. I know it is used
>> elsewhere within git-subtree, but it is specifically discouraged.
> 
> There are 7 existing uses of `local`.  I think I prefer to use it here
> too.  In practice I think there are no shells we might want to use
> that don't have local.  The alternative is to change all the variable
> names to be obviously globally unique, which is clumsy and also seems
> to me to put us at greater risk of bugs.
> 
>>> +	if git rev-parse --verify -q "$rev:$config"; then
>>
>> For subtree split, should we also test for this file in tree you are
>> splitting: i.e., "$dir/$config"? The answer might be no.
> 
> You're right that we should consider this question.  The answer is:
> no, we should not.  Briefly, whether to use the new or old algorithms
> depends on whether the downstream has adopted the new git-subtree, not
> on whether the upstream has added some optional config.
> 
> https://codeberg.org/diziet/git-subtree/src/branch/main/DATA-MODEL.md#control-of-unmarked-subtree-merges-guessing-config
> 
>> I think that subtree merge should only test the top-level project, as
>> this patch does now.
> 
> By "top-level" I think you mean what I've taken to calling the
> "downstream": the project where the subtree is in a subdir, and whose
> top-level has other stuff.  In which case I agree.
> 
>> On 7/6/26 06:58, Ian Jackson wrote:
>>> Another, bigger, reason is that current git-subtree generates unmarked
>>> subtree merges (ie, without any git-subtree trailers)
>>
>> Subtree merges can be performed without git-subtree, via the `-X
>> subtree` merge strategy option. While the design of RIIR git-subtree is
>> outside the scope of this patch series, this may be worth thinking about
>> in your rewrite.
> 
> This is what I'm calling an "unmarked subtree merge".  My rewrite is
> not going to support this user behaviour.  The problem is that it is
> not possible to reliably determine whetheer something is an unmarked
> subtree merge.
> 
> It is possible to guess based on tree similarity, but that's a
> heuristic.  It's also possible to guess based on root commits.
> Both of these approaches can go wrong in some cases.  I prefer to
> write reliable software, which doesn't guess.
> 
> I'll advise against this practice in the documentation, but I'm
> reasonably confident that if a user does this anyway the results won't
> be terrible.  The upstream input to an unmarked subtree merge in a
> downstream that has already used my rewrite, will be treated as if it
> were a downstream branch that predates the subtree addition.  The
> effect on split (in most cases) is a missing parent relationship,
> which is undesirable but not catastrophic.I've made a note to add a
> test case for this scenario.
> 
> Combining manual -X subtree merges with git-subtree --squash merges
> could easily produce quite weird and wrong results in the tree (even
> before anyone tries split, or something).  I don't think I can even
> reliably detect this situation after the user has done it, and of
> course since that user is using plain git, I certainly can't prevent
> it.  This is another reason why manual use of -X subtree should be
> discouraged.
> 
> Regards,
> Ian.
>