From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: "Jeff King" <peff@peff.net>,
"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
"Leho Kraav" <leho@conversionready.com>,
git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>
Subject: [PATCHv2 6/7] versioncmp: use earliest-longest contained suffix to determine sorting order
Date: Thu, 8 Dec 2016 15:24:00 +0100 [thread overview]
Message-ID: <20161208142401.1329-7-szeder.dev@gmail.com> (raw)
In-Reply-To: <20161208142401.1329-1-szeder.dev@gmail.com>
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>
---
Documentation/config.txt | 6 ++++--
t/t7004-tag.sh | 31 +++++++++++++++++++++++++++++++
versioncmp.c | 33 +++++++++++++++++++++++++--------
3 files changed, 60 insertions(+), 10 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index c1a9616e9..58009c70c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -3010,8 +3010,10 @@ 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).
If more than one suffixes match the same tagname, then that tagname will
-be sorted according to the matching suffix which comes first in the
-configuration.
+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.
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index c7aaace8c..e2efe312d 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1564,6 +1564,37 @@ test_expect_success 'version sort with prerelease reordering, multiple suffixes
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 very long prerelease suffix' '
test_config versionsort.prereleaseSuffix -very-looooooooooooooooooooooooong-prerelease-suffix &&
git tag -l --sort=version:refname
diff --git a/versioncmp.c b/versioncmp.c
index f86ac562e..ae0a9199b 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -27,14 +27,18 @@ static int initialized;
/*
* 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, then that string will be forced to be on top.
+ * that offset or a suffix ends right before that offset, then that
+ * string will be forced to be on top.
*
* 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 comes first in the configuration.
+ * 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()
*/
@@ -44,27 +48,40 @@ static int swap_prereleases(const char *s1,
int *diff)
{
int i, i1 = -1, i2 = -1;
+ int start_at1 = off, start_at2 = off, match_len1 = -1, match_len2 = -1;
for (i = 0; i < prereleases->nr; i++) {
const char *suffix = prereleases->items[i].string;
- int j, start, suffix_len = strlen(suffix);
+ int j, start, end, suffix_len = strlen(suffix);
if (suffix_len < off)
- start = off - suffix_len + 1;
+ start = off - suffix_len;
else
start = 0;
- for (j = start; j <= off; j++)
- if (i1 == -1 && starts_with(s1 + j, suffix)) {
+ end = match_len1 < suffix_len ? start_at1 : start_at1-1;
+ for (j = start; j <= end; j++)
+ if (starts_with(s1 + j, suffix)) {
i1 = i;
+ start_at1 = j;
+ match_len1 = suffix_len;
break;
}
- for (j = start; j <= off; j++)
- if (i2 == -1 && starts_with(s2 + j, suffix)) {
+ end = match_len2 < suffix_len ? start_at2 : start_at2-1;
+ for (j = start; j <= end; j++)
+ if (starts_with(s2 + j, suffix)) {
i2 = i;
+ start_at2 = j;
+ match_len2 = suffix_len;
break;
}
}
if (i1 == -1 && i2 == -1)
return 0;
+ if (i1 == i2)
+ /* 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 (i1 >= 0 && i2 >= 0)
*diff = i1 - i2;
else if (i1 >= 0)
--
2.11.0.78.g5a2d011
next prev parent reply other threads:[~2016-12-08 14:24 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-05 22:42 2.10.0: multiple versionsort.prereleasesuffix buggy? Leho Kraav (Conversion Ready)
2016-09-05 23:21 ` Jeff King
2016-09-06 1:07 ` SZEDER Gábor
2016-09-06 4:07 ` Jeff King
2016-09-06 19:45 ` SZEDER Gábor
2016-09-07 15:12 ` [PATCH 0/5] Fix version sort prerelease reordering bug SZEDER Gábor
2016-09-07 15:12 ` [PATCH 1/5] t7004-tag: delete unnecessary tags with test_when_finished SZEDER Gábor
2016-09-07 15:12 ` [PATCH 2/5] t7004-tag: use test_config helper SZEDER Gábor
2016-09-07 15:12 ` [PATCH 3/5] t7004-tag: add version sort tests to show prerelease reordering issues SZEDER Gábor
2016-09-07 15:12 ` [PATCH 4/5] versioncmp: pass full tagnames to swap_prereleases() SZEDER Gábor
2016-09-08 17:49 ` Junio C Hamano
2016-09-08 20:37 ` SZEDER Gábor
2016-09-08 21:31 ` Junio C Hamano
2016-09-07 15:12 ` [PATCH 5/5] versioncmp: cope with common leading parts in versionsort.prereleaseSuffix SZEDER Gábor
2016-09-07 15:48 ` SZEDER Gábor
2016-09-09 10:43 ` Duy Nguyen
2016-10-05 1:33 ` SZEDER Gábor
2016-10-05 17:01 ` Junio C Hamano
2016-10-05 21:26 ` SZEDER Gábor
2016-10-05 22:15 ` Junio C Hamano
2016-10-06 0:40 ` Jacob Keller
2016-10-06 5:48 ` Duy Nguyen
2016-12-08 14:23 ` [PATCHv2 0/7] Fix and generalize version sort reordering SZEDER Gábor
2016-12-08 14:23 ` [PATCHv2 1/7] t7004-tag: delete unnecessary tags with test_when_finished SZEDER Gábor
2016-12-08 14:23 ` [PATCHv2 2/7] t7004-tag: use test_config helper SZEDER Gábor
2016-12-08 14:23 ` [PATCHv2 3/7] t7004-tag: add version sort tests to show prerelease reordering issues SZEDER Gábor
2016-12-08 14:23 ` [PATCHv2 4/7] versioncmp: pass full tagnames to swap_prereleases() SZEDER Gábor
2016-12-08 14:23 ` [PATCHv2 5/7] versioncmp: cope with common part overlapping with prerelease suffix SZEDER Gábor
2016-12-12 21:27 ` Junio C Hamano
2016-12-13 0:27 ` SZEDER Gábor
2016-12-13 6:39 ` Junio C Hamano
2016-12-08 14:24 ` SZEDER Gábor [this message]
2016-12-08 14:48 ` [PATCHv2 6.5/7] squash! versioncmp: use earliest-longest contained suffix to determine sorting order SZEDER Gábor
2016-12-08 14:24 ` [PATCHv2 7/7] versioncmp: generalize version sort suffix reordering SZEDER Gábor
2016-12-08 19:36 ` Junio C Hamano
2016-12-14 17:08 ` [PATCHv2 0/7] Fix and generalize version sort reordering Jeff King
2016-12-14 17:36 ` Junio C Hamano
2016-12-20 8:50 ` SZEDER Gábor
2016-12-20 16:49 ` Jeff King
2016-09-06 7:12 ` 2.10.0: multiple versionsort.prereleasesuffix buggy? Leho Kraav (Conversion Ready)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161208142401.1329-7-szeder.dev@gmail.com \
--to=szeder.dev@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=leho@conversionready.com \
--cc=pclouds@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).