[PATCH] ls-remote: create option to sort by versions
Create the options '-V ' and '--version-sort' to sort 'git ls-remote' output by version semantics. This is useful e.g. for the Go repository after the release of version 1.10, where otherwise v1.10 is sorted before v1.2. See: $ git ls-remote -t https://go.googlesource.com/go ... 205f850ceacfc39d1e9d76a9569416284594ce8crefs/tags/go1.1 d260448f6b6ac10efe4ae7f6dfe944e72bc2a676refs/tags/go1.1.1 1d6d8fca241bb611af51e265c1b5a2e9ae904702refs/tags/go1.1.2 bf86aec25972f3a100c3aa58a6abcbcc35bdea49refs/tags/go1.10 ac7c0ee26dda18076d5f6c151d8f920b43340ae3refs/tags/go1.10.1 9ce6b5c2ed5d3d5251b9a6a0c548d5fb2c8567e8refs/tags/go1.10beta1 594668a5a96267a46282ce3007a584ec07adf705refs/tags/go1.10beta2 5348aed83e39bd1d450d92d7f627e994c2db6ebfrefs/tags/go1.10rc1 20e228f2fdb44350c858de941dff4aea9f3127b8refs/tags/go1.10rc2 1c5438aae896edcd1e9f9618f4776517f08053b3refs/tags/go1.1rc2 46a6097aa7943a490e9bd2e04274845d0e5e200frefs/tags/go1.1rc3 402d3590b54e4a0df9fb51ed14b2999e85ce0b76refs/tags/go1.2 9c9802fad57c1bcb72ea98c5c55ea2652efc5772refs/tags/go1.2.1 ... Signed-off-by: Harald Nordgren --- builtin/ls-remote.c | 27 +-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c index 540d56429..740c6f117 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[] = { @@ -33,11 +34,20 @@ static int tail_match(const char **pattern, const char *path) return 0; } +static int cmp_ref_versions(const void *_a, const void *_b) +{ + const struct ref *a = *(const struct ref **)_a; + const struct ref *b = *(const struct ref **)_b; + + return versioncmp(a->name, b->name); +} + int cmd_ls_remote(int argc, const char **argv, const char *prefix) { const char *dest = NULL; unsigned flags = 0; int get_url = 0; + int version_sort = 0; int quiet = 0; int status = 0; int show_symref_target = 0; @@ -47,6 +57,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) struct remote *remote; struct transport *transport; const struct ref *ref; + const struct ref **refs = NULL; + int nr = 0; struct option options[] = { OPT__QUIET(&quiet, N_("do not print remote URL")), @@ -60,6 +72,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..insteadOf into account")), + OPT_BOOL('V', "version-sort", &version_sort, +N_("sort tags by version numbers")), OPT_SET_INT_F(0, "exit-code", &status, N_("exit with exit code 2 if no matching refs are found"), 2, PARSE_OPT_NOCOMPLETE), @@ -101,13 +115,22 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) if (transport_disconnect(transport)) return 1; - if (!dest && !quiet) - fprintf(stderr, "From %s\n", *remote->url); for ( ; ref; ref = ref->next) { if (!check_ref_type(ref, flags)) continue; if (!tail_match(pattern, ref->name)) continue; + REALLOC_ARRAY(refs, nr + 1); + refs[nr++] = ref; + } + + if (version_sort) + QSORT(refs, nr, cmp_ref_versions); + + if (!dest && !quiet) + fprintf(stderr, "From %s\n", *remote->url); + for (int i = 0; i < nr; i++) { + const struct ref *ref = refs[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); -- 2.14.3 (Apple Git-98)
Re: [PATCH] ls-remote: create option to sort by versions
On Mon, Apr 02 2018, Harald Nordgren wrote: > Create the options '-V ' and '--version-sort' to sort > 'git ls-remote' output by version semantics. This is useful e.g. for > the Go repository after the release of version 1.10, where otherwise > v1.10 is sorted before v1.2. See: > > $ git ls-remote -t https://go.googlesource.com/go > ... > 205f850ceacfc39d1e9d76a9569416284594ce8crefs/tags/go1.1 > d260448f6b6ac10efe4ae7f6dfe944e72bc2a676refs/tags/go1.1.1 > 1d6d8fca241bb611af51e265c1b5a2e9ae904702refs/tags/go1.1.2 > bf86aec25972f3a100c3aa58a6abcbcc35bdea49refs/tags/go1.10 > ac7c0ee26dda18076d5f6c151d8f920b43340ae3refs/tags/go1.10.1 > 9ce6b5c2ed5d3d5251b9a6a0c548d5fb2c8567e8refs/tags/go1.10beta1 > 594668a5a96267a46282ce3007a584ec07adf705refs/tags/go1.10beta2 > 5348aed83e39bd1d450d92d7f627e994c2db6ebfrefs/tags/go1.10rc1 > 20e228f2fdb44350c858de941dff4aea9f3127b8refs/tags/go1.10rc2 > 1c5438aae896edcd1e9f9618f4776517f08053b3refs/tags/go1.1rc2 > 46a6097aa7943a490e9bd2e04274845d0e5e200frefs/tags/go1.1rc3 > 402d3590b54e4a0df9fb51ed14b2999e85ce0b76refs/tags/go1.2 > 9c9802fad57c1bcb72ea98c5c55ea2652efc5772refs/tags/go1.2.1 > ... This is a sensible thing to want, but why not follow the UI we have for this with git-tag? I.e. --sort= & -i (or --ignore-case)? Of course ls-remote doesn't just show tags, so maybe we'd want --tag-sort= and --ignore-tag-case or something, but the rest should be equivalent, no? > [...] > @@ -101,13 +115,22 @@ int cmd_ls_remote(int argc, const char **argv, const > char *prefix) > if (transport_disconnect(transport)) > return 1; > > - if (!dest && !quiet) > - fprintf(stderr, "From %s\n", *remote->url); > for ( ; ref; ref = ref->next) { > if (!check_ref_type(ref, flags)) > continue; > if (!tail_match(pattern, ref->name)) > continue; > + REALLOC_ARRAY(refs, nr + 1); > + refs[nr++] = ref; > + } > + > + if (version_sort) > + QSORT(refs, nr, cmp_ref_versions); > + > + if (!dest && !quiet) > + fprintf(stderr, "From %s\n", *remote->url); Is there some subtlety here I'm missing which means that when sorting we'd now need to print this "From" line later (i.e. after sorting?
Re: [PATCH] ls-remote: create option to sort by versions
Thanks for your comment Ævar! In regards the the print statement, it was only moved down according to the diff because I added more logic above. Basically there is 1) the unrolling of the linked list to an array and 2) the printing logic. I could move it and make the diff smaller, but that probably makes the code a tiny bit more complicated. It would be nice to have a uniform option like '--sort=version:refname'. But spending a few hours to look over the code, it seems that ls-remote.c would require a lot of rewrites if we wanted to start using `ref_array` and `ref_array_item` for storing the refs. Which seems necessary in order to hook in to the sorting flow used in other subcommands. That, or reimplement `cmp_ref_sorting`. But maybe I'm missing something? On Mon, Apr 2, 2018 at 8:37 AM, Ævar Arnfjörð Bjarmason wrote: > > On Mon, Apr 02 2018, Harald Nordgren wrote: > >> Create the options '-V ' and '--version-sort' to sort >> 'git ls-remote' output by version semantics. This is useful e.g. for >> the Go repository after the release of version 1.10, where otherwise >> v1.10 is sorted before v1.2. See: >> >> $ git ls-remote -t https://go.googlesource.com/go >> ... >> 205f850ceacfc39d1e9d76a9569416284594ce8crefs/tags/go1.1 >> d260448f6b6ac10efe4ae7f6dfe944e72bc2a676refs/tags/go1.1.1 >> 1d6d8fca241bb611af51e265c1b5a2e9ae904702refs/tags/go1.1.2 >> bf86aec25972f3a100c3aa58a6abcbcc35bdea49refs/tags/go1.10 >> ac7c0ee26dda18076d5f6c151d8f920b43340ae3refs/tags/go1.10.1 >> 9ce6b5c2ed5d3d5251b9a6a0c548d5fb2c8567e8refs/tags/go1.10beta1 >> 594668a5a96267a46282ce3007a584ec07adf705refs/tags/go1.10beta2 >> 5348aed83e39bd1d450d92d7f627e994c2db6ebfrefs/tags/go1.10rc1 >> 20e228f2fdb44350c858de941dff4aea9f3127b8refs/tags/go1.10rc2 >> 1c5438aae896edcd1e9f9618f4776517f08053b3refs/tags/go1.1rc2 >> 46a6097aa7943a490e9bd2e04274845d0e5e200frefs/tags/go1.1rc3 >> 402d3590b54e4a0df9fb51ed14b2999e85ce0b76refs/tags/go1.2 >> 9c9802fad57c1bcb72ea98c5c55ea2652efc5772refs/tags/go1.2.1 >> ... > > This is a sensible thing to want, but why not follow the UI we have for > this with git-tag? I.e. --sort= & -i (or --ignore-case)? Of course > ls-remote doesn't just show tags, so maybe we'd want --tag-sort= > and --ignore-tag-case or something, but the rest should be equivalent, > no? > >> [...] >> @@ -101,13 +115,22 @@ int cmd_ls_remote(int argc, const char **argv, const >> char *prefix) >> if (transport_disconnect(transport)) >> return 1; >> >> - if (!dest && !quiet) >> - fprintf(stderr, "From %s\n", *remote->url); >> for ( ; ref; ref = ref->next) { >> if (!check_ref_type(ref, flags)) >> continue; >> if (!tail_match(pattern, ref->name)) >> continue; >> + REALLOC_ARRAY(refs, nr + 1); >> + refs[nr++] = ref; >> + } >> + >> + if (version_sort) >> + QSORT(refs, nr, cmp_ref_versions); >> + >> + if (!dest && !quiet) >> + fprintf(stderr, "From %s\n", *remote->url); > > Is there some subtlety here I'm missing which means that when sorting > we'd now need to print this "From" line later (i.e. after sorting?
Re: [PATCH] ls-remote: create option to sort by versions
On Mon, Apr 02 2018, Harald Nordgren wrote: > In regards the the print statement, it was only moved down according > to the diff because I added more logic above. Basically there is 1) > the unrolling of the linked list to an array and 2) the printing > logic. I could move it and make the diff smaller, but that probably > makes the code a tiny bit more complicated. I was just wondering since it wasn't explained in the commit message, makes sense to copy this explanation into v2, or lead with a purely code re-arrangement patch. > It would be nice to have a uniform option like > '--sort=version:refname'. But spending a few hours to look over the > code, it seems that ls-remote.c would require a lot of rewrites if we > wanted to start using `ref_array` and `ref_array_item` for storing the > refs. > > Which seems necessary in order to hook in to the sorting flow used in > other subcommands. That, or reimplement `cmp_ref_sorting`. But maybe > I'm missing something? I'm thinking just in terms of UI. If it's the case that porting this to whatever guts git-tag uses for sorting would be hard, then we could still use the same command-line option convention (and perhaps just die if anything except --sort=version:refname is supplied). Changing the underlying implementation is easier than cleaning up UI-differences that (seemingly) only arose due to underlying implementation details at the time. > On Mon, Apr 2, 2018 at 8:37 AM, Ævar Arnfjörð Bjarmason > wrote: >> >> On Mon, Apr 02 2018, Harald Nordgren wrote: >> >>> Create the options '-V ' and '--version-sort' to sort >>> 'git ls-remote' output by version semantics. This is useful e.g. for >>> the Go repository after the release of version 1.10, where otherwise >>> v1.10 is sorted before v1.2. See: >>> >>> $ git ls-remote -t https://go.googlesource.com/go >>> ... >>> 205f850ceacfc39d1e9d76a9569416284594ce8crefs/tags/go1.1 >>> d260448f6b6ac10efe4ae7f6dfe944e72bc2a676refs/tags/go1.1.1 >>> 1d6d8fca241bb611af51e265c1b5a2e9ae904702refs/tags/go1.1.2 >>> bf86aec25972f3a100c3aa58a6abcbcc35bdea49refs/tags/go1.10 >>> ac7c0ee26dda18076d5f6c151d8f920b43340ae3refs/tags/go1.10.1 >>> 9ce6b5c2ed5d3d5251b9a6a0c548d5fb2c8567e8refs/tags/go1.10beta1 >>> 594668a5a96267a46282ce3007a584ec07adf705refs/tags/go1.10beta2 >>> 5348aed83e39bd1d450d92d7f627e994c2db6ebfrefs/tags/go1.10rc1 >>> 20e228f2fdb44350c858de941dff4aea9f3127b8refs/tags/go1.10rc2 >>> 1c5438aae896edcd1e9f9618f4776517f08053b3refs/tags/go1.1rc2 >>> 46a6097aa7943a490e9bd2e04274845d0e5e200frefs/tags/go1.1rc3 >>> 402d3590b54e4a0df9fb51ed14b2999e85ce0b76refs/tags/go1.2 >>> 9c9802fad57c1bcb72ea98c5c55ea2652efc5772refs/tags/go1.2.1 >>> ... >> >> This is a sensible thing to want, but why not follow the UI we have for >> this with git-tag? I.e. --sort= & -i (or --ignore-case)? Of course >> ls-remote doesn't just show tags, so maybe we'd want --tag-sort= >> and --ignore-tag-case or something, but the rest should be equivalent, >> no? >> >>> [...] >>> @@ -101,13 +115,22 @@ int cmd_ls_remote(int argc, const char **argv, const >>> char *prefix) >>> if (transport_disconnect(transport)) >>> return 1; >>> >>> - if (!dest && !quiet) >>> - fprintf(stderr, "From %s\n", *remote->url); >>> for ( ; ref; ref = ref->next) { >>> if (!check_ref_type(ref, flags)) >>> continue; >>> if (!tail_match(pattern, ref->name)) >>> continue; >>> + REALLOC_ARRAY(refs, nr + 1); >>> + refs[nr++] = ref; >>> + } >>> + >>> + if (version_sort) >>> + QSORT(refs, nr, cmp_ref_versions); >>> + >>> + if (!dest && !quiet) >>> + fprintf(stderr, "From %s\n", *remote->url); >> >> Is there some subtlety here I'm missing which means that when sorting >> we'd now need to print this "From" line later (i.e. after sorting?
Re: [PATCH] ls-remote: create option to sort by versions
Both points make sense and it sounds like a very pragmatic approach. I'll look into it! On Mon, Apr 2, 2018 at 7:32 PM, Ævar Arnfjörð Bjarmason wrote: > > On Mon, Apr 02 2018, Harald Nordgren wrote: > >> In regards the the print statement, it was only moved down according >> to the diff because I added more logic above. Basically there is 1) >> the unrolling of the linked list to an array and 2) the printing >> logic. I could move it and make the diff smaller, but that probably >> makes the code a tiny bit more complicated. > > I was just wondering since it wasn't explained in the commit message, > makes sense to copy this explanation into v2, or lead with a purely code > re-arrangement patch. > >> It would be nice to have a uniform option like >> '--sort=version:refname'. But spending a few hours to look over the >> code, it seems that ls-remote.c would require a lot of rewrites if we >> wanted to start using `ref_array` and `ref_array_item` for storing the >> refs. >> >> Which seems necessary in order to hook in to the sorting flow used in >> other subcommands. That, or reimplement `cmp_ref_sorting`. But maybe >> I'm missing something? > > I'm thinking just in terms of UI. If it's the case that porting this to > whatever guts git-tag uses for sorting would be hard, then we could > still use the same command-line option convention (and perhaps just die > if anything except --sort=version:refname is supplied). Changing the > underlying implementation is easier than cleaning up UI-differences that > (seemingly) only arose due to underlying implementation details at the > time. > >> On Mon, Apr 2, 2018 at 8:37 AM, Ævar Arnfjörð Bjarmason >> wrote: >>> >>> On Mon, Apr 02 2018, Harald Nordgren wrote: >>> Create the options '-V ' and '--version-sort' to sort 'git ls-remote' output by version semantics. This is useful e.g. for the Go repository after the release of version 1.10, where otherwise v1.10 is sorted before v1.2. See: $ git ls-remote -t https://go.googlesource.com/go ... 205f850ceacfc39d1e9d76a9569416284594ce8crefs/tags/go1.1 d260448f6b6ac10efe4ae7f6dfe944e72bc2a676refs/tags/go1.1.1 1d6d8fca241bb611af51e265c1b5a2e9ae904702refs/tags/go1.1.2 bf86aec25972f3a100c3aa58a6abcbcc35bdea49refs/tags/go1.10 ac7c0ee26dda18076d5f6c151d8f920b43340ae3refs/tags/go1.10.1 9ce6b5c2ed5d3d5251b9a6a0c548d5fb2c8567e8refs/tags/go1.10beta1 594668a5a96267a46282ce3007a584ec07adf705refs/tags/go1.10beta2 5348aed83e39bd1d450d92d7f627e994c2db6ebfrefs/tags/go1.10rc1 20e228f2fdb44350c858de941dff4aea9f3127b8refs/tags/go1.10rc2 1c5438aae896edcd1e9f9618f4776517f08053b3refs/tags/go1.1rc2 46a6097aa7943a490e9bd2e04274845d0e5e200frefs/tags/go1.1rc3 402d3590b54e4a0df9fb51ed14b2999e85ce0b76refs/tags/go1.2 9c9802fad57c1bcb72ea98c5c55ea2652efc5772refs/tags/go1.2.1 ... >>> >>> This is a sensible thing to want, but why not follow the UI we have for >>> this with git-tag? I.e. --sort= & -i (or --ignore-case)? Of course >>> ls-remote doesn't just show tags, so maybe we'd want --tag-sort= >>> and --ignore-tag-case or something, but the rest should be equivalent, >>> no? >>> [...] @@ -101,13 +115,22 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix) if (transport_disconnect(transport)) return 1; - if (!dest && !quiet) - fprintf(stderr, "From %s\n", *remote->url); for ( ; ref; ref = ref->next) { if (!check_ref_type(ref, flags)) continue; if (!tail_match(pattern, ref->name)) continue; + REALLOC_ARRAY(refs, nr + 1); + refs[nr++] = ref; + } + + if (version_sort) + QSORT(refs, nr, cmp_ref_versions); + + if (!dest && !quiet) + fprintf(stderr, "From %s\n", *remote->url); >>> >>> Is there some subtlety here I'm missing which means that when sorting >>> we'd now need to print this "From" line later (i.e. after sorting?
Re: [PATCH] ls-remote: create option to sort by versions
On Mon, Apr 02, 2018 at 06:26:49PM +0200, Harald Nordgren wrote: > It would be nice to have a uniform option like > '--sort=version:refname'. But spending a few hours to look over the > code, it seems that ls-remote.c would require a lot of rewrites if we > wanted to start using `ref_array` and `ref_array_item` for storing the > refs. > > Which seems necessary in order to hook in to the sorting flow used in > other subcommands. That, or reimplement `cmp_ref_sorting`. But maybe > I'm missing something? I haven't looked at how painful it might be to use ref-filter.c, but it would buy us even more if we could. That would open up other options like --format, I think (OTOH there may be some funny corner cases; that code assumes we're talking about local refs, so if you were to ask for "%(committerdate)" or something, we might have to more cleanly handle the case where we don't actually have the object). -Peff
Re: [PATCH] ls-remote: create option to sort by versions
Ævar Arnfjörð Bjarmason writes: > This is a sensible thing to want, but why not follow the UI we have for > this with git-tag? I.e. --sort= & -i (or --ignore-case)? Of course > ls-remote doesn't just show tags, so maybe we'd want --tag-sort= > and --ignore-tag-case or something, but the rest should be equivalent, > no? Yeah, and if we can reuse more of ref-filter.c machinery (which was factored out of for-each-ref and tag you suggested borrows from), that would be even better. In the context of ls-remote, however, we cannot inspect the object (we typically do not have them yet), so it may not be practical, but I agree with your suggestion---we should match the behaviour at the UI level at least when we can.
Re: [PATCH] ls-remote: create option to sort by versions
Thanks for all the discussion! I think I figured out a way to reuse more ref-filter.c machinery. I will submit another patchset shortly. On Mon, Apr 2, 2018 at 8:32 PM, Junio C Hamano wrote: > Ævar Arnfjörð Bjarmason writes: > >> This is a sensible thing to want, but why not follow the UI we have for >> this with git-tag? I.e. --sort= & -i (or --ignore-case)? Of course >> ls-remote doesn't just show tags, so maybe we'd want --tag-sort= >> and --ignore-tag-case or something, but the rest should be equivalent, >> no? > > Yeah, and if we can reuse more of ref-filter.c machinery (which was > factored out of for-each-ref and tag you suggested borrows from), > that would be even better. In the context of ls-remote, however, we > cannot inspect the object (we typically do not have them yet), so it > may not be practical, but I agree with your suggestion---we should > match the behaviour at the UI level at least when we can. >