{"thread":{"id":"58623","subject":"Re: git: Expected output regression due to 0353c6881890db1302f0f1bdf85c6076eed61113","startedAt":"2022-10-13T18:41:23Z","lastAt":"2022-10-13T18:41:23Z","messageCount":1,"participants":["Glen Choo"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"464878","messageId":"kl6l1qrb1tt6.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"58623","inReplyTo":"6F557A7B-F670-4840-9616-FD54B189FBF0@anodized.com","subject":"Re: git: Expected output regression due to 0353c6881890db1302f0f1bdf85c6076eed61113","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-10-13T18:07:17Z","receivedAt":"2022-10-13T18:41:23Z","isPatch":false,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Hi Christopher!\n\nThanks for the report. Cc-ing Git mailing list; do keep the mailing list\nin the loop in the future, it'll also let you get a response from\nsomeone other than Junio (who is probably busy with post 2.38 work) or\nme :)\n\nI have responded in-line, but otherwise the email I am responding to is\nintact.\n\nChristopher Layne <clayne@anodized.com> writes:\n\n>> On Oct 12, 2022, at 1359 PT, Christopher Layne <clayne@anodized.com> wrote:\n>> \n>> This change causes a regression in expected output for the single remote case:\n>> \n>> e.g.:\n>> \n>> gclayne@dorian:~/git/cmake (master =) $ git fetch --all\n>> From https://gitlab.kitware.com/cmake/cmake\n>>   0a45aa7525..f6af01b53d  master     -> origin/master\n>> \n>> vs:\n>> \n>> clayne@i860:~/git/cmake (master=) $ git fetch --all\n>> Fetching origin\n>> From https://gitlab.kitware.com/cmake/cmake\n>>   0a45aa7525..f6af01b53d  master     -> origin/master\n>> \n>> There may be any manner of output parsers involved in automation or other scripts looking\n>> for the remote to be emitted before the actual url portion and in fact it broke one of mine,\n>> hence how I even stumbled upon this. Since the remote name is no longer emitted in the\n>> single remote case it deviates from the expected output that's been present for years now\n>> and is likely to trip up all manner of people and automation which may depend on it.\n\nThe commit in question (which you've identified correctly) is 0353c68818\n(fetch: do not run a redundant fetch from submodule, 2022-05-16). \"git\ndescribe\" tells me that this has been with us since 2.36, so it is not a\nrecent regression at least.\n\nThis commit changes \"git fetch --all\" so that if a single remote would\nbe fetched, we call fetch_one() (which does not print \"Fetching\nremote....\") instead of fetch_multiple() (which prints the message you\nexpect).\n\nIn [1], Junio spotted that we performed exactly this 'optimization' when\na remote group expands to only one remote. So, in a sense, this commit\nmakes \"git fetch\" more self consistent between \"--all\" and remote\ngroups, but this more consistent behavior seems undesired by at least\none bug reporter.\n\n[1] https://lore.kernel.org/git/xmqqwnel1eqb.fsf@gitster.g/\n\n>\n> Note: it also implicitly makes the output \"quiet\" in the single remote case:\n>\n> Original approach:\n>\n> clayne@i860:/tmp/git-test/cmake (master<) $ git fetch --all\n> Fetching origin\n>\n> clayne@i860:/tmp/git-test/cmake (master<) $ git fetch --all --quiet\n> clayne@i860:/tmp/git-test/cmake (master<) $ \n>\n> New approach:\n>\n> clayne@dorian:~/git/cmake (master <) $ git fetch --all\n> clayne@dorian:~/git/cmake (master <) $ \n>\n> clayne@dorian:~/git/cmake (master <) $ git remote add origin2 -f https://gitlab.kitware.com/cmake/cmake.git\n> Updating origin2\n> From https://gitlab.kitware.com/cmake/cmake\n>  * [new branch]            master     -> origin2/master\n>  * [new branch]            release    -> origin2/release\n>\n> clayne@dorian:~/git/cmake (master <) $ git fetch --all\n> Fetching origin\n> Fetching origin2\n> clayne@dorian:~/git/cmake (master <) $ git fetch --all --quiet\n> clayne@dorian:~/git/cmake (master <) $ \n\nThis is the same behavior reported above; the input isn't becoming\nquiet, only that \"--quiet\" suppresses that same message that is no\nlonger being output.\n\n> The output is now inconsistent and unpredictable because it depends on the amount of remotes\n> present for a given repo. If there is a single remote, \"Fetching %s\" is absent. If there are\n> multiple remotes it behaves as normal. This breaks parsers and since there is no official\n> spec on the output, other than the man page, people have come to rely on parsing the output\n> as a means of determining progress/status for their own automation involving git.\n\n> multiple remotes it behaves as normal. This breaks parsers and since there is no official\n> spec on the output, other than the man page, people have come to rely on parsing the output\n\nI'm not sure whether to treat a change like this as a regression. I\nsuspect that the lack of an official spec is by design, as it signals to\nend users that this is not behavior that has been promised and is prone\nto change. I personally don't think we should start promising that the\n\"git fetch\" output will be well-structured and will not change either,\nbecause the output is meant to be read by humans, who tend to be much\nmore forgiving than scripts.\n\nAll said, I do empathize with your use case. This isn't just \"your\nchange in the output format broke my script\", but \"information that used\nto be there isn't there any more, so I can't even update my script\". A\nreasonable compromise might be to log \"Fetching %s\" when fetching a\nsingle remote in \"git fetch --verbose\", e.g.\n\n  diff --git a/builtin/fetch.c b/builtin/fetch.c\n  index a0fca93bb6..7f811e115c 100644\n  --- a/builtin/fetch.c\n  +++ b/builtin/fetch.c\n  @@ -2085,6 +2085,10 @@ static int fetch_one(struct remote *remote, int argc, const char **argv,\n    if (server_options.nr)\n      gtransport->server_options = &server_options;\n\n  +\n  +\tif (verbosity >= 1)\n  +\t\tprintf(_(\"Fetching %s\\n\"), remote->name);\n  +\n    sigchain_push_common(unlock_pack_on_signal);\n    atexit(unlock_pack_atexit);\n    sigchain_push(SIGPIPE, SIG_IGN);\n\nI could see this as a reasonable QoL improvement for some humans. This\nis _still_ only meant for humans, and doesn't change my opinion that\nthis is unpromised behavior.\n\nIf you are looking for promised, well-behaved output, you're welcome to\npropose something like \"git status --porcelain\" for \"git fetch\", where\nthe output format is well-documented and stable.\n\n>\n> -cl\n"}]}