{"thread":{"id":"55250","subject":"[PATCH] branch: return error when --list finds no matches","startedAt":"2021-03-04T00:23:34Z","lastAt":"2021-03-05T01:05:40Z","messageCount":3,"participants":["Josh Hunt via GitGitGadget","Junio C Hamano","Josh Hunt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"418167","messageId":"pull.892.git.1614793491538.gitgitgadget@gmail.com","threadId":"55250","inReplyTo":null,"subject":"[PATCH] branch: return error when --list finds no matches","fromName":"Josh Hunt via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2021-03-03T17:44:51Z","receivedAt":"2021-03-04T00:23:34Z","isPatch":true,"sender":{"key":"johunt@akamai.com","avatar":null},"body":"From: Josh Hunt \n\nCurrently git branch --list foo always returns an exit status of 0 even\nwhen the branch being searched for does not exist. Now an error is printed\nand returns a non-zero exit status.\n\nSigned-off-by: Josh Hunt \n---\n branch: return error when --list finds no matches\n \n Currently git branch --list foo always returns an exit status of 0 even\n when the branch being searched for does not exist. Now an error is\n printed and returns a non-zero exit status.\n \n Signed-off-by: Josh Hunt johunt@akamai.com\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-892%2Fjoshuahunt%2Fjohunt%2Fgit-branch-list-return-error-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-892/joshuahunt/johunt/git-branch-list-return-error-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/892\n\n builtin/branch.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex bcc00bcf182d..5573175734fe 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -426,6 +426,8 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin\n \tmemset(&array, 0, sizeof(array));\n \n \tfilter_refs(&array, filter, filter->kind | FILTER_REFS_INCLUDE_BROKEN);\n+\tif (!array.nr)\n+\t\tdie(_(\"no branches found\"));\n \n \tif (filter->verbose)\n \t\tmaxwidth = calc_maxwidth(&array, strlen(remote_prefix));\n\nbase-commit: ec125d1bc1053df7e4103f71ebd24fe48dd624a2\n-- \ngitgitgadget\n"},{"id":"418202","messageId":"xmqqr1kvemid.fsf@gitster.c.googlers.com","threadId":"55250","inReplyTo":"pull.892.git.1614793491538.gitgitgadget@gmail.com","subject":"Re: [PATCH] branch: return error when --list finds no matches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-03-04T01:27:38Z","receivedAt":"2021-03-04T01:28:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Josh Hunt via GitGitGadget\" writes:\n\n> From: Josh Hunt \n>\n> Currently git branch --list foo always returns an exit status of 0 even\n> when the branch being searched for does not exist. Now an error is printed\n> and returns a non-zero exit status.\n\nExplaining what happens in the current code upfront is a good thing\nand is in line with the convention used in our project, which is\ngood. But drop \"currently\" from there.\n\nStrictly speaking, it is not \"always\". In a corrupt repository, it\nis likely to show a proper error message and die.\n\nAlso explaining what you want to happen before the end of the log\nmessage is good.\n\nBut the proposed log message lacks why it is a good idea to make\nsuch a change, which is the most important part.\n\nIf you ask me, I would say that the command was asked to show any\nbranches, if exist, that match the given pattern, and did what it\nwas asked to do without encountering any error---it just happened to\nhave seen 0 branch that matched. So I think returning non-zero\nstatus would be a bug.\n\nThanks.\n\n\n\n"},{"id":"418293","messageId":"4f8b0030-0e15-0035-a9e8-224f64cb48f5@akamai.com","threadId":"55250","inReplyTo":"xmqqr1kvemid.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] branch: return error when --list finds no matches","fromName":"Josh Hunt","fromEmail":"johunt@akamai.com","sentAt":"2021-03-05T00:46:39Z","receivedAt":"2021-03-05T01:05:40Z","isPatch":true,"sender":{"key":"johunt@akamai.com","avatar":null},"body":"On 3/3/21 5:27 PM, Junio C Hamano wrote:\n> \"Josh Hunt via GitGitGadget\" writes:\n> \n>> From: Josh Hunt \n>>\n>> Currently git branch --list foo always returns an exit status of 0 even\n>> when the branch being searched for does not exist. Now an error is printed\n>> and returns a non-zero exit status.\n> \n\nJunio\n\nThanks so much for the review.\n\n> Explaining what happens in the current code upfront is a good thing\n> and is in line with the convention used in our project, which is\n> good. But drop \"currently\" from there.\n> \n> Strictly speaking, it is not \"always\". In a corrupt repository, it\n> is likely to show a proper error message and die.\n> \n> Also explaining what you want to happen before the end of the log\n> message is good.\n\nThank you. I will make the above changes to the commit message if this \nmoves forward.\n\n> \n> But the proposed log message lacks why it is a good idea to make\n> such a change, which is the most important part.\n\nOK sure. To provide better context I came across this when using 'git \nbranch --list foo' in a script and there was an expectation (possibly \nincorrect) when the search was not successful it would return an \nnon-zero exit status.\n\nI don't know if there's a convention that git follows, but I have the \nfollowing examples where a similar type of command does return a \nnon-zero exit status:\n\njohunt@johunt-ThinkPad-T480s:~$ ls foo\nls: cannot access 'foo': No such file or directory\njohunt@johunt-ThinkPad-T480s:~$ echo $?\n2\n\njohunt@johunt-ThinkPad-T480s:~$ touch foo\njohunt@johunt-ThinkPad-T480s:~$ grep bar foo\njohunt@johunt-ThinkPad-T480s:~$ echo $?\n1\n\njohunt@johunt-ThinkPad-T480s:~$ grep . foo\ngrep: foo: No such file or directory\njohunt@johunt-ThinkPad-T480s:~$ echo $?\n2\n\nNot just these tools, but even in git itself:\n\njohunt@johunt-ThinkPad-T480s:~$ git init foo\nInitialized empty Git repository in /home/johunt/foo/.git/\njohunt@johunt-ThinkPad-T480s:~/foo$ touch bar\njohunt@johunt-ThinkPad-T480s:~/foo$ git add bar\njohunt@johunt-ThinkPad-T480s:~/foo$ git commit -am 'initial commit'\n[master (root-commit) 771510f] initial commit\n 1 file changed, 0 insertions(+), 0 deletions(-)\n create mode 100644 bar\njohunt@johunt-ThinkPad-T480s:~/foo$ git log foo\nfatal: ambiguous argument 'foo': unknown revision or path not in the \nworking tree.\nUse '--' to separate paths from revisions, like this:\n'git [...] -- [...]'\njohunt@johunt-ThinkPad-T480s:~/foo$ echo $?\n128\n\njohunt@johunt-ThinkPad-T480s:~/foo$ git grep foo .\njohunt@johunt-ThinkPad-T480s:~/foo$ echo $?\n1\n\nHowever it does seem like 'tag --list' follows the branch behavior:\n\njohunt@johunt-ThinkPad-T480s:~/foo$ git tag --list foo\njohunt@johunt-ThinkPad-T480s:~/foo$ echo $?\n0\n\nThere are likely other examples of git commands showing both behaviors. \nIs it that branch and tag are a certain type of command which allows \nthem to behave differently than say log or grep?\n\n> \n> If you ask me, I would say that the command was asked to show any\n> branches, if exist, that match the given pattern, and did what it\n> was asked to do without encountering any error---it just happened to\n> have seen 0 branch that matched. So I think returning non-zero\n> status would be a bug.\n\nInteresting. Going back to the scripting context then your suggestion \nwould just be to check if output is NULL? This is what I've converted my \nscripts to do for now, but still feel like if --list can't find the \nbranch I'm looking for then it should return non-zero as it matches \nbehavior of other tools :)\n\nJosh\n"}]}