Skip to content

deb: split Debian revision at the last hyphen #1646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
twaik wants to merge 1 commit into
base: master
Choose a base branch
from
Open

deb: split Debian revision at the last hyphen #1646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion deb/version.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func parseVersion(ver string) (epoch, upstream, debian string) {
epoch, ver = ver[:i], ver[i+1:]
}

i = strings.Index(ver, "-")
i = strings.LastIndex(ver, "-")

@thunder-coding thunder-coding Oct 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You sure? From lib/dpkg/parsehelp.c (https://git.dpkg.org/cgit/dpkg/dpkg.git/tree/lib/dpkg/parsehelp.c):

	colon = strchr(string, ':');
	if (colon) {
		long epoch;

		errno = 0;
		epoch = strtol(string, &eepochcolon, 10);
		if (string == eepochcolon)
			return dpkg_put_error(err, _("epoch in version is empty"));
		if (colon != eepochcolon)
			return dpkg_put_error(err, _("epoch in version is not number"));
		if (epoch < 0)
			return dpkg_put_error(err, _("epoch in version is negative"));
		if (epoch > INT_MAX || errno == ERANGE)
			return dpkg_put_error(err, _("epoch in version is too big"));
		if (!*++colon)
			return dpkg_put_error(err, _("nothing after colon in version number"));
		string = colon;
		rversion->epoch = epoch;
	} else {
		rversion->epoch = 0;
	}

	rversion->version = nfstrnsave(string, end - string);


  // [SEETHIS] this is the part where the revision is extracted
	hyphen = strrchr(rversion->version, '-');
	if (hyphen) {
		*hyphen++ = '\0';

		if (*hyphen == '\0')
			return dpkg_put_error(err, _("revision number is empty"));
	}
	rversion->revision = hyphen ? hyphen : "";

Look at the comment which I added with [SEETHIS] in dpkg's source, it seems to actually split at the first hyphen (unless I am missing something else).

@thunder-coding thunder-coding Oct 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking again slightly deeper. Yeah the docs to mention that it should be splitted at last hyphen, but dpkg is doing something different internally.

The incompatibility I believe exists in dpkg's lib/dpkg/version.c:

static int
verrevcmp(const char *a, const char *b)
{
	if (a == NULL)
		a = "";
	if (b == NULL)
		b = "";

	while (*a || *b) {
		int first_diff = 0;

		while ((*a && !c_isdigit(*a)) || (*b && !c_isdigit(*b))) {
			int ac = order(*a);
			int bc = order(*b);

			if (ac != bc)
				return ac - bc;

			a++;
			b++;
		}
		while (*a == '0')
			a++;
		while (*b == '0')
			b++;
		while (c_isdigit(*a) && c_isdigit(*b)) {
			if (!first_diff)
				first_diff = *a - *b;
			a++;
			b++;
		}

		if (c_isdigit(*a))
			return 1;
		if (c_isdigit(*b))
			return -1;
		if (first_diff)
			return first_diff;
	}

	return 0;
}

and aptly's CompareVersionPart deb/version.go:

func compareVersionPart(part1, part2 string) int {
	i1, i2 := 0, 0
	l1, l2 := len(part1), len(part2)

	for {
		j1, j2 := i1, i2
		for j1 < l1 && !unicode.IsDigit(rune(part1[j1])) {
			j1++
		}

		for j2 < l2 && !unicode.IsDigit(rune(part2[j2])) {
			j2++
		}

		s1, s2 := part1[i1:j1], part2[i2:j2]
		r := compareLexicographic(s1, s2)
		if r != 0 {
			return r
		}

		i1, i2 = j1, j2

		for j1 < l1 && unicode.IsDigit(rune(part1[j1])) {
			j1++
		}

		for j2 < l2 && unicode.IsDigit(rune(part2[j2])) {
			j2++
		}

		s1, s2 = part1[i1:j1], part2[i2:j2]
		n1, _ := strconv.Atoi(s1)
		n2, _ := strconv.Atoi(s2)

		if n1 < n2 {
			return -1
		}
		if n1 > n2 {
			return 1
		}

		i1, i2 = j1, j2

		if i1 == l1 && i2 == l2 {
			break
		}
	}
	return 0
}

@twaik twaik Oct 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

// [SEETHIS] this is the part where the revision is extracted
hyphen = strrchr(rversion->version, '-');

strrchr looks for the last occurence unlike strchr which looks for the first one.

@thunder-coding thunder-coding Oct 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah okay, makes sense then (somehow I didn't read it properly). But the test case I added back in 836d9f3 for 4.3.5a and 4.3.5rc1-1 needs to be left as is.

$ dpkg --compare-versions 4.3.5a lt 4.3.5-rc1-1; echo $?
0

So for CompareVersions() it should return 1 (-1 for left side is greater, 0 for equal, 1 for right side)

Yeah I was wrong in that commit for how the hyphen is supposed to split, but I still believe that CompareVersionPart() behaviour differs from that of dpkg's behaviour looking at how this breaks

@twaik twaik Oct 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit unsure about the sign here, so correct me if I'm wrong
dpkg --compare-versions 4.3.5a lt 4.3.5-rc3-1 is true, so I'd expect -1 (the old 1 passed only because of the first-hyphen split).

For the sign convention: CompareVersions("1.0", "2.0") returns -1 in aptly, and dpkg --compare-versions 1.0 lt 2.0 is true, so -1 means "first is lower" in both.

I also ran every pair from TestCompareVersions through dpkg --compare-versions manually, and with this PR all of them match dpkg (before it, three didn't: 4.3.5a vs 4.3.5-rc3-1, 1.0+a-1 vs 1.0-a-1, and 1.6.0-rc1+really1.6.0-0 vs 1.6.0-rc1-0).

if i != -1 {
debian, ver = ver[i+1:], ver[:i]
}
Expand Down
17 changes: 14 additions & 3 deletions deb/version_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ func (s *VersionSuite) TestParseVersion(c *C) {
c.Check([]string{e, u, d}, DeepEquals, []string{"", "1.3.4", "1"})

e, u, d = parseVersion("1.3-pre4-1")
c.Check([]string{e, u, d}, DeepEquals, []string{"", "1.3", "pre4-1"})
c.Check([]string{e, u, d}, DeepEquals, []string{"", "1.3-pre4", "1"})

e, u, d = parseVersion("4:1.3-pre4-1")
c.Check([]string{e, u, d}, DeepEquals, []string{"4", "1.3", "pre4-1"})
c.Check([]string{e, u, d}, DeepEquals, []string{"4", "1.3-pre4", "1"})
}

func (s *VersionSuite) TestCompareLexicographic(c *C) {
Expand Down Expand Up @@ -100,7 +100,18 @@ func (s *VersionSuite) TestCompareVersions(c *C) {
c.Check(CompareVersions("1.0-133-avc", "1.0"), Equals, 1)

c.Check(CompareVersions("5.2.0.3", "5.2.0.283"), Equals, -1)
c.Check(CompareVersions("4.3.5a", "4.3.5-rc3-1"), Equals, 1)
c.Check(CompareVersions("4.3.5a", "4.3.5-rc3-1"), Equals, -1)
c.Check(CompareVersions("1.6.0-rc1+really1.6.0-0", "1.6.0-rc1-0"), Equals, 1)
c.Check(CompareVersions("1.2-3-4", "1.2-3-5"), Equals, -1)
c.Check(CompareVersions("1.0-1-1", "1.0-1"), Equals, 1)
c.Check(CompareVersions("1.0+1", "1.0"), Equals, 1)
c.Check(CompareVersions("1.0+a-1", "1.0-a-1"), Equals, -1)
c.Check(CompareVersions("1.0+1-1", "1.0-1"), Equals, 1)
c.Check(CompareVersions("1.0-1+b1", "1.0-1"), Equals, 1)
c.Check(CompareVersions("1.0-1+b1", "1.0-1~bpo1"), Equals, 1)
c.Check(CompareVersions("1.0-1+b2", "1.0-1+b10"), Equals, -1)
c.Check(CompareVersions("1.0.1+dfsg-1", "1.0.1-1"), Equals, 1)
c.Check(CompareVersions("1.0-1+b1", "1.0-2"), Equals, -1)
}

func (s *VersionSuite) TestParseDependency(c *C) {
Expand Down
Loading