Fix #342: don't use distutils.version.LooseVersion anymore (#364)

Previously, distutils.version.LooseVersion was used, because it was
sort of close enough to how Alpine parses versions.

This new version uses the exact same algorithm, as `apk` does, and
it passes *all* of `apk`'s testcases for version checking (previously
we simply skipped the ones, that did not pass).

* Remove pmb/helpers/version.py left-over (it is in parse now)
* Make asserts consistent, do not use unnecessary parenthesis
This commit is contained in:
Oliver Smith 2017-08-12 14:03:40 +00:00 committed by GitHub
parent aaaceff297
commit f3f21d3152
6 changed files with 317 additions and 50 deletions

View File

@ -26,6 +26,7 @@ import pmb.chroot
import pmb.helpers.run
import pmb.helpers.file
import pmb.parse.apkindex
import pmb.parse.version
def find_aport(args, package, must_exist=True):
@ -194,8 +195,7 @@ def is_necessary(args, arch, apkbuild, apkindex_path=None):
# a) Binary repo has a newer version
version_old = index_data["version"]
if pmb.parse.apkindex.compare_version(version_old,
version_new) == 1:
if pmb.parse.version.compare(version_old, version_new) == 1:
logging.warning("WARNING: Package '" + package + "' in your aports folder"
" has version " + version_new + ", but the binary package"
" repositories already have version " + version_old + "!")

View File

@ -25,6 +25,7 @@ import pmb.config
import pmb.parse.apkindex
import pmb.parse.arch
import pmb.parse.depends
import pmb.parse.version
def update_repository_list(args, suffix="native", check=False):
@ -93,8 +94,8 @@ def check_min_version(args, suffix="native"):
# Compare
version_installed = installed(args, suffix)["apk-tools"]["version"]
version_min = pmb.config.apk_tools_static_min_version
if pmb.parse.apkindex.compare_version(
version_installed, version_min) == -1:
if pmb.parse.version.compare(version_installed,
version_min) == -1:
raise RuntimeError("You have an outdated version of the 'apk' package"
" manager installed (your version: " + version_installed +
", expected at least: " + version_min + "). Delete"
@ -137,8 +138,8 @@ def install_is_necessary(args, build, arch, package, packages_installed):
# Compare the installed version vs. the version in the repos
data_installed = packages_installed[package]
compare = pmb.parse.apkindex.compare_version(data_installed["version"],
data_repo["version"])
compare = pmb.parse.version.compare(data_installed["version"],
data_repo["version"])
# a) Installed newer (should not happen normally)
if compare == 1:
logging.info("WARNING: " + arch + " package '" + package +

View File

@ -28,6 +28,7 @@ import pmb.config
import pmb.config.load
import pmb.parse.apkindex
import pmb.helpers.http
import pmb.parse.version
def read_signature_info(tar):
@ -162,7 +163,7 @@ def init(args):
version = index_data["version"]
version_min = pmb.config.apk_tools_static_min_version
apk_name = "apk-tools-static-" + version + ".apk"
if pmb.parse.apkindex.compare_version(version, version_min) == -1:
if pmb.parse.version.compare(version, version_min) == -1:
raise RuntimeError("You have an outdated version of apk-tools-static"
" (your version: " + version +
", expected at least:"

View File

@ -16,36 +16,12 @@ GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with pmbootstrap. If not, see <http://www.gnu.org/licenses/>.
"""
import distutils.version
import logging
import os
import tarfile
import pmb.chroot.apk
import pmb.helpers.repo
def compare_version(a_str, b_str):
"""
http://stackoverflow.com/a/11887885
LooseVersion behaves just like apk's version check, at least
for all package versions, that have "-r".
:returns:
(a < b): -1
(a == b): 0
(a > b): 1
"""
if a_str is None:
a_str = "0"
if b_str is None:
b_str = "0"
a = distutils.version.LooseVersion(a_str)
b = distutils.version.LooseVersion(b_str)
if a < b:
return -1
if a == b:
return 0
return 1
import pmb.parse.version
def parse_next_block(args, path, lines, start):
@ -160,7 +136,7 @@ def parse_add_block(path, strict, ret, block, pkgname=None):
# version
version_old = ret[pkgname]["version"]
version_new = block["version"]
if compare_version(version_old, version_new) == 1:
if pmb.parse.version.compare(version_old, version_new) == 1:
return
# Add it to the result set

280
pmb/parse/version.py Normal file
View File

@ -0,0 +1,280 @@
"""
Copyright 2017 Oliver Smith
This file is part of pmbootstrap.
pmbootstrap is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.
pmbootstrap is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.
You should have received a copy of the GNU General Public License
along with pmbootstrap. If not, see <http://www.gnu.org/licenses/>.
"""
"""
In order to stay as compatible to Alpine's apk as possible, this code
is heavily based on:
https://git.alpinelinux.org/cgit/apk-tools/tree/src/version.c
"""
def token_value(string):
"""
Return the associated value for a given token string (we parse
through the version string one token at a time).
:param string: a token string
:returns: integer associated to the token (so we can compare them in
functions further below, a digit (1) looses against a
letter (2), because "letter" has a higher value).
C equivalent: enum PARTS
"""
order = {
"invalid": -1,
"digit_or_zero": 0,
"digit": 1,
"letter": 2,
"suffix": 3,
"suffix_no": 4,
"revision_no": 5,
"end": 6
}
return order[string]
def next_token(previous, rest):
"""
Parse the next token in the rest of the version string, we're
currently looking at.
We do *not* get the value of the token, or advance the rest string
beyond the whole token, that is what the get_token() function does
(see below).
:param previous: the token before
:param rest: of the version string
:returns: (next, rest) next is the upcoming token, rest is the
input "rest" string with one leading '.', '_' or '-'
character removed (if there was any).
C equivalent: next_token()
"""
next = "invalid"
char = rest[:1]
# Tokes, which do not change rest
if not len(rest):
next = "end"
elif previous in ["digit", "digit_or_zero"] and char.islower():
next = "letter"
elif previous == "letter" and char.isdigit():
next = "digit"
elif previous == "suffix" and char.isdigit():
next = "suffix_no"
# Tokens, which remove the first character of rest
else:
if char == ".":
next = "digit_or_zero"
elif char == "_":
next = "suffix"
elif rest.startswith("-r"):
next = "revision_no"
rest = rest[1:]
elif char == "-":
next = "invalid"
rest = rest[1:]
# Validate current token
# Check if the transition from previous to current is valid
if token_value(next) < token_value(previous):
if not ((next == "digit_or_zero" and previous == "digit") or
(next == "suffix" and previous == "suffix_no") or
(next == "digit" and previous == "letter")):
next = "invalid"
return (next, rest)
def parse_suffix(rest):
"""
Cut off the suffix of rest (which is now at the beginning of the
rest variable, but regarding the whole version string, it is a
suffix), and return a value integer (so it can be compared later,
"beta" > "alpha" etc).
:param rest: what is left of the version string, that we are
currently parsing, starts with a "suffix" value
(see below for valid suffixes).
:returns: (rest, value) rest is the input "rest" string without the
suffix, value is a signed integer (negative for pre-,
positive for post-suffixes).
C equivalent: get_token(), case TOKEN_SUFFIX
"""
suffixes = {
"pre": ["alpha", "beta", "pre", "rc"],
"post": ["cvs", "svn", "git", "hg", "p"]
}
for name, suffixes in suffixes.items():
for i, suffix in enumerate(suffixes):
if not rest.startswith(suffix):
continue
rest = rest[len(suffix):]
value = i
if name == "pre":
value = value - len(suffixes)
return (rest, value)
return (rest, 0)
def get_token(previous, rest):
"""
This function does three things:
* get the next token
* get the token value
* cut-off the whole token from rest
:param previous: the token before
:param rest: of the version string
:returns: (next, value, rest) next is the new token string,
value is an integer for comparing, rest is the rest of the
input string.
C equivalent: get_token()
"""
# Set defaults
value = 0
next = "invalid"
# Bail out if at the end
if not len(rest):
return ("end", 0, rest)
# Cut off leading zero digits
if previous == "digit_or_zero" and rest.startswith("0"):
while rest.startswith("0"):
rest = rest[1:]
value -= 1
next = "digit"
# Add up numeric values
elif previous in ["digit_or_zero", "digit", "suffix_no",
"revision_no"]:
for i in range(len(rest)):
while len(rest) and rest[0].isdigit():
value *= 10
value += int(rest[i])
rest = rest[1:]
# Append chars or parse suffix
elif previous == "letter":
value = rest[0]
rest = rest[1:]
elif previous == "suffix":
(rest, value) = parse_suffix(rest)
# Invalid previous token
else:
value = -1
# Get the next token (for non-leading zeros)
if(not len(rest)):
next = "end"
elif(next == "invalid"):
(next, rest) = next_token(previous, rest)
return (next, value, rest)
def validate(version):
"""
Check whether one version string is valid.
:param version: full version string
:returns: True when the version string is valid
C equivalent: apk_version_validate()
"""
current = "digit"
rest = version
while current != "end":
(current, value, rest) = get_token(current, rest)
if current == "invalid":
return False
return True
def compare(a_version, b_version, fuzzy=False):
"""
Compare two versions A and B to find out which one is higher, or if
both are equal.
:param a_version: full version string A
:param b_version: full version string B
:param fuzzy: treat version strings, which end in different token
types as equal
:returns:
(a < b): -1
(a == b): 0
(a > b): 1
C equivalent: apk_version_compare_blob_fuzzy()
"""
# Defaults
a_token = "digit"
b_token = "digit"
a_value = 0
b_value = 0
a_rest = a_version
b_rest = b_version
# Parse A and B one token at a time, until one string ends, or the
# current token has a different type/value
while (a_token == b_token and a_token not in ["end", "invalid"] and
a_value == b_value):
(a_token, a_value, a_rest) = get_token(a_token, a_rest)
(b_token, b_value, b_rest) = get_token(b_token, b_rest)
# Compare the values inside the last tokens
if a_value < b_value:
return -1
if a_value > b_value:
return 1
# Equal: When tokens are the same strings, or when the value
# is the same and fuzzy compare is enabled
if a_token == b_token or fuzzy:
return 0
# Leading version components and their values are equal, now the
# non-terminating version is greater unless it's a suffix
# indicating pre-release
if a_token == "suffix":
(a_token, a_value, a_rest) = get_token(a_token, a_rest)
if a_value < 0:
return -1
if b_token == "suffix":
(b_token, b_value, b_rest) = get_token(b_token, b_rest)
if b_value < 0:
return 1
# Compare the token value (e.g. digit < letter)
if token_value(a_token) > token_value(b_token):
return -1
if token_value(a_token) < token_value(b_token):
return 1
# The tokens are not the same, but previous checks revealed that it
# is equal anyway (e.g. "1.0" == "1").
return 0

View File

@ -23,9 +23,9 @@ import pytest
# Import from parent directory
sys.path.append(os.path.abspath(
os.path.join(os.path.dirname(__file__) + "/..")))
import pmb.parse.apkindex
import pmb.helpers.git
import pmb.helpers.logging
import pmb.parse.version
@pytest.fixture
@ -40,30 +40,39 @@ def args(request):
def test_version(args):
# clone official test file from apk-tools
# Fail after the first error or print a grand total of failures
keep_going = False
# Clone official test file from apk-tools
pmb.helpers.git.clone(args, "apk-tools")
path = args.work + "/cache_git/apk-tools/test/version.data"
# Iterate over the cases from the list
mapping = {-1: "<", 0: "=", 1: ">"}
count = 0
errors = []
with open(path) as handle:
for line in handle:
split = line.split(" ")
a = split[0]
b = split[2].rstrip()
b = split[2].split("#")[0].rstrip()
expected = split[1]
print("(#" + str(count) + ") " + line.rstrip())
result = pmb.parse.version.compare(a, b)
real = mapping[result]
# Alpine packages nowadays always have '-r' in their version
if "-r" not in a or "-r" not in b:
continue
count += 1
if real != expected:
if keep_going:
errors.append(line.rstrip() + " (got: '" + real +
"')")
else:
assert real == expected
print(line.rstrip())
try:
result = pmb.parse.apkindex.compare_version(a, b)
real = mapping[result]
except TypeError:
# FIXME: Bug in Python:
# https://bugs.python.org/issue14894
# When this happens in pmbootstrap, it will also raise the
# TypeError exception.
continue
assert(real == expected)
print("---")
print("total: " + str(count))
print("errors: " + str(len(errors)))
print("---")
for error in errors:
print(error)
assert errors == []