[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

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

[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 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.
> 

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

[Colin Stagner]

On 7/9/26 04:36, Ian Jackson wrote:

> Colin Stagner writes ("Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite"):
>
>> 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.

Yes, I think we're talking about the same thing.

In retrospect, "top-level" is ambiguous. "Upstream" and "downstream" may 
be as well. Within git-branch(1), the phrase "upstream" refers to the 
remote tracking branch set by

     git branch --set-upstream-to=<upstream>

git-merge(1) is consistent with this.

     "If no commit is given from the command line, merge the
      remote-tracking branches that the current branch is
      configured to use as its upstream."

git-subtree.sh doesn't really deal in "upstreams" in the git-branch or 
git-merge sense.

Less ambiguous language is available:

For merge commits, there is the "first parent" and "second parent" (or 
3rd or higher parents).

For trees, there is the "root tree" and "sub-trees," like `git ls-tree -r`

     -r     Recurse into sub-trees.

Both of these deliberately ignore the dependency relationship between 
the various projects and branches in question, which can potentially get 
messy.

>>> +	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.

Very well-reasoned; I like it.

Let me ask this question in a slightly different way: does RIIR subtree 
honor config files in locations other than the one you test for above? 
That's

     ${rev}:.git-subtree/config

which is `.git-subtree/config` within the root tree of the rev that is 
being manipulated?

If this is the only config file RIIR subtree honors, the patch is 
probably correct. If RIIR subtree honors config from other places, such as

* the working tree
* HEAD:.git-subtree/config
* HEAD:./.git-subtree/config

then consider testing for those if appropriate.

>> Subtree merges can be performed without git-subtree, via the `-X
>> subtree` merge strategy option.
> 
> 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.

Thanks for looking at this.

> Combining manual -X subtree merges with git-subtree --squash merges
> could easily produce quite weird and wrong results in the tree

I haven't tried it, but I think if --squash is in use, then attempting 
an unmarked subtree merge will probably die with "unrelated history" 
warnings.

Looking forward to v2,

Colin

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

[Ian Jackson]

Colin Stagner writes ("Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite [and 1 more messages]"):
> In retrospect, "top-level" is ambiguous. "Upstream" and "downstream" may 
> be as well. Within git-branch(1), the phrase "upstream" refers to the 
> remote tracking branch set by

Upstream and downstream are of course relative terms.  I think the
git usage you cite isn't quite central.

> git-subtree.sh doesn't really deal in "upstreams" in the git-branch or 
> git-merge sense.

I'm using "upstream" in the wider sense; here, when you import a
depedency you're downstream of it.

I'm open to better terminology and now is a good time to be debating
this, but I don't like the other suggestions so far.

I want a term that talks about the logical (even, social) relationship
between the two projects; and it should be one that makes sense from
the point of view of the upstream.  Talking about the file position
within the downstream tree doesn't make sense from the upstream's
point of view.

> Both of these deliberately ignore the dependency relationship between 
> the various projects and branches in question, which can potentially get 
> messy.

I think the dependency relationship is inherent in git-subtree's usual
use cases: suppose a project A gets merged with git-subtree into a
subdirectory S of project B, so that B.git:/S/ is a copy of A.git:/

Then I think almost invariably, this is because A has B as a
dependency.  And A has B as an upstream:

Code that's part of B flows from B to A, and can be edited in A, but
the canonical version is that in B itself.  If there are multiple As
incorporating the same B, they share via "split", which produces
history "within" B.  Thios seems a classic upstream/downstream
relationship.

As I say, I'm open to other terminology but I don't think "root tree"
and "subtree" are the general terms I need to describe the
relationship.  In particular, from the point of view of the upstream
project, it is its own root tree.

> Very well-reasoned; I like it.
> 
> Let me ask this question in a slightly different way: does RIIR subtree 
> honor config files in locations other than the one you test for above? 
> That's
> 
>      ${rev}:.git-subtree/config

Yes, but not relevantly.  Different information is taken from
different places (the design gets a little complex to make sure
everything works in all the use cases).

> > Combining manual -X subtree merges with git-subtree --squash merges
> > could easily produce quite weird and wrong results in the tree
> 
> I haven't tried it, but I think if --squash is in use, then attempting 
> an unmarked subtree merge will probably die with "unrelated history" 
> warnings.

I think that's not guaranteed if squash merges and non-squash merges
are interleaved.

> Looking forward to v2,

Thanks for your support, and your critical consideration of the design
questions.

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]

[D. Ben Knoble]

Hi Ian,

On Thu, Jul 9, 2026 at 5:48 AM Ian Jackson
<ijackson@chiark.greenend.org.uk> wrote:
[snip]
> > 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.

Just to make sure I understand you (I regularly use -X subtree with
one project): the Rust rewrite won't support -X subtree merges, but we
don't intend to discourage folks from using -X subtree merges in toto,
right? Merely not support a mix of the 2?

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

[Ian Jackson]

D. Ben Knoble writes ("Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite [and 1 more messages]"):
> Just to make sure I understand you (I regularly use -X subtree with
> one project): the Rust rewrite won't support -X subtree merges, but we
> don't intend to discourage folks from using -X subtree merges in toto,
> right? Merely not support a mix of the 2?

Precisely so.

I think you may find my git-subtree rewrite superior in ergonomics to
git merge -X subtree so you might want to switch to it, when it exists
and supports that transition.

I haven't yet thought about how that transition ought to go but I
think it might look like what I'm calling an "unmarked subtree
merge~.  I've made a TODO note in my working branch to remind myself
to consider this situation.

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]

[Junio C Hamano]

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

> 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.

So, is there a conclusion after reviewing this?

I think this is the only thing outstanding item among the review
comments this thread received.  Specifically, regarding the use of
'local' discussed in the thread, our coding guidelines explicitly
state:

 - Even though "local" is not part of POSIX, we make heavy use of it
   in our test suite.  We do not use it in scripted Porcelains, and
   hopefully nobody starts using "local" before all shells that matter
   support it (notably, ksh from AT&T Research does not support it yet).

Thus, we are fine there.

Just responding belatedly as I was scanning topics that are marked
as "Expecting a reroll" in my draft copy of the "What's cooking"
report that I work from.

Thanks.

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

[Colin Stagner]

On 7/11/26 18:04, Junio C Hamano wrote:

> So, is there a conclusion after reviewing this?

I think we're expecting a reroll, but this looks like the way forward.

Colin

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

[Ian Jackson]

Colin Stagner writes ("Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite [and 1 more messages]"):
> On 7/11/26 18:04, Junio C Hamano wrote:
> > So, is there a conclusion after reviewing this?
> 
> I think we're expecting a reroll, but this looks like the way forward.

Yes.  Please bear with me, I'm travelling for a few days.

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]

[Junio C Hamano]

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

> Colin Stagner writes ("Re: [PATCH 1/2] git-subtree: Bail out if we find output from Rust rewrite [and 1 more messages]"):
>> On 7/11/26 18:04, Junio C Hamano wrote:
>> > So, is there a conclusion after reviewing this?
>> 
>> I think we're expecting a reroll, but this looks like the way forward.
>
> Yes.  Please bear with me, I'm travelling for a few days.
>
> Ian.

No worries, and take your time.  I was just updating the status of
the various topics in the "What's cooking" draft.

Thanks.