Failing tests with WITH_BREAKING_CHANGES

Failing tests with WITH_BREAKING_CHANGES

[brian m. carlson]

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

I have the following in `config.mak`:

----
DEVELOPER=1
CC=clang
GENERATE_COMPILATION_DATABASE=yes
WITH_RUST=1
USE_ASCIIDOCTOR=1
WITH_BREAKING_CHANGES=1
----

In this configuration, I've noticed some tests failing:

----
t0014-alias.sh                                   (Wstat: 256 (exited 1) Tests: 23 Failed: 2)
  Failed tests:  4, 8
  Non-zero exit status: 1
t1517-outside-repo.sh                            (Wstat: 256 (exited 1) Tests: 404 Failed: 2)
  Failed tests:  248-249
  Non-zero exit status: 1
----

These don't occur if I remove `WITH_BREAKING_CHANGES=1`, so they appear
to be related to that option.  However, I know we have a CI job for that
case, so it's unclear to me why these tests are failing; perhaps the CI
job is not testing what we think it's testing.

I noticed this because I plan to send out a series soon based on that
option and obviously I want to run the testsuite first.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]

Re: Failing tests with WITH_BREAKING_CHANGES

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> I have the following in `config.mak`:
>
> ----
> DEVELOPER=1
> CC=clang
> GENERATE_COMPILATION_DATABASE=yes
> WITH_RUST=1
> USE_ASCIIDOCTOR=1
> WITH_BREAKING_CHANGES=1
> ----
>
> In this configuration, I've noticed some tests failing:
>
> ----
> t0014-alias.sh                                   (Wstat: 256 (exited 1) Tests: 23 Failed: 2)
>   Failed tests:  4, 8
>   Non-zero exit status: 1
> t1517-outside-repo.sh                            (Wstat: 256 (exited 1) Tests: 404 Failed: 2)
>   Failed tests:  248-249
>   Non-zero exit status: 1
> ----
>
> These don't occur if I remove `WITH_BREAKING_CHANGES=1`, so they appear
> to be related to that option.  However, I know we have a CI job for that
> case, so it's unclear to me why these tests are failing; perhaps the CI
> job is not testing what we think it's testing.

Does not immediately ring a bell for me.  All four integration
branches are OK in my builds.

> I noticed this because I plan to send out a series soon based on that
> option and obviously I want to run the testsuite first.

Re: Failing tests with WITH_BREAKING_CHANGES

[Phillip Wood]

Hi brian

On 28/07/2026 01:46, brian m. carlson wrote:
> I have the following in `config.mak`:
> 
> ----
> DEVELOPER=1
> CC=clang
> GENERATE_COMPILATION_DATABASE=yes
> WITH_RUST=1
> USE_ASCIIDOCTOR=1
> WITH_BREAKING_CHANGES=1
> ----
> 
> In this configuration, I've noticed some tests failing:
> 
> ----
> t0014-alias.sh                                   (Wstat: 256 (exited 1) Tests: 23 Failed: 2)
>    Failed tests:  4, 8
>    Non-zero exit status: 1
> t1517-outside-repo.sh                            (Wstat: 256 (exited 1) Tests: 404 Failed: 2)
>    Failed tests:  248-249
>    Non-zero exit status: 1
> ----

I find t1517 fails quite often for me due to cruft from a previous build 
when a different branch was checked out. I wonder if there is a command 
that is no-longer built by WITH_BREAKING_CHANGES whose executable still 
exists in the build directory from a previous build. Its not clear to me 
why the alias tests might be failing though.

Thanks

Phillip

> These don't occur if I remove `WITH_BREAKING_CHANGES=1`, so they appear
> to be related to that option.  However, I know we have a CI job for that
> case, so it's unclear to me why these tests are failing; perhaps the CI
> job is not testing what we think it's testing.
> 
> I noticed this because I plan to send out a series soon based on that
> option and obviously I want to run the testsuite first.

Re: Failing tests with WITH_BREAKING_CHANGES

[Jeff King]

On Tue, Jul 28, 2026 at 02:31:03PM +0100, Phillip Wood wrote:

> I find t1517 fails quite often for me due to cruft from a previous build
> when a different branch was checked out. I wonder if there is a command that
> is no-longer built by WITH_BREAKING_CHANGES whose executable still exists in
> the build directory from a previous build. Its not clear to me why the alias
> tests might be failing though.

It's the same reason. We test looping through deprecated aliases using
whatchanged and pack-redundant. When those are builtin but deprecated
(like now) we allow aliases. After the breaking-changes split, those
names are not special at all, and they are subject to the usual alias
rules. If there is crufty git-whatchanged in your build directory, then
that is an "external command" unknown to Git and you are not allowed to
alias over it.

The test in t0014 that covers this should be removed after the breaking
changes actually land (those commands won't handled specially, so it's
not different than the normal alias loop detection).

But we are in a funny limbo now for WITH_BREAKING_CHANGES. Possibly we
could pull the value out of GIT-BUILD-OPTIONS (which I guess happens
already via the environment) and use a prereq to skip the test.

-Peff

[PATCH 0/2] fix serial tests without/with breaking-changes

[Jeff King]

On Tue, Jul 28, 2026 at 09:55:32AM -0400, Jeff King wrote:

> But we are in a funny limbo now for WITH_BREAKING_CHANGES. Possibly we
> could pull the value out of GIT-BUILD-OPTIONS (which I guess happens
> already via the environment) and use a prereq to skip the test.

I think we can do even a bit better. Leaving aside
WITH_BREAKING_CHANGES, we should consider what will eventually happen to
these tests when those deprecated commands go away. I think we want to
keep them in preparation for when we have more deprecated commands.

So here's a patch.

It doesn't address the t1517 issue at all. That test can also be
confused by older build products, but I don't think deprecation is
particularly related.  It comes from going to an old version where some
now-vanished command doesn't pass the "-h" test and then traveling back
to the present. Probably it could be more careful about what it
considers a valid command. Right now it checks "git --list-cmds", but in
theory it could be using a list generated from the Makefile.

  [1/2]: t0014: factor out choice of deprecated commands
  [2/2]: t0014: generate deprecated command names dynamically

 t/t0014-alias.sh | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

-Peff

[PATCH 1/2] t0014: factor out choice of deprecated commands

[Jeff King]

We have a few tests related to aliasing deprecated commands which use
"whatchanged" and "pack-redundant", as these are the only two deprecated
commands we have. Let's pull those names into variables so that we can
refactor the tests without relying on the specific names.

Signed-off-by: Jeff King <peff@peff.net>
---
I pulled this into its own patch because it's so noisy, but it could be
squashed with the subsequent one.

 t/t0014-alias.sh | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh
index 5144b0effd..9d7c737355 100755
--- a/t/t0014-alias.sh
+++ b/t/t0014-alias.sh
@@ -27,17 +27,20 @@ test_expect_success 'looping aliases - internal execution' '
 	test_grep "^fatal: alias loop detected: expansion of" output
 '
 
+deprecated1=whatchanged
+deprecated2=pack-redundant
+
 test_expect_success 'looping aliases - deprecated builtins' '
-	test_config alias.whatchanged pack-redundant &&
-	test_config alias.pack-redundant whatchanged &&
+	test_config alias.$deprecated1 $deprecated2 &&
+	test_config alias.$deprecated2 $deprecated1 &&
 	cat >expect <<-EOF &&
-	${SQ}whatchanged${SQ} is aliased to ${SQ}pack-redundant${SQ}
-	${SQ}pack-redundant${SQ} is aliased to ${SQ}whatchanged${SQ}
-	fatal: alias loop detected: expansion of ${SQ}whatchanged${SQ} does not terminate:
-	  whatchanged <==
-	  pack-redundant ==>
+	${SQ}$deprecated1${SQ} is aliased to ${SQ}$deprecated2${SQ}
+	${SQ}$deprecated2${SQ} is aliased to ${SQ}$deprecated1${SQ}
+	fatal: alias loop detected: expansion of ${SQ}$deprecated1${SQ} does not terminate:
+	  $deprecated1 <==
+	  $deprecated2 ==>
 	EOF
-	test_must_fail git whatchanged -h 2>actual &&
+	test_must_fail git $deprecated1 -h 2>actual &&
 	test_cmp expect actual
 '
 
@@ -90,8 +93,8 @@ test_expect_success 'can alias-shadow via two deprecated builtins' '
 	# some git(1) commands will fail... (see above)
 	test_might_fail git status -h >expect &&
 	test_file_not_empty expect &&
-	test_might_fail git -c alias.whatchanged=pack-redundant \
-		-c alias.pack-redundant=status whatchanged -h >actual &&
+	test_might_fail git -c alias.$deprecated1=$deprecated2 \
+		-c alias.$deprecated2=status $deprecated1 -h >actual &&
 	test_cmp expect actual
 '
 
-- 
2.55.0.749.g30c495c7a6

Re: [PATCH 1/2] t0014: factor out choice of deprecated commands

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> We have a few tests related to aliasing deprecated commands which use
> "whatchanged" and "pack-redundant", as these are the only two deprecated
> commands we have. Let's pull those names into variables so that we can
> refactor the tests without relying on the specific names.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I pulled this into its own patch because it's so noisy, but it could be
> squashed with the subsequent one.

The knee-jerk reaction I got after reading the above explanation
before the morning caffeine fully taking effect and without looking
at [2/2] is "we may have parameterized the exact command names, but
I cannot tell what value this change has, as the fact that we have
exactly two deprecated commands is still hardcoded in the test".

If the point of this change is that even if we ever deprecated a
third command, this test does not need to care about it, then I can
understand it is perfectly fine to have the hardcoded "this test
uses two deprecated commands" while parameterizing which two
commands are used.  But then the log message may be a bit
misleading.  I dunno.

I am very sure that I will be enlightened when I read [2/2], though
;-)

>  t/t0014-alias.sh | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh
> index 5144b0effd..9d7c737355 100755
> --- a/t/t0014-alias.sh
> +++ b/t/t0014-alias.sh
> @@ -27,17 +27,20 @@ test_expect_success 'looping aliases - internal execution' '
>  	test_grep "^fatal: alias loop detected: expansion of" output
>  '
>  
> +deprecated1=whatchanged
> +deprecated2=pack-redundant
> +
>  test_expect_success 'looping aliases - deprecated builtins' '
> -	test_config alias.whatchanged pack-redundant &&
> -	test_config alias.pack-redundant whatchanged &&
> +	test_config alias.$deprecated1 $deprecated2 &&
> +	test_config alias.$deprecated2 $deprecated1 &&
>  	cat >expect <<-EOF &&
> -	${SQ}whatchanged${SQ} is aliased to ${SQ}pack-redundant${SQ}
> -	${SQ}pack-redundant${SQ} is aliased to ${SQ}whatchanged${SQ}
> -	fatal: alias loop detected: expansion of ${SQ}whatchanged${SQ} does not terminate:
> -	  whatchanged <==
> -	  pack-redundant ==>
> +	${SQ}$deprecated1${SQ} is aliased to ${SQ}$deprecated2${SQ}
> +	${SQ}$deprecated2${SQ} is aliased to ${SQ}$deprecated1${SQ}
> +	fatal: alias loop detected: expansion of ${SQ}$deprecated1${SQ} does not terminate:
> +	  $deprecated1 <==
> +	  $deprecated2 ==>
>  	EOF
> -	test_must_fail git whatchanged -h 2>actual &&
> +	test_must_fail git $deprecated1 -h 2>actual &&
>  	test_cmp expect actual
>  '
>  
> @@ -90,8 +93,8 @@ test_expect_success 'can alias-shadow via two deprecated builtins' '
>  	# some git(1) commands will fail... (see above)
>  	test_might_fail git status -h >expect &&
>  	test_file_not_empty expect &&
> -	test_might_fail git -c alias.whatchanged=pack-redundant \
> -		-c alias.pack-redundant=status whatchanged -h >actual &&
> +	test_might_fail git -c alias.$deprecated1=$deprecated2 \
> +		-c alias.$deprecated2=status $deprecated1 -h >actual &&
>  	test_cmp expect actual
>  '

[PATCH 2/2] t0014: generate deprecated command names dynamically

[Jeff King]

We have a few tests related to aliasing of deprecated commands. They use
whatchanged and pack-redundant because those are the only two deprecated
commands we have. Eventually those commands will be removed, at which
point these tests will be checking nothing useful (they'll just be
regular aliases, which we already cover in other tests).

We could remove them at that point, but the code to handle deprecated
commands will still remain. We probably do want to keep the tests around
for the eventual day that we deprecate more commands. So let's ask Git
for its list of deprecated commands, and if we don't have any, skip
those tests.

This also prevents an annoying corner case when your build directory
contains old build products. Right now those commands are marked as
deprecated builtins and treated specially; we allow aliases and never
look for them as dashed external commands. But after they are removed,
they aren't special anymore. If your directory happens to contain
hardlinks from the build of an older version, that confuses Git: it sees
the old hardlinks in place, thinks those are actual external commands,
and refuses to allow aliasing.

You can see that today like this:

  make
  make WITH_BREAKING_CHANGES=1 test

The first "make" creates git-whatchanged as a hardlink to Git, and the
second does not clean it up (it doesn't know about the whatchanged
command at all anymore). t0014 fails because Git won't create an alias
to the "external" whatchanged command.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t0014-alias.sh | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh
index 9d7c737355..cbc447b481 100755
--- a/t/t0014-alias.sh
+++ b/t/t0014-alias.sh
@@ -27,10 +27,15 @@ test_expect_success 'looping aliases - internal execution' '
 	test_grep "^fatal: alias loop detected: expansion of" output
 '
 
-deprecated1=whatchanged
-deprecated2=pack-redundant
+test_expect_success 'detect deprecated commands' '
+	git --list-cmds=deprecated >deprecated &&
+	if read deprecated1 && read deprecated2
+	then
+		test_set_prereq HAVE_DEPRECATED
+	fi <deprecated
+'
 
-test_expect_success 'looping aliases - deprecated builtins' '
+test_expect_success HAVE_DEPRECATED 'looping aliases - deprecated builtins' '
 	test_config alias.$deprecated1 $deprecated2 &&
 	test_config alias.$deprecated2 $deprecated1 &&
 	cat >expect <<-EOF &&
@@ -89,7 +94,7 @@ test_expect_success 'can alias-shadow deprecated builtins' '
 	done
 '
 
-test_expect_success 'can alias-shadow via two deprecated builtins' '
+test_expect_success HAVE_DEPRECATED 'can alias-shadow via two deprecated builtins' '
 	# some git(1) commands will fail... (see above)
 	test_might_fail git status -h >expect &&
 	test_file_not_empty expect &&
-- 
2.55.0.749.g30c495c7a6

Re: [PATCH 2/2] t0014: generate deprecated command names dynamically

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> We have a few tests related to aliasing of deprecated commands. They use
> whatchanged and pack-redundant because those are the only two deprecated
> commands we have. Eventually those commands will be removed, at which
> point these tests will be checking nothing useful (they'll just be
> regular aliases, which we already cover in other tests).
>
> We could remove them at that point, but the code to handle deprecated
> commands will still remain. We probably do want to keep the tests around
> for the eventual day that we deprecate more commands. So let's ask Git
> for its list of deprecated commands, and if we don't have any, skip
> those tests.

Ah, now I understand.  So HAVE_DEPRECATED prerequisite guards tests
that require at least two deprecated commands, so that we can test
cases with aliases that involve two commands among deprecated ones
referring to each other.  Obviously, with 0 or 1 deprecated commands,
there is no point to perform such tests.

Makes sense.

Thanks.

> diff --git a/t/t0014-alias.sh b/t/t0014-alias.sh
> index 9d7c737355..cbc447b481 100755
> --- a/t/t0014-alias.sh
> +++ b/t/t0014-alias.sh
> @@ -27,10 +27,15 @@ test_expect_success 'looping aliases - internal execution' '
>  	test_grep "^fatal: alias loop detected: expansion of" output
>  '
>  
> -deprecated1=whatchanged
> -deprecated2=pack-redundant
> +test_expect_success 'detect deprecated commands' '
> +	git --list-cmds=deprecated >deprecated &&
> +	if read deprecated1 && read deprecated2
> +	then
> +		test_set_prereq HAVE_DEPRECATED
> +	fi <deprecated
> +'
>  
> -test_expect_success 'looping aliases - deprecated builtins' '
> +test_expect_success HAVE_DEPRECATED 'looping aliases - deprecated builtins' '
>  	test_config alias.$deprecated1 $deprecated2 &&
>  	test_config alias.$deprecated2 $deprecated1 &&
>  	cat >expect <<-EOF &&
> @@ -89,7 +94,7 @@ test_expect_success 'can alias-shadow deprecated builtins' '
>  	done
>  '
>  
> -test_expect_success 'can alias-shadow via two deprecated builtins' '
> +test_expect_success HAVE_DEPRECATED 'can alias-shadow via two deprecated builtins' '
>  	# some git(1) commands will fail... (see above)
>  	test_might_fail git status -h >expect &&
>  	test_file_not_empty expect &&

Re: [PATCH 2/2] t0014: generate deprecated command names dynamically

[Jeff King]

On Tue, Jul 28, 2026 at 09:01:53AM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > We have a few tests related to aliasing of deprecated commands. They use
> > whatchanged and pack-redundant because those are the only two deprecated
> > commands we have. Eventually those commands will be removed, at which
> > point these tests will be checking nothing useful (they'll just be
> > regular aliases, which we already cover in other tests).
> >
> > We could remove them at that point, but the code to handle deprecated
> > commands will still remain. We probably do want to keep the tests around
> > for the eventual day that we deprecate more commands. So let's ask Git
> > for its list of deprecated commands, and if we don't have any, skip
> > those tests.
> 
> Ah, now I understand.  So HAVE_DEPRECATED prerequisite guards tests
> that require at least two deprecated commands, so that we can test
> cases with aliases that involve two commands among deprecated ones
> referring to each other.  Obviously, with 0 or 1 deprecated commands,
> there is no point to perform such tests.

Yeah. Sorry, maybe splitting the two just made it more confusing (it was
really to make the diff a bit less heinous). I'm OK if you want to just
squash them together (using the commit message from the second).

I suspect we could _probably_ rewrite the "looping aliases" test to also
run when there's only 1 deprecated command (just looping on itself). But
since we have two now, and plan to have zero later, I don't know that
it's worth the effort of doing so.

-Peff

Re: [PATCH 2/2] t0014: generate deprecated command names dynamically

[brian m. carlson]

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

On 2026-07-28 at 14:38:45, Jeff King wrote:
> We have a few tests related to aliasing of deprecated commands. They use
> whatchanged and pack-redundant because those are the only two deprecated
> commands we have. Eventually those commands will be removed, at which
> point these tests will be checking nothing useful (they'll just be
> regular aliases, which we already cover in other tests).
> 
> We could remove them at that point, but the code to handle deprecated
> commands will still remain. We probably do want to keep the tests around
> for the eventual day that we deprecate more commands. So let's ask Git
> for its list of deprecated commands, and if we don't have any, skip
> those tests.
> 
> This also prevents an annoying corner case when your build directory
> contains old build products. Right now those commands are marked as
> deprecated builtins and treated specially; we allow aliases and never
> look for them as dashed external commands. But after they are removed,
> they aren't special anymore. If your directory happens to contain
> hardlinks from the build of an older version, that confuses Git: it sees
> the old hardlinks in place, thinks those are actual external commands,
> and refuses to allow aliasing.
> 
> You can see that today like this:
> 
>   make
>   make WITH_BREAKING_CHANGES=1 test
> 
> The first "make" creates git-whatchanged as a hardlink to Git, and the
> second does not clean it up (it doesn't know about the whatchanged
> command at all anymore). t0014 fails because Git won't create an alias
> to the "external" whatchanged command.

These patches look sensible.  I was planning to spend some time this
morning investigating more since I woke up early, but I appreciate you
sending some patches in to fix them.
-- 
brian m. carlson (they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 325 bytes --]