{"thread":{"id":"59047","subject":"should git rev-parse -q --verify on a range produce output?","startedAt":"2023-01-07T23:04:17Z","lastAt":"2023-01-09T09:11:10Z","messageCount":5,"participants":["demerphq","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"469894","messageId":"CANgJU+Vo3B=YuqgWVgiRMMiBwTFEh98O0LSJJ+ES6EM=MP4Cew@mail.gmail.com","threadId":"59047","inReplyTo":null,"subject":"should git rev-parse -q --verify on a range produce output?","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2023-01-07T23:03:55Z","receivedAt":"2023-01-07T23:04:17Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"Hi all,\n\nI was curious if it is a bug that `rev-parse -q --verify` produces\noutput for a commit range, and only reveals it is supposed to be used\nwith a single commit as an error message? When combined with -q it\nmake it seem like it has silently worked if you aren't careful to\ncheck $? afterwards. (Which obviously some script wasn't or I wouldn't\nbe posting this. :-)\n\nFor example:\n\ngit rev-parse --verify -q HEAD^..HEAD; echo $?\n869400a03fff1b3dcff82f3357d37ab506af2788\n^13073008cfc15016d862c630d56323bb7c9d775e\n1\n\n\"looks\" like it has worked, producing two output refs, but as it shows\nthe exit code is 1. We have also seen it output a single line from a\nrange, when the entire range wasn't cloned.\n\nIt seems weird that it produces output at all in this case. Shouldn't\nit see that it is a range and exit with a nonzero exit code\nimmediately? The docs say:\n\n       --verify\n           Verify that exactly one parameter is provided, and that it\ncan be turned into a raw 20-byte SHA-1 that\n           can be used to access the object database. If so, emit it\nto the standard output; otherwise, error out.\n           ...\n       -q, --quiet\n           Only meaningful in --verify mode. Do not output an error\nmessage if the first argument is not a valid\n           object name; instead exit with non-zero status silently.\nSHA-1s for valid object names are printed to\n           stdout on success.\n\nIt seems to me that the second sentence from --verify indicates that\nit should not produce output from a range at all, although I guess the\nexact meaning of \"error out\" is ambiguous, and the last sentence of\nthe --quiet documentation is not incompatible with it also producing\noutput when it fails,  but it seems like if this is expected behavior\nit should be documented more explicitly. Which makes me think it is a\nbug. :-)\n\nI am testing with git version 2.25.1 but this was encountered on\nGitHub with whatever git they used as well. (We noticed this in a CI\npipeline using a shallow clone.)\n\ncheers\nYves\n\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"469901","messageId":"xmqqmt6tzrcx.fsf@gitster.g","threadId":"59047","inReplyTo":"CANgJU+Vo3B=YuqgWVgiRMMiBwTFEh98O0LSJJ+ES6EM=MP4Cew@mail.gmail.com","subject":"Re: should git rev-parse -q --verify on a range produce output?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-08T02:34:38Z","receivedAt":"2023-01-08T02:46:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"demerphq <demerphq@gmail.com> writes:\n\n> I was curious if it is a bug that `rev-parse -q --verify` produces\n> output for a commit range, and only reveals it is supposed to be used\n> with a single commit as an error message?\n\nI know that the original scenario that the combination of \"--verify\"\nand \"--quiet\" was invented for was \"I have a string that ought to\nresolve to a single object name, but the object may be missing\", and\n\n\tif git cat-file -e \"$name\" 2>/dev/null\n\tthen\n\t\trawname=$(git rev-parse --verify \"$name\")\n\t\ttrue\n\telse\n\t\tfalse\n\tfi &&\n\t... do something that uses $rawname here ...\n\t\nis a mouthful.  It becomes easier to use if we can say\n\n\trawname=$(git rev-parse -q --verify \"$name\") &&\n\t... do something that uses $rawname here ...\n\nI do not think the behaviour in usecase outside that was carefully\ndesigned to the details.\n"},{"id":"469914","messageId":"CANgJU+UeZJP=tBx7ALd8_X=b25RkAdQ1NkQpueSL-p+kpCO17Q@mail.gmail.com","threadId":"59047","inReplyTo":"xmqqmt6tzrcx.fsf@gitster.g","subject":"Re: should git rev-parse -q --verify on a range produce output?","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2023-01-08T10:45:13Z","receivedAt":"2023-01-08T10:45:30Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On Sun, 8 Jan 2023 at 03:34, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> demerphq <demerphq@gmail.com> writes:\n>\n> > I was curious if it is a bug that `rev-parse -q --verify` produces\n> > output for a commit range, and only reveals it is supposed to be used\n> > with a single commit as an error message?\n>\n> I know that the original scenario that the combination of \"--verify\"\n> and \"--quiet\" was invented for was \"I have a string that ought to\n> resolve to a single object name, but the object may be missing\", and\n>\n>         if git cat-file -e \"$name\" 2>/dev/null\n>         then\n>                 rawname=$(git rev-parse --verify \"$name\")\n>                 true\n>         else\n>                 false\n>         fi &&\n>         ... do something that uses $rawname here ...\n>\n> is a mouthful.  It becomes easier to use if we can say\n>\n>         rawname=$(git rev-parse -q --verify \"$name\") &&\n>         ... do something that uses $rawname here ...\n>\n> I do not think the behaviour in usecase outside that was carefully\n> designed to the details.\n\nIs this something you think should be fixed? I would give it a go if\nthere was some direction on what it should do in this case. Just error\nearly and produce no output?\n\nBTW, the weird behavior of it is documented here:\nhttps://github.com/Perl/perl5/pull/20657\n\nWe have some tooling which we use to generate lists of contributors\nfor each release from the git commits, and we test this code each\nbuild on specific known commit ranges. We noticed this because some of\nour tests run in shallow clones and we were using rev-parse to disable\nthe test if the clone didnt have the right commit range. But it was\nfailing sometimes when it didnt have the complete range and we were\n\"doing it wrong\" by not checking the error code and just checking to\nsee if it output anything.\n\ncheers,\nYves\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"},{"id":"469935","messageId":"xmqqpmbowek7.fsf@gitster.g","threadId":"59047","inReplyTo":"CANgJU+UeZJP=tBx7ALd8_X=b25RkAdQ1NkQpueSL-p+kpCO17Q@mail.gmail.com","subject":"Re: should git rev-parse -q --verify on a range produce output?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-01-09T03:51:36Z","receivedAt":"2023-01-09T03:54:21Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"demerphq <demerphq@gmail.com> writes:\n\n> Is this something you think should be fixed? I would give it a go if\n> there was some direction on what it should do in this case. Just error\n> early and produce no output?\n\nI do not mind if the error case gets changed to behave differently,\nas long as the updated behaviour is something everybody thinks an\nimprovement over the current behaviour.  I do not offhand know what\nthe \"fixed\" behaviour should be.\n\nI do not mind if nothing changed and documentation gets updated to\nreduce end-user confusion, either.\n\nThanks.\n\n"},{"id":"469956","messageId":"CANgJU+VsAZ6Dz05ZFRQfGLgti7WssdYqGaEYEQuACoCdN+nvew@mail.gmail.com","threadId":"59047","inReplyTo":"xmqqpmbowek7.fsf@gitster.g","subject":"Re: should git rev-parse -q --verify on a range produce output?","fromName":"demerphq","fromEmail":"demerphq@gmail.com","sentAt":"2023-01-09T09:04:13Z","receivedAt":"2023-01-09T09:11:10Z","isPatch":false,"sender":{"key":"demerphq@gmail.com","avatar":null},"body":"On Mon, 9 Jan 2023 at 04:51, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> demerphq <demerphq@gmail.com> writes:\n>\n> > Is this something you think should be fixed? I would give it a go if\n> > there was some direction on what it should do in this case. Just error\n> > early and produce no output?\n>\n> I do not mind if the error case gets changed to behave differently,\n> as long as the updated behaviour is something everybody thinks an\n> improvement over the current behaviour.  I do not offhand know what\n> the \"fixed\" behaviour should be.\n>\n> I do not mind if nothing changed and documentation gets updated to\n> reduce end-user confusion, either.\n\nOk, I'll wait a bit to see if someone can suggest a good alternative\nbehavior, and if nobody does ill write a doc patch.\n\nCheers!\nYves\n\n-- \nperl -Mre=debug -e \"/just|another|perl|hacker/\"\n"}]}