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
This commit is contained in:
Junio C Hamano 2017-01-23 15:59:21 -08:00
commit 1ac244d5b2
4 changed files with 220 additions and 57 deletions

View file

@ -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".
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.
+
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.
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.

View file

@ -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

View file

@ -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
'

View file

@ -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.
* 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.
*
* If both p1 and p2 start with (different) suffix, the order is
* determined by config file.
*
* 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)))];