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

Re: [PATCH v8] ls-remote: create '--sort' option

From
Harald Nordgren <haraldnordgren@gmail.com>
Date
Apr 4, 2018, 23:11 UTC
Message-ID
<CAHwyqnU4jv=wW_no4ff_E2TgvGtga1GOhZoDKd93J5MMUtLFCA@mail.gmail.com>
In-Reply-To
<20180404230110.9950-1-haraldnordgren@gmail.com>

Without digging to much into the `ref-filter` code itself, it seems like there is an opportunity to generalize and unfify the logic between these two cases. As well as using `ALLOC_GROW`. But maybe that is best left as a follow-up task? Especially since this patch focuses on `ls-remote`. Seems possibly like too big of a change to start changing a different sub-command.

Wouldn't a `ref_array_push()` also require `ref->symref`, maybe then we could pass the whole ref? It needs to be very clear that it's a `ref` and not a `ref_array_item` that is being pushed. Much of my logic here deals specifically with trying to treat a ref as ref_array_item.

From my viewpoint as implementer, I was very happy that I could implement the feature *without* invoking `filter_refs` since that `filter->kind` switching looks a pretty daunting. I'm not exactly sure what a `git ls-remote --contains HEAD` would do, maybe you could explain a bit more?

On Thu, Apr 5, 2018 at 1:01 AM, Harald Nordgren <haraldnordgren@gmail.com> wrote:

Show 199 quoted lines
> Create a '--sort' option for ls-remote, based on the one from
> for-each-ref. This e.g. allows ref names to be sorted by version
> semantics, so that v1.2 is sorted before v1.10.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>
> Notes:
>     Partial fixes from Jeff King's comments
>
>  Documentation/git-ls-remote.txt | 15 ++++++++++++++-
>  builtin/ls-remote.c             | 27 +++++++++++++++++++++++++--
>  t/t5512-ls-remote.sh            | 41 ++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 79 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-ls-remote.txt b/Documentation/git-ls-remote.txt
> index 5f2628c8f..fa4505fd7 100644
> --- a/Documentation/git-ls-remote.txt
> +++ b/Documentation/git-ls-remote.txt
> @@ -10,7 +10,7 @@ SYNOPSIS
>  --------
>  [verse]
>  'git ls-remote' [--heads] [--tags] [--refs] [--upload-pack=<exec>]
> -             [-q | --quiet] [--exit-code] [--get-url]
> +             [-q | --quiet] [--exit-code] [--get-url] [--sort=<key>]
>               [--symref] [<repository> [<refs>...]]
>
>  DESCRIPTION
> @@ -60,6 +60,19 @@ OPTIONS
>         upload-pack only shows the symref HEAD, so it will be the only
>         one shown by ls-remote.
>
> +--sort=<key>::
> +       Sort based on the key given.  Prefix `-` to sort in
> +       descending order of the value. You may use the --sort=<key> option
> +       multiple times, in which case the last key becomes the primary
> +       key. Also supports "version:refname" or "v:refname" (tag
> +       names are treated as versions). The "version:refname" sort
> +       order can also be affected by the "versionsort.suffix"
> +       configuration variable.
> +       The keys supported are the same as those in `git for-each-ref`,
> +       except that because `ls-remote` deals only with remotes, keys like
> +       `committerdate` that require access to the objects themselves will
> +       not work.
> +
>  <repository>::
>         The "remote" repository to query.  This parameter can be
>         either a URL or the name of a remote (see the GIT URLS and
> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index 540d56429..fbec2bc95 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c
> @@ -1,6 +1,7 @@
>  #include "builtin.h"
>  #include "cache.h"
>  #include "transport.h"
> +#include "ref-filter.h"
>  #include "remote.h"
>
>  static const char * const ls_remote_usage[] = {
> @@ -43,10 +44,13 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>         int show_symref_target = 0;
>         const char *uploadpack = NULL;
>         const char **pattern = NULL;
> +       int i;
>
>         struct remote *remote;
>         struct transport *transport;
>         const struct ref *ref;
> +       struct ref_array array;
> +       static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
>
>         struct option options[] = {
>                 OPT__QUIET(&quiet, N_("do not print remote URL")),
> @@ -60,6 +64,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>                 OPT_BIT(0, "refs", &flags, N_("do not show peeled tags"), REF_NORMAL),
>                 OPT_BOOL(0, "get-url", &get_url,
>                          N_("take url.<base>.insteadOf into account")),
> +               OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
> +                            N_("field name to sort on"), &parse_opt_ref_sorting),
>                 OPT_SET_INT_F(0, "exit-code", &status,
>                               N_("exit with exit code 2 if no matching refs are found"),
>                               2, PARSE_OPT_NOCOMPLETE),
> @@ -68,6 +74,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>                 OPT_END()
>         };
>
> +       memset(&array, 0, sizeof(array));
> +
>         argc = parse_options(argc, argv, prefix, options, ls_remote_usage,
>                              PARSE_OPT_STOP_AT_NON_OPTION);
>         dest = argv[0];
> @@ -104,13 +112,28 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
>         if (!dest && !quiet)
>                 fprintf(stderr, "From %s\n", *remote->url);
>         for ( ; ref; ref = ref->next) {
> +               struct ref_array_item *item;
>                 if (!check_ref_type(ref, flags))
>                         continue;
>                 if (!tail_match(pattern, ref->name))
>                         continue;
> +
> +               FLEX_ALLOC_STR(item, refname, ref->name);
> +               item->symref = xstrdup_or_null(ref->symref);
> +               oidcpy(&item->objectname, &ref->old_oid);
> +
> +               ALLOC_GROW(array.items, array.nr + 1, array.alloc);
> +               array.items[array.nr++] = item;
> +       }
> +
> +       if (sorting)
> +               ref_array_sort(sorting, &array);
> +
> +       for (i = 0; i < array.nr; i++) {
> +               const struct ref_array_item *ref = array.items[i];
>                 if (show_symref_target && ref->symref)
> -                       printf("ref: %s\t%s\n", ref->symref, ref->name);
> -               printf("%s\t%s\n", oid_to_hex(&ref->old_oid), ref->name);
> +                       printf("ref: %s\t%s\n", ref->symref, ref->refname);
> +               printf("%s\t%s\n", oid_to_hex(&ref->objectname), ref->refname);
>                 status = 0; /* we found something */
>         }
>         return status;
> diff --git a/t/t5512-ls-remote.sh b/t/t5512-ls-remote.sh
> index 02106c922..66370cd88 100755
> --- a/t/t5512-ls-remote.sh
> +++ b/t/t5512-ls-remote.sh
> @@ -10,6 +10,9 @@ test_expect_success setup '
>         test_tick &&
>         git commit -m initial &&
>         git tag mark &&
> +       git tag mark1.1 &&
> +       git tag mark1.2 &&
> +       git tag mark1.10 &&
>         git show-ref --tags -d | sed -e "s/ /   /" >expected.tag &&
>         (
>                 echo "$(git rev-parse HEAD)     HEAD"
> @@ -39,6 +42,39 @@ test_expect_success 'ls-remote self' '
>         test_cmp expected.all actual
>  '
>
> +test_expect_success 'ls-remote --sort="version:refname" --tags self' '
> +       cat >expect <<-\EOF &&
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark1.1
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark1.2
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark1.10
> +       EOF
> +       git ls-remote --sort="version:refname" --tags self >actual &&
> +       test_cmp expect actual
> +'
> +
> +test_expect_success 'ls-remote --sort="-version:refname" --tags self' '
> +       cat >expect <<-\EOF &&
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark1.10
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark1.2
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark1.1
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark
> +       EOF
> +       git ls-remote --sort="-version:refname" --tags self >actual &&
> +       test_cmp expect actual
> +'
> +
> +test_expect_success 'ls-remote --sort="-refname" --tags self' '
> +       cat >expect <<-\EOF &&
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark1.2
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark1.10
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark1.1
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark
> +       EOF
> +       git ls-remote --sort="-refname" --tags self >actual &&
> +       test_cmp expect actual
> +'
> +
>  test_expect_success 'dies when no remote specified and no default remotes found' '
>         test_must_fail git ls-remote
>  '
> @@ -131,7 +167,7 @@ test_expect_success 'Report no-match with --exit-code' '
>
>  test_expect_success 'Report match with --exit-code' '
>         git ls-remote --exit-code other.git "refs/tags/*" >actual &&
> -       git ls-remote . tags/mark >expect &&
> +       git ls-remote . tags/mark* >expect &&
>         test_cmp expect actual
>  '
>
> @@ -178,6 +214,9 @@ test_expect_success 'ls-remote --symref' '
>         1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/remotes/origin/HEAD
>         1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/remotes/origin/master
>         1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark1.1
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark1.10
> +       1bd44cb9d13204b0fe1958db0082f5028a16eb3a        refs/tags/mark1.2
>         EOF
>         git ls-remote --symref >actual &&
>         test_cmp expect actual
> --
> 2.14.3 (Apple Git-98)
>
Previous: Harald NordgrenNext: Jeff King
Message 20 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.