{"thread":{"id":"49403","subject":"Coredump on ls-remote + --sort","startedAt":"2018-09-22T10:42:28Z","lastAt":"2018-11-16T13:16:50Z","messageCount":15,"participants":["H.Merijn Brand","Ævar Arnfjörð Bjarmason","SZEDER Gábor","Junio C Hamano","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"358695","messageId":"20180922124215.0c8172d1@pc09.procura.nl","threadId":"49403","inReplyTo":null,"subject":"Coredump on ls-remote + --sort","fromName":"H.Merijn Brand","fromEmail":"h.m.brand@xs4all.nl","sentAt":"2018-09-22T10:42:15Z","receivedAt":"2018-09-22T10:42:28Z","isPatch":false,"sender":{"key":"h.m.brand@xs4all.nl","avatar":"https://gravatar.com/avatar/5b8f83ee35c427a646cbea3b104346e00ab3663b99bbf435cddeb75cd4b3857b?d=mp&s=160"},"body":"A small background why I wanted this: I need to build a new version of\nsome software hosted in git, add a lot of shared/static stuff and\nautomatically test it. I want to get the most recent *tag* and create a\nfolder with the tagname in it, go into that folder and clone the repo,\ncheck out the tag, add the rest of the stuff, build and test\n\nAs the default outpout of «git ls-remote --tags» or «git ls-remote» is\ntopologically sorted by tagname, the most recent tag is likely to be in\nthe middle.\n\nLinux 4.12.14-lp150.12.16-default [openSUSE Leap 15.0]\n\n$ git --version\ngit version 2.19.0\n\n$ git ls-remote --tags github.com:Tux/App-ccdiff\n5e2513ab6dd4a24c8f3a3ace0a3faba6a291d818        refs/tags/0.04\n2f7ea0f1e751dc20c1ddb15f6d61c6fa62d5d6f1        refs/tags/0.05\na3802907be5b10383c7438f1d1c660fe13a05d3f        refs/tags/0.06\n3e4bfa7cde75fba221650b9d3aa5555b706803df        refs/tags/0.07\n05829d1ac5b49bbdd2167bc363b94f8a12e752b3        refs/tags/0.08\n9c6e5861ea9c6e50c501663d43c5a9f6d31b54bc        refs/tags/0.09\ne815b059f6326da936c3a92272ba67e273b1dc3e        refs/tags/0.10\ne6b40e331c945449bb8e71023de4920ca5574adc        refs/tags/0.20\nbe55e6336b1db5ffad23a6a0a663763e2f5da779        refs/tags/0.21\ne283d563f02bb8d2131e8b95852072ac204b28b4        refs/tags/0.22\n0d3d1830f542121bfef1d984f21343c6d9c774f8        refs/tags/0.23\nd7bf195a92095a4f0b810584810450e4001b1a2c        refs/tags/0.24\n5c517cf3f79cb18173714e63bc5b80a3e3f888f1        refs/tags/0.25\n\nWhether or not supported, it should not dump core\n\n$ git ls-remote --tags --sort=authordate github.com:Tux/App-ccdiff\nSegmentation fault (core dumped)\n\n(gdb) where\n#0  0x00007ffff74784a6 in __strlen_sse2 () from /lib64/libc.so.6\n#1  0x000000000057a956 in for_each_replace_ref ()\n#2  0x0000000000596cec in do_lookup_replace_object ()\n#3  0x00000000005c14eb in oid_object_info_extended ()\n#4  0x000000000058b984 in get_object ()\n#5  0x000000000058ddde in populate_value ()\n#6  0x000000000058e36b in compare_refs ()\n#7  0x000000000061447a in msort_with_tmp.part ()\n#8  0x0000000000614505 in msort_with_tmp.part ()\n#9  0x0000000000614518 in msort_with_tmp.part ()\n#10 0x0000000000614518 in msort_with_tmp.part ()\n#11 0x000000000061459e in git_qsort_s ()\n#12 0x000000000058ed40 in ref_array_sort ()\n#13 0x000000000044ef66 in cmd_ls_remote ()\n#14 0x000000000040784f in handle_builtin ()\n#15 0x0000000000407bb0 in cmd_main ()\n#16 0x0000000000406b04 in main ()\n\nLinux 3.10.0-862.6.3.el7.x86_64 [CentOS Linux 7.5.1804 (Core)]\n\n$ git --version\ngit version 2.18.0\n\n$ git ls-remote --tags https://github.com/Tux/App-ccdiff\n5e2513ab6dd4a24c8f3a3ace0a3faba6a291d818        refs/tags/0.04\n2f7ea0f1e751dc20c1ddb15f6d61c6fa62d5d6f1        refs/tags/0.05\na3802907be5b10383c7438f1d1c660fe13a05d3f        refs/tags/0.06\n3e4bfa7cde75fba221650b9d3aa5555b706803df        refs/tags/0.07\n05829d1ac5b49bbdd2167bc363b94f8a12e752b3        refs/tags/0.08\n9c6e5861ea9c6e50c501663d43c5a9f6d31b54bc        refs/tags/0.09\ne815b059f6326da936c3a92272ba67e273b1dc3e        refs/tags/0.10\ne6b40e331c945449bb8e71023de4920ca5574adc        refs/tags/0.20\nbe55e6336b1db5ffad23a6a0a663763e2f5da779        refs/tags/0.21\ne283d563f02bb8d2131e8b95852072ac204b28b4        refs/tags/0.22\n0d3d1830f542121bfef1d984f21343c6d9c774f8        refs/tags/0.23\nd7bf195a92095a4f0b810584810450e4001b1a2c        refs/tags/0.24\n5c517cf3f79cb18173714e63bc5b80a3e3f888f1        refs/tags/0.25\n\n$ git ls-remote --tags --sort=authordate https://github.com/Tux/App-ccdiff\nSegmentation fault\n\n(gdb) where\n#0  0x00007ffff751a67f in __strlen_sse42 () from /lib64/libc.so.6\n#1  0x0000000000561c06 in for_each_replace_ref ()\n#2  0x000000000057c3fa in do_lookup_replace_object ()\n#3  0x00000000005a6aa8 in read_object_file_extended ()\n#4  0x00000000005731e5 in get_object ()\n#5  0x00000000005749df in populate_value ()\n#6  0x0000000000574e9d in compare_refs ()\n#7  0x00000000005efe57 in msort_with_tmp.part.0 ()\n#8  0x00000000005efe31 in msort_with_tmp.part.0 ()\n#9  0x00000000005efe0e in msort_with_tmp.part.0 ()\n#10 0x00000000005efe0e in msort_with_tmp.part.0 ()\n#11 0x00000000005eff5c in git_qsort_s ()\n#12 0x00000000005757e0 in ref_array_sort ()\n#13 0x000000000044c6b6 in cmd_ls_remote ()\n#14 0x000000000040730e in handle_builtin ()\n#15 0x000000000040760e in cmd_main ()\n#16 0x0000000000406554 in main ()\n\n\n-- \nH.Merijn Brand  http://tux.nl   Perl Monger  http://amsterdam.pm.org/\nusing perl5.00307 .. 5.29   porting perl5 on HP-UX, AIX, and openSUSE\nhttp://mirrors.develooper.com/hpux/        http://www.test-smoke.org/\nhttp://qa.perl.org   http://www.goldmark.org/jeff/stupid-disclaimers/\n"},{"id":"358696","messageId":"87in2xk8zc.fsf@evledraar.gmail.com","threadId":"49403","inReplyTo":"20180922124215.0c8172d1@pc09.procura.nl","subject":"Re: Coredump on ls-remote + --sort","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-09-22T12:33:11Z","receivedAt":"2018-09-22T12:33:17Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Sat, Sep 22 2018, H.Merijn Brand wrote:\n\n> A small background why I wanted this: I need to build a new version of\n> some software hosted in git, add a lot of shared/static stuff and\n> automatically test it. I want to get the most recent *tag* and create a\n> folder with the tagname in it, go into that folder and clone the repo,\n> check out the tag, add the rest of the stuff, build and test\n>\n> As the default outpout of «git ls-remote --tags» or «git ls-remote» is\n> topologically sorted by tagname, the most recent tag is likely to be in\n> the middle.\n>\n> Linux 4.12.14-lp150.12.16-default [openSUSE Leap 15.0]\n>\n> $ git --version\n> git version 2.19.0\n>\n> $ git ls-remote --tags github.com:Tux/App-ccdiff\n> 5e2513ab6dd4a24c8f3a3ace0a3faba6a291d818        refs/tags/0.04\n> 2f7ea0f1e751dc20c1ddb15f6d61c6fa62d5d6f1        refs/tags/0.05\n> a3802907be5b10383c7438f1d1c660fe13a05d3f        refs/tags/0.06\n> 3e4bfa7cde75fba221650b9d3aa5555b706803df        refs/tags/0.07\n> 05829d1ac5b49bbdd2167bc363b94f8a12e752b3        refs/tags/0.08\n> 9c6e5861ea9c6e50c501663d43c5a9f6d31b54bc        refs/tags/0.09\n> e815b059f6326da936c3a92272ba67e273b1dc3e        refs/tags/0.10\n> e6b40e331c945449bb8e71023de4920ca5574adc        refs/tags/0.20\n> be55e6336b1db5ffad23a6a0a663763e2f5da779        refs/tags/0.21\n> e283d563f02bb8d2131e8b95852072ac204b28b4        refs/tags/0.22\n> 0d3d1830f542121bfef1d984f21343c6d9c774f8        refs/tags/0.23\n> d7bf195a92095a4f0b810584810450e4001b1a2c        refs/tags/0.24\n> 5c517cf3f79cb18173714e63bc5b80a3e3f888f1        refs/tags/0.25\n>\n> Whether or not supported, it should not dump core\n>\n> $ git ls-remote --tags --sort=authordate github.com:Tux/App-ccdiff\n> Segmentation fault (core dumped)\n>\n> (gdb) where\n> #0  0x00007ffff74784a6 in __strlen_sse2 () from /lib64/libc.so.6\n> #1  0x000000000057a956 in for_each_replace_ref ()\n> #2  0x0000000000596cec in do_lookup_replace_object ()\n> #3  0x00000000005c14eb in oid_object_info_extended ()\n> #4  0x000000000058b984 in get_object ()\n> #5  0x000000000058ddde in populate_value ()\n> #6  0x000000000058e36b in compare_refs ()\n> #7  0x000000000061447a in msort_with_tmp.part ()\n> #8  0x0000000000614505 in msort_with_tmp.part ()\n> #9  0x0000000000614518 in msort_with_tmp.part ()\n> #10 0x0000000000614518 in msort_with_tmp.part ()\n> #11 0x000000000061459e in git_qsort_s ()\n> #12 0x000000000058ed40 in ref_array_sort ()\n> #13 0x000000000044ef66 in cmd_ls_remote ()\n> #14 0x000000000040784f in handle_builtin ()\n> #15 0x0000000000407bb0 in cmd_main ()\n> #16 0x0000000000406b04 in main ()\n>\n> Linux 3.10.0-862.6.3.el7.x86_64 [CentOS Linux 7.5.1804 (Core)]\n>\n> $ git --version\n> git version 2.18.0\n>\n> $ git ls-remote --tags https://github.com/Tux/App-ccdiff\n> 5e2513ab6dd4a24c8f3a3ace0a3faba6a291d818        refs/tags/0.04\n> 2f7ea0f1e751dc20c1ddb15f6d61c6fa62d5d6f1        refs/tags/0.05\n> a3802907be5b10383c7438f1d1c660fe13a05d3f        refs/tags/0.06\n> 3e4bfa7cde75fba221650b9d3aa5555b706803df        refs/tags/0.07\n> 05829d1ac5b49bbdd2167bc363b94f8a12e752b3        refs/tags/0.08\n> 9c6e5861ea9c6e50c501663d43c5a9f6d31b54bc        refs/tags/0.09\n> e815b059f6326da936c3a92272ba67e273b1dc3e        refs/tags/0.10\n> e6b40e331c945449bb8e71023de4920ca5574adc        refs/tags/0.20\n> be55e6336b1db5ffad23a6a0a663763e2f5da779        refs/tags/0.21\n> e283d563f02bb8d2131e8b95852072ac204b28b4        refs/tags/0.22\n> 0d3d1830f542121bfef1d984f21343c6d9c774f8        refs/tags/0.23\n> d7bf195a92095a4f0b810584810450e4001b1a2c        refs/tags/0.24\n> 5c517cf3f79cb18173714e63bc5b80a3e3f888f1        refs/tags/0.25\n>\n> $ git ls-remote --tags --sort=authordate https://github.com/Tux/App-ccdiff\n> Segmentation fault\n>\n> (gdb) where\n> #0  0x00007ffff751a67f in __strlen_sse42 () from /lib64/libc.so.6\n> #1  0x0000000000561c06 in for_each_replace_ref ()\n> #2  0x000000000057c3fa in do_lookup_replace_object ()\n> #3  0x00000000005a6aa8 in read_object_file_extended ()\n> #4  0x00000000005731e5 in get_object ()\n> #5  0x00000000005749df in populate_value ()\n> #6  0x0000000000574e9d in compare_refs ()\n> #7  0x00000000005efe57 in msort_with_tmp.part.0 ()\n> #8  0x00000000005efe31 in msort_with_tmp.part.0 ()\n> #9  0x00000000005efe0e in msort_with_tmp.part.0 ()\n> #10 0x00000000005efe0e in msort_with_tmp.part.0 ()\n> #11 0x00000000005eff5c in git_qsort_s ()\n> #12 0x00000000005757e0 in ref_array_sort ()\n> #13 0x000000000044c6b6 in cmd_ls_remote ()\n> #14 0x000000000040730e in handle_builtin ()\n> #15 0x000000000040760e in cmd_main ()\n> #16 0x0000000000406554 in main ()\n\nI can't reproduce this, I just get for both ssh and https:\n\n    $ ~/g/git/git --exec-path=$PWD ls-remote --tags --sort=authordate https://github.com/Tux/App-ccdiff\n    fatal: missing object 2f7ea0f1e751dc20c1ddb15f6d61c6fa62d5d6f1 for refs/tags/0.05\n    $ ~/g/git/git --exec-path=$PWD version\n    git version 2.18.0\n\nSame thing on latest 'master' (v2.19.0-221-g150f307afc).\n\nBut maybe that's just a symptom of the same bug, when I clone the repo I\nget a working 0.05 tag, and it passes fsck, and I do get the\n2f7ea0f1e751dc20c1ddb15f6d61c6fa62d5d6f1 object (which is the 0.05 tag\nobject).\n"},{"id":"358699","messageId":"20180922141145.10558-1-szeder.dev@gmail.com","threadId":"49403","inReplyTo":"20180922124215.0c8172d1@pc09.procura.nl","subject":"[PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-09-22T14:11:45Z","receivedAt":"2018-09-22T14:12:19Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The command 'git ls-remote --sort=authordate <remote>' segfaults when\nrun outside of a repository, ever since the introduction of its\n'--sort' option in 1fb20dfd8e (ls-remote: create '--sort' option,\n2018-04-09).\n\nWhile in general the 'git ls-remote' command can be run outside of a\nrepository just fine, its '--sort=<key>' option with certain keys does\nrequire access to the referenced objects.  This sorting is implemented\nusing the generic ref-filter sorting facility, which already handles\nmissing objects gracefully with the appropriate 'missing object\ndeadbeef for HEAD' message.  However, being generic means that it\nchecks replace refs while trying to retrieve an object, and while\ndoing so it accesses the 'git_replace_ref_base' variable, which has\nnot been initialized and is still a NULL pointer when outside of a\nrepository, thus causing the segfault.\n\nMake ref-filter more careful and only attempt to retrieve an object\nwhen we are in a repository.  Also add a test to ensure that 'git\nls-remote --sort' fails gracefully when executed outside of a\nrepository.\n\nReported-by: H.Merijn Brand <h.m.brand@xs4all.nl>\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n\nI'm not quite sure that this is the best place to add this check...\nbut hey, it's a Saturday afternoon after all ;)\n\n ref-filter.c         | 3 ++-\n t/t5512-ls-remote.sh | 6 ++++++\n 2 files changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex e1bcb4ca8a..3555bc29e7 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1473,7 +1473,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj\n \t\toi->info.sizep = &oi->size;\n \t\toi->info.typep = &oi->type;\n \t}\n-\tif (oid_object_info_extended(the_repository, &oi->oid, &oi->info,\n+\tif (!have_git_dir() ||\n+\t    oid_object_info_extended(the_repository, &oi->oid, &oi->info,\n \t\t\t\t     OBJECT_INFO_LOOKUP_REPLACE))\n \t\treturn strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n \t\t\t\t       oid_to_hex(&oi->oid), ref->refname);\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex bc5703ff9b..7dd081da01 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -302,4 +302,10 @@ test_expect_success 'ls-remote works outside repository' '\n \tnongit git ls-remote dst.git\n '\n \n+test_expect_success 'ls-remote --sort fails gracefully outside repository' '\n+\t# Use a sort key that requires access to the referenced objects.\n+\tnongit test_must_fail git ls-remote --sort=authordate \"$TRASH_DIRECTORY\" 2>err &&\n+\ttest_i18ngrep \"^fatal: missing object\" err\n+'\n+\n test_done\n-- \n2.19.0.355.geb876cd9d6\n\n"},{"id":"358763","messageId":"xmqqzhw6swhf.fsf@gitster-ct.c.googlers.com","threadId":"49403","inReplyTo":"20180922141145.10558-1-szeder.dev@gmail.com","subject":"Re: [PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-24T16:15:08Z","receivedAt":"2018-09-24T16:15:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> The command 'git ls-remote --sort=authordate <remote>' segfaults when\n> run outside of a repository, ever since the introduction of its\n> '--sort' option in 1fb20dfd8e (ls-remote: create '--sort' option,\n> 2018-04-09).\n>\n> While in general the 'git ls-remote' command can be run outside of a\n> repository just fine, its '--sort=<key>' option with certain keys does\n> require access to the referenced objects.  This sorting is implemented\n> using the generic ref-filter sorting facility, which already handles\n> missing objects gracefully with the appropriate 'missing object\n> deadbeef for HEAD' message.  However, being generic means that it\n> checks replace refs while trying to retrieve an object, and while\n> doing so it accesses the 'git_replace_ref_base' variable, which has\n> not been initialized and is still a NULL pointer when outside of a\n> repository, thus causing the segfault.\n>\n> Make ref-filter more careful and only attempt to retrieve an object\n> when we are in a repository.  Also add a test to ensure that 'git\n> ls-remote --sort' fails gracefully when executed outside of a\n> repository.\n\nOK.  So by forcing get_object() return an error, we do the same to\npopulate_value() which in turn will make get_ref_atgom_value return\nan error and cmp_ref_sorting() will notice and die.\n\nI think that is the best we could do.\n\n>\n> Reported-by: H.Merijn Brand <h.m.brand@xs4all.nl>\n> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n> ---\n>\n> I'm not quite sure that this is the best place to add this check...\n> but hey, it's a Saturday afternoon after all ;)\n>\n>  ref-filter.c         | 3 ++-\n>  t/t5512-ls-remote.sh | 6 ++++++\n>  2 files changed, 8 insertions(+), 1 deletion(-)\n>\n> diff --git a/ref-filter.c b/ref-filter.c\n> index e1bcb4ca8a..3555bc29e7 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1473,7 +1473,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj\n>  \t\toi->info.sizep = &oi->size;\n>  \t\toi->info.typep = &oi->type;\n>  \t}\n> -\tif (oid_object_info_extended(the_repository, &oi->oid, &oi->info,\n> +\tif (!have_git_dir() ||\n> +\t    oid_object_info_extended(the_repository, &oi->oid, &oi->info,\n>  \t\t\t\t     OBJECT_INFO_LOOKUP_REPLACE))\n>  \t\treturn strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n>  \t\t\t\t       oid_to_hex(&oi->oid), ref->refname);\n> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\n> index bc5703ff9b..7dd081da01 100755\n> --- a/t/t5512-ls-remote.sh\n> +++ b/t/t5512-ls-remote.sh\n> @@ -302,4 +302,10 @@ test_expect_success 'ls-remote works outside repository' '\n>  \tnongit git ls-remote dst.git\n>  '\n>  \n> +test_expect_success 'ls-remote --sort fails gracefully outside repository' '\n> +\t# Use a sort key that requires access to the referenced objects.\n> +\tnongit test_must_fail git ls-remote --sort=authordate \"$TRASH_DIRECTORY\" 2>err &&\n> +\ttest_i18ngrep \"^fatal: missing object\" err\n> +'\n> +\n>  test_done\n"},{"id":"358770","messageId":"20180924181722.GA25341@sigill.intra.peff.net","threadId":"49403","inReplyTo":"20180922141145.10558-1-szeder.dev@gmail.com","subject":"Re: [PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-09-24T18:17:22Z","receivedAt":"2018-09-24T18:17:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Sep 22, 2018 at 04:11:45PM +0200, SZEDER Gábor wrote:\n\n> The command 'git ls-remote --sort=authordate <remote>' segfaults when\n> run outside of a repository, ever since the introduction of its\n> '--sort' option in 1fb20dfd8e (ls-remote: create '--sort' option,\n> 2018-04-09).\n> \n> While in general the 'git ls-remote' command can be run outside of a\n> repository just fine, its '--sort=<key>' option with certain keys does\n> require access to the referenced objects.  This sorting is implemented\n> using the generic ref-filter sorting facility, which already handles\n> missing objects gracefully with the appropriate 'missing object\n> deadbeef for HEAD' message.  However, being generic means that it\n> checks replace refs while trying to retrieve an object, and while\n> doing so it accesses the 'git_replace_ref_base' variable, which has\n> not been initialized and is still a NULL pointer when outside of a\n> repository, thus causing the segfault.\n> \n> Make ref-filter more careful and only attempt to retrieve an object\n> when we are in a repository.  Also add a test to ensure that 'git\n> ls-remote --sort' fails gracefully when executed outside of a\n> repository.\n\nThis all makes sense, and I think your fix is going in the right\ndirection.\n\nBut...\n\n> I'm not quite sure that this is the best place to add this check...\n> but hey, it's a Saturday afternoon after all ;)\n\nI also wonder about this. For refs, we already catch these cases at a\nlow-level and BUG(). That's better than a segfault, and I suspect we\nshould be doing the same here in oid_object_info_extended(). But that\njust shifts the segfault to a BUG().\n\nFor the refs code, we've generally tried to catch things at a high-level\nand report a more human-friendly error explaining the situation. So\ndoing the same thing here would mean adding code to ls-remote. But I\nthink the plumbing gets pretty tricky, since it has no way to ask\nref-filter \"hey, are we doing to need to look at objects?\".\n\nThat's a thing that I think ref-filter _should_ support (it knows it\nafter having parsed the format string). But it probably ought to come\nalong with other refactoring, and shouldn't hold up this fix.\n\nSo this probably _is_ a reasonable place to check it. However...\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index e1bcb4ca8a..3555bc29e7 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1473,7 +1473,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj\n>  \t\toi->info.sizep = &oi->size;\n>  \t\toi->info.typep = &oi->type;\n>  \t}\n> -\tif (oid_object_info_extended(the_repository, &oi->oid, &oi->info,\n> +\tif (!have_git_dir() ||\n> +\t    oid_object_info_extended(the_repository, &oi->oid, &oi->info,\n>  \t\t\t\t     OBJECT_INFO_LOOKUP_REPLACE))\n>  \t\treturn strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n>  \t\t\t\t       oid_to_hex(&oi->oid), ref->refname);\n\nWould we perhaps want to give the user a hint that the object is not\nreally missing, but rather that we're not in a repository? E.g.,\nsomething like:\n\n  if (!have_git_dir())\n\treturn strbuf_addf_ret(err, -1, \"format specifier requires a repository\");\n  if (oid_object_info_extended(...))\n\treturn ...;\n\n?\n\n-Peff\n"},{"id":"358793","messageId":"20180924212034.GF27036@localhost","threadId":"49403","inReplyTo":"20180924181722.GA25341@sigill.intra.peff.net","subject":"Re: [PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-09-24T21:20:34Z","receivedAt":"2018-09-24T21:20:40Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Sep 24, 2018 at 02:17:22PM -0400, Jeff King wrote:\n> On Sat, Sep 22, 2018 at 04:11:45PM +0200, SZEDER Gábor wrote:\n \n> > diff --git a/ref-filter.c b/ref-filter.c\n> > index e1bcb4ca8a..3555bc29e7 100644\n> > --- a/ref-filter.c\n> > +++ b/ref-filter.c\n> > @@ -1473,7 +1473,8 @@ static int get_object(struct ref_array_item *ref, int deref, struct object **obj\n> >  \t\toi->info.sizep = &oi->size;\n> >  \t\toi->info.typep = &oi->type;\n> >  \t}\n> > -\tif (oid_object_info_extended(the_repository, &oi->oid, &oi->info,\n> > +\tif (!have_git_dir() ||\n> > +\t    oid_object_info_extended(the_repository, &oi->oid, &oi->info,\n> >  \t\t\t\t     OBJECT_INFO_LOOKUP_REPLACE))\n> >  \t\treturn strbuf_addf_ret(err, -1, _(\"missing object %s for %s\"),\n> >  \t\t\t\t       oid_to_hex(&oi->oid), ref->refname);\n> \n> Would we perhaps want to give the user a hint that the object is not\n> really missing, but rather that we're not in a repository? E.g.,\n> something like:\n> \n>   if (!have_git_dir())\n> \treturn strbuf_addf_ret(err, -1, \"format specifier requires a repository\");\n>   if (oid_object_info_extended(...))\n> \treturn ...;\n> \n> ?\n\nI think it makes sense.\n\nI wanted to preserve the error message, because the description of\n'--sort=<key>' in 'Documentation/git-ls-remote.txt' explicitly\nmentions it, and I added the condition at this place because I didn't\nwant to duplicate the construction of the error message.\n\nHowever, if we go for a more informative error message, then wouldn't\nit be better to add this condition in populate_value() before it even\ncalls get_object()?  Then we could also add the problematic format\nspecifier to the error message (I think, but didn't actually check),\njust in case someone specified multiple sort keys.\n\n\n"},{"id":"358795","messageId":"20180924213000.GA7047@sigill.intra.peff.net","threadId":"49403","inReplyTo":"20180924212034.GF27036@localhost","subject":"Re: [PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-09-24T21:30:01Z","receivedAt":"2018-09-24T21:30:04Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Sep 24, 2018 at 11:20:34PM +0200, SZEDER Gábor wrote:\n\n> > Would we perhaps want to give the user a hint that the object is not\n> > really missing, but rather that we're not in a repository? E.g.,\n> > something like:\n> > \n> >   if (!have_git_dir())\n> > \treturn strbuf_addf_ret(err, -1, \"format specifier requires a repository\");\n> >   if (oid_object_info_extended(...))\n> > \treturn ...;\n> > \n> > ?\n> \n> I think it makes sense.\n> \n> I wanted to preserve the error message, because the description of\n> '--sort=<key>' in 'Documentation/git-ls-remote.txt' explicitly\n> mentions it, and I added the condition at this place because I didn't\n> want to duplicate the construction of the error message.\n\nAh, I didn't realize we actually documented that. And perhaps it is more\nconsistent, too: you'd get different results from running \"ls-remote\"\noutside a repository versus one that just doesn't have the objects from\nthe other side.\n\n> However, if we go for a more informative error message, then wouldn't\n> it be better to add this condition in populate_value() before it even\n> calls get_object()?  Then we could also add the problematic format\n> specifier to the error message (I think, but didn't actually check),\n> just in case someone specified multiple sort keys.\n\nYeah, that probably would be a better place. Though your response also\nhas made me think that maybe just sticking with the \"missing object\"\nresponse is reasonable. I don't have a strong opinion between the two.\n\n-Peff\n"},{"id":"358871","messageId":"xmqq5zytpa65.fsf@gitster-ct.c.googlers.com","threadId":"49403","inReplyTo":"20180924212034.GF27036@localhost","subject":"Re: [PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-09-25T20:57:38Z","receivedAt":"2018-09-25T20:57:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> However, if we go for a more informative error message, then wouldn't\n> it be better to add this condition in populate_value() before it even\n> calls get_object()?  Then we could also add the problematic format\n> specifier to the error message (I think, but didn't actually check),\n> just in case someone specified multiple sort keys.\n\nEven though I suspect that verify_ref_format() is the logically the\nright place to do this (after all, it is about seeing if the format\nmakes sense, and a format that requires an object access used\noutside a repository should trigger an verification error), doing\nthat in populate_value() probably strikes the best balance, I would\nthink.\n\n"},{"id":"363350","messageId":"20181114122725.18659-1-szeder.dev@gmail.com","threadId":"49403","inReplyTo":"xmqq5zytpa65.fsf@gitster-ct.c.googlers.com","subject":"[PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-14T12:27:25Z","receivedAt":"2018-11-14T12:28:25Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"The command 'git ls-remote --sort=authordate <remote>' segfaults when\nrun outside of a repository, ever since the introduction of its\n'--sort' option in 1fb20dfd8e (ls-remote: create '--sort' option,\n2018-04-09).\n\nWhile in general the 'git ls-remote' command can be run outside of a\nrepository just fine, its '--sort=<key>' option with certain keys does\nrequire access to the referenced objects.  This sorting is implemented\nusing the generic ref-filter sorting facility, which already handles\nmissing objects gracefully with the appropriate 'missing object\ndeadbeef for HEAD' message.  However, being generic means that it\nchecks replace refs while trying to retrieve an object, and while\ndoing so it accesses the 'git_replace_ref_base' variable, which has\nnot been initialized and is still a NULL pointer when outside of a\nrepository, thus causing the segfault.\n\nMake ref-filter more careful upfront while parsing the format string,\nand make it error out when encountering a format atom requiring object\naccess when we are not in a repository.  Also add a test to ensure\nthat 'git ls-remote --sort' fails gracefully when executed outside of\na repository.\n\nReported-by: H.Merijn Brand <h.m.brand@xs4all.nl>\nSigned-off-by: SZEDER Gábor <szeder.dev@gmail.com>\n---\n\nOn Tue, Sep 25, 2018 at 01:57:38PM -0700, Junio C Hamano wrote:\n> SZEDER Gábor <szeder.dev@gmail.com> writes:\n> \n> > However, if we go for a more informative error message, then wouldn't\n> > it be better to add this condition in populate_value() before it even\n> > calls get_object()?  Then we could also add the problematic format\n> > specifier to the error message (I think, but didn't actually check),\n> > just in case someone specified multiple sort keys.\n> \n> Even though I suspect that verify_ref_format() is the logically the\n> right place to do this (after all, it is about seeing if the format\n> makes sense, and a format that requires an object access used\n> outside a repository should trigger an verification error), doing\n> that in populate_value() probably strikes the best balance, I would\n> think.\n\nWe are dealing with format specifiers used for sorting here, and those\ndon't go through verify_ref_format().\n\nSo how about this patch instead?\n\nI think it will catch all cases where a user would try to use a format\nspecifier, for any purpose, requiring object access outside of a\nrepository (though I don't know whether there are any other cases\nbesides 'git ls-remote --sort=...'; but perhaps in the future\n'ls-remote' will get a '--format' option as well), and it does so\nbefore performing a potentially expensive query to the remote.  OTOH,\nit won't change the documented \"missing object\" error message when run\ninside a repo but the necessary object is indeed missing.\n\n\n ref-filter.c         | 4 ++++\n t/t5512-ls-remote.sh | 6 ++++++\n 2 files changed, 10 insertions(+)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 0c45ed9d94..a1290659af 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -534,6 +534,10 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n \tif (ARRAY_SIZE(valid_atom) <= i)\n \t\treturn strbuf_addf_ret(err, -1, _(\"unknown field name: %.*s\"),\n \t\t\t\t       (int)(ep-atom), atom);\n+\tif (valid_atom[i].source != SOURCE_NONE && !have_git_dir())\n+\t\treturn strbuf_addf_ret(err, -1,\n+\t\t\t\t       _(\"not a git repository, but the field '%.*s' requires access to object data\"),\n+\t\t\t\t       (int)(ep-atom), atom);\n \n \t/* Add it in, including the deref prefix */\n \tat = used_atom_cnt;\ndiff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\nindex 91ee6841c1..32e722db2e 100755\n--- a/t/t5512-ls-remote.sh\n+++ b/t/t5512-ls-remote.sh\n@@ -302,6 +302,12 @@ test_expect_success 'ls-remote works outside repository' '\n \tnongit git ls-remote dst.git\n '\n \n+test_expect_success 'ls-remote --sort fails gracefully outside repository' '\n+\t# Use a sort key that requires access to the referenced objects.\n+\tnongit test_must_fail git ls-remote --sort=authordate \"$TRASH_DIRECTORY\" 2>err &&\n+\ttest_i18ngrep \"^fatal: not a git repository, but the field '\\''authordate'\\'' requires access to object data\" err\n+'\n+\n test_expect_success 'ls-remote patterns work with all protocol versions' '\n \tgit for-each-ref --format=\"%(objectname)\t%(refname)\" \\\n \t\trefs/heads/master refs/remotes/origin/master >expect &&\n-- \n2.19.1.1182.gbfcc7ed3e6\n\n"},{"id":"363416","messageId":"20181115093844.GA14218@sigill.intra.peff.net","threadId":"49403","inReplyTo":"20181114122725.18659-1-szeder.dev@gmail.com","subject":"Re: [PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-15T09:38:44Z","receivedAt":"2018-11-15T09:38:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 14, 2018 at 01:27:25PM +0100, SZEDER Gábor wrote:\n\n> The command 'git ls-remote --sort=authordate <remote>' segfaults when\n> run outside of a repository, ever since the introduction of its\n> '--sort' option in 1fb20dfd8e (ls-remote: create '--sort' option,\n> 2018-04-09).\n> \n> While in general the 'git ls-remote' command can be run outside of a\n> repository just fine, its '--sort=<key>' option with certain keys does\n> require access to the referenced objects.  This sorting is implemented\n> using the generic ref-filter sorting facility, which already handles\n> missing objects gracefully with the appropriate 'missing object\n> deadbeef for HEAD' message.  However, being generic means that it\n> checks replace refs while trying to retrieve an object, and while\n> doing so it accesses the 'git_replace_ref_base' variable, which has\n> not been initialized and is still a NULL pointer when outside of a\n> repository, thus causing the segfault.\n> \n> Make ref-filter more careful upfront while parsing the format string,\n> and make it error out when encountering a format atom requiring object\n> access when we are not in a repository.  Also add a test to ensure\n> that 'git ls-remote --sort' fails gracefully when executed outside of\n> a repository.\n\nThanks for picking up this loose end. I like the general approach here,\nbut...\n\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 0c45ed9d94..a1290659af 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -534,6 +534,10 @@ static int parse_ref_filter_atom(const struct ref_format *format,\n>  \tif (ARRAY_SIZE(valid_atom) <= i)\n>  \t\treturn strbuf_addf_ret(err, -1, _(\"unknown field name: %.*s\"),\n>  \t\t\t\t       (int)(ep-atom), atom);\n> +\tif (valid_atom[i].source != SOURCE_NONE && !have_git_dir())\n> +\t\treturn strbuf_addf_ret(err, -1,\n> +\t\t\t\t       _(\"not a git repository, but the field '%.*s' requires access to object data\"),\n> +\t\t\t\t       (int)(ep-atom), atom);\n\nIs SOURCE_NONE a complete match for what we want?\n\nI see problems in both directions:\n\n - sorting by \"objectname\" works now, but it's marked with SOURCE_OBJ,\n   and would be forbidden with your patch.  I'm actually not sure if\n   SOURCE_OBJ is accurate; we shouldn't need to access the object to\n   show it (and we are probably wasting effort loading the full contents\n   for tools like for-each-ref).\n\n   However, that's not the full story. For objectname:short, it _does_ call\n   find_unique_abbrev(). So we expect to have an object directory.\n\n - sorting by \"HEAD\" hits a BUG(), and would still be allowed with your\n   patch.\n\nSo I like the idea here that the particular atoms would tell us whether\nthey're going to need to be in a repository or not, but I think the\nannotations have to be cleaned up first.\n\n> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh\n> index 91ee6841c1..32e722db2e 100755\n> --- a/t/t5512-ls-remote.sh\n> +++ b/t/t5512-ls-remote.sh\n> @@ -302,6 +302,12 @@ test_expect_success 'ls-remote works outside repository' '\n>  \tnongit git ls-remote dst.git\n>  '\n>  \n> +test_expect_success 'ls-remote --sort fails gracefully outside repository' '\n> +\t# Use a sort key that requires access to the referenced objects.\n> +\tnongit test_must_fail git ls-remote --sort=authordate \"$TRASH_DIRECTORY\" 2>err &&\n> +\ttest_i18ngrep \"^fatal: not a git repository, but the field '\\''authordate'\\'' requires access to object data\" err\n> +'\n\nRegardless of our solution, we probably want to add an extra test making\nsure that something vanilla like:\n\n  nongit git ls-remote --sort=v:refname \"$TRASH_DIRECTORY\"\n\ncontinues to work (we do test ls-remote outside a repo already, but not\nwith a sort specifier).\n\n-Peff\n"},{"id":"363418","messageId":"20181115094320.GA18790@sigill.intra.peff.net","threadId":"49403","inReplyTo":"20181115093844.GA14218@sigill.intra.peff.net","subject":"Re: [PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-15T09:43:20Z","receivedAt":"2018-11-15T09:43:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 15, 2018 at 04:38:44AM -0500, Jeff King wrote:\n\n> Is SOURCE_NONE a complete match for what we want?\n> \n> I see problems in both directions:\n> \n>  - sorting by \"objectname\" works now, but it's marked with SOURCE_OBJ,\n>    and would be forbidden with your patch.  I'm actually not sure if\n>    SOURCE_OBJ is accurate; we shouldn't need to access the object to\n>    show it (and we are probably wasting effort loading the full contents\n>    for tools like for-each-ref).\n> \n>    However, that's not the full story. For objectname:short, it _does_ call\n>    find_unique_abbrev(). So we expect to have an object directory.\n\nOops, I'm apparently bad at reading. It is in fact SOURCE_OTHER, which\nmakes sense (outside of this whole \"--sort outside a repo thing\").\n\nBut we'd ideally distinguish between \"objectname\" (which should be OK\noutside a repo) and \"objectname:short\" (which currently segfaults).\n\n-Peff\n"},{"id":"363465","messageId":"xmqq36s1libw.fsf@gitster-ct.c.googlers.com","threadId":"49403","inReplyTo":"20181115094320.GA18790@sigill.intra.peff.net","subject":"Re: [PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-16T05:09:07Z","receivedAt":"2018-11-16T05:09:11Z","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> On Thu, Nov 15, 2018 at 04:38:44AM -0500, Jeff King wrote:\n>\n>> Is SOURCE_NONE a complete match for what we want?\n>> \n>> I see problems in both directions:\n>> \n>>  - sorting by \"objectname\" works now, but it's marked with SOURCE_OBJ,\n>>    and would be forbidden with your patch.  I'm actually not sure if\n>>    SOURCE_OBJ is accurate; we shouldn't need to access the object to\n>>    show it (and we are probably wasting effort loading the full contents\n>>    for tools like for-each-ref).\n>> \n>>    However, that's not the full story. For objectname:short, it _does_ call\n>>    find_unique_abbrev(). So we expect to have an object directory.\n>\n> Oops, I'm apparently bad at reading. It is in fact SOURCE_OTHER, which\n> makes sense (outside of this whole \"--sort outside a repo thing\").\n>\n> But we'd ideally distinguish between \"objectname\" (which should be OK\n> outside a repo) and \"objectname:short\" (which currently segfaults).\n\nArguably, use of ref-filter machinery in ls-remote, whether it is\ngiven from inside or outside a repo, was a mistake in 1fb20dfd\n(\"ls-remote: create '--sort' option\", 2018-04-09), as the whole\npoint of \"ls-remote\" is to peek the list of refs and it is perfectly\nnormal that the objects listed are not available.\n\n\"ls-remote --sort=authorname\" that is run in a repository may not\nsegfault on a ref that points at a yet-to-be-fetched commit, but it\ncannot be doing anything sensible.  Is it still better to silently\nproduce a nonsense result than refusing to --sort no matter what the\nsort keys are, whether we are inside or outside a repository?\n\n"},{"id":"363488","messageId":"20181116085602.GB20828@sigill.intra.peff.net","threadId":"49403","inReplyTo":"xmqq36s1libw.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-11-16T08:56:03Z","receivedAt":"2018-11-16T08:56:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Nov 16, 2018 at 02:09:07PM +0900, Junio C Hamano wrote:\n\n> >> I see problems in both directions:\n> >> \n> >>  - sorting by \"objectname\" works now, but it's marked with SOURCE_OBJ,\n> >>    and would be forbidden with your patch.  I'm actually not sure if\n> >>    SOURCE_OBJ is accurate; we shouldn't need to access the object to\n> >>    show it (and we are probably wasting effort loading the full contents\n> >>    for tools like for-each-ref).\n> >> \n> >>    However, that's not the full story. For objectname:short, it _does_ call\n> >>    find_unique_abbrev(). So we expect to have an object directory.\n> >\n> > Oops, I'm apparently bad at reading. It is in fact SOURCE_OTHER, which\n> > makes sense (outside of this whole \"--sort outside a repo thing\").\n> >\n> > But we'd ideally distinguish between \"objectname\" (which should be OK\n> > outside a repo) and \"objectname:short\" (which currently segfaults).\n> \n> Arguably, use of ref-filter machinery in ls-remote, whether it is\n> given from inside or outside a repo, was a mistake in 1fb20dfd\n> (\"ls-remote: create '--sort' option\", 2018-04-09), as the whole\n> point of \"ls-remote\" is to peek the list of refs and it is perfectly\n> normal that the objects listed are not available.\n\nI think it's conceptually reasonable to use the ref-filter machinery.\nIt's just that it was underprepared to handle this out-of-repo case. I\nthink we're not too far off, though.\n\n> \"ls-remote --sort=authorname\" that is run in a repository may not\n> segfault on a ref that points at a yet-to-be-fetched commit, but it\n> cannot be doing anything sensible.  Is it still better to silently\n> produce a nonsense result than refusing to --sort no matter what the\n> sort keys are, whether we are inside or outside a repository?\n\nI don't think we produce silent nonsense in the current code (or after\nany of the discussed solutions), either in a repo or out. We say \"fatal:\nmissing object ...\" inside a repo if the request cannot be fulfilled.\nThat's not incredibly illuminating, perhaps, but it means we fulfill\nwhatever we _can_ on behalf of the user's request, and bail otherwise.\n\nIf you are arguing that even in a repo we should reject \"authorname\"\nearly (just as we would outside of a repo), I could buy that.\nTechnically we can make it work sometimes (if we happen to have fetched\neverything the other side has), but behaving consistently (and with a\ndecent error message) may trump that.\n\n-Peff\n"},{"id":"363493","messageId":"xmqqwopdibeg.fsf@gitster-ct.c.googlers.com","threadId":"49403","inReplyTo":"20181116085602.GB20828@sigill.intra.peff.net","subject":"Re: [PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-11-16T10:07:03Z","receivedAt":"2018-11-16T10:07:08Z","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> If you are arguing that even in a repo we should reject \"authorname\"\n> early (just as we would outside of a repo), I could buy that.\n\nYup, that (and replace 'authorname' with anything that won't work\nwith missing objects) for consistency was what I meant.\n\n"},{"id":"363512","messageId":"20181116131644.GM30222@szeder.dev","threadId":"49403","inReplyTo":"xmqq36s1libw.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH] ref-filter: don't look for objects when outside of a repository","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2018-11-16T13:16:44Z","receivedAt":"2018-11-16T13:16:50Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Fri, Nov 16, 2018 at 02:09:07PM +0900, Junio C Hamano wrote:\n> Jeff King <peff@peff.net> writes:\n> \n> > On Thu, Nov 15, 2018 at 04:38:44AM -0500, Jeff King wrote:\n> >\n> >> Is SOURCE_NONE a complete match for what we want?\n> >> \n> >> I see problems in both directions:\n> >> \n> >>  - sorting by \"objectname\" works now, but it's marked with SOURCE_OBJ,\n> >>    and would be forbidden with your patch.  I'm actually not sure if\n> >>    SOURCE_OBJ is accurate; we shouldn't need to access the object to\n> >>    show it (and we are probably wasting effort loading the full contents\n> >>    for tools like for-each-ref).\n> >> \n> >>    However, that's not the full story. For objectname:short, it _does_ call\n> >>    find_unique_abbrev(). So we expect to have an object directory.\n> >\n> > Oops, I'm apparently bad at reading. It is in fact SOURCE_OTHER, which\n> > makes sense (outside of this whole \"--sort outside a repo thing\").\n> >\n> > But we'd ideally distinguish between \"objectname\" (which should be OK\n> > outside a repo) and \"objectname:short\" (which currently segfaults).\n> \n> Arguably, use of ref-filter machinery in ls-remote, whether it is\n> given from inside or outside a repo, was a mistake in 1fb20dfd\n> (\"ls-remote: create '--sort' option\", 2018-04-09), as the whole\n> point of \"ls-remote\" is to peek the list of refs and it is perfectly\n> normal that the objects listed are not available.\n\nI hope that one day 'git ls-remote' will learn to '--format=...' its\noutput, and I think that (re)using the ref-filter machinery would be\nthe right way to go to achive that.  Sure, ref-filter supports a lot\nof format specifiers that don't at all make sense in the context of\n'ls-remote' (perhaps we should have a dedicated set of valid_atoms for\nthat), but I think it's perfectly reasonable to do something like:\n\n  git ls-remote --format=%(refname:strip=2) remote\n\nA concrete use case for that could be to eliminate the last remaining\nshell loops from refs completion.\n\n"}]}