threads / patch / 63090

patch, 4 partsdrop "name-rev --stdin" support

Subject: [PATCH v1 0/4] drop "name-rev --stdin" support

## tl;dr

20 messages between Mar 10, 2025 and Mar 13, 2025. Diffs are folded; open one to read it.

replies: 19people: 3as markdown or json

Junio C Hamano· Mar 10, 2025, 23:16 UTC · lore
[Administrivia]
    Yes, yes, I know I shouldn't be playing with shiny new toys
    during the pre-release freeze time.  But just to show others who
    may still be doing so that the first patch in this series has
    already be written to avoid duplicated and conflicting work, I
    am sending them out.  I have no intention to push the topic
    further before the final.

During Git 2.40 timeframe, we deprecated "--stdin" option of the "name-rev" command in preference to "--annotate-stdin", and removed the mention of it from the documentation.

Let's prepare for Git 3.0 to stop supporting it.

The real motive of these patches is not really about that option but make sure we have, with WITH_BREAKING_CHANGES compilation knob, enough support to keep preparing these changes. The first patch renames the WITHOUT_BREAKING_CHANGES prerequisite that unfortunately invites double negations easily and changes existing users of it, then two patches to a test script minimally modernizes it. The last step introduces the real change, guarded by WITH_BREAKING_CHANGES compilation knob.

Junio C Hamano (4):
  t: introduce WITH_BREAKING_CHANGES prerequisite
  t6120: avoid hiding "git" exit status
  t6120: further modernize
  name-rev: remove "--stdin" support
 Documentation/BreakingChanges.adoc |  6 ++++++
 builtin/name-rev.c                 |  2 ++
 t/t5323-pack-redundant.sh          |  2 +-
 t/t5505-remote.sh                  |  6 +++---
 t/t5515-fetch-merge-logic.sh       |  2 +-
 t/t5516-fetch-push.sh              |  8 ++++----
 t/t6120-describe.sh                | 18 +++++++++++++-----
 t/test-lib.sh                      |  5 +++++
 8 files changed, 35 insertions(+), 14 deletions(-)
-- 
2.49.0-rc2-173-g4d16673c2b
Junio C Hamano· Mar 10, 2025, 23:16 UTC · re: Junio C Hamano · lore

[PATCH v1 1/4] t: introduce WITH_BREAKING_CHANGES prerequisite

Earlier c5bc9a7f (Makefile: wire up build option for deprecated features, 2025-01-22) made an unfortunate decision to introduce the WITHOUT_BREAKING_CHANGES prerequisite to perform tests that ensure the historical behaviour that may be different from what we will have in the future. It would inevitably invite doulbe negation when we need to add tests to ensure the behaviour we want to have in the future.

Introduce WITH_BREAKING_CHANGES prerequisite and replace the existing uses of WITHOUT_BREAKING_CHANGES prerequisite. Some in-flight topics that add more uses of WITHOUT_BREAKING_CHANGES would still need the old prerequisite, so let's keep its definition for now while we'll eradicate its use.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5323-pack-redundant.sh    | 2 +-
 t/t5505-remote.sh            | 6 +++---
 t/t5515-fetch-merge-logic.sh | 2 +-
 t/t5516-fetch-push.sh        | 8 ++++----
 t/test-lib.sh                | 5 +++++
 5 files changed, 14 insertions(+), 9 deletions(-)
Show changes to 5 files +14 −9

t/t5323-pack-redundant.sh, t/t5505-remote.sh, t/t5515-fetch-merge-logic.sh, t/t5516-fetch-push.sh, t/test-lib.sh

diff --git a/t/t5323-pack-redundant.sh b/t/t5323-pack-redundant.sh
index 688cd9706c..bc30bc9652 100755
--- a/t/t5323-pack-redundant.sh
+++ b/t/t5323-pack-redundant.sh
@@ -36,7 +36,7 @@ relationship between packs and objects is as follows:
 
 . ./test-lib.sh
 
-if ! test_have_prereq WITHOUT_BREAKING_CHANGES
+if test_have_prereq WITH_BREAKING_CHANGES
 then
 	skip_all='skipping git-pack-redundant tests; built with breaking changes'
 	test_done
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index bb7e0c6879..82fccf8e36 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -1123,7 +1123,7 @@ Pull: refs/heads/main:refs/heads/origin
 Pull: refs/heads/next:refs/heads/origin2
 EOF
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/remotes' '
+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/remotes' '
 	git clone one five &&
 	origin_url=$(pwd)/one &&
 	(
@@ -1149,7 +1149,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file i
 	)
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches' '
+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches' '
 	git clone --template= one six &&
 	origin_url=$(pwd)/one &&
 	(
@@ -1165,7 +1165,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file i
 	)
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches (2)' '
+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches (2)' '
 	git clone --template= one seven &&
 	(
 		cd seven &&
diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
index 4e6026c611..8ac04d742c 100755
--- a/t/t5515-fetch-merge-logic.sh
+++ b/t/t5515-fetch-merge-logic.sh
@@ -104,7 +104,7 @@ test_expect_success setup '
 	git config remote.config-glob.fetch refs/heads/*:refs/remotes/rem/* &&
 	remotes="$remotes config-glob" &&
 
-	if test_have_prereq WITHOUT_BREAKING_CHANGES
+	if ! test_have_prereq WITH_BREAKING_CHANGES
 	then
 		mkdir -p .git/remotes &&
 		cat >.git/remotes/remote-explicit <<-\EOF &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 85ed049627..6e2b233157 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -975,7 +975,7 @@ test_expect_success 'allow push to HEAD of non-bare repository (config)' '
 	! grep "warning: updating the current branch" stderr
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches' '
+test_expect_success !WITH_BREAKING_CHANGES 'fetch with branches' '
 	mk_empty testrepo &&
 	git branch second $the_first_commit &&
 	git checkout second &&
@@ -991,7 +991,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches' '
 	git checkout main
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches containing #' '
+test_expect_success !WITH_BREAKING_CHANGES 'fetch with branches containing #' '
 	mk_empty testrepo &&
 	mkdir testrepo/.git/branches &&
 	echo "..#second" > testrepo/.git/branches/branch2 &&
@@ -1005,7 +1005,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches containing #'
 	git checkout main
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches' '
+test_expect_success !WITH_BREAKING_CHANGES 'push with branches' '
 	mk_empty testrepo &&
 	git checkout second &&
 
@@ -1022,7 +1022,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches' '
 	)
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches containing #' '
+test_expect_success !WITH_BREAKING_CHANGES 'push with branches containing #' '
 	mk_empty testrepo &&
 
 	test_when_finished "rm -rf .git/branches" &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9001ed3a64..12fe82f660 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1862,6 +1862,11 @@ test_lazy_prereq CURL '
 	curl --version
 '
 
+test_lazy_prereq WITH_BREAKING_CHANGES '
+	test -n "$WITH_BREAKING_CHANGES"
+'
+
+# DEPRECATED; DO NOT USE THIS IN NEW TESTS
 test_lazy_prereq WITHOUT_BREAKING_CHANGES '
 	test -z "$WITH_BREAKING_CHANGES"
 '
-- 
2.49.0-rc2-173-g4d16673c2b
Eric Sunshine· Mar 10, 2025, 23:53 UTC · re: Junio C Hamano · lore

Re: [PATCH v1 1/4] t: introduce WITH_BREAKING_CHANGES prerequisite

On Mon, Mar 10, 2025 at 7:17 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> Earlier c5bc9a7f (Makefile: wire up build option for deprecated
> features, 2025-01-22) made an unfortunate decision to introduce the
> WITHOUT_BREAKING_CHANGES prerequisite to perform tests that ensure
> the historical behaviour that may be different from what we will
> have in the future.  It would inevitably invite doulbe negation when
> we need to add tests to ensure the behaviour we want to have in the
> future.
s/doulbe/double/
Show 7 quoted lines
> Introduce WITH_BREAKING_CHANGES prerequisite and replace the
> existing uses of WITHOUT_BREAKING_CHANGES prerequisite.  Some
> in-flight topics that add more uses of WITHOUT_BREAKING_CHANGES
> would still need the old prerequisite, so let's keep its definition
> for now while we'll eradicate its use.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Patrick Steinhardt· Mar 11, 2025, 12:57 UTC · re: Junio C Hamano · lore

Re: [PATCH v1 1/4] t: introduce WITH_BREAKING_CHANGES prerequisite

On Mon, Mar 10, 2025 at 04:16:49PM -0700, Junio C Hamano wrote:
Show 16 quoted lines
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 9001ed3a64..12fe82f660 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -1862,6 +1862,11 @@ test_lazy_prereq CURL '
>  	curl --version
>  '
>  
> +test_lazy_prereq WITH_BREAKING_CHANGES '
> +	test -n "$WITH_BREAKING_CHANGES"
> +'
> +
> +# DEPRECATED; DO NOT USE THIS IN NEW TESTS
>  test_lazy_prereq WITHOUT_BREAKING_CHANGES '
>  	test -z "$WITH_BREAKING_CHANGES"
>  '

Do we maybe want to state that this can be removed once the next release cycle is over? I find it to be a bit more actionable when stating hard dates after which something can be dropped as it allows any drive-by developers to act when they notice that Git v2.50 has been released already.

Patrick
Junio C Hamano· Mar 11, 2025, 17:07 UTC · re: Patrick Steinhardt · lore

Re: [PATCH v1 1/4] t: introduce WITH_BREAKING_CHANGES prerequisite

Patrick Steinhardt <ps@pks.im> writes:
Show 20 quoted lines
> On Mon, Mar 10, 2025 at 04:16:49PM -0700, Junio C Hamano wrote:
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index 9001ed3a64..12fe82f660 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -1862,6 +1862,11 @@ test_lazy_prereq CURL '
>>  	curl --version
>>  '
>>  
>> +test_lazy_prereq WITH_BREAKING_CHANGES '
>> +	test -n "$WITH_BREAKING_CHANGES"
>> +'
>> +
>> +# DEPRECATED; DO NOT USE THIS IN NEW TESTS
>>  test_lazy_prereq WITHOUT_BREAKING_CHANGES '
>>  	test -z "$WITH_BREAKING_CHANGES"
>>  '
>
> Do we maybe want to state that this can be removed once the next release
> cycle is over?

Perhaps. As 'seen' is pretty-much closed at this point and there is nothing in flight that uses WITHOUT_ variant in there, v2 of this series can just do without it, which may be simpler.

> I find it to be a bit more actionable when stating hard
> dates after which something can be dropped 

True, that is a good strategy for a transition that takes longer time.

Junio C Hamano· Mar 10, 2025, 23:16 UTC · re: Junio C Hamano · lore

[PATCH v1 2/4] t6120: avoid hiding "git" exit status

A handful of tests invoke "git" on the upstream side of a pipe, hiding its exit status. Correct them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t6120-describe.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Show changes to t/t6120-describe.sh +4 −2
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 76843a6169..dcb526e37d 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -292,13 +292,15 @@ test_expect_success 'name-rev --annotate-stdin' '
 		echo "$rev ($name)" >>expect.unsorted || return 1
 	done &&
 	sort <expect.unsorted >expect &&
-	git rev-list --all | git name-rev --annotate-stdin >actual.unsorted &&
+	git rev-list --all >list &&
+	git name-rev --annotate-stdin <list >actual.unsorted &&
 	sort <actual.unsorted >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'name-rev --stdin deprecated' "
-	git rev-list --all | git name-rev --stdin 2>actual &&
+	git rev-list --all >list &&
+	git name-rev --stdin <list 2>actual &&
 	grep -E 'warning: --stdin is deprecated' actual
 "
 
-- 
2.49.0-rc2-173-g4d16673c2b
Junio C Hamano· Mar 10, 2025, 23:16 UTC · re: Junio C Hamano · lore

[PATCH v1 3/4] t6120: further modernize

There is absolutely no reason why a pattern given to grep to find 'warning: --stdin is deprecated' must be quoted within a pair of single quotes, or the pattern to look for the literal string as ERE.

Quote the test body with a pair of single quotes like everybody else, and quote the needle string in a pair of double quotes. Also use test_grep instead of "grep -E".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t6120-describe.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to t/t6120-describe.sh +3 −3
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index dcb526e37d..71e261394a 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -298,11 +298,11 @@ test_expect_success 'name-rev --annotate-stdin' '
 	test_cmp expect actual
 '
 
-test_expect_success 'name-rev --stdin deprecated' "
+test_expect_success 'name-rev --stdin deprecated' '
 	git rev-list --all >list &&
 	git name-rev --stdin <list 2>actual &&
-	grep -E 'warning: --stdin is deprecated' actual
-"
+	test_grep "warning: --stdin is deprecated" actual
+'
 
 test_expect_success 'describe --contains with the exact tags' '
 	echo "A^0" >expect &&
-- 
2.49.0-rc2-173-g4d16673c2b
Junio C Hamano· Mar 10, 2025, 23:16 UTC · re: Junio C Hamano · lore

[PATCH v1 4/4] name-rev: remove "--stdin" support

As part of Git 3.0, remove the hidden synonym for "--annotate-stdin" for real. As this does not change the fact that it used to be called "--stdin" in older version of Git, keep that passage in the documentation for "--annotate-stdin".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/BreakingChanges.adoc |  6 ++++++
 builtin/name-rev.c                 |  2 ++
 t/t6120-describe.sh                | 10 ++++++++--
 3 files changed, 16 insertions(+), 2 deletions(-)
Show changes to 3 files +16 −2

Documentation/BreakingChanges.adoc, builtin/name-rev.c, t/t6120-describe.sh

diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc
index bdfad29d8a..61bdd586b9 100644
--- a/Documentation/BreakingChanges.adoc
+++ b/Documentation/BreakingChanges.adoc
@@ -178,6 +178,12 @@ references.
 +
 These features will be removed.
 
+* Support for "--stdin" option in the "name-rev" command was
+  deprecated (and hidden from the documentation) in the Git 2.40
+  timeframe, in preference to its synonym "--annotate-stdin".  Git 3.0
+  removes the support for "--stdin" altogether.
+
+
 == Superseded features that will not be deprecated
 
 Some features have gained newer replacements that aim to improve the design in
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index beac166b5c..3f49138551 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -578,11 +578,13 @@ int cmd_name_rev(int argc,
 				   N_("ignore refs matching <pattern>")),
 		OPT_GROUP(""),
 		OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
+#ifndef WITH_BREAKING_CHANGES
 		OPT_BOOL_F(0,
 			   "stdin",
 			   &transform_stdin,
 			   N_("deprecated: use --annotate-stdin instead"),
 			   PARSE_OPT_HIDDEN),
+#endif /* WITH_BREAKING_CHANGES */
 		OPT_BOOL(0, "annotate-stdin", &annotate_stdin, N_("annotate text from stdin")),
 		OPT_BOOL(0, "undefined", &allow_undefined, N_("allow to print `undefined` names (default)")),
 		OPT_BOOL(0, "always",     &always,
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 71e261394a..256ccaefb7 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -300,8 +300,14 @@ test_expect_success 'name-rev --annotate-stdin' '
 
 test_expect_success 'name-rev --stdin deprecated' '
 	git rev-list --all >list &&
-	git name-rev --stdin <list 2>actual &&
-	test_grep "warning: --stdin is deprecated" actual
+	if ! test_have_prereq WITH_BREAKING_CHANGES
+	then
+		git name-rev --stdin <list 2>actual &&
+		test_grep "warning: --stdin is deprecated" actual
+	else
+		test_must_fail git name-rev --stdin <list 2>actual &&
+		test_grep "unknown option .stdin." actual
+	fi
 '
 
 test_expect_success 'describe --contains with the exact tags' '
-- 
2.49.0-rc2-173-g4d16673c2b
Patrick Steinhardt· Mar 11, 2025, 12:57 UTC · re: Junio C Hamano · lore

Re: [PATCH v1 4/4] name-rev: remove "--stdin" support

On Mon, Mar 10, 2025 at 04:16:52PM -0700, Junio C Hamano wrote:
Show 18 quoted lines
> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
> index beac166b5c..3f49138551 100644
> --- a/builtin/name-rev.c
> +++ b/builtin/name-rev.c
> @@ -578,11 +578,13 @@ int cmd_name_rev(int argc,
>  				   N_("ignore refs matching <pattern>")),
>  		OPT_GROUP(""),
>  		OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
> +#ifndef WITH_BREAKING_CHANGES
>  		OPT_BOOL_F(0,
>  			   "stdin",
>  			   &transform_stdin,
>  			   N_("deprecated: use --annotate-stdin instead"),
>  			   PARSE_OPT_HIDDEN),
> +#endif /* WITH_BREAKING_CHANGES */
>  		OPT_BOOL(0, "annotate-stdin", &annotate_stdin, N_("annotate text from stdin")),
>  		OPT_BOOL(0, "undefined", &allow_undefined, N_("allow to print `undefined` names (default)")),
>  		OPT_BOOL(0, "always",     &always,

I was wondering whether we should also #ifdef `transform_stdin` and its single user to more closely reflect what we would have once the feature is fully removed.

Patrick
Junio C Hamano· Mar 11, 2025, 17:07 UTC · re: Patrick Steinhardt · lore

Re: [PATCH v1 4/4] name-rev: remove "--stdin" support

Patrick Steinhardt <ps@pks.im> writes:
Show 23 quoted lines
> On Mon, Mar 10, 2025 at 04:16:52PM -0700, Junio C Hamano wrote:
>> diff --git a/builtin/name-rev.c b/builtin/name-rev.c
>> index beac166b5c..3f49138551 100644
>> --- a/builtin/name-rev.c
>> +++ b/builtin/name-rev.c
>> @@ -578,11 +578,13 @@ int cmd_name_rev(int argc,
>>  				   N_("ignore refs matching <pattern>")),
>>  		OPT_GROUP(""),
>>  		OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
>> +#ifndef WITH_BREAKING_CHANGES
>>  		OPT_BOOL_F(0,
>>  			   "stdin",
>>  			   &transform_stdin,
>>  			   N_("deprecated: use --annotate-stdin instead"),
>>  			   PARSE_OPT_HIDDEN),
>> +#endif /* WITH_BREAKING_CHANGES */
>>  		OPT_BOOL(0, "annotate-stdin", &annotate_stdin, N_("annotate text from stdin")),
>>  		OPT_BOOL(0, "undefined", &allow_undefined, N_("allow to print `undefined` names (default)")),
>>  		OPT_BOOL(0, "always",     &always,
>
> I was wondering whether we should also #ifdef `transform_stdin` and its
> single user to more closely reflect what we would have once the feature
> is fully removed.
Absolutely.  That's a great point.
Thanks.
Junio C Hamano· Mar 11, 2025, 21:24 UTC · re: Junio C Hamano · lore

[PATCH v2 0/6] drop "name-rev --stdin" support

During Git 2.40 timeframe, we deprecated the "--stdin" option of the "name-rev" command in preference to the "--annotate-stdin" option, and removed the mention of the former from the documentation.

Let's prepare for Git 3.0 to stop supporting it.

The real motive of these patches is not really about that particular option but make sure we have, with WITH_BREAKING_CHANGES compilation knob, enough support to keep preparing for these changes.

The first two patches are preliminary clean-up and enhancement. We lacked documentation on test_lazy_prereq and we did not have a good way to signal a prerequisite that no longer should be used, both of which are remedied.

The third patch renames the WITHOUT_BREAKING_CHANGES prerequisite that unfortunately invites double negations easily and changes existing users of it.

Then two patches to a test script minimally modernizes it.  

The last step introduces the real change, guarded by WITH_BREAKING_CHANGES compilation knob. The resulting code is more littered with #if[n]def compared to the previous round but it should be clear which section of the code should go away once we do Git 3.0, which is the whole point of the WITH_BREAKING_CHANGES exercise.

Junio C Hamano (6):
  t: document test_lazy_prereq
  t: extend test_lazy_prereq
  t: introduce WITH_BREAKING_CHANGES prerequisite
  t6120: avoid hiding "git" exit status
  t6120: further modernize
  name-rev: remove "--stdin" support
 Documentation/BreakingChanges.adoc |  6 ++++++
 builtin/name-rev.c                 | 10 +++++++++-
 t/README                           | 25 ++++++++++++++++++++++++-
 t/t5323-pack-redundant.sh          |  2 +-
 t/t5505-remote.sh                  |  6 +++---
 t/t5515-fetch-merge-logic.sh       |  2 +-
 t/t5516-fetch-push.sh              |  8 ++++----
 t/t6120-describe.sh                | 18 +++++++++++++-----
 t/test-lib-functions.sh            |  5 +++++
 t/test-lib.sh                      |  7 ++++++-
 10 files changed, 72 insertions(+), 17 deletions(-)
-- 
2.49.0-rc2-181-g28e223d67e
Junio C Hamano· Mar 11, 2025, 21:25 UTC · re: Junio C Hamano · lore

[PATCH v2 1/6] t: document test_lazy_prereq

The t/README file talked about test_set_prereq but lacked explanation on test_lazy_prereq, which is a more modern way to define prerequisites.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/README | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)
Show changes to t/README +22 −1
diff --git a/t/README b/t/README
index 53e5b4a710..3ce9f5a393 100644
--- a/t/README
+++ b/t/README
@@ -818,7 +818,7 @@ Skipping tests
 --------------
 
 If you need to skip tests you should do so by using the three-arg form
-of the test_* functions (see the "Test harness library" section
+of the test_expect_* functions (see the "Test harness library" section
 below), e.g.:
 
     test_expect_success PERL 'I need Perl' '
@@ -965,6 +965,27 @@ see test-lib-functions.sh for the full list and their options.
 	    test_done
 	fi
 
+ - test_lazy_prereq <prereq> <script>
+
+   Declare the way to determine if a test prerequisite <prereq> is
+   satisified or not, but delay the actual determination until the
+   prerequisite is actually used by "test_have_prereq" or the
+   three-arg form of the test_expect_* functions.  For example, this
+   is how the SYMLINKS prerequisite is declared to see if the platform
+   supports symbolic links:
+
+	test_lazy_prereq SYMLINKS '
+		ln -s x y && test -h y
+	'
+
+   The script is lazily invoked when SYMLINKS prerequisite is first
+   queried by either "test_have_prereq SYMLINKS" or "test_expect_*
+   SYMLINKS ...".  The script is run in a temporary directory inside
+   a subshell, so you do not have to worry about removing temporary
+   files you create there.  When the script exits with status 0, the
+   prerequisite is set.  Exiting with non-zero status makes the
+   prerequisite unsatisified.
+
  - test_expect_code <exit-code> <command>
 
    Run a command and ensure that it exits with the given exit code.
-- 
2.49.0-rc2-181-g28e223d67e
Junio C Hamano· Mar 11, 2025, 21:25 UTC · re: Junio C Hamano · lore

[PATCH v2 2/6] t: extend test_lazy_prereq

Allow test_lazy_prereq script to signal a programming error by exiting with status 125 (like how bisect scripts do). This is used to signal a deprecated-and-then-removed prerequisite that should never be used in tests anymore.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/README                | 6 ++++--
 t/test-lib-functions.sh | 5 +++++
 2 files changed, 9 insertions(+), 2 deletions(-)
Show changes to 2 files +9 −2

t/README, t/test-lib-functions.sh

diff --git a/t/README b/t/README
index 3ce9f5a393..e9ffd9a81c 100644
--- a/t/README
+++ b/t/README
@@ -983,8 +983,10 @@ see test-lib-functions.sh for the full list and their options.
    SYMLINKS ...".  The script is run in a temporary directory inside
    a subshell, so you do not have to worry about removing temporary
    files you create there.  When the script exits with status 0, the
-   prerequisite is set.  Exiting with non-zero status makes the
-   prerequisite unsatisified.
+   prerequisite is set.  Exiting with non-zero status other than 125
+   makes the prerequisite unsatisified.  Exiting the script with 125
+   signals a programming error and is used to mark a prerequisite that
+   should not be used by test scripts.
 
  - test_expect_code <exit-code> <command>
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index 79377bc0fc..16eaaaf4c3 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -773,6 +773,8 @@ mkdir -p "$TRASH_DIRECTORY/prereq-test-dir-'"$1"'" &&
 	rm -rf "$TRASH_DIRECTORY/prereq-test-dir-$1"
 	if test "$eval_ret" = 0; then
 		say >&3 "prerequisite $1 ok"
+	elif test "$eval_ret" = 125; then
+		:;
 	else
 		say >&3 "prerequisite $1 not satisfied"
 	fi
@@ -811,6 +813,9 @@ test_have_prereq () {
 				if test_run_lazy_prereq_ "$prerequisite" "$script"
 				then
 					test_set_prereq $prerequisite
+				elif test $? = 125
+				then
+					BUG "Do not use $prerequisite"
 				fi
 				lazily_tested_prereq="$lazily_tested_prereq$prerequisite "
 			esac
-- 
2.49.0-rc2-181-g28e223d67e
Patrick Steinhardt· Mar 12, 2025, 07:01 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 2/6] t: extend test_lazy_prereq

On Tue, Mar 11, 2025 at 02:25:01PM -0700, Junio C Hamano wrote:
Show 13 quoted lines
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 79377bc0fc..16eaaaf4c3 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -773,6 +773,8 @@ mkdir -p "$TRASH_DIRECTORY/prereq-test-dir-'"$1"'" &&
>  	rm -rf "$TRASH_DIRECTORY/prereq-test-dir-$1"
>  	if test "$eval_ret" = 0; then
>  		say >&3 "prerequisite $1 ok"
> +	elif test "$eval_ret" = 125; then
> +		:;
>  	else
>  		say >&3 "prerequisite $1 not satisfied"
>  	fi

The semicolon in ":;" threw me off a bit. Am I missing why we need it or is it superfluous?

Show 10 quoted lines
> @@ -811,6 +813,9 @@ test_have_prereq () {
>  				if test_run_lazy_prereq_ "$prerequisite" "$script"
>  				then
>  					test_set_prereq $prerequisite
> +				elif test $? = 125
> +				then
> +					BUG "Do not use $prerequisite"
>  				fi
>  				lazily_tested_prereq="$lazily_tested_prereq$prerequisite "
>  			esac

Hm, okay. It feels quite close to overthinking the whole deprecation cycle around prerequisites as it's nothing that we tend to do very often. But on the other hand the implementation is trivial enough, so I don't mind it much.

Patrick
Junio C Hamano· Mar 13, 2025, 11:56 UTC · re: Patrick Steinhardt · lore

Re: [PATCH v2 2/6] t: extend test_lazy_prereq

Patrick Steinhardt <ps@pks.im> writes:
Show 8 quoted lines
>> +	elif test "$eval_ret" = 125; then
>> +		:;
>>  	else
>>  		say >&3 "prerequisite $1 not satisfied"
>>  	fi
>
> The semicolon in ":;" threw me off a bit. Am I missing why we need it or
> is it superfluous?
The latter, of course -).
Show 15 quoted lines
>> @@ -811,6 +813,9 @@ test_have_prereq () {
>>  				if test_run_lazy_prereq_ "$prerequisite" "$script"
>>  				then
>>  					test_set_prereq $prerequisite
>> +				elif test $? = 125
>> +				then
>> +					BUG "Do not use $prerequisite"
>>  				fi
>>  				lazily_tested_prereq="$lazily_tested_prereq$prerequisite "
>>  			esac
>
> Hm, okay. It feels quite close to overthinking the whole deprecation
> cycle around prerequisites as it's nothing that we tend to do very
> often. But on the other hand the implementation is trivial enough, so I
> don't mind it much.

I agree that this has nothing to do with breaking changes at the Git 3.0 boundary. We just did not have good documentation for lazy prerequisites, and we just did not have any good support for marking a prerequisite should no longer be used. [1/6] is for the former, and [2/6] is for the latter.

We can avoid the magic 125 by adding a new helper like test_removed_prereq and do this instead, which may be cleaner and simpler to reason about.

Another alternative that may make writing tests even be less error prone but a bit more verbose is to introduce test_unset_prereq and be explicit about unsatisfied prerequisites. The effect of the resulting system becomes larger to include detecting misspelled prerequisites, and removed prerequisites would be detected as a natural fallout from the same mechanism.

As we have >50 prerequisites defined with test_set_prereq and we'd need to add 50 calls to test_unset_prereq to mark them as "known but not satisified on this platform" to differentiate them from the ones that are misspelt or removed, if we go that route. I am not sure if that is worth it. Certainly not in the short term, but for a longer term, if people ever misspelt a prerequisite SYMLINKS as SYMLINK and wasted time wondering why their tests didn't trigger, it might be worth it. I dunno.

 t/test-lib-functions.sh | 14 ++++++++++++++
 t/test-lib.sh           |  5 +++--
 2 files changed, 17 insertions(+), 2 deletions(-)
Show changes to diff +17 −2
diff --git c/t/test-lib-functions.sh w/t/test-lib-functions.sh
index 79377bc0fc..3903344fc1 100644
--- c/t/test-lib-functions.sh
+++ w/t/test-lib-functions.sh
@@ -751,7 +751,15 @@ test_set_prereq () {
 		;;
 	esac
 }
+
 satisfied_prereq=" "
+
+removed_prereq=
+# Mark a prerequisite deprecated-and-then-removed
+test_removed_prereq () {
+	removed_prereq="$removed_prereq$1 "
+}
+
 lazily_testable_prereq= lazily_tested_prereq=
 
 # Usage: test_lazy_prereq PREREQ 'script'
@@ -801,6 +809,12 @@ test_have_prereq () {
 			negative_prereq=
 		esac
 
+		case " $removed_prereq " in
+		*" $prerequisite "*)
+			BUG "Do not use $prerequisite"
+			;;
+		esac
+
 		case " $lazily_tested_prereq " in
 		*" $prerequisite "*)
 			;;
diff --git c/t/test-lib.sh w/t/test-lib.sh
index 9001ed3a64..c2c96f5e7a 100644
--- c/t/test-lib.sh
+++ w/t/test-lib.sh
@@ -1862,8 +1862,9 @@ test_lazy_prereq CURL '
 	curl --version
 '
 
-test_lazy_prereq WITHOUT_BREAKING_CHANGES '
-	test -z "$WITH_BREAKING_CHANGES"
+test_removed_prereq WITHOUT_BREAKING_CHANGES
+test_lazy_prereq WITH_BREAKING_CHANGES '
+	test -n "$WITH_BREAKING_CHANGES"
 '
 
 # SHA1 is a test if the hash algorithm in use is SHA-1.  This is both for tests
Junio C Hamano· Mar 11, 2025, 21:25 UTC · re: Junio C Hamano · lore

[PATCH v2 3/6] t: introduce WITH_BREAKING_CHANGES prerequisite

Earlier c5bc9a7f (Makefile: wire up build option for deprecated features, 2025-01-22) made an unfortunate decision to introduce the WITHOUT_BREAKING_CHANGES prerequisite to perform tests that ensure the historical behaviour that may be different from what we will have in the future. It would inevitably invite double-negation when we need to add tests to ensure the behaviour we want to have in the future.

Introduce WITH_BREAKING_CHANGES prerequisite and replace the existing uses of WITHOUT_BREAKING_CHANGES prerequisite. To catch any future topics that add more uses of WITHOUT_BREAKING_CHANGES, introduce a mechanism to mark a prerequisite not to be used, and use it to mark the removed prerequisite as such.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t5323-pack-redundant.sh    | 2 +-
 t/t5505-remote.sh            | 6 +++---
 t/t5515-fetch-merge-logic.sh | 2 +-
 t/t5516-fetch-push.sh        | 8 ++++----
 t/test-lib.sh                | 7 ++++++-
 5 files changed, 15 insertions(+), 10 deletions(-)
Show changes to 5 files +15 −10

t/t5323-pack-redundant.sh, t/t5505-remote.sh, t/t5515-fetch-merge-logic.sh, t/t5516-fetch-push.sh, t/test-lib.sh

diff --git a/t/t5323-pack-redundant.sh b/t/t5323-pack-redundant.sh
index 688cd9706c..bc30bc9652 100755
--- a/t/t5323-pack-redundant.sh
+++ b/t/t5323-pack-redundant.sh
@@ -36,7 +36,7 @@ relationship between packs and objects is as follows:
 
 . ./test-lib.sh
 
-if ! test_have_prereq WITHOUT_BREAKING_CHANGES
+if test_have_prereq WITH_BREAKING_CHANGES
 then
 	skip_all='skipping git-pack-redundant tests; built with breaking changes'
 	test_done
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index bb7e0c6879..82fccf8e36 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -1123,7 +1123,7 @@ Pull: refs/heads/main:refs/heads/origin
 Pull: refs/heads/next:refs/heads/origin2
 EOF
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/remotes' '
+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/remotes' '
 	git clone one five &&
 	origin_url=$(pwd)/one &&
 	(
@@ -1149,7 +1149,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file i
 	)
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches' '
+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches' '
 	git clone --template= one six &&
 	origin_url=$(pwd)/one &&
 	(
@@ -1165,7 +1165,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file i
 	)
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches (2)' '
+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches (2)' '
 	git clone --template= one seven &&
 	(
 		cd seven &&
diff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh
index 4e6026c611..8ac04d742c 100755
--- a/t/t5515-fetch-merge-logic.sh
+++ b/t/t5515-fetch-merge-logic.sh
@@ -104,7 +104,7 @@ test_expect_success setup '
 	git config remote.config-glob.fetch refs/heads/*:refs/remotes/rem/* &&
 	remotes="$remotes config-glob" &&
 
-	if test_have_prereq WITHOUT_BREAKING_CHANGES
+	if ! test_have_prereq WITH_BREAKING_CHANGES
 	then
 		mkdir -p .git/remotes &&
 		cat >.git/remotes/remote-explicit <<-\EOF &&
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 85ed049627..6e2b233157 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -975,7 +975,7 @@ test_expect_success 'allow push to HEAD of non-bare repository (config)' '
 	! grep "warning: updating the current branch" stderr
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches' '
+test_expect_success !WITH_BREAKING_CHANGES 'fetch with branches' '
 	mk_empty testrepo &&
 	git branch second $the_first_commit &&
 	git checkout second &&
@@ -991,7 +991,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches' '
 	git checkout main
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches containing #' '
+test_expect_success !WITH_BREAKING_CHANGES 'fetch with branches containing #' '
 	mk_empty testrepo &&
 	mkdir testrepo/.git/branches &&
 	echo "..#second" > testrepo/.git/branches/branch2 &&
@@ -1005,7 +1005,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches containing #'
 	git checkout main
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches' '
+test_expect_success !WITH_BREAKING_CHANGES 'push with branches' '
 	mk_empty testrepo &&
 	git checkout second &&
 
@@ -1022,7 +1022,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches' '
 	)
 '
 
-test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches containing #' '
+test_expect_success !WITH_BREAKING_CHANGES 'push with branches containing #' '
 	mk_empty testrepo &&
 
 	test_when_finished "rm -rf .git/branches" &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9001ed3a64..fffbfb89ef 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -1862,8 +1862,13 @@ test_lazy_prereq CURL '
 	curl --version
 '
 
+test_lazy_prereq WITH_BREAKING_CHANGES '
+	test -n "$WITH_BREAKING_CHANGES"
+'
+
 test_lazy_prereq WITHOUT_BREAKING_CHANGES '
-	test -z "$WITH_BREAKING_CHANGES"
+	# Signal that this prereq should not be used.
+	exit 125
 '
 
 # SHA1 is a test if the hash algorithm in use is SHA-1.  This is both for tests
-- 
2.49.0-rc2-181-g28e223d67e
Patrick Steinhardt· Mar 12, 2025, 07:01 UTC · re: Junio C Hamano · lore

Re: [PATCH v2 3/6] t: introduce WITH_BREAKING_CHANGES prerequisite

On Tue, Mar 11, 2025 at 02:25:02PM -0700, Junio C Hamano wrote:
Show 13 quoted lines
> Earlier c5bc9a7f (Makefile: wire up build option for deprecated
> features, 2025-01-22) made an unfortunate decision to introduce the
> WITHOUT_BREAKING_CHANGES prerequisite to perform tests that ensure
> the historical behaviour that may be different from what we will
> have in the future.  It would inevitably invite double-negation when
> we need to add tests to ensure the behaviour we want to have in the
> future.
> 
> Introduce WITH_BREAKING_CHANGES prerequisite and replace the
> existing uses of WITHOUT_BREAKING_CHANGES prerequisite.  To catch
> any future topics that add more uses of WITHOUT_BREAKING_CHANGES,
> introduce a mechanism to mark a prerequisite not to be used, and
> use it to mark the removed prerequisite as such.
Nit: the mechanism has already been introduced in the preceding commit.
Patrick
Junio C Hamano· Mar 11, 2025, 21:25 UTC · re: Junio C Hamano · lore

[PATCH v2 4/6] t6120: avoid hiding "git" exit status

A handful of tests invoke "git" on the upstream side of a pipe, hiding its exit status. Correct them.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t6120-describe.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Show changes to t/t6120-describe.sh +4 −2
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 76843a6169..dcb526e37d 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -292,13 +292,15 @@ test_expect_success 'name-rev --annotate-stdin' '
 		echo "$rev ($name)" >>expect.unsorted || return 1
 	done &&
 	sort <expect.unsorted >expect &&
-	git rev-list --all | git name-rev --annotate-stdin >actual.unsorted &&
+	git rev-list --all >list &&
+	git name-rev --annotate-stdin <list >actual.unsorted &&
 	sort <actual.unsorted >actual &&
 	test_cmp expect actual
 '
 
 test_expect_success 'name-rev --stdin deprecated' "
-	git rev-list --all | git name-rev --stdin 2>actual &&
+	git rev-list --all >list &&
+	git name-rev --stdin <list 2>actual &&
 	grep -E 'warning: --stdin is deprecated' actual
 "
 
-- 
2.49.0-rc2-181-g28e223d67e
Junio C Hamano· Mar 11, 2025, 21:25 UTC · re: Junio C Hamano · lore

[PATCH v2 5/6] t6120: further modernize

There is absolutely no reason why a pattern given to grep to find 'warning: --stdin is deprecated' must be quoted within a pair of single quotes, or the pattern to look for the literal string as ERE.

Quote the test body with a pair of single quotes like everybody else, and quote the needle string in a pair of double quotes. Also use test_grep instead of "grep -E".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t6120-describe.sh | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to t/t6120-describe.sh +3 −3
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index dcb526e37d..71e261394a 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -298,11 +298,11 @@ test_expect_success 'name-rev --annotate-stdin' '
 	test_cmp expect actual
 '
 
-test_expect_success 'name-rev --stdin deprecated' "
+test_expect_success 'name-rev --stdin deprecated' '
 	git rev-list --all >list &&
 	git name-rev --stdin <list 2>actual &&
-	grep -E 'warning: --stdin is deprecated' actual
-"
+	test_grep "warning: --stdin is deprecated" actual
+'
 
 test_expect_success 'describe --contains with the exact tags' '
 	echo "A^0" >expect &&
-- 
2.49.0-rc2-181-g28e223d67e
Junio C Hamano· Mar 11, 2025, 21:25 UTC · re: Junio C Hamano · lore

[PATCH v2 6/6] name-rev: remove "--stdin" support

As part of Git 3.0, remove the hidden synonym for "--annotate-stdin" for real. As this does not change the fact that it used to be called "--stdin" in older version of Git, keep that passage in the documentation for "--annotate-stdin".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/BreakingChanges.adoc |  6 ++++++
 builtin/name-rev.c                 | 10 +++++++++-
 t/t6120-describe.sh                | 10 ++++++++--
 3 files changed, 23 insertions(+), 3 deletions(-)
Show changes to 3 files +23 −3

Documentation/BreakingChanges.adoc, builtin/name-rev.c, t/t6120-describe.sh

diff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc
index bdfad29d8a..61bdd586b9 100644
--- a/Documentation/BreakingChanges.adoc
+++ b/Documentation/BreakingChanges.adoc
@@ -178,6 +178,12 @@ references.
 +
 These features will be removed.
 
+* Support for "--stdin" option in the "name-rev" command was
+  deprecated (and hidden from the documentation) in the Git 2.40
+  timeframe, in preference to its synonym "--annotate-stdin".  Git 3.0
+  removes the support for "--stdin" altogether.
+
+
 == Superseded features that will not be deprecated
 
 Some features have gained newer replacements that aim to improve the design in
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index beac166b5c..65f867d7a4 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -567,7 +567,11 @@ int cmd_name_rev(int argc,
 {
 	struct mem_pool string_pool;
 	struct object_array revs = OBJECT_ARRAY_INIT;
-	int all = 0, annotate_stdin = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
+
+#ifndef WITH_BREAKING_CHANGES
+	int transform_stdin = 0;
+#endif
+	int all = 0, annotate_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;
 	struct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };
 	struct option opts[] = {
 		OPT_BOOL(0, "name-only", &data.name_only, N_("print only ref-based names (no object names)")),
@@ -578,11 +582,13 @@ int cmd_name_rev(int argc,
 				   N_("ignore refs matching <pattern>")),
 		OPT_GROUP(""),
 		OPT_BOOL(0, "all", &all, N_("list all commits reachable from all refs")),
+#ifndef WITH_BREAKING_CHANGES
 		OPT_BOOL_F(0,
 			   "stdin",
 			   &transform_stdin,
 			   N_("deprecated: use --annotate-stdin instead"),
 			   PARSE_OPT_HIDDEN),
+#endif /* WITH_BREAKING_CHANGES */
 		OPT_BOOL(0, "annotate-stdin", &annotate_stdin, N_("annotate text from stdin")),
 		OPT_BOOL(0, "undefined", &allow_undefined, N_("allow to print `undefined` names (default)")),
 		OPT_BOOL(0, "always",     &always,
@@ -597,12 +603,14 @@ int cmd_name_rev(int argc,
 	git_config(git_default_config, NULL);
 	argc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);
 
+#ifndef WITH_BREAKING_CHANGES
 	if (transform_stdin) {
 		warning("--stdin is deprecated. Please use --annotate-stdin instead, "
 					"which is functionally equivalent.\n"
 					"This option will be removed in a future release.");
 		annotate_stdin = 1;
 	}
+#endif
 
 	if (all + annotate_stdin + !!argc > 1) {
 		error("Specify either a list, or --all, not both!");
diff --git a/t/t6120-describe.sh b/t/t6120-describe.sh
index 71e261394a..256ccaefb7 100755
--- a/t/t6120-describe.sh
+++ b/t/t6120-describe.sh
@@ -300,8 +300,14 @@ test_expect_success 'name-rev --annotate-stdin' '
 
 test_expect_success 'name-rev --stdin deprecated' '
 	git rev-list --all >list &&
-	git name-rev --stdin <list 2>actual &&
-	test_grep "warning: --stdin is deprecated" actual
+	if ! test_have_prereq WITH_BREAKING_CHANGES
+	then
+		git name-rev --stdin <list 2>actual &&
+		test_grep "warning: --stdin is deprecated" actual
+	else
+		test_must_fail git name-rev --stdin <list 2>actual &&
+		test_grep "unknown option .stdin." actual
+	fi
 '
 
 test_expect_success 'describe --contains with the exact tags' '
-- 
2.49.0-rc2-181-g28e223d67e

← back to recent threads