[PATCH 5/5] versioncmp: cope with common leading parts in versionsort.prereleaseSuffix

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Version sort with prerelease reordering sometimes puts tagnames in the
wrong order, when the common part of two compared tagnames ends with
the leading character(s) of one or more configured prerelease
suffixes.

  $ git config --get-all versionsort.prereleaseSuffix
  -beta
  $ git tag -l --sort=version:refname v2.1.*
  v2.1.0-beta-2
  v2.1.0-beta-3
  v2.1.0
  v2.1.0-RC1
  v2.1.0-RC2
  v2.1.0-beta-1
  v2.1.1
  v2.1.2

The reason is that when comparing a pair of tagnames, first
versioncmp() looks for the first different character in a pair of
tagnames, and then the swap_prereleases() helper function checks for
prerelease suffixes _starting at_ that character.  Thus, when in the
above example the sorting algorithm happens to compare the tagnames
"v2.1.0-beta-1" and "v2.1.0-RC2", swap_prereleases() will try to match
the suffix "-beta" against "beta-1" to no avail, and the two tagnames
erroneously end up being ordered lexicographically.

To fix this issue change swap_prereleases() to look for configured
prerelease suffixes containing that first different character.

Reported-by: Leho Kraav <leho@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: SZEDER Gábor <szeder@xxxxxxxxxx>
---
 t/t7004-tag.sh |  4 ++--
 versioncmp.c   | 24 +++++++++++++++++++-----
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index d69ac4940388..f0cfe1fa3d8b 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -1538,7 +1538,7 @@ test_expect_success 'reverse version sort with prerelease reordering' '
 	test_cmp expect actual
 '
 
-test_expect_failure 'version sort with prerelease reordering and common leading character' '
+test_expect_success 'version sort with prerelease reordering and common leading character' '
 	test_config versionsort.prereleaseSuffix -beta &&
 	git tag foo1.6-after1 &&
 	git tag -l --sort=version:refname "foo*" >actual &&
@@ -1555,7 +1555,7 @@ test_expect_failure 'version sort with prerelease reordering and common leading
 
 # Capitalization of suffixes is important here, because "-RC" would normally
 # be sorted before "-beta" and the config settings should override that.
-test_expect_failure 'version sort with prerelease reordering, multiple suffixes and common leading character' '
+test_expect_success 'version sort with prerelease reordering, multiple suffixes and common leading character' '
 	test_config versionsort.prereleaseSuffix -beta &&
 	git config --add versionsort.prereleaseSuffix -RC &&
 	git tag foo1.6-RC1 &&
diff --git a/versioncmp.c b/versioncmp.c
index fed02d2a2878..87b49a622423 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -27,7 +27,8 @@ 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 starting
- * at that offset, it will be forced to be on top.
+ * at that offset or the character at that offset is part of a
+ * prerelease suffix, then that string will be forced to be on top.
  *
  * If both s1 and s2 contain a (different) suffix at that position, the
  * order is determined by config file.
@@ -47,10 +48,23 @@ static int swap_prereleases(const char *s1,
 
 	for (i = 0; i < prereleases->nr; i++) {
 		const char *suffix = prereleases->items[i].string;
-		if (i1 == -1 && starts_with(s1 + off, suffix))
-			i1 = i;
-		if (i2 == -1 && starts_with(s2 + off, suffix))
-			i2 = i;
+		int j, start, suffix_len = strlen(suffix);
+		if (suffix_len < off)
+			start = off - suffix_len + 1;
+		else
+			start = 0;
+		for (j = start; j <= off; j++) {
+			if (i1 == -1 && starts_with(s1 + j, suffix)) {
+				i1 = i;
+				break;
+			}
+		}
+		for (j = start; j <= off; j++) {
+			if (i2 == -1 && starts_with(s2 + j, suffix)) {
+				i2 = i;
+				break;
+			}
+		}
 	}
 	if (i1 == -1 && i2 == -1)
 		return 0;
-- 
2.10.0.74.g6632b1b




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]