@matheusbsbs / matheus-bagliano / commits / 51acfa9db5

versioncmp: use earliest-longest contained suffix to determine sorting order

When comparing tagnames, it is possible that a tagname contains more than one of the configured prerelease suffixes around the first different character. After fixing a bug in the previous commit such a tagname is sorted according to the contained suffix which comes first in the configuration. This is, however, not quite the right thing to do in the following corner cases: 1. $ git -c versionsort.suffix=-bar -c versionsort.suffix=-foo-baz -c versionsort.suffix=-foo-bar tag -l --sort=version:refname 'v1*' v1.0-foo-bar v1.0-foo-baz The suffix of the tagname 'v1.0-foo-bar' is clearly '-foo-bar', so it should be listed last. However, as it also contains '-bar' around the first different character, it is listed first instead, because that '-bar' suffix comes first the configuration. 2. One of the configured suffixes starts with the other: $ git -c versionsort.prereleasesuffix=-pre \ -c versionsort.prereleasesuffix=-prerelease \ tag -l --sort=version:refname 'v2*' v2.0-prerelease1 v2.0-pre1 v2.0-pre2 Here the tagname 'v2.0-prerelease1' should be the last. When comparing 'v2.0-pre1' and 'v2.0-prerelease1' the first different characters are '1' and 'r', respectively. Since this first different character must be part of the configured suffix, the '-pre' suffix is not recognized in the first tagname. OTOH, the '-prerelease' suffix is properly recognized in 'v2.0-prerelease1', thus it is listed first. Improve version sort in these corner cases, and - look for a configured prerelease suffix containing the first different character or ending right before it, so the '-pre' suffixes are recognized in case (2). This also means that when comparing tagnames 'v2.0-pre1' and 'v2.0-pre2', swap_prereleases() would find the '-pre' suffix in both, but then it will return "undecided" and the caller will do the right thing by sorting based in '1' and '2'. - If the tagname contains more than one suffix, then give precedence to the contained suffix that starts at the earliest offset in the tagname to address (1). - If there are more than one suffixes starting at that earliest position, then give precedence to the longest of those suffixes, thus ensuring that in (2) the tagname 'v2.0-prerelease1' won't be sorted based on the '-pre' suffix. Add tests for these corner cases and adjust the documentation accordingly. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

SZEDER Gábor committed Dec 8, 2016 at 15:24 UTC 51acfa9db588ddd8c3aff2acbc1a85ae7ac239f6
3 files changed +60 -10
Documentation/config.txt
+4 -2
@@ -3049,8 +3049,10 @@ order of suffixes in the config file determines the sorting order
3049 (e.g. if "-pre" appears before "-rc" in the config file then 1.0-preXX
3050 is sorted before 1.0-rcXX).
3051 If more than one suffixes match the same tagname, then that tagname will
3052 -be sorted according to the matching suffix which comes first in the
3053 -configuration.
3052 +be sorted according to the suffix which starts at the earliest position in
3053 +the tagname. If more than one different matching suffixes start at
3054 +that earliest position, then that tagname will be sorted according to the
3055 +longest of those suffixes.
3056 The sorting order between different suffixes is undefined if they are
3057 in multiple config files.
3058
t/t7004-tag.sh
+31
@@ -1564,6 +1564,37 @@ test_expect_success 'version sort with prerelease reordering, multiple suffixes
1564 test_cmp expect actual
1565 '
1566
1567 +test_expect_success 'version sort with prerelease reordering, multiple suffixes match the same tag' '
1568 + test_config versionsort.prereleaseSuffix -bar &&
1569 + git config --add versionsort.prereleaseSuffix -foo-baz &&
1570 + git config --add versionsort.prereleaseSuffix -foo-bar &&
1571 + git tag foo1.8-foo-bar &&
1572 + git tag foo1.8-foo-baz &&
1573 + git tag foo1.8 &&
1574 + git tag -l --sort=version:refname "foo1.8*" >actual &&
1575 + cat >expect <<-\EOF &&
1576 + foo1.8-foo-baz
1577 + foo1.8-foo-bar
1578 + foo1.8
1579 + EOF
1580 + test_cmp expect actual
1581 +'
1582 +
1583 +test_expect_success 'version sort with prerelease reordering, multiple suffixes match starting at the same position' '
1584 + test_config versionsort.prereleaseSuffix -pre &&
1585 + git config --add versionsort.prereleaseSuffix -prerelease &&
1586 + git tag foo1.9-pre1 &&
1587 + git tag foo1.9-pre2 &&
1588 + git tag foo1.9-prerelease1 &&
1589 + git tag -l --sort=version:refname "foo1.9*" >actual &&
1590 + cat >expect <<-\EOF &&
1591 + foo1.9-pre1
1592 + foo1.9-pre2
1593 + foo1.9-prerelease1
1594 + EOF
1595 + test_cmp expect actual
1596 +'
1597 +
1598 test_expect_success 'version sort with very long prerelease suffix' '
1599 test_config versionsort.prereleaseSuffix -very-looooooooooooooooooooooooong-prerelease-suffix &&
1600 git tag -l --sort=version:refname
versioncmp.c
+25 -8
@@ -27,14 +27,18 @@ static int initialized;
27 /*
28 * off is the offset of the first different character in the two strings
29 * s1 and s2. If either s1 or s2 contains a prerelease suffix containing
30 - * that offset, then that string will be forced to be on top.
30 + * that offset or a suffix ends right before that offset, then that
31 + * string will be forced to be on top.
32 *
33 * If both s1 and s2 contain a (different) suffix around that position,
34 * their order is determined by the order of those two suffixes in the
35 * configuration.
36 * If any of the strings contains more than one different suffixes around
37 * that position, then that string is sorted according to the contained
37 - * suffix which comes first in the configuration.
38 + * suffix which starts at the earliest offset in that string.
39 + * If more than one different contained suffixes start at that earliest
40 + * offset, then that string is sorted according to the longest of those
41 + * suffixes.
42 *
43 * Return non-zero if *diff contains the return value for versioncmp()
44 */
@@ -44,27 +48,40 @@ static int swap_prereleases(const char *s1,
48 int *diff)
49 {
50 int i, i1 = -1, i2 = -1;
51 + int start_at1 = off, start_at2 = off, match_len1 = -1, match_len2 = -1;
52
53 for (i = 0; i < prereleases->nr; i++) {
54 const char *suffix = prereleases->items[i].string;
50 - int j, start, suffix_len = strlen(suffix);
55 + int j, start, end, suffix_len = strlen(suffix);
56 if (suffix_len < off)
52 - start = off - suffix_len + 1;
57 + start = off - suffix_len;
58 else
59 start = 0;
55 - for (j = start; j <= off; j++)
56 - if (i1 == -1 && starts_with(s1 + j, suffix)) {
60 + end = match_len1 < suffix_len ? start_at1 : start_at1-1;
61 + for (j = start; j <= end; j++)
62 + if (starts_with(s1 + j, suffix)) {
63 i1 = i;
64 + start_at1 = j;
65 + match_len1 = suffix_len;
66 break;
67 }
60 - for (j = start; j <= off; j++)
61 - if (i2 == -1 && starts_with(s2 + j, suffix)) {
68 + end = match_len2 < suffix_len ? start_at2 : start_at2-1;
69 + for (j = start; j <= end; j++)
70 + if (starts_with(s2 + j, suffix)) {
71 i2 = i;
72 + start_at2 = j;
73 + match_len2 = suffix_len;
74 break;
75 }
76 }
77 if (i1 == -1 && i2 == -1)
78 return 0;
79 + if (i1 == i2)
80 + /* Found the same suffix in both, e.g. "-rc" in "v1.0-rcX"
81 + * and "v1.0-rcY": the caller should decide based on "X"
82 + * and "Y". */
83 + return 0;
84 +
85 if (i1 >= 0 && i2 >= 0)
86 *diff = i1 - i2;
87 else if (i1 >= 0)