{"thread":{"id":"63090","subject":"[PATCH v1 0/4] drop \"name-rev --stdin\" support","startedAt":"2025-03-10T23:16:54Z","lastAt":"2025-03-13T11:56:42Z","messageCount":20,"participants":["Junio C Hamano","Eric Sunshine","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"513955","messageId":"20250310231652.3742490-1-gitster@pobox.com","threadId":"63090","inReplyTo":null,"subject":"[PATCH v1 0/4] drop \"name-rev --stdin\" support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-10T23:16:48Z","receivedAt":"2025-03-10T23:16:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"[Administrivia]\n    Yes, yes, I know I shouldn't be playing with shiny new toys\n    during the pre-release freeze time.  But just to show others who\n    may still be doing so that the first patch in this series has\n    already be written to avoid duplicated and conflicting work, I\n    am sending them out.  I have no intention to push the topic\n    further before the final.\n\nDuring Git 2.40 timeframe, we deprecated \"--stdin\" option of the\n\"name-rev\" command in preference to \"--annotate-stdin\", and removed\nthe mention of it from the documentation.\n\nLet's prepare for Git 3.0 to stop supporting it.\n\nThe real motive of these patches is not really about that option but\nmake sure we have, with WITH_BREAKING_CHANGES compilation knob,\nenough support to keep preparing these changes.  The first patch\nrenames the WITHOUT_BREAKING_CHANGES prerequisite that unfortunately\ninvites double negations easily and changes existing users of it,\nthen two patches to a test script minimally modernizes it.  The last\nstep introduces the real change, guarded by WITH_BREAKING_CHANGES\ncompilation knob.\n\nJunio C Hamano (4):\n  t: introduce WITH_BREAKING_CHANGES prerequisite\n  t6120: avoid hiding \"git\" exit status\n  t6120: further modernize\n  name-rev: remove \"--stdin\" support\n\n Documentation/BreakingChanges.adoc |  6 ++++++\n builtin/name-rev.c                 |  2 ++\n t/t5323-pack-redundant.sh          |  2 +-\n t/t5505-remote.sh                  |  6 +++---\n t/t5515-fetch-merge-logic.sh       |  2 +-\n t/t5516-fetch-push.sh              |  8 ++++----\n t/t6120-describe.sh                | 18 +++++++++++++-----\n t/test-lib.sh                      |  5 +++++\n 8 files changed, 35 insertions(+), 14 deletions(-)\n\n-- \n2.49.0-rc2-173-g4d16673c2b\n\n"},{"id":"513956","messageId":"20250310231652.3742490-2-gitster@pobox.com","threadId":"63090","inReplyTo":"20250310231652.3742490-1-gitster@pobox.com","subject":"[PATCH v1 1/4] t: introduce WITH_BREAKING_CHANGES prerequisite","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-10T23:16:49Z","receivedAt":"2025-03-10T23:16:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Earlier c5bc9a7f (Makefile: wire up build option for deprecated\nfeatures, 2025-01-22) made an unfortunate decision to introduce the\nWITHOUT_BREAKING_CHANGES prerequisite to perform tests that ensure\nthe historical behaviour that may be different from what we will\nhave in the future.  It would inevitably invite doulbe negation when\nwe need to add tests to ensure the behaviour we want to have in the\nfuture.\n\nIntroduce WITH_BREAKING_CHANGES prerequisite and replace the\nexisting uses of WITHOUT_BREAKING_CHANGES prerequisite.  Some\nin-flight topics that add more uses of WITHOUT_BREAKING_CHANGES\nwould still need the old prerequisite, so let's keep its definition\nfor now while we'll eradicate its use.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t5323-pack-redundant.sh    | 2 +-\n t/t5505-remote.sh            | 6 +++---\n t/t5515-fetch-merge-logic.sh | 2 +-\n t/t5516-fetch-push.sh        | 8 ++++----\n t/test-lib.sh                | 5 +++++\n 5 files changed, 14 insertions(+), 9 deletions(-)\n\ndiff --git a/t/t5323-pack-redundant.sh b/t/t5323-pack-redundant.sh\nindex 688cd9706c..bc30bc9652 100755\n--- a/t/t5323-pack-redundant.sh\n+++ b/t/t5323-pack-redundant.sh\n@@ -36,7 +36,7 @@ relationship between packs and objects is as follows:\n \n . ./test-lib.sh\n \n-if ! test_have_prereq WITHOUT_BREAKING_CHANGES\n+if test_have_prereq WITH_BREAKING_CHANGES\n then\n \tskip_all='skipping git-pack-redundant tests; built with breaking changes'\n \ttest_done\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex bb7e0c6879..82fccf8e36 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -1123,7 +1123,7 @@ Pull: refs/heads/main:refs/heads/origin\n Pull: refs/heads/next:refs/heads/origin2\n EOF\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/remotes' '\n+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/remotes' '\n \tgit clone one five &&\n \torigin_url=$(pwd)/one &&\n \t(\n@@ -1149,7 +1149,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file i\n \t)\n '\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches' '\n+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches' '\n \tgit clone --template= one six &&\n \torigin_url=$(pwd)/one &&\n \t(\n@@ -1165,7 +1165,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file i\n \t)\n '\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches (2)' '\n+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches (2)' '\n \tgit clone --template= one seven &&\n \t(\n \t\tcd seven &&\ndiff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh\nindex 4e6026c611..8ac04d742c 100755\n--- a/t/t5515-fetch-merge-logic.sh\n+++ b/t/t5515-fetch-merge-logic.sh\n@@ -104,7 +104,7 @@ test_expect_success setup '\n \tgit config remote.config-glob.fetch refs/heads/*:refs/remotes/rem/* &&\n \tremotes=\"$remotes config-glob\" &&\n \n-\tif test_have_prereq WITHOUT_BREAKING_CHANGES\n+\tif ! test_have_prereq WITH_BREAKING_CHANGES\n \tthen\n \t\tmkdir -p .git/remotes &&\n \t\tcat >.git/remotes/remote-explicit <<-\\EOF &&\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 85ed049627..6e2b233157 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -975,7 +975,7 @@ test_expect_success 'allow push to HEAD of non-bare repository (config)' '\n \t! grep \"warning: updating the current branch\" stderr\n '\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches' '\n+test_expect_success !WITH_BREAKING_CHANGES 'fetch with branches' '\n \tmk_empty testrepo &&\n \tgit branch second $the_first_commit &&\n \tgit checkout second &&\n@@ -991,7 +991,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches' '\n \tgit checkout main\n '\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches containing #' '\n+test_expect_success !WITH_BREAKING_CHANGES 'fetch with branches containing #' '\n \tmk_empty testrepo &&\n \tmkdir testrepo/.git/branches &&\n \techo \"..#second\" > testrepo/.git/branches/branch2 &&\n@@ -1005,7 +1005,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches containing #'\n \tgit checkout main\n '\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches' '\n+test_expect_success !WITH_BREAKING_CHANGES 'push with branches' '\n \tmk_empty testrepo &&\n \tgit checkout second &&\n \n@@ -1022,7 +1022,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches' '\n \t)\n '\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches containing #' '\n+test_expect_success !WITH_BREAKING_CHANGES 'push with branches containing #' '\n \tmk_empty testrepo &&\n \n \ttest_when_finished \"rm -rf .git/branches\" &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 9001ed3a64..12fe82f660 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1862,6 +1862,11 @@ test_lazy_prereq CURL '\n \tcurl --version\n '\n \n+test_lazy_prereq WITH_BREAKING_CHANGES '\n+\ttest -n \"$WITH_BREAKING_CHANGES\"\n+'\n+\n+# DEPRECATED; DO NOT USE THIS IN NEW TESTS\n test_lazy_prereq WITHOUT_BREAKING_CHANGES '\n \ttest -z \"$WITH_BREAKING_CHANGES\"\n '\n-- \n2.49.0-rc2-173-g4d16673c2b\n\n"},{"id":"513957","messageId":"20250310231652.3742490-3-gitster@pobox.com","threadId":"63090","inReplyTo":"20250310231652.3742490-1-gitster@pobox.com","subject":"[PATCH v1 2/4] t6120: avoid hiding \"git\" exit status","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-10T23:16:50Z","receivedAt":"2025-03-10T23:16:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A handful of tests invoke \"git\" on the upstream side of a pipe,\nhiding its exit status.  Correct them.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t6120-describe.sh | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 76843a6169..dcb526e37d 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -292,13 +292,15 @@ test_expect_success 'name-rev --annotate-stdin' '\n \t\techo \"$rev ($name)\" >>expect.unsorted || return 1\n \tdone &&\n \tsort <expect.unsorted >expect &&\n-\tgit rev-list --all | git name-rev --annotate-stdin >actual.unsorted &&\n+\tgit rev-list --all >list &&\n+\tgit name-rev --annotate-stdin <list >actual.unsorted &&\n \tsort <actual.unsorted >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'name-rev --stdin deprecated' \"\n-\tgit rev-list --all | git name-rev --stdin 2>actual &&\n+\tgit rev-list --all >list &&\n+\tgit name-rev --stdin <list 2>actual &&\n \tgrep -E 'warning: --stdin is deprecated' actual\n \"\n \n-- \n2.49.0-rc2-173-g4d16673c2b\n\n"},{"id":"513958","messageId":"20250310231652.3742490-4-gitster@pobox.com","threadId":"63090","inReplyTo":"20250310231652.3742490-1-gitster@pobox.com","subject":"[PATCH v1 3/4] t6120: further modernize","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-10T23:16:51Z","receivedAt":"2025-03-10T23:17:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There is absolutely no reason why a pattern given to grep to find\n'warning: --stdin is deprecated' must be quoted within a pair of\nsingle quotes, or the pattern to look for the literal string as ERE.\n\nQuote the test body with a pair of single quotes like everybody\nelse, and quote the needle string in a pair of double quotes.  Also\nuse test_grep instead of \"grep -E\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t6120-describe.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex dcb526e37d..71e261394a 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -298,11 +298,11 @@ test_expect_success 'name-rev --annotate-stdin' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'name-rev --stdin deprecated' \"\n+test_expect_success 'name-rev --stdin deprecated' '\n \tgit rev-list --all >list &&\n \tgit name-rev --stdin <list 2>actual &&\n-\tgrep -E 'warning: --stdin is deprecated' actual\n-\"\n+\ttest_grep \"warning: --stdin is deprecated\" actual\n+'\n \n test_expect_success 'describe --contains with the exact tags' '\n \techo \"A^0\" >expect &&\n-- \n2.49.0-rc2-173-g4d16673c2b\n\n"},{"id":"513959","messageId":"20250310231652.3742490-5-gitster@pobox.com","threadId":"63090","inReplyTo":"20250310231652.3742490-1-gitster@pobox.com","subject":"[PATCH v1 4/4] name-rev: remove \"--stdin\" support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-10T23:16:52Z","receivedAt":"2025-03-10T23:17:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As part of Git 3.0, remove the hidden synonym for \"--annotate-stdin\"\nfor real.  As this does not change the fact that it used to be\ncalled \"--stdin\" in older version of Git, keep that passage in the\ndocumentation for \"--annotate-stdin\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/BreakingChanges.adoc |  6 ++++++\n builtin/name-rev.c                 |  2 ++\n t/t6120-describe.sh                | 10 ++++++++--\n 3 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc\nindex bdfad29d8a..61bdd586b9 100644\n--- a/Documentation/BreakingChanges.adoc\n+++ b/Documentation/BreakingChanges.adoc\n@@ -178,6 +178,12 @@ references.\n +\n These features will be removed.\n \n+* Support for \"--stdin\" option in the \"name-rev\" command was\n+  deprecated (and hidden from the documentation) in the Git 2.40\n+  timeframe, in preference to its synonym \"--annotate-stdin\".  Git 3.0\n+  removes the support for \"--stdin\" altogether.\n+\n+\n == Superseded features that will not be deprecated\n \n Some features have gained newer replacements that aim to improve the design in\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex beac166b5c..3f49138551 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -578,11 +578,13 @@ int cmd_name_rev(int argc,\n \t\t\t\t   N_(\"ignore refs matching <pattern>\")),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL(0, \"all\", &all, N_(\"list all commits reachable from all refs\")),\n+#ifndef WITH_BREAKING_CHANGES\n \t\tOPT_BOOL_F(0,\n \t\t\t   \"stdin\",\n \t\t\t   &transform_stdin,\n \t\t\t   N_(\"deprecated: use --annotate-stdin instead\"),\n \t\t\t   PARSE_OPT_HIDDEN),\n+#endif /* WITH_BREAKING_CHANGES */\n \t\tOPT_BOOL(0, \"annotate-stdin\", &annotate_stdin, N_(\"annotate text from stdin\")),\n \t\tOPT_BOOL(0, \"undefined\", &allow_undefined, N_(\"allow to print `undefined` names (default)\")),\n \t\tOPT_BOOL(0, \"always\",     &always,\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 71e261394a..256ccaefb7 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -300,8 +300,14 @@ test_expect_success 'name-rev --annotate-stdin' '\n \n test_expect_success 'name-rev --stdin deprecated' '\n \tgit rev-list --all >list &&\n-\tgit name-rev --stdin <list 2>actual &&\n-\ttest_grep \"warning: --stdin is deprecated\" actual\n+\tif ! test_have_prereq WITH_BREAKING_CHANGES\n+\tthen\n+\t\tgit name-rev --stdin <list 2>actual &&\n+\t\ttest_grep \"warning: --stdin is deprecated\" actual\n+\telse\n+\t\ttest_must_fail git name-rev --stdin <list 2>actual &&\n+\t\ttest_grep \"unknown option .stdin.\" actual\n+\tfi\n '\n \n test_expect_success 'describe --contains with the exact tags' '\n-- \n2.49.0-rc2-173-g4d16673c2b\n\n"},{"id":"513960","messageId":"CAPig+cTRNts9JO-17rfkmmwyTJojJSv-mS1kvxCJV+L2F2WgzA@mail.gmail.com","threadId":"63090","inReplyTo":"20250310231652.3742490-2-gitster@pobox.com","subject":"Re: [PATCH v1 1/4] t: introduce WITH_BREAKING_CHANGES prerequisite","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2025-03-10T23:53:28Z","receivedAt":"2025-03-10T23:53:40Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 10, 2025 at 7:17 PM Junio C Hamano <gitster@pobox.com> wrote:\n> Earlier c5bc9a7f (Makefile: wire up build option for deprecated\n> features, 2025-01-22) made an unfortunate decision to introduce the\n> WITHOUT_BREAKING_CHANGES prerequisite to perform tests that ensure\n> the historical behaviour that may be different from what we will\n> have in the future.  It would inevitably invite doulbe negation when\n> we need to add tests to ensure the behaviour we want to have in the\n> future.\n\ns/doulbe/double/\n\n> Introduce WITH_BREAKING_CHANGES prerequisite and replace the\n> existing uses of WITHOUT_BREAKING_CHANGES prerequisite.  Some\n> in-flight topics that add more uses of WITHOUT_BREAKING_CHANGES\n> would still need the old prerequisite, so let's keep its definition\n> for now while we'll eradicate its use.\n>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n"},{"id":"513977","messageId":"Z9AzMaXMGq8BbENb@pks.im","threadId":"63090","inReplyTo":"20250310231652.3742490-5-gitster@pobox.com","subject":"Re: [PATCH v1 4/4] name-rev: remove \"--stdin\" support","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-11T12:57:21Z","receivedAt":"2025-03-11T12:57:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Mar 10, 2025 at 04:16:52PM -0700, Junio C Hamano wrote:\n> diff --git a/builtin/name-rev.c b/builtin/name-rev.c\n> index beac166b5c..3f49138551 100644\n> --- a/builtin/name-rev.c\n> +++ b/builtin/name-rev.c\n> @@ -578,11 +578,13 @@ int cmd_name_rev(int argc,\n>  \t\t\t\t   N_(\"ignore refs matching <pattern>\")),\n>  \t\tOPT_GROUP(\"\"),\n>  \t\tOPT_BOOL(0, \"all\", &all, N_(\"list all commits reachable from all refs\")),\n> +#ifndef WITH_BREAKING_CHANGES\n>  \t\tOPT_BOOL_F(0,\n>  \t\t\t   \"stdin\",\n>  \t\t\t   &transform_stdin,\n>  \t\t\t   N_(\"deprecated: use --annotate-stdin instead\"),\n>  \t\t\t   PARSE_OPT_HIDDEN),\n> +#endif /* WITH_BREAKING_CHANGES */\n>  \t\tOPT_BOOL(0, \"annotate-stdin\", &annotate_stdin, N_(\"annotate text from stdin\")),\n>  \t\tOPT_BOOL(0, \"undefined\", &allow_undefined, N_(\"allow to print `undefined` names (default)\")),\n>  \t\tOPT_BOOL(0, \"always\",     &always,\n\nI was wondering whether we should also #ifdef `transform_stdin` and its\nsingle user to more closely reflect what we would have once the feature\nis fully removed.\n\nPatrick\n"},{"id":"513978","messageId":"Z9AzKoWLeOJ63j_E@pks.im","threadId":"63090","inReplyTo":"20250310231652.3742490-2-gitster@pobox.com","subject":"Re: [PATCH v1 1/4] t: introduce WITH_BREAKING_CHANGES prerequisite","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-11T12:57:14Z","receivedAt":"2025-03-11T12:57:23Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Mar 10, 2025 at 04:16:49PM -0700, Junio C Hamano wrote:\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 9001ed3a64..12fe82f660 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -1862,6 +1862,11 @@ test_lazy_prereq CURL '\n>  \tcurl --version\n>  '\n>  \n> +test_lazy_prereq WITH_BREAKING_CHANGES '\n> +\ttest -n \"$WITH_BREAKING_CHANGES\"\n> +'\n> +\n> +# DEPRECATED; DO NOT USE THIS IN NEW TESTS\n>  test_lazy_prereq WITHOUT_BREAKING_CHANGES '\n>  \ttest -z \"$WITH_BREAKING_CHANGES\"\n>  '\n\nDo we maybe want to state that this can be removed once the next release\ncycle is over? I find it to be a bit more actionable when stating hard\ndates after which something can be dropped as it allows any drive-by\ndevelopers to act when they notice that Git v2.50 has been released\nalready.\n\nPatrick\n"},{"id":"513990","messageId":"xmqq4izz4hte.fsf@gitster.g","threadId":"63090","inReplyTo":"Z9AzKoWLeOJ63j_E@pks.im","subject":"Re: [PATCH v1 1/4] t: introduce WITH_BREAKING_CHANGES prerequisite","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-11T17:07:25Z","receivedAt":"2025-03-11T17:07:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Mon, Mar 10, 2025 at 04:16:49PM -0700, Junio C Hamano wrote:\n>> diff --git a/t/test-lib.sh b/t/test-lib.sh\n>> index 9001ed3a64..12fe82f660 100644\n>> --- a/t/test-lib.sh\n>> +++ b/t/test-lib.sh\n>> @@ -1862,6 +1862,11 @@ test_lazy_prereq CURL '\n>>  \tcurl --version\n>>  '\n>>  \n>> +test_lazy_prereq WITH_BREAKING_CHANGES '\n>> +\ttest -n \"$WITH_BREAKING_CHANGES\"\n>> +'\n>> +\n>> +# DEPRECATED; DO NOT USE THIS IN NEW TESTS\n>>  test_lazy_prereq WITHOUT_BREAKING_CHANGES '\n>>  \ttest -z \"$WITH_BREAKING_CHANGES\"\n>>  '\n>\n> Do we maybe want to state that this can be removed once the next release\n> cycle is over?\n\nPerhaps.  As 'seen' is pretty-much closed at this point and there is\nnothing in flight that uses WITHOUT_ variant in there, v2 of this\nseries can just do without it, which may be simpler.\n\n> I find it to be a bit more actionable when stating hard\n> dates after which something can be dropped \n\nTrue, that is a good strategy for a transition that takes longer\ntime.\n"},{"id":"513991","messageId":"xmqqzfhr3384.fsf@gitster.g","threadId":"63090","inReplyTo":"Z9AzMaXMGq8BbENb@pks.im","subject":"Re: [PATCH v1 4/4] name-rev: remove \"--stdin\" support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-11T17:07:55Z","receivedAt":"2025-03-11T17:07:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Mon, Mar 10, 2025 at 04:16:52PM -0700, Junio C Hamano wrote:\n>> diff --git a/builtin/name-rev.c b/builtin/name-rev.c\n>> index beac166b5c..3f49138551 100644\n>> --- a/builtin/name-rev.c\n>> +++ b/builtin/name-rev.c\n>> @@ -578,11 +578,13 @@ int cmd_name_rev(int argc,\n>>  \t\t\t\t   N_(\"ignore refs matching <pattern>\")),\n>>  \t\tOPT_GROUP(\"\"),\n>>  \t\tOPT_BOOL(0, \"all\", &all, N_(\"list all commits reachable from all refs\")),\n>> +#ifndef WITH_BREAKING_CHANGES\n>>  \t\tOPT_BOOL_F(0,\n>>  \t\t\t   \"stdin\",\n>>  \t\t\t   &transform_stdin,\n>>  \t\t\t   N_(\"deprecated: use --annotate-stdin instead\"),\n>>  \t\t\t   PARSE_OPT_HIDDEN),\n>> +#endif /* WITH_BREAKING_CHANGES */\n>>  \t\tOPT_BOOL(0, \"annotate-stdin\", &annotate_stdin, N_(\"annotate text from stdin\")),\n>>  \t\tOPT_BOOL(0, \"undefined\", &allow_undefined, N_(\"allow to print `undefined` names (default)\")),\n>>  \t\tOPT_BOOL(0, \"always\",     &always,\n>\n> I was wondering whether we should also #ifdef `transform_stdin` and its\n> single user to more closely reflect what we would have once the feature\n> is fully removed.\n\nAbsolutely.  That's a great point.\n\nThanks.\n"},{"id":"514001","messageId":"20250311212505.2920181-1-gitster@pobox.com","threadId":"63090","inReplyTo":"20250310231652.3742490-1-gitster@pobox.com","subject":"[PATCH v2 0/6] drop \"name-rev --stdin\" support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-11T21:24:59Z","receivedAt":"2025-03-11T21:25:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"During Git 2.40 timeframe, we deprecated the \"--stdin\" option of the\n\"name-rev\" command in preference to the \"--annotate-stdin\" option,\nand removed the mention of the former from the documentation.\n\nLet's prepare for Git 3.0 to stop supporting it.\n\nThe real motive of these patches is not really about that particular\noption but make sure we have, with WITH_BREAKING_CHANGES compilation\nknob, enough support to keep preparing for these changes.\n\nThe first two patches are preliminary clean-up and enhancement.\nWe lacked documentation on test_lazy_prereq and we did not have a\ngood way to signal a prerequisite that no longer should be used,\nboth of which are remedied.\n\nThe third patch renames the WITHOUT_BREAKING_CHANGES prerequisite\nthat unfortunately invites double negations easily and changes\nexisting users of it.\n\nThen two patches to a test script minimally modernizes it.  \n\nThe last step introduces the real change, guarded by\nWITH_BREAKING_CHANGES compilation knob.  The resulting code is more\nlittered with #if[n]def compared to the previous round but it should\nbe clear which section of the code should go away once we do Git\n3.0, which is the whole point of the WITH_BREAKING_CHANGES exercise.\n\nJunio C Hamano (6):\n  t: document test_lazy_prereq\n  t: extend test_lazy_prereq\n  t: introduce WITH_BREAKING_CHANGES prerequisite\n  t6120: avoid hiding \"git\" exit status\n  t6120: further modernize\n  name-rev: remove \"--stdin\" support\n\n Documentation/BreakingChanges.adoc |  6 ++++++\n builtin/name-rev.c                 | 10 +++++++++-\n t/README                           | 25 ++++++++++++++++++++++++-\n t/t5323-pack-redundant.sh          |  2 +-\n t/t5505-remote.sh                  |  6 +++---\n t/t5515-fetch-merge-logic.sh       |  2 +-\n t/t5516-fetch-push.sh              |  8 ++++----\n t/t6120-describe.sh                | 18 +++++++++++++-----\n t/test-lib-functions.sh            |  5 +++++\n t/test-lib.sh                      |  7 ++++++-\n 10 files changed, 72 insertions(+), 17 deletions(-)\n\n-- \n2.49.0-rc2-181-g28e223d67e\n\n"},{"id":"514002","messageId":"20250311212505.2920181-2-gitster@pobox.com","threadId":"63090","inReplyTo":"20250311212505.2920181-1-gitster@pobox.com","subject":"[PATCH v2 1/6] t: document test_lazy_prereq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-11T21:25:00Z","receivedAt":"2025-03-11T21:25:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The t/README file talked about test_set_prereq but lacked\nexplanation on test_lazy_prereq, which is a more modern way to\ndefine prerequisites.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/README | 23 ++++++++++++++++++++++-\n 1 file changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/t/README b/t/README\nindex 53e5b4a710..3ce9f5a393 100644\n--- a/t/README\n+++ b/t/README\n@@ -818,7 +818,7 @@ Skipping tests\n --------------\n \n If you need to skip tests you should do so by using the three-arg form\n-of the test_* functions (see the \"Test harness library\" section\n+of the test_expect_* functions (see the \"Test harness library\" section\n below), e.g.:\n \n     test_expect_success PERL 'I need Perl' '\n@@ -965,6 +965,27 @@ see test-lib-functions.sh for the full list and their options.\n \t    test_done\n \tfi\n \n+ - test_lazy_prereq <prereq> <script>\n+\n+   Declare the way to determine if a test prerequisite <prereq> is\n+   satisified or not, but delay the actual determination until the\n+   prerequisite is actually used by \"test_have_prereq\" or the\n+   three-arg form of the test_expect_* functions.  For example, this\n+   is how the SYMLINKS prerequisite is declared to see if the platform\n+   supports symbolic links:\n+\n+\ttest_lazy_prereq SYMLINKS '\n+\t\tln -s x y && test -h y\n+\t'\n+\n+   The script is lazily invoked when SYMLINKS prerequisite is first\n+   queried by either \"test_have_prereq SYMLINKS\" or \"test_expect_*\n+   SYMLINKS ...\".  The script is run in a temporary directory inside\n+   a subshell, so you do not have to worry about removing temporary\n+   files you create there.  When the script exits with status 0, the\n+   prerequisite is set.  Exiting with non-zero status makes the\n+   prerequisite unsatisified.\n+\n  - test_expect_code <exit-code> <command>\n \n    Run a command and ensure that it exits with the given exit code.\n-- \n2.49.0-rc2-181-g28e223d67e\n\n"},{"id":"514003","messageId":"20250311212505.2920181-3-gitster@pobox.com","threadId":"63090","inReplyTo":"20250311212505.2920181-1-gitster@pobox.com","subject":"[PATCH v2 2/6] t: extend test_lazy_prereq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-11T21:25:01Z","receivedAt":"2025-03-11T21:25:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Allow test_lazy_prereq script to signal a programming error by\nexiting with status 125 (like how bisect scripts do).  This is used\nto signal a deprecated-and-then-removed prerequisite that should\nnever be used in tests anymore.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/README                | 6 ++++--\n t/test-lib-functions.sh | 5 +++++\n 2 files changed, 9 insertions(+), 2 deletions(-)\n\ndiff --git a/t/README b/t/README\nindex 3ce9f5a393..e9ffd9a81c 100644\n--- a/t/README\n+++ b/t/README\n@@ -983,8 +983,10 @@ see test-lib-functions.sh for the full list and their options.\n    SYMLINKS ...\".  The script is run in a temporary directory inside\n    a subshell, so you do not have to worry about removing temporary\n    files you create there.  When the script exits with status 0, the\n-   prerequisite is set.  Exiting with non-zero status makes the\n-   prerequisite unsatisified.\n+   prerequisite is set.  Exiting with non-zero status other than 125\n+   makes the prerequisite unsatisified.  Exiting the script with 125\n+   signals a programming error and is used to mark a prerequisite that\n+   should not be used by test scripts.\n \n  - test_expect_code <exit-code> <command>\n \ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 79377bc0fc..16eaaaf4c3 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -773,6 +773,8 @@ mkdir -p \"$TRASH_DIRECTORY/prereq-test-dir-'\"$1\"'\" &&\n \trm -rf \"$TRASH_DIRECTORY/prereq-test-dir-$1\"\n \tif test \"$eval_ret\" = 0; then\n \t\tsay >&3 \"prerequisite $1 ok\"\n+\telif test \"$eval_ret\" = 125; then\n+\t\t:;\n \telse\n \t\tsay >&3 \"prerequisite $1 not satisfied\"\n \tfi\n@@ -811,6 +813,9 @@ test_have_prereq () {\n \t\t\t\tif test_run_lazy_prereq_ \"$prerequisite\" \"$script\"\n \t\t\t\tthen\n \t\t\t\t\ttest_set_prereq $prerequisite\n+\t\t\t\telif test $? = 125\n+\t\t\t\tthen\n+\t\t\t\t\tBUG \"Do not use $prerequisite\"\n \t\t\t\tfi\n \t\t\t\tlazily_tested_prereq=\"$lazily_tested_prereq$prerequisite \"\n \t\t\tesac\n-- \n2.49.0-rc2-181-g28e223d67e\n\n"},{"id":"514004","messageId":"20250311212505.2920181-4-gitster@pobox.com","threadId":"63090","inReplyTo":"20250311212505.2920181-1-gitster@pobox.com","subject":"[PATCH v2 3/6] t: introduce WITH_BREAKING_CHANGES prerequisite","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-11T21:25:02Z","receivedAt":"2025-03-11T21:25:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Earlier c5bc9a7f (Makefile: wire up build option for deprecated\nfeatures, 2025-01-22) made an unfortunate decision to introduce the\nWITHOUT_BREAKING_CHANGES prerequisite to perform tests that ensure\nthe historical behaviour that may be different from what we will\nhave in the future.  It would inevitably invite double-negation when\nwe need to add tests to ensure the behaviour we want to have in the\nfuture.\n\nIntroduce WITH_BREAKING_CHANGES prerequisite and replace the\nexisting uses of WITHOUT_BREAKING_CHANGES prerequisite.  To catch\nany future topics that add more uses of WITHOUT_BREAKING_CHANGES,\nintroduce a mechanism to mark a prerequisite not to be used, and\nuse it to mark the removed prerequisite as such.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t5323-pack-redundant.sh    | 2 +-\n t/t5505-remote.sh            | 6 +++---\n t/t5515-fetch-merge-logic.sh | 2 +-\n t/t5516-fetch-push.sh        | 8 ++++----\n t/test-lib.sh                | 7 ++++++-\n 5 files changed, 15 insertions(+), 10 deletions(-)\n\ndiff --git a/t/t5323-pack-redundant.sh b/t/t5323-pack-redundant.sh\nindex 688cd9706c..bc30bc9652 100755\n--- a/t/t5323-pack-redundant.sh\n+++ b/t/t5323-pack-redundant.sh\n@@ -36,7 +36,7 @@ relationship between packs and objects is as follows:\n \n . ./test-lib.sh\n \n-if ! test_have_prereq WITHOUT_BREAKING_CHANGES\n+if test_have_prereq WITH_BREAKING_CHANGES\n then\n \tskip_all='skipping git-pack-redundant tests; built with breaking changes'\n \ttest_done\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex bb7e0c6879..82fccf8e36 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -1123,7 +1123,7 @@ Pull: refs/heads/main:refs/heads/origin\n Pull: refs/heads/next:refs/heads/origin2\n EOF\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/remotes' '\n+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/remotes' '\n \tgit clone one five &&\n \torigin_url=$(pwd)/one &&\n \t(\n@@ -1149,7 +1149,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file i\n \t)\n '\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches' '\n+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches' '\n \tgit clone --template= one six &&\n \torigin_url=$(pwd)/one &&\n \t(\n@@ -1165,7 +1165,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file i\n \t)\n '\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches (2)' '\n+test_expect_success !WITH_BREAKING_CHANGES 'migrate a remote from named file in $GIT_DIR/branches (2)' '\n \tgit clone --template= one seven &&\n \t(\n \t\tcd seven &&\ndiff --git a/t/t5515-fetch-merge-logic.sh b/t/t5515-fetch-merge-logic.sh\nindex 4e6026c611..8ac04d742c 100755\n--- a/t/t5515-fetch-merge-logic.sh\n+++ b/t/t5515-fetch-merge-logic.sh\n@@ -104,7 +104,7 @@ test_expect_success setup '\n \tgit config remote.config-glob.fetch refs/heads/*:refs/remotes/rem/* &&\n \tremotes=\"$remotes config-glob\" &&\n \n-\tif test_have_prereq WITHOUT_BREAKING_CHANGES\n+\tif ! test_have_prereq WITH_BREAKING_CHANGES\n \tthen\n \t\tmkdir -p .git/remotes &&\n \t\tcat >.git/remotes/remote-explicit <<-\\EOF &&\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 85ed049627..6e2b233157 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -975,7 +975,7 @@ test_expect_success 'allow push to HEAD of non-bare repository (config)' '\n \t! grep \"warning: updating the current branch\" stderr\n '\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches' '\n+test_expect_success !WITH_BREAKING_CHANGES 'fetch with branches' '\n \tmk_empty testrepo &&\n \tgit branch second $the_first_commit &&\n \tgit checkout second &&\n@@ -991,7 +991,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches' '\n \tgit checkout main\n '\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches containing #' '\n+test_expect_success !WITH_BREAKING_CHANGES 'fetch with branches containing #' '\n \tmk_empty testrepo &&\n \tmkdir testrepo/.git/branches &&\n \techo \"..#second\" > testrepo/.git/branches/branch2 &&\n@@ -1005,7 +1005,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'fetch with branches containing #'\n \tgit checkout main\n '\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches' '\n+test_expect_success !WITH_BREAKING_CHANGES 'push with branches' '\n \tmk_empty testrepo &&\n \tgit checkout second &&\n \n@@ -1022,7 +1022,7 @@ test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches' '\n \t)\n '\n \n-test_expect_success WITHOUT_BREAKING_CHANGES 'push with branches containing #' '\n+test_expect_success !WITH_BREAKING_CHANGES 'push with branches containing #' '\n \tmk_empty testrepo &&\n \n \ttest_when_finished \"rm -rf .git/branches\" &&\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 9001ed3a64..fffbfb89ef 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -1862,8 +1862,13 @@ test_lazy_prereq CURL '\n \tcurl --version\n '\n \n+test_lazy_prereq WITH_BREAKING_CHANGES '\n+\ttest -n \"$WITH_BREAKING_CHANGES\"\n+'\n+\n test_lazy_prereq WITHOUT_BREAKING_CHANGES '\n-\ttest -z \"$WITH_BREAKING_CHANGES\"\n+\t# Signal that this prereq should not be used.\n+\texit 125\n '\n \n # SHA1 is a test if the hash algorithm in use is SHA-1.  This is both for tests\n-- \n2.49.0-rc2-181-g28e223d67e\n\n"},{"id":"514005","messageId":"20250311212505.2920181-5-gitster@pobox.com","threadId":"63090","inReplyTo":"20250311212505.2920181-1-gitster@pobox.com","subject":"[PATCH v2 4/6] t6120: avoid hiding \"git\" exit status","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-11T21:25:03Z","receivedAt":"2025-03-11T21:25:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A handful of tests invoke \"git\" on the upstream side of a pipe,\nhiding its exit status.  Correct them.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t6120-describe.sh | 6 ++++--\n 1 file changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 76843a6169..dcb526e37d 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -292,13 +292,15 @@ test_expect_success 'name-rev --annotate-stdin' '\n \t\techo \"$rev ($name)\" >>expect.unsorted || return 1\n \tdone &&\n \tsort <expect.unsorted >expect &&\n-\tgit rev-list --all | git name-rev --annotate-stdin >actual.unsorted &&\n+\tgit rev-list --all >list &&\n+\tgit name-rev --annotate-stdin <list >actual.unsorted &&\n \tsort <actual.unsorted >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'name-rev --stdin deprecated' \"\n-\tgit rev-list --all | git name-rev --stdin 2>actual &&\n+\tgit rev-list --all >list &&\n+\tgit name-rev --stdin <list 2>actual &&\n \tgrep -E 'warning: --stdin is deprecated' actual\n \"\n \n-- \n2.49.0-rc2-181-g28e223d67e\n\n"},{"id":"514006","messageId":"20250311212505.2920181-6-gitster@pobox.com","threadId":"63090","inReplyTo":"20250311212505.2920181-1-gitster@pobox.com","subject":"[PATCH v2 5/6] t6120: further modernize","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-11T21:25:04Z","receivedAt":"2025-03-11T21:25:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There is absolutely no reason why a pattern given to grep to find\n'warning: --stdin is deprecated' must be quoted within a pair of\nsingle quotes, or the pattern to look for the literal string as ERE.\n\nQuote the test body with a pair of single quotes like everybody\nelse, and quote the needle string in a pair of double quotes.  Also\nuse test_grep instead of \"grep -E\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t6120-describe.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex dcb526e37d..71e261394a 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -298,11 +298,11 @@ test_expect_success 'name-rev --annotate-stdin' '\n \ttest_cmp expect actual\n '\n \n-test_expect_success 'name-rev --stdin deprecated' \"\n+test_expect_success 'name-rev --stdin deprecated' '\n \tgit rev-list --all >list &&\n \tgit name-rev --stdin <list 2>actual &&\n-\tgrep -E 'warning: --stdin is deprecated' actual\n-\"\n+\ttest_grep \"warning: --stdin is deprecated\" actual\n+'\n \n test_expect_success 'describe --contains with the exact tags' '\n \techo \"A^0\" >expect &&\n-- \n2.49.0-rc2-181-g28e223d67e\n\n"},{"id":"514007","messageId":"20250311212505.2920181-7-gitster@pobox.com","threadId":"63090","inReplyTo":"20250311212505.2920181-1-gitster@pobox.com","subject":"[PATCH v2 6/6] name-rev: remove \"--stdin\" support","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-11T21:25:05Z","receivedAt":"2025-03-11T21:25:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"As part of Git 3.0, remove the hidden synonym for \"--annotate-stdin\"\nfor real.  As this does not change the fact that it used to be\ncalled \"--stdin\" in older version of Git, keep that passage in the\ndocumentation for \"--annotate-stdin\".\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n Documentation/BreakingChanges.adoc |  6 ++++++\n builtin/name-rev.c                 | 10 +++++++++-\n t/t6120-describe.sh                | 10 ++++++++--\n 3 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/BreakingChanges.adoc b/Documentation/BreakingChanges.adoc\nindex bdfad29d8a..61bdd586b9 100644\n--- a/Documentation/BreakingChanges.adoc\n+++ b/Documentation/BreakingChanges.adoc\n@@ -178,6 +178,12 @@ references.\n +\n These features will be removed.\n \n+* Support for \"--stdin\" option in the \"name-rev\" command was\n+  deprecated (and hidden from the documentation) in the Git 2.40\n+  timeframe, in preference to its synonym \"--annotate-stdin\".  Git 3.0\n+  removes the support for \"--stdin\" altogether.\n+\n+\n == Superseded features that will not be deprecated\n \n Some features have gained newer replacements that aim to improve the design in\ndiff --git a/builtin/name-rev.c b/builtin/name-rev.c\nindex beac166b5c..65f867d7a4 100644\n--- a/builtin/name-rev.c\n+++ b/builtin/name-rev.c\n@@ -567,7 +567,11 @@ int cmd_name_rev(int argc,\n {\n \tstruct mem_pool string_pool;\n \tstruct object_array revs = OBJECT_ARRAY_INIT;\n-\tint all = 0, annotate_stdin = 0, transform_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;\n+\n+#ifndef WITH_BREAKING_CHANGES\n+\tint transform_stdin = 0;\n+#endif\n+\tint all = 0, annotate_stdin = 0, allow_undefined = 1, always = 0, peel_tag = 0;\n \tstruct name_ref_data data = { 0, 0, STRING_LIST_INIT_NODUP, STRING_LIST_INIT_NODUP };\n \tstruct option opts[] = {\n \t\tOPT_BOOL(0, \"name-only\", &data.name_only, N_(\"print only ref-based names (no object names)\")),\n@@ -578,11 +582,13 @@ int cmd_name_rev(int argc,\n \t\t\t\t   N_(\"ignore refs matching <pattern>\")),\n \t\tOPT_GROUP(\"\"),\n \t\tOPT_BOOL(0, \"all\", &all, N_(\"list all commits reachable from all refs\")),\n+#ifndef WITH_BREAKING_CHANGES\n \t\tOPT_BOOL_F(0,\n \t\t\t   \"stdin\",\n \t\t\t   &transform_stdin,\n \t\t\t   N_(\"deprecated: use --annotate-stdin instead\"),\n \t\t\t   PARSE_OPT_HIDDEN),\n+#endif /* WITH_BREAKING_CHANGES */\n \t\tOPT_BOOL(0, \"annotate-stdin\", &annotate_stdin, N_(\"annotate text from stdin\")),\n \t\tOPT_BOOL(0, \"undefined\", &allow_undefined, N_(\"allow to print `undefined` names (default)\")),\n \t\tOPT_BOOL(0, \"always\",     &always,\n@@ -597,12 +603,14 @@ int cmd_name_rev(int argc,\n \tgit_config(git_default_config, NULL);\n \targc = parse_options(argc, argv, prefix, opts, name_rev_usage, 0);\n \n+#ifndef WITH_BREAKING_CHANGES\n \tif (transform_stdin) {\n \t\twarning(\"--stdin is deprecated. Please use --annotate-stdin instead, \"\n \t\t\t\t\t\"which is functionally equivalent.\\n\"\n \t\t\t\t\t\"This option will be removed in a future release.\");\n \t\tannotate_stdin = 1;\n \t}\n+#endif\n \n \tif (all + annotate_stdin + !!argc > 1) {\n \t\terror(\"Specify either a list, or --all, not both!\");\ndiff --git a/t/t6120-describe.sh b/t/t6120-describe.sh\nindex 71e261394a..256ccaefb7 100755\n--- a/t/t6120-describe.sh\n+++ b/t/t6120-describe.sh\n@@ -300,8 +300,14 @@ test_expect_success 'name-rev --annotate-stdin' '\n \n test_expect_success 'name-rev --stdin deprecated' '\n \tgit rev-list --all >list &&\n-\tgit name-rev --stdin <list 2>actual &&\n-\ttest_grep \"warning: --stdin is deprecated\" actual\n+\tif ! test_have_prereq WITH_BREAKING_CHANGES\n+\tthen\n+\t\tgit name-rev --stdin <list 2>actual &&\n+\t\ttest_grep \"warning: --stdin is deprecated\" actual\n+\telse\n+\t\ttest_must_fail git name-rev --stdin <list 2>actual &&\n+\t\ttest_grep \"unknown option .stdin.\" actual\n+\tfi\n '\n \n test_expect_success 'describe --contains with the exact tags' '\n-- \n2.49.0-rc2-181-g28e223d67e\n\n"},{"id":"514023","messageId":"Z9ExMHf9CkcDwEt1@pks.im","threadId":"63090","inReplyTo":"20250311212505.2920181-3-gitster@pobox.com","subject":"Re: [PATCH v2 2/6] t: extend test_lazy_prereq","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-12T07:01:04Z","receivedAt":"2025-03-12T07:01:09Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Mar 11, 2025 at 02:25:01PM -0700, Junio C Hamano wrote:\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index 79377bc0fc..16eaaaf4c3 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -773,6 +773,8 @@ mkdir -p \"$TRASH_DIRECTORY/prereq-test-dir-'\"$1\"'\" &&\n>  \trm -rf \"$TRASH_DIRECTORY/prereq-test-dir-$1\"\n>  \tif test \"$eval_ret\" = 0; then\n>  \t\tsay >&3 \"prerequisite $1 ok\"\n> +\telif test \"$eval_ret\" = 125; then\n> +\t\t:;\n>  \telse\n>  \t\tsay >&3 \"prerequisite $1 not satisfied\"\n>  \tfi\n\nThe semicolon in \":;\" threw me off a bit. Am I missing why we need it or\nis it superfluous?\n\n> @@ -811,6 +813,9 @@ test_have_prereq () {\n>  \t\t\t\tif test_run_lazy_prereq_ \"$prerequisite\" \"$script\"\n>  \t\t\t\tthen\n>  \t\t\t\t\ttest_set_prereq $prerequisite\n> +\t\t\t\telif test $? = 125\n> +\t\t\t\tthen\n> +\t\t\t\t\tBUG \"Do not use $prerequisite\"\n>  \t\t\t\tfi\n>  \t\t\t\tlazily_tested_prereq=\"$lazily_tested_prereq$prerequisite \"\n>  \t\t\tesac\n\nHm, okay. It feels quite close to overthinking the whole deprecation\ncycle around prerequisites as it's nothing that we tend to do very\noften. But on the other hand the implementation is trivial enough, so I\ndon't mind it much.\n\nPatrick\n"},{"id":"514024","messageId":"Z9ExM8cyOvVHHIjR@pks.im","threadId":"63090","inReplyTo":"20250311212505.2920181-4-gitster@pobox.com","subject":"Re: [PATCH v2 3/6] t: introduce WITH_BREAKING_CHANGES prerequisite","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-03-12T07:01:07Z","receivedAt":"2025-03-12T07:01:10Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Mar 11, 2025 at 02:25:02PM -0700, Junio C Hamano wrote:\n> Earlier c5bc9a7f (Makefile: wire up build option for deprecated\n> features, 2025-01-22) made an unfortunate decision to introduce the\n> WITHOUT_BREAKING_CHANGES prerequisite to perform tests that ensure\n> the historical behaviour that may be different from what we will\n> have in the future.  It would inevitably invite double-negation when\n> we need to add tests to ensure the behaviour we want to have in the\n> future.\n> \n> Introduce WITH_BREAKING_CHANGES prerequisite and replace the\n> existing uses of WITHOUT_BREAKING_CHANGES prerequisite.  To catch\n> any future topics that add more uses of WITHOUT_BREAKING_CHANGES,\n> introduce a mechanism to mark a prerequisite not to be used, and\n> use it to mark the removed prerequisite as such.\n\nNit: the mechanism has already been introduced in the preceding commit.\n\nPatrick\n"},{"id":"514173","messageId":"xmqqsenhrvns.fsf@gitster.g","threadId":"63090","inReplyTo":"Z9ExMHf9CkcDwEt1@pks.im","subject":"Re: [PATCH v2 2/6] t: extend test_lazy_prereq","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-03-13T11:56:39Z","receivedAt":"2025-03-13T11:56:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n>> +\telif test \"$eval_ret\" = 125; then\n>> +\t\t:;\n>>  \telse\n>>  \t\tsay >&3 \"prerequisite $1 not satisfied\"\n>>  \tfi\n>\n> The semicolon in \":;\" threw me off a bit. Am I missing why we need it or\n> is it superfluous?\n\nThe latter, of course -).\n\n>> @@ -811,6 +813,9 @@ test_have_prereq () {\n>>  \t\t\t\tif test_run_lazy_prereq_ \"$prerequisite\" \"$script\"\n>>  \t\t\t\tthen\n>>  \t\t\t\t\ttest_set_prereq $prerequisite\n>> +\t\t\t\telif test $? = 125\n>> +\t\t\t\tthen\n>> +\t\t\t\t\tBUG \"Do not use $prerequisite\"\n>>  \t\t\t\tfi\n>>  \t\t\t\tlazily_tested_prereq=\"$lazily_tested_prereq$prerequisite \"\n>>  \t\t\tesac\n>\n> Hm, okay. It feels quite close to overthinking the whole deprecation\n> cycle around prerequisites as it's nothing that we tend to do very\n> often. But on the other hand the implementation is trivial enough, so I\n> don't mind it much.\n\nI agree that this has nothing to do with breaking changes at the Git\n3.0 boundary.  We just did not have good documentation for lazy\nprerequisites, and we just did not have any good support for marking\na prerequisite should no longer be used.  [1/6] is for the former,\nand [2/6] is for the latter.\n\nWe can avoid the magic 125 by adding a new helper like test_removed_prereq\nand do this instead, which may be cleaner and simpler to reason about.\n\nAnother alternative that may make writing tests even be less error\nprone but a bit more verbose is to introduce test_unset_prereq and\nbe explicit about unsatisfied prerequisites.  The effect of the\nresulting system becomes larger to include detecting misspelled\nprerequisites, and removed prerequisites would be detected as a\nnatural fallout from the same mechanism.\n\nAs we have >50 prerequisites defined with test_set_prereq and we'd\nneed to add 50 calls to test_unset_prereq to mark them as \"known but\nnot satisified on this platform\" to differentiate them from the ones\nthat are misspelt or removed, if we go that route.  I am not sure if\nthat is worth it.  Certainly not in the short term, but for a longer\nterm, if people ever misspelt a prerequisite SYMLINKS as SYMLINK and\nwasted time wondering why their tests didn't trigger, it might be\nworth it.  I dunno.\n\n t/test-lib-functions.sh | 14 ++++++++++++++\n t/test-lib.sh           |  5 +++--\n 2 files changed, 17 insertions(+), 2 deletions(-)\n\ndiff --git c/t/test-lib-functions.sh w/t/test-lib-functions.sh\nindex 79377bc0fc..3903344fc1 100644\n--- c/t/test-lib-functions.sh\n+++ w/t/test-lib-functions.sh\n@@ -751,7 +751,15 @@ test_set_prereq () {\n \t\t;;\n \tesac\n }\n+\n satisfied_prereq=\" \"\n+\n+removed_prereq=\n+# Mark a prerequisite deprecated-and-then-removed\n+test_removed_prereq () {\n+\tremoved_prereq=\"$removed_prereq$1 \"\n+}\n+\n lazily_testable_prereq= lazily_tested_prereq=\n \n # Usage: test_lazy_prereq PREREQ 'script'\n@@ -801,6 +809,12 @@ test_have_prereq () {\n \t\t\tnegative_prereq=\n \t\tesac\n \n+\t\tcase \" $removed_prereq \" in\n+\t\t*\" $prerequisite \"*)\n+\t\t\tBUG \"Do not use $prerequisite\"\n+\t\t\t;;\n+\t\tesac\n+\n \t\tcase \" $lazily_tested_prereq \" in\n \t\t*\" $prerequisite \"*)\n \t\t\t;;\ndiff --git c/t/test-lib.sh w/t/test-lib.sh\nindex 9001ed3a64..c2c96f5e7a 100644\n--- c/t/test-lib.sh\n+++ w/t/test-lib.sh\n@@ -1862,8 +1862,9 @@ test_lazy_prereq CURL '\n \tcurl --version\n '\n \n-test_lazy_prereq WITHOUT_BREAKING_CHANGES '\n-\ttest -z \"$WITH_BREAKING_CHANGES\"\n+test_removed_prereq WITHOUT_BREAKING_CHANGES\n+test_lazy_prereq WITH_BREAKING_CHANGES '\n+\ttest -n \"$WITH_BREAKING_CHANGES\"\n '\n \n # SHA1 is a test if the hash algorithm in use is SHA-1.  This is both for tests\n"}]}