git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v12 4/4] ls-remote: create '--sort' option

From
Harald Nordgren <haraldnordgren@gmail.com>
Date
Apr 9, 2018, 00:09 UTC
Message-ID
<CAHwyqnWe=KmmdGJJ_DnCK+D_FoC0sAQG_8=_p+4+Yxmr8s8sPA@mail.gmail.com>
In-Reply-To
<xmqq7eph9wds.fsf@gitster-ct.c.googlers.com>
On Mon, Apr 9, 2018 at 12:16 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 25 quoted lines
>
> I can connect "because it deals only with remotes" and "access to
> the object may fail", but I wonder if we can clarify the former a
> bit better to make it easier to understand for those who are not yet
> familiar with Git.
>
>     Keys like `committerdate` that require access to the object will
>     not work [***HOW??? Do we get a segfault or something???***] for
>     refs whose objects have not yet been fetched from the remote.
>
> I added "for refs whose objects have not yet been fetched", whose
> explicit-ness made "will not work" to be an OK expression, but
> without it I would have suggested to rephrase it to "may not work".
>
> This is a tangent but I suspect that the description of --sort in
> git-for-each-ref documentation is wrong on the use of multiple
> options, or I am misreading parse_opt_ref_sorting(), which I think
> appends later keys to the end of the list, and compare_refs() which
> I think yields results when an earlier key in the last decides two
> are not equal and ignores the later keys.  The commit that added the
> description was c0f6dc9b ("git-for-each-ref.txt: minor
> improvements", 2008-06-05), and I think even back then the code in
> builtin-for-each-ref.c appended later keys at the end, and it seems
> nobody complained, so it is possible I am not reading the code right
> before fully adjusting to timezone change ;-)
Updated the docs.
Show 8 quoted lines
>
> It is not too big a deal, but I find that liberal sprinkling of
> UNLEAK() is somewhat unsightly.  From the code we leave with this
> change, it is clear what we'd need to do when we make the code fully
> leak-proof (i.e. we'd have a several lines to free resources held by
> these two variables here, and then UNLEAK() that appear earlier in
> the function will become a "goto cleanup;" after setting appropriate
> value to "status"), so let's not worry about it.

Removed the "extra" unleak annotations within the if-return blocks, but kept them at the end.

Show 19 quoted lines
> Please do *NOT* step outside the pair of single quotes that protects
> the body of the test scriptlet and execute commands like "git
> rev-parse".  These execute in order to prepare the command line
> argument strings BEFORE test_expect_success can run (or the test
> framework decides if the test will be skipped via GIT_TEST_SKIP).
>
> Instead do it like so:
>
>         cat >expect <<-EOF &&
>         $(git rev-parse mark)   refs/tags/mark
>         $(git rev-parse mark1.1)        refs/tags/mark1.1
>         $(git rev-parse mark1.2)        refs/tags/mark1.2
>         $(git rev-parse mark1.10)       refs/tags/mark1.10
>         EOF
>
> I.e. the end token for << that is not quoted allows $var and $(cmd)
> to be substituted.
>
> Same comment applies throughout the remainder of this file.

Ah, thanks! I was looking for some way to allow the values to expand within the proper test context.

So turns out '<<-\EOF' vs '<<-EOF' makes all the difference :)
> Other than that, this patch was a very pleasant read.  Thanks.
Thank you!
Previous: Junio C HamanoNext: Junio C Hamano
Message 42 of 62 in “ls-remote: create option to sort by versions”
  1. ls-remote: create option to sort by versionsHarald Nordgren, Apr 2, 2018
  2. Ævar Arnfjörð BjarmasonApr 2, 2018
  3. Harald NordgrenApr 2, 2018
  4. Ævar Arnfjörð BjarmasonApr 2, 2018
  5. Harald NordgrenApr 2, 2018
  6. Jeff KingApr 2, 2018
  7. Junio C HamanoApr 2, 2018
  8. Harald NordgrenApr 2, 2018
  9. ls-remote: create '--sort' optionHarald Nordgren, Apr 2, 2018
  10. Eric SunshineApr 2, 2018
  11. Eric SunshineApr 2, 2018
  12. ls-remote: create '--sort' optionHarald Nordgren, Apr 3, 2018
  13. ls-remote: create '--sort' optionHarald Nordgren, Apr 2, 2018
  14. ls-remote: create '--sort' optionHarald Nordgren, Apr 4, 2018
  15. Harald NordgrenApr 4, 2018
  16. Harald NordgrenApr 4, 2018
  17. Jeff KingApr 4, 2018
  18. Jeff KingApr 4, 2018
  19. ls-remote: create '--sort' optionHarald Nordgren, Apr 4, 2018
  20. Harald NordgrenApr 4, 2018
  21. Jeff KingApr 6, 2018
  22. 1/3 ref-filter: use "struct object_id" consistentlyJeff King, Apr 6, 2018
  23. 2/3 ref-filter: make ref_array_item allocation more consistentJeff King, Apr 6, 2018
  24. 3/3 ref-filter: factor ref_array pushing into its own functionJeff King, Apr 6, 2018
  25. Derrick StoleeApr 6, 2018
  26. Harald NordgrenApr 7, 2018
  27. Junio C HamanoApr 8, 2018
  28. Jeff KingApr 9, 2018
  29. ls-remote: create '--sort' optionHarald Nordgren, Apr 4, 2018
  30. ls-remote: create '--sort' optionHarald Nordgren, Apr 5, 2018
  31. 1/4 ref-filter: use "struct object_id" consistentlyHarald Nordgren, Apr 7, 2018
  32. 2/4 ref-filter: make ref_array_item allocation more consistentHarald Nordgren, Apr 7, 2018
  33. 3/4 ref-filter: factor ref_array pushing into its own functionHarald Nordgren, Apr 7, 2018
  34. 4/4 ls-remote: create '--sort' optionHarald Nordgren, Apr 7, 2018
  35. Eric SunshineApr 8, 2018
  36. Eric SunshineApr 8, 2018
  37. Harald NordgrenApr 8, 2018
  38. 1/4 ref-filter: use "struct object_id" consistentlyHarald Nordgren, Apr 8, 2018
  39. 3/4 ref-filter: factor ref_array pushing into its own functionHarald Nordgren, Apr 8, 2018
  40. 4/4 ls-remote: create '--sort' optionHarald Nordgren, Apr 8, 2018
  41. Junio C HamanoApr 8, 2018
  42. Harald NordgrenApr 9, 2018
  43. Junio C HamanoApr 9, 2018
  44. Eric SunshineApr 9, 2018
  45. 2/4 ref-filter: make ref_array_item allocation more consistentHarald Nordgren, Apr 8, 2018
  46. 1/4 ref-filter: use "struct object_id" consistentlyHarald Nordgren, Apr 8, 2018
  47. 2/4 ref-filter: make ref_array_item allocation more consistentHarald Nordgren, Apr 8, 2018
  48. 4/4 ls-remote: create '--sort' optionHarald Nordgren, Apr 8, 2018
  49. Junio C HamanoApr 9, 2018
  50. Harald NordgrenApr 9, 2018
  51. 3/4 ref-filter: factor ref_array pushing into its own functionHarald Nordgren, Apr 8, 2018
  52. 1/4 ref-filter: use "struct object_id" consistentlyHarald Nordgren, Apr 9, 2018
  53. 2/4 ref-filter: make ref_array_item allocation more consistentHarald Nordgren, Apr 9, 2018
  54. Harald NordgrenApr 11, 2018
  55. Stefan BellerApr 11, 2018
  56. Todd ZullingerApr 11, 2018
  57. Eric SunshineApr 11, 2018
  58. Junio C HamanoApr 11, 2018
  59. 3/4 ref-filter: factor ref_array pushing into its own functionHarald Nordgren, Apr 9, 2018
  60. 4/4 ls-remote: create '--sort' optionHarald Nordgren, Apr 9, 2018
  61. René ScharfeMay 12, 2018
  62. Jeff KingMay 12, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.