{"thread":{"id":"54508","subject":"[PATCH] remote: add meaningful exit code on missing/existing","startedAt":"2020-10-26T14:48:42Z","lastAt":"2020-10-27T09:41:53Z","messageCount":7,"participants":["Ævar Arnfjörð Bjarmason","Eric Sunshine","Junio C Hamano","Jeff King","Bert Wesarg"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"408395","messageId":"20201026144825.26537-1-avarab@gmail.com","threadId":"54508","inReplyTo":null,"subject":"[PATCH] remote: add meaningful exit code on missing/existing","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2020-10-26T14:48:25Z","receivedAt":"2020-10-26T14:48:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the exit code for the likes of \"git remote add/rename\" to exit\nwith 2 if the remote in question doesn't exist, and 3 if it\ndoes. Before we'd just die() and exit with the general 128 exit code.\n\nThis changes the output message from e.g.:\n\n    fatal: remote origin already exists.\n\nTo:\n\n    error: remote origin already exists.\n\nWhich I believe is a feature, since we generally use \"fatal\" for the\ngeneric errors, and \"error\" for the more specific ones with a custom\nexit code, but this part of the change may break code that already\nrelies on stderr parsing (not that we ever supported that...).\n\nThe motivation for this is a discussion around some code in GitLab's\ngitaly which wanted to check this, and had to parse stderr to do so:\nhttps://gitlab.com/gitlab-org/gitaly/-/merge_requests/2695\n\nIt's worth noting as an aside that a method of checking this that\ndoesn't rely on that is to check with \"git config\" whether the value\nin question does or doesn't exist. That introduces a TOCTOU race\ncondition, but on the other hand this code (e.g. \"git remote add\")\nalready has a TOCTOU race.\n\nWe go through the config.lock for the actual setting of the config,\nbut the pseudocode logic is:\n\n    read_config();\n    check_config_and_arg_sanity();\n    save_config();\n\nSo e.g. if a sleep() is added right after the remote_is_configured()\ncheck in add() we'll clobber rermote.NAME.url, and add\nanother (usually duplicate) remote.NAME.fetch entry (and other values,\ndepending on invocation).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Documentation/git-remote.txt | 11 ++++++++++\n builtin/remote.c             | 42 ++++++++++++++++++++++++------------\n t/t5505-remote.sh            | 16 +++++++-------\n 3 files changed, 47 insertions(+), 22 deletions(-)\n\ndiff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\nindex 9659abbf8e..c1f1355f52 100644\n--- a/Documentation/git-remote.txt\n+++ b/Documentation/git-remote.txt\n@@ -203,6 +203,17 @@ The remote configuration is achieved using the `remote.origin.url` and\n `remote.origin.fetch` configuration variables.  (See\n linkgit:git-config[1]).\n \n+EXIT STATUS\n+-----------\n+\n+On success, the exit status is `0`.\n+\n+When subcommands such as 'add', 'rename' and 'remove' can't find the\n+remote in question the exit status is `2`, when the remote already\n+exists the exit status is `3`.\n+\n+On any other error, the exit status may be any other non-zero value.\n+\n EXAMPLES\n --------\n \ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 64b4b551eb..c1828ca7d2 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -191,8 +191,10 @@ static int add(int argc, const char **argv)\n \turl = argv[1];\n \n \tremote = remote_get(name);\n-\tif (remote_is_configured(remote, 1))\n-\t\tdie(_(\"remote %s already exists.\"), name);\n+\tif (remote_is_configured(remote, 1)) {\n+\t\terror(_(\"remote %s already exists.\"), name);\n+\t\texit(3);\n+\t}\n \n \tstrbuf_addf(&buf2, \"refs/heads/test:refs/remotes/%s/test\", name);\n \tif (!valid_fetch_refspec(buf2.buf))\n@@ -686,15 +688,19 @@ static int mv(int argc, const char **argv)\n \trename.remote_branches = &remote_branches;\n \n \toldremote = remote_get(rename.old_name);\n-\tif (!remote_is_configured(oldremote, 1))\n-\t\tdie(_(\"No such remote: '%s'\"), rename.old_name);\n+\tif (!remote_is_configured(oldremote, 1)) {\n+\t\terror(_(\"No such remote: '%s'\"), rename.old_name);\n+\t\texit(2);\n+\t}\n \n \tif (!strcmp(rename.old_name, rename.new_name) && oldremote->origin != REMOTE_CONFIG)\n \t\treturn migrate_file(oldremote);\n \n \tnewremote = remote_get(rename.new_name);\n-\tif (remote_is_configured(newremote, 1))\n-\t\tdie(_(\"remote %s already exists.\"), rename.new_name);\n+\tif (remote_is_configured(newremote, 1)) {\n+\t\terror(_(\"remote %s already exists.\"), rename.new_name);\n+\t\texit(3);\n+\t}\n \n \tstrbuf_addf(&buf, \"refs/heads/test:refs/remotes/%s/test\", rename.new_name);\n \tif (!valid_fetch_refspec(buf.buf))\n@@ -829,8 +835,10 @@ static int rm(int argc, const char **argv)\n \t\tusage_with_options(builtin_remote_rm_usage, options);\n \n \tremote = remote_get(argv[1]);\n-\tif (!remote_is_configured(remote, 1))\n-\t\tdie(_(\"No such remote: '%s'\"), argv[1]);\n+\tif (!remote_is_configured(remote, 1)) {\n+\t\terror(_(\"No such remote: '%s'\"), argv[1]);\n+\t\texit(2);\n+\t}\n \n \tknown_remotes.to_delete = remote;\n \tfor_each_remote(add_known_remote, &known_remotes);\n@@ -1511,8 +1519,10 @@ static int set_remote_branches(const char *remotename, const char **branches,\n \tstrbuf_addf(&key, \"remote.%s.fetch\", remotename);\n \n \tremote = remote_get(remotename);\n-\tif (!remote_is_configured(remote, 1))\n-\t\tdie(_(\"No such remote '%s'\"), remotename);\n+\tif (!remote_is_configured(remote, 1)) {\n+\t\terror(_(\"No such remote '%s'\"), remotename);\n+\t\texit(2);\n+\t}\n \n \tif (!add_mode && remove_all_fetch_refspecs(key.buf)) {\n \t\tstrbuf_release(&key);\n@@ -1565,8 +1575,10 @@ static int get_url(int argc, const char **argv)\n \tremotename = argv[0];\n \n \tremote = remote_get(remotename);\n-\tif (!remote_is_configured(remote, 1))\n-\t\tdie(_(\"No such remote '%s'\"), remotename);\n+\tif (!remote_is_configured(remote, 1)) {\n+\t\terror(_(\"No such remote '%s'\"), remotename);\n+\t\texit(2);\n+\t}\n \n \turl_nr = 0;\n \tif (push_mode) {\n@@ -1633,8 +1645,10 @@ static int set_url(int argc, const char **argv)\n \t\toldurl = newurl;\n \n \tremote = remote_get(remotename);\n-\tif (!remote_is_configured(remote, 1))\n-\t\tdie(_(\"No such remote '%s'\"), remotename);\n+\tif (!remote_is_configured(remote, 1)) {\n+\t\terror(_(\"No such remote '%s'\"), remotename);\n+\t\texit(2);\n+\t}\n \n \tif (push_mode) {\n \t\tstrbuf_addf(&name_buf, \"remote.%s.pushurl\", remotename);\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 8d62edd98b..c99659d39e 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -145,8 +145,8 @@ test_expect_success 'remove remote protects local branches' '\n test_expect_success 'remove errors out early when deleting non-existent branch' '\n \t(\n \t\tcd test &&\n-\t\techo \"fatal: No such remote: '\\''foo'\\''\" >expect &&\n-\t\ttest_must_fail git remote rm foo 2>actual &&\n+\t\techo \"error: No such remote: '\\''foo'\\''\" >expect &&\n+\t\ttest_expect_code 2 git remote rm foo 2>actual &&\n \t\ttest_i18ncmp expect actual\n \t)\n '\n@@ -173,24 +173,24 @@ test_expect_success 'remove remote with a branch without configured merge' '\n test_expect_success 'rename errors out early when deleting non-existent branch' '\n \t(\n \t\tcd test &&\n-\t\techo \"fatal: No such remote: '\\''foo'\\''\" >expect &&\n-\t\ttest_must_fail git remote rename foo bar 2>actual &&\n+\t\techo \"error: No such remote: '\\''foo'\\''\" >expect &&\n+\t\ttest_expect_code 2 git remote rename foo bar 2>actual &&\n \t\ttest_i18ncmp expect actual\n \t)\n '\n \n test_expect_success 'add existing foreign_vcs remote' '\n \ttest_config remote.foo.vcs bar &&\n-\techo \"fatal: remote foo already exists.\" >expect &&\n-\ttest_must_fail git remote add foo bar 2>actual &&\n+\techo \"error: remote foo already exists.\" >expect &&\n+\ttest_expect_code 3 git remote add foo bar 2>actual &&\n \ttest_i18ncmp expect actual\n '\n \n test_expect_success 'add existing foreign_vcs remote' '\n \ttest_config remote.foo.vcs bar &&\n \ttest_config remote.bar.vcs bar &&\n-\techo \"fatal: remote bar already exists.\" >expect &&\n-\ttest_must_fail git remote rename foo bar 2>actual &&\n+\techo \"error: remote bar already exists.\" >expect &&\n+\ttest_expect_code 3 git remote rename foo bar 2>actual &&\n \ttest_i18ncmp expect actual\n '\n \n-- \n2.28.0.297.g1956fa8f8d\n\n"},{"id":"408396","messageId":"CAPig+cRvCPaUxoCojiHvwte4Ty9iRmOaYGrcKs7WHXQN=bid6g@mail.gmail.com","threadId":"54508","inReplyTo":"20201026144825.26537-1-avarab@gmail.com","subject":"Re: [PATCH] remote: add meaningful exit code on missing/existing","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2020-10-26T14:52:36Z","receivedAt":"2020-10-26T14:52:50Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Oct 26, 2020 at 10:48 AM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> Change the exit code for the likes of \"git remote add/rename\" to exit\n> with 2 if the remote in question doesn't exist, and 3 if it\n> does. Before we'd just die() and exit with the general 128 exit code.\n> [...]\n> So e.g. if a sleep() is added right after the remote_is_configured()\n> check in add() we'll clobber rermote.NAME.url, and add\n\ns/rermote/remote/\n\n> another (usually duplicate) remote.NAME.fetch entry (and other values,\n> depending on invocation).\n>\n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n"},{"id":"408434","messageId":"xmqqlffset85.fsf@gitster.c.googlers.com","threadId":"54508","inReplyTo":"20201026144825.26537-1-avarab@gmail.com","subject":"Re: [PATCH] remote: add meaningful exit code on missing/existing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-26T18:29:30Z","receivedAt":"2020-10-26T18:29:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> Change the exit code for the likes of \"git remote add/rename\" to exit\n> with 2 if the remote in question doesn't exist, and 3 if it\n> does. Before we'd just die() and exit with the general 128 exit code.\n>\n> This changes the output message from e.g.:\n>\n>     fatal: remote origin already exists.\n>\n> To:\n>\n>     error: remote origin already exists.\n>\n> Which I believe is a feature, since we generally use \"fatal\" for the\n> generic errors, and \"error\" for the more specific ones with a custom\n> exit code, but this part of the change may break code that already\n> relies on stderr parsing (not that we ever supported that...).\n\nSounds like a worthy goal.  One thing that disturbs me somewhat,\nwhich the time will solve without any of our effort, is how the\ncalling scripts and machinery decides when they can start relying on\nthe exit codes.  A \"git remote tell-me-what-you-can-do\" command that\nmay or may not say \"exitcode\" to its standard output is added?\n\"test $(git version) -ge 2.30 || old_way\"?\n\nThe changes to the code all looked quite straight-forward and \nthe tests look sensible, too.\n\nThanks.\n"},{"id":"408449","messageId":"20201026194519.GB16150@coredump.intra.peff.net","threadId":"54508","inReplyTo":"20201026144825.26537-1-avarab@gmail.com","subject":"Re: [PATCH] remote: add meaningful exit code on missing/existing","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-10-26T19:45:19Z","receivedAt":"2020-10-26T19:45:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Oct 26, 2020 at 03:48:25PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> Change the exit code for the likes of \"git remote add/rename\" to exit\n> with 2 if the remote in question doesn't exist, and 3 if it\n> does. Before we'd just die() and exit with the general 128 exit code.\n\nI can't think of a particular reason not to do this. Though I suspect\nanything scripting may generally be better off using git-config directly\nfor more flexibility and power.\n\n> The motivation for this is a discussion around some code in GitLab's\n> gitaly which wanted to check this, and had to parse stderr to do so:\n> https://gitlab.com/gitlab-org/gitaly/-/merge_requests/2695\n\nI didn't immediately see there the use cases you'd put this to. If it's\nsomething like \"add an origin remote for fetching from a mirror\", then\nthat it is probably reasonable. But if you're thinking of something that\nmay grow without bound (e.g., one remote per fork of the repository),\nkeep in mind that modifying the config file is linear to its size (so\ncreating or deleting N remotes is N^2).\n\n>  Documentation/git-remote.txt | 11 ++++++++++\n>  builtin/remote.c             | 42 ++++++++++++++++++++++++------------\n>  t/t5505-remote.sh            | 16 +++++++-------\n>  3 files changed, 47 insertions(+), 22 deletions(-)\n\nThe patch itself looks good to me, with one minor nit:\n\n> +EXIT STATUS\n> +-----------\n> +\n> +On success, the exit status is `0`.\n> +\n> +When subcommands such as 'add', 'rename' and 'remove' can't find the\n> +remote in question the exit status is `2`, when the remote already\n> +exists the exit status is `3`.\n\nThis is a run-on sentence; the comma after `2` should be a period.\n\nI'd also put an Oxford comma after `rename`, but that's a stylistic\nchoice. :)\n\n-Peff\n"},{"id":"408451","messageId":"CAKPyHN0Dbk+fqYR-5-RMUanJXFqjzh_w6zd0QXnM1fMpSUDJ3Q@mail.gmail.com","threadId":"54508","inReplyTo":"20201026144825.26537-1-avarab@gmail.com","subject":"Re: [PATCH] remote: add meaningful exit code on missing/existing","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2020-10-26T19:55:36Z","receivedAt":"2020-10-26T19:55:51Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Mon, Oct 26, 2020 at 3:48 PM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n> diff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\n> index 9659abbf8e..c1f1355f52 100644\n> --- a/Documentation/git-remote.txt\n> +++ b/Documentation/git-remote.txt\n> @@ -203,6 +203,17 @@ The remote configuration is achieved using the `remote.origin.url` and\n>  `remote.origin.fetch` configuration variables.  (See\n>  linkgit:git-config[1]).\n>\n> +EXIT STATUS\n> +-----------\n> +\n> +On success, the exit status is `0`.\n> +\n> +When subcommands such as 'add', 'rename' and 'remove' can't find the\n\nat least a comma before 'and', but I consider 'or' better here anyway:\n\nWhen subcommands such as 'add', 'rename', or 'remove' can't find the\n\nBert\n\n> +remote in question the exit status is `2`, when the remote already\n> +exists the exit status is `3`.\n> +\n> +On any other error, the exit status may be any other non-zero value.\n> +\n>  EXAMPLES\n>  --------\n>\n"},{"id":"408456","messageId":"xmqq5z6wem82.fsf@gitster.c.googlers.com","threadId":"54508","inReplyTo":"20201026194519.GB16150@coredump.intra.peff.net","subject":"Re: [PATCH] remote: add meaningful exit code on missing/existing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-10-26T21:00:45Z","receivedAt":"2020-10-26T21:00:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> This is a run-on sentence; the comma after `2` should be a period.\n>\n> I'd also put an Oxford comma after `rename`, but that's a stylistic\n> choice. :)\n\nWill queue with this squashed in.\n\n Documentation/git-remote.txt | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git c/Documentation/git-remote.txt w/Documentation/git-remote.txt\nindex 7c84631343..31c29c9b31 100644\n--- c/Documentation/git-remote.txt\n+++ w/Documentation/git-remote.txt\n@@ -208,9 +208,9 @@ EXIT STATUS\n \n On success, the exit status is `0`.\n \n-When subcommands such as 'add', 'rename' and 'remove' can't find the\n-remote in question the exit status is `2`, when the remote already\n-exists the exit status is `3`.\n+When subcommands such as 'add', 'rename', and 'remove' can't find the\n+remote in question, the exit status is `2`. When the remote already\n+exists, the exit status is `3`.\n \n On any other error, the exit status may be any other non-zero value.\n \n"},{"id":"408500","messageId":"20201027094136.7994-1-avarab@gmail.com","threadId":"54508","inReplyTo":"xmqq5z6wem82.fsf@gitster.c.googlers.com","subject":"[PATCH v2] remote: add meaningful exit code on missing/existing","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2020-10-27T09:41:36Z","receivedAt":"2020-10-27T09:41:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the exit code for the likes of \"git remote add/rename\" to exit\nwith 2 if the remote in question doesn't exist, and 3 if it\ndoes. Before we'd just die() and exit with the general 128 exit code.\n\nThis changes the output message from e.g.:\n\n    fatal: remote origin already exists.\n\nTo:\n\n    error: remote origin already exists.\n\nWhich I believe is a feature, since we generally use \"fatal\" for the\ngeneric errors, and \"error\" for the more specific ones with a custom\nexit code, but this part of the change may break code that already\nrelies on stderr parsing (not that we ever supported that...).\n\nThe motivation for this is a discussion around some code in GitLab's\ngitaly which wanted to check this, and had to parse stderr to do so:\nhttps://gitlab.com/gitlab-org/gitaly/-/merge_requests/2695\n\nIt's worth noting as an aside that a method of checking this that\ndoesn't rely on that is to check with \"git config\" whether the value\nin question does or doesn't exist. That introduces a TOCTOU race\ncondition, but on the other hand this code (e.g. \"git remote add\")\nalready has a TOCTOU race.\n\nWe go through the config.lock for the actual setting of the config,\nbut the pseudocode logic is:\n\n    read_config();\n    check_config_and_arg_sanity();\n    save_config();\n\nSo e.g. if a sleep() is added right after the remote_is_configured()\ncheck in add() we'll clobber remote.NAME.url, and add another (usually\nduplicate) remote.NAME.fetch entry (and other values, depending on\ninvocation).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nOn Mon, Oct 26 2020, Junio C Hamano wrote:\n  \n> Jeff King <peff@peff.net> writes:\n>\n>> This is a run-on sentence; the comma after `2` should be a period.\n>>\n>> I'd also put an Oxford comma after `rename`, but that's a stylistic\n>> choice. :)\n>\n> Will queue with this squashed in.\n\nThanks, here's that version plus Eric's suggested typo fix in the\ncommit message.\n\nRange-diff:\n1:  26037f6294 ! 1:  4e44aed049 remote: add meaningful exit code on missing/existing\n    @@ Commit message\n             save_config();\n     \n         So e.g. if a sleep() is added right after the remote_is_configured()\n    -    check in add() we'll clobber rermote.NAME.url, and add\n    -    another (usually duplicate) remote.NAME.fetch entry (and other values,\n    -    depending on invocation).\n    +    check in add() we'll clobber remote.NAME.url, and add another (usually\n    +    duplicate) remote.NAME.fetch entry (and other values, depending on\n    +    invocation).\n     \n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n    @@ Documentation/git-remote.txt: The remote configuration is achieved using the `re\n     +\n     +On success, the exit status is `0`.\n     +\n    -+When subcommands such as 'add', 'rename' and 'remove' can't find the\n    -+remote in question the exit status is `2`, when the remote already\n    -+exists the exit status is `3`.\n    ++When subcommands such as 'add', 'rename', and 'remove' can't find the\n    ++remote in question, the exit status is `2`. When the remote already\n    ++exists, the exit status is `3`.\n     +\n     +On any other error, the exit status may be any other non-zero value.\n     +\n\n Documentation/git-remote.txt | 11 ++++++++++\n builtin/remote.c             | 42 ++++++++++++++++++++++++------------\n t/t5505-remote.sh            | 16 +++++++-------\n 3 files changed, 47 insertions(+), 22 deletions(-)\n\ndiff --git a/Documentation/git-remote.txt b/Documentation/git-remote.txt\nindex ea73386c81..31c29c9b31 100644\n--- a/Documentation/git-remote.txt\n+++ b/Documentation/git-remote.txt\n@@ -203,6 +203,17 @@ The remote configuration is achieved using the `remote.origin.url` and\n `remote.origin.fetch` configuration variables.  (See\n linkgit:git-config[1]).\n \n+EXIT STATUS\n+-----------\n+\n+On success, the exit status is `0`.\n+\n+When subcommands such as 'add', 'rename', and 'remove' can't find the\n+remote in question, the exit status is `2`. When the remote already\n+exists, the exit status is `3`.\n+\n+On any other error, the exit status may be any other non-zero value.\n+\n EXAMPLES\n --------\n \ndiff --git a/builtin/remote.c b/builtin/remote.c\nindex 64b4b551eb..c1828ca7d2 100644\n--- a/builtin/remote.c\n+++ b/builtin/remote.c\n@@ -191,8 +191,10 @@ static int add(int argc, const char **argv)\n \turl = argv[1];\n \n \tremote = remote_get(name);\n-\tif (remote_is_configured(remote, 1))\n-\t\tdie(_(\"remote %s already exists.\"), name);\n+\tif (remote_is_configured(remote, 1)) {\n+\t\terror(_(\"remote %s already exists.\"), name);\n+\t\texit(3);\n+\t}\n \n \tstrbuf_addf(&buf2, \"refs/heads/test:refs/remotes/%s/test\", name);\n \tif (!valid_fetch_refspec(buf2.buf))\n@@ -686,15 +688,19 @@ static int mv(int argc, const char **argv)\n \trename.remote_branches = &remote_branches;\n \n \toldremote = remote_get(rename.old_name);\n-\tif (!remote_is_configured(oldremote, 1))\n-\t\tdie(_(\"No such remote: '%s'\"), rename.old_name);\n+\tif (!remote_is_configured(oldremote, 1)) {\n+\t\terror(_(\"No such remote: '%s'\"), rename.old_name);\n+\t\texit(2);\n+\t}\n \n \tif (!strcmp(rename.old_name, rename.new_name) && oldremote->origin != REMOTE_CONFIG)\n \t\treturn migrate_file(oldremote);\n \n \tnewremote = remote_get(rename.new_name);\n-\tif (remote_is_configured(newremote, 1))\n-\t\tdie(_(\"remote %s already exists.\"), rename.new_name);\n+\tif (remote_is_configured(newremote, 1)) {\n+\t\terror(_(\"remote %s already exists.\"), rename.new_name);\n+\t\texit(3);\n+\t}\n \n \tstrbuf_addf(&buf, \"refs/heads/test:refs/remotes/%s/test\", rename.new_name);\n \tif (!valid_fetch_refspec(buf.buf))\n@@ -829,8 +835,10 @@ static int rm(int argc, const char **argv)\n \t\tusage_with_options(builtin_remote_rm_usage, options);\n \n \tremote = remote_get(argv[1]);\n-\tif (!remote_is_configured(remote, 1))\n-\t\tdie(_(\"No such remote: '%s'\"), argv[1]);\n+\tif (!remote_is_configured(remote, 1)) {\n+\t\terror(_(\"No such remote: '%s'\"), argv[1]);\n+\t\texit(2);\n+\t}\n \n \tknown_remotes.to_delete = remote;\n \tfor_each_remote(add_known_remote, &known_remotes);\n@@ -1511,8 +1519,10 @@ static int set_remote_branches(const char *remotename, const char **branches,\n \tstrbuf_addf(&key, \"remote.%s.fetch\", remotename);\n \n \tremote = remote_get(remotename);\n-\tif (!remote_is_configured(remote, 1))\n-\t\tdie(_(\"No such remote '%s'\"), remotename);\n+\tif (!remote_is_configured(remote, 1)) {\n+\t\terror(_(\"No such remote '%s'\"), remotename);\n+\t\texit(2);\n+\t}\n \n \tif (!add_mode && remove_all_fetch_refspecs(key.buf)) {\n \t\tstrbuf_release(&key);\n@@ -1565,8 +1575,10 @@ static int get_url(int argc, const char **argv)\n \tremotename = argv[0];\n \n \tremote = remote_get(remotename);\n-\tif (!remote_is_configured(remote, 1))\n-\t\tdie(_(\"No such remote '%s'\"), remotename);\n+\tif (!remote_is_configured(remote, 1)) {\n+\t\terror(_(\"No such remote '%s'\"), remotename);\n+\t\texit(2);\n+\t}\n \n \turl_nr = 0;\n \tif (push_mode) {\n@@ -1633,8 +1645,10 @@ static int set_url(int argc, const char **argv)\n \t\toldurl = newurl;\n \n \tremote = remote_get(remotename);\n-\tif (!remote_is_configured(remote, 1))\n-\t\tdie(_(\"No such remote '%s'\"), remotename);\n+\tif (!remote_is_configured(remote, 1)) {\n+\t\terror(_(\"No such remote '%s'\"), remotename);\n+\t\texit(2);\n+\t}\n \n \tif (push_mode) {\n \t\tstrbuf_addf(&name_buf, \"remote.%s.pushurl\", remotename);\ndiff --git a/t/t5505-remote.sh b/t/t5505-remote.sh\nindex 8d62edd98b..c99659d39e 100755\n--- a/t/t5505-remote.sh\n+++ b/t/t5505-remote.sh\n@@ -145,8 +145,8 @@ test_expect_success 'remove remote protects local branches' '\n test_expect_success 'remove errors out early when deleting non-existent branch' '\n \t(\n \t\tcd test &&\n-\t\techo \"fatal: No such remote: '\\''foo'\\''\" >expect &&\n-\t\ttest_must_fail git remote rm foo 2>actual &&\n+\t\techo \"error: No such remote: '\\''foo'\\''\" >expect &&\n+\t\ttest_expect_code 2 git remote rm foo 2>actual &&\n \t\ttest_i18ncmp expect actual\n \t)\n '\n@@ -173,24 +173,24 @@ test_expect_success 'remove remote with a branch without configured merge' '\n test_expect_success 'rename errors out early when deleting non-existent branch' '\n \t(\n \t\tcd test &&\n-\t\techo \"fatal: No such remote: '\\''foo'\\''\" >expect &&\n-\t\ttest_must_fail git remote rename foo bar 2>actual &&\n+\t\techo \"error: No such remote: '\\''foo'\\''\" >expect &&\n+\t\ttest_expect_code 2 git remote rename foo bar 2>actual &&\n \t\ttest_i18ncmp expect actual\n \t)\n '\n \n test_expect_success 'add existing foreign_vcs remote' '\n \ttest_config remote.foo.vcs bar &&\n-\techo \"fatal: remote foo already exists.\" >expect &&\n-\ttest_must_fail git remote add foo bar 2>actual &&\n+\techo \"error: remote foo already exists.\" >expect &&\n+\ttest_expect_code 3 git remote add foo bar 2>actual &&\n \ttest_i18ncmp expect actual\n '\n \n test_expect_success 'add existing foreign_vcs remote' '\n \ttest_config remote.foo.vcs bar &&\n \ttest_config remote.bar.vcs bar &&\n-\techo \"fatal: remote bar already exists.\" >expect &&\n-\ttest_must_fail git remote rename foo bar 2>actual &&\n+\techo \"error: remote bar already exists.\" >expect &&\n+\ttest_expect_code 3 git remote rename foo bar 2>actual &&\n \ttest_i18ncmp expect actual\n '\n \n-- \n2.28.0.297.g1956fa8f8d\n\n"}]}