diff options
author | Junio C Hamano <gitster@pobox.com> | 2017-01-23 15:59:21 -0800 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2017-01-23 15:59:21 -0800 |
commit | 1ac244d5b20a9ea4a640e2ee80b9818f564aa237 (patch) | |
tree | 0b2be044f570e6ba9cdcd429aca5c62745ffd6db | |
parent | 8ec68d1ae2863823b74d67c5e92297e38bbf97bc (diff) | |
parent | c026557a37361b7019acca28f240a19f546739e9 (diff) | |
download | git-1ac244d5b20a9ea4a640e2ee80b9818f564aa237.tar.gz |
Merge branch 'sg/fix-versioncmp-with-common-suffix'
The prereleaseSuffix feature of version comparison that is used in
"git tag -l" did not correctly when two or more prereleases for the
same release were present (e.g. when 2.0, 2.0-beta1, and 2.0-beta2
are there and the code needs to compare 2.0-beta1 and 2.0-beta2).
* sg/fix-versioncmp-with-common-suffix:
versioncmp: generalize version sort suffix reordering
versioncmp: factor out helper for suffix matching
versioncmp: use earliest-longest contained suffix to determine sorting order
versioncmp: cope with common part overlapping with prerelease suffix
versioncmp: pass full tagnames to swap_prereleases()
t7004-tag: add version sort tests to show prerelease reordering issues
t7004-tag: use test_config helper
t7004-tag: delete unnecessary tags with test_when_finished
-rw-r--r-- | Documentation/config.txt | 44 | ||||
-rw-r--r-- | Documentation/git-tag.txt | 4 | ||||
-rwxr-xr-x | t/t7004-tag.sh | 132 | ||||
-rw-r--r-- | versioncmp.c | 99 |
4 files changed, 221 insertions, 58 deletions
diff --git a/Documentation/config.txt b/Documentation/config.txt index 506431267e..af2ae4cc02 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -3113,17 +3113,39 @@ user.signingKey:: This option is passed unchanged to gpg's --local-user parameter, so you may specify a key using any method that gpg supports. -versionsort.prereleaseSuffix:: - When version sort is used in linkgit:git-tag[1], prerelease - tags (e.g. "1.0-rc1") may appear after the main release - "1.0". By specifying the suffix "-rc" in this variable, - "1.0-rc1" will appear before "1.0". -+ -This variable can be specified multiple times, once per suffix. The -order of suffixes in the config file determines the sorting order -(e.g. if "-pre" appears before "-rc" in the config file then 1.0-preXX -is sorted before 1.0-rcXX). The sorting order between different -suffixes is undefined if they are in multiple config files. +versionsort.prereleaseSuffix (deprecated):: + Deprecated alias for `versionsort.suffix`. Ignored if + `versionsort.suffix` is set. + +versionsort.suffix:: + Even when version sort is used in linkgit:git-tag[1], tagnames + with the same base version but different suffixes are still sorted + lexicographically, resulting e.g. in prerelease tags appearing + after the main release (e.g. "1.0-rc1" after "1.0"). This + variable can be specified to determine the sorting order of tags + with different suffixes. ++ +By specifying a single suffix in this variable, any tagname containing +that suffix will appear before the corresponding main release. E.g. if +the variable is set to "-rc", then all "1.0-rcX" tags will appear before +"1.0". If specified multiple times, once per suffix, then the order of +suffixes in the configuration will determine the sorting order of tagnames +with those suffixes. E.g. if "-pre" appears before "-rc" in the +configuration, then all "1.0-preX" tags will be listed before any +"1.0-rcX" tags. The placement of the main release tag relative to tags +with various suffixes can be determined by specifying the empty suffix +among those other suffixes. E.g. if the suffixes "-rc", "", "-ck" and +"-bfs" appear in the configuration in this order, then all "v4.8-rcX" tags +are listed first, followed by "v4.8", then "v4.8-ckX" and finally +"v4.8-bfsX". ++ +If more than one suffixes match the same tagname, then that tagname will +be sorted according to the suffix which starts at the earliest position in +the tagname. If more than one different matching suffixes start at +that earliest position, then that tagname will be sorted according to the +longest of those suffixes. +The sorting order between different suffixes is undefined if they are +in multiple config files. web.browser:: Specify a web browser that may be used by some commands. diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 76cfe40d96..5055a96823 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -101,8 +101,8 @@ OPTIONS 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.prereleaseSuffix" configuration variable. + order can also be affected by the "versionsort.suffix" + configuration variable. The keys supported are the same as those in `git for-each-ref`. Sort order defaults to the value configured for the `tag.sort` variable if it exists, or lexicographic order otherwise. See diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index 07869b0c09..1cfa8a21d2 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -149,11 +149,11 @@ test_expect_success '--force can create a tag with the name of one existing' ' tag_exists mytag' test_expect_success '--force is moot with a non-existing tag name' ' + test_when_finished git tag -d newtag forcetag && git tag newtag >expect && git tag --force forcetag >actual && test_cmp expect actual ' -git tag -d newtag forcetag # deleting tags: @@ -324,11 +324,9 @@ EOF ' test_expect_success 'listing tags in column with column.*' ' - git config column.tag row && - git config column.ui dense && + test_config column.tag row && + test_config column.ui dense && COLUMNS=40 git tag -l >actual && - git config --unset column.ui && - git config --unset column.tag && cat >expected <<\EOF && a1 aa1 cba t210 t211 v0.2.1 v1.0 v1.0.1 v1.1.3 @@ -341,9 +339,8 @@ test_expect_success 'listing tag with -n --column should fail' ' ' test_expect_success 'listing tags -n in column with column.ui ignored' ' - git config column.ui "row dense" && + test_config column.ui "row dense" && COLUMNS=40 git tag -l -n >actual && - git config --unset column.ui && cat >expected <<\EOF && a1 Foo aa1 Foo @@ -1227,11 +1224,10 @@ test_expect_success GPG,RFC1991 \ ' # try to sign with bad user.signingkey -git config user.signingkey BobTheMouse test_expect_success GPG \ 'git tag -s fails if gpg is misconfigured (bad key)' \ - 'test_must_fail git tag -s -m tail tag-gpg-failure' -git config --unset user.signingkey + 'test_config user.signingkey BobTheMouse && + test_must_fail git tag -s -m tail tag-gpg-failure' # try to produce invalid signature test_expect_success GPG \ @@ -1511,7 +1507,7 @@ test_expect_success 'reverse lexical sort' ' ' test_expect_success 'configured lexical sort' ' - git config tag.sort "v:refname" && + test_config tag.sort "v:refname" && git tag -l "foo*" >actual && cat >expect <<-\EOF && foo1.3 @@ -1522,6 +1518,7 @@ test_expect_success 'configured lexical sort' ' ' test_expect_success 'option override configured sort' ' + test_config tag.sort "v:refname" && git tag -l --sort=-refname "foo*" >actual && cat >expect <<-\EOF && foo1.6 @@ -1536,13 +1533,12 @@ test_expect_success 'invalid sort parameter on command line' ' ' test_expect_success 'invalid sort parameter in configuratoin' ' - git config tag.sort "v:notvalid" && + test_config tag.sort "v:notvalid" && test_must_fail git tag -l "foo*" ' test_expect_success 'version sort with prerelease reordering' ' - git config --unset tag.sort && - git config versionsort.prereleaseSuffix -rc && + test_config versionsort.prereleaseSuffix -rc && git tag foo1.6-rc1 && git tag foo1.6-rc2 && git tag -l --sort=version:refname "foo*" >actual && @@ -1557,6 +1553,7 @@ test_expect_success 'version sort with prerelease reordering' ' ' test_expect_success 'reverse version sort with prerelease reordering' ' + test_config versionsort.prereleaseSuffix -rc && git tag -l --sort=-version:refname "foo*" >actual && cat >expect <<-\EOF && foo1.10 @@ -1568,6 +1565,103 @@ test_expect_success 'reverse version sort with prerelease reordering' ' test_cmp expect actual ' +test_expect_success 'version sort with prerelease reordering and common leading character' ' + test_config versionsort.prereleaseSuffix -before && + git tag foo1.7-before1 && + git tag foo1.7 && + git tag foo1.7-after1 && + git tag -l --sort=version:refname "foo1.7*" >actual && + cat >expect <<-\EOF && + foo1.7-before1 + foo1.7 + foo1.7-after1 + EOF + test_cmp expect actual +' + +test_expect_success 'version sort with prerelease reordering, multiple suffixes and common leading character' ' + test_config versionsort.prereleaseSuffix -before && + git config --add versionsort.prereleaseSuffix -after && + git tag -l --sort=version:refname "foo1.7*" >actual && + cat >expect <<-\EOF && + foo1.7-before1 + foo1.7-after1 + foo1.7 + EOF + test_cmp expect actual +' + +test_expect_success 'version sort with prerelease reordering, multiple suffixes match the same tag' ' + test_config versionsort.prereleaseSuffix -bar && + git config --add versionsort.prereleaseSuffix -foo-baz && + git config --add versionsort.prereleaseSuffix -foo-bar && + git tag foo1.8-foo-bar && + git tag foo1.8-foo-baz && + git tag foo1.8 && + git tag -l --sort=version:refname "foo1.8*" >actual && + cat >expect <<-\EOF && + foo1.8-foo-baz + foo1.8-foo-bar + foo1.8 + EOF + test_cmp expect actual +' + +test_expect_success 'version sort with prerelease reordering, multiple suffixes match starting at the same position' ' + test_config versionsort.prereleaseSuffix -pre && + git config --add versionsort.prereleaseSuffix -prerelease && + git tag foo1.9-pre1 && + git tag foo1.9-pre2 && + git tag foo1.9-prerelease1 && + git tag -l --sort=version:refname "foo1.9*" >actual && + cat >expect <<-\EOF && + foo1.9-pre1 + foo1.9-pre2 + foo1.9-prerelease1 + EOF + test_cmp expect actual +' + +test_expect_success 'version sort with general suffix reordering' ' + test_config versionsort.suffix -alpha && + git config --add versionsort.suffix -beta && + git config --add versionsort.suffix "" && + git config --add versionsort.suffix -gamma && + git config --add versionsort.suffix -delta && + git tag foo1.10-alpha && + git tag foo1.10-beta && + git tag foo1.10-gamma && + git tag foo1.10-delta && + git tag foo1.10-unlisted-suffix && + git tag -l --sort=version:refname "foo1.10*" >actual && + cat >expect <<-\EOF && + foo1.10-alpha + foo1.10-beta + foo1.10 + foo1.10-unlisted-suffix + foo1.10-gamma + foo1.10-delta + EOF + test_cmp expect actual +' + +test_expect_success 'versionsort.suffix overrides versionsort.prereleaseSuffix' ' + test_config versionsort.suffix -before && + test_config versionsort.prereleaseSuffix -after && + git tag -l --sort=version:refname "foo1.7*" >actual && + cat >expect <<-\EOF && + foo1.7-before1 + foo1.7 + foo1.7-after1 + EOF + test_cmp expect actual +' + +test_expect_success 'version sort with very long prerelease suffix' ' + test_config versionsort.prereleaseSuffix -very-looooooooooooooooooooooooong-prerelease-suffix && + git tag -l --sort=version:refname +' + run_with_limited_stack () { (ulimit -s 128 && "$@") } @@ -1596,13 +1690,11 @@ EOF" test_expect_success '--format should list tags as per format given' ' cat >expect <<-\EOF && - refname : refs/tags/foo1.10 - refname : refs/tags/foo1.3 - refname : refs/tags/foo1.6 - refname : refs/tags/foo1.6-rc1 - refname : refs/tags/foo1.6-rc2 + refname : refs/tags/v1.0 + refname : refs/tags/v1.0.1 + refname : refs/tags/v1.1.3 EOF - git tag -l --format="refname : %(refname)" "foo*" >actual && + git tag -l --format="refname : %(refname)" "v1*" >actual && test_cmp expect actual ' diff --git a/versioncmp.c b/versioncmp.c index 80bfd109fa..9f81dc1062 100644 --- a/versioncmp.c +++ b/versioncmp.c @@ -24,42 +24,83 @@ static const struct string_list *prereleases; static int initialized; +struct suffix_match { + int conf_pos; + int start; + int len; +}; + +static void find_better_matching_suffix(const char *tagname, const char *suffix, + int suffix_len, int start, int conf_pos, + struct suffix_match *match) +{ + /* + * A better match either starts earlier or starts at the same offset + * but is longer. + */ + int end = match->len < suffix_len ? match->start : match->start-1; + int i; + for (i = start; i <= end; i++) + if (starts_with(tagname + i, suffix)) { + match->conf_pos = conf_pos; + match->start = i; + match->len = suffix_len; + break; + } +} + /* - * p1 and p2 point to the first different character in two strings. If - * either p1 or p2 starts with a prerelease suffix, it will be forced - * to be on top. - * - * If both p1 and p2 start with (different) suffix, the order is - * determined by config file. + * off is the offset of the first different character in the two strings + * s1 and s2. If either s1 or s2 contains a prerelease suffix containing + * that offset or a suffix ends right before that offset, then that + * string will be forced to be on top. * - * Note that we don't have to deal with the situation when both p1 and - * p2 start with the same suffix because the common part is already - * consumed by the caller. + * If both s1 and s2 contain a (different) suffix around that position, + * their order is determined by the order of those two suffixes in the + * configuration. + * If any of the strings contains more than one different suffixes around + * that position, then that string is sorted according to the contained + * suffix which starts at the earliest offset in that string. + * If more than one different contained suffixes start at that earliest + * offset, then that string is sorted according to the longest of those + * suffixes. * * Return non-zero if *diff contains the return value for versioncmp() */ -static int swap_prereleases(const void *p1_, - const void *p2_, +static int swap_prereleases(const char *s1, + const char *s2, + int off, int *diff) { - const char *p1 = p1_; - const char *p2 = p2_; - int i, i1 = -1, i2 = -1; + int i; + struct suffix_match match1 = { -1, off, -1 }; + struct suffix_match match2 = { -1, off, -1 }; for (i = 0; i < prereleases->nr; i++) { const char *suffix = prereleases->items[i].string; - if (i1 == -1 && starts_with(p1, suffix)) - i1 = i; - if (i2 == -1 && starts_with(p2, suffix)) - i2 = i; + int start, suffix_len = strlen(suffix); + if (suffix_len < off) + start = off - suffix_len; + else + start = 0; + find_better_matching_suffix(s1, suffix, suffix_len, start, + i, &match1); + find_better_matching_suffix(s2, suffix, suffix_len, start, + i, &match2); } - if (i1 == -1 && i2 == -1) + if (match1.conf_pos == -1 && match2.conf_pos == -1) return 0; - if (i1 >= 0 && i2 >= 0) - *diff = i1 - i2; - else if (i1 >= 0) + if (match1.conf_pos == match2.conf_pos) + /* Found the same suffix in both, e.g. "-rc" in "v1.0-rcX" + * and "v1.0-rcY": the caller should decide based on "X" + * and "Y". */ + return 0; + + if (match1.conf_pos >= 0 && match2.conf_pos >= 0) + *diff = match1.conf_pos - match2.conf_pos; + else if (match1.conf_pos >= 0) *diff = -1; - else /* if (i2 >= 0) */ + else /* if (match2.conf_pos >= 0) */ *diff = 1; return 1; } @@ -118,10 +159,18 @@ int versioncmp(const char *s1, const char *s2) } if (!initialized) { + const struct string_list *deprecated_prereleases; initialized = 1; - prereleases = git_config_get_value_multi("versionsort.prereleasesuffix"); + prereleases = git_config_get_value_multi("versionsort.suffix"); + deprecated_prereleases = git_config_get_value_multi("versionsort.prereleasesuffix"); + if (prereleases) { + if (deprecated_prereleases) + warning("ignoring versionsort.prereleasesuffix because versionsort.suffix is set"); + } else + prereleases = deprecated_prereleases; } - if (prereleases && swap_prereleases(p1 - 1, p2 - 1, &diff)) + if (prereleases && swap_prereleases(s1, s2, (const char *) p1 - s1 - 1, + &diff)) return diff; state = result_type[state * 3 + (((c2 == '0') + (isdigit (c2) != 0)))]; |