From fc07468051b831286661905b4882f7ac36af356e Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sat, 2 May 2020 06:08:54 +0200 Subject: [PATCH 1/6] Simplify / optimize reading version of yunohost packages... --- debian/control | 2 +- .../0003_migrate_to_stretch.py | 4 +- src/yunohost/utils/packages.py | 89 +++++-------------- 3 files changed, 27 insertions(+), 68 deletions(-) diff --git a/debian/control b/debian/control index 5061ad4f2..8274197ae 100644 --- a/debian/control +++ b/debian/control @@ -13,7 +13,7 @@ Architecture: all Depends: ${python:Depends}, ${misc:Depends} , moulinette (>= 3.7), ssowat (>= 3.7) , python-psutil, python-requests, python-dnspython, python-openssl - , python-apt, python-miniupnpc, python-dbus, python-jinja2 + , python-miniupnpc, python-dbus, python-jinja2 , python-toml , apt, apt-transport-https , nginx, nginx-extras (>=1.6.2) diff --git a/src/yunohost/data_migrations/0003_migrate_to_stretch.py b/src/yunohost/data_migrations/0003_migrate_to_stretch.py index 60b26169a..e916b1ae8 100644 --- a/src/yunohost/data_migrations/0003_migrate_to_stretch.py +++ b/src/yunohost/data_migrations/0003_migrate_to_stretch.py @@ -14,7 +14,7 @@ from yunohost.service import _run_service_command from yunohost.regenconf import (manually_modified_files, manually_modified_files_compared_to_debian_default) from yunohost.utils.filesystem import free_space_in_directory -from yunohost.utils.packages import get_installed_version +from yunohost.utils.packages import get_ynh_package_version from yunohost.utils.network import get_network_interfaces from yunohost.firewall import firewall_allow, firewall_disallow @@ -94,7 +94,7 @@ class MyMigration(Migration): return int(check_output("grep VERSION_ID /etc/os-release | head -n 1 | tr '\"' ' ' | cut -d ' ' -f2")) def yunohost_major_version(self): - return int(get_installed_version("yunohost").split('.')[0]) + return int(get_ynh_package_version("yunohost")["version"].split('.')[0]) def check_assertions(self): diff --git a/src/yunohost/utils/packages.py b/src/yunohost/utils/packages.py index debba70f4..23da08129 100644 --- a/src/yunohost/utils/packages.py +++ b/src/yunohost/utils/packages.py @@ -21,15 +21,12 @@ import re import os import logging -from collections import OrderedDict -import apt -from apt_pkg import version_compare - -from moulinette import m18n +from moulinette.utils.process import check_output logger = logging.getLogger('yunohost.utils.packages') +YUNOHOST_PACKAGES = ['yunohost', 'yunohost-admin', 'moulinette', 'ssowat'] # Exceptions ----------------------------------------------------------------- @@ -368,66 +365,29 @@ class SpecifierSet(object): # Packages and cache helpers ------------------------------------------------- -def get_installed_version(*pkgnames, **kwargs): - """Get the installed version of package(s) +def get_ynh_package_version(package): - Retrieve one or more packages named `pkgnames` and return their installed - version as a dict or as a string if only one is requested. + # Returns the installed version and release version ('stable' or 'testing' + # or 'unstable') - """ - versions = OrderedDict() - cache = apt.Cache() - - # Retrieve options - with_repo = kwargs.get('with_repo', False) - - for pkgname in pkgnames: - try: - pkg = cache[pkgname] - except KeyError: - logger.warning(m18n.n('package_unknown', pkgname=pkgname)) - if with_repo: - versions[pkgname] = { - "version": None, - "repo": None, - } - else: - versions[pkgname] = None - continue - - try: - version = pkg.installed.version - except AttributeError: - version = None - - try: - # stable, testing, unstable - repo = pkg.installed.origins[0].component - except AttributeError: - repo = "" - - if repo == "now": - repo = "local" - - if with_repo: - versions[pkgname] = { - "version": version, - # when we don't have component it's because it's from a local - # install or from an image (like in vagrant) - "repo": repo if repo else "local", - } - else: - versions[pkgname] = version - - if len(pkgnames) == 1: - return versions[pkgnames[0]] - return versions + # NB: this is designed for yunohost packages only ! + # Not tested for any arbitrary packages that + # may handle changelog differently ! + changelog = "/usr/share/doc/%s/changelog.gz" % package + cmd = "gzip -cd %s | head -n1" % changelog + if not os.path.exists(changelog): + return {"version": "?", "repo": "?"} + out = check_output(cmd).split() + # Output looks like : "yunohost (1.2.3) testing; urgency=medium" + return {"version": out[1].strip("()"), + "repo": out[2].strip(";")} def meets_version_specifier(pkgname, specifier): """Check if a package installed version meets specifier""" - spec = SpecifierSet(specifier) - return get_installed_version(pkgname) in spec + # In practice, this function is only used to check the yunohost version installed + assert pkgname in YUNOHOST_PACKAGES + return get_ynh_package_version(pkgname) in SpecifierSet(specifier) # YunoHost related methods --------------------------------------------------- @@ -437,10 +397,11 @@ def ynh_packages_version(*args, **kwargs): # (Namespace(_callbacks=deque([]), _tid='_global', _to_return={}), []) {} # they don't seem to serve any purpose """Return the version of each YunoHost package""" - return get_installed_version( - 'yunohost', 'yunohost-admin', 'moulinette', 'ssowat', - with_repo=True - ) + from collections import OrderedDict + packages = OrderedDict() + for package in YUNOHOST_PACKAGES: + packages[package] = get_ynh_package_version(package) + return packages def dpkg_is_broken(): @@ -457,8 +418,6 @@ def dpkg_lock_available(): def _list_upgradable_apt_packages(): - from moulinette.utils.process import check_output - # List upgradable packages # LC_ALL=C is here to make sure the results are in english upgradable_raw = check_output("LC_ALL=C apt list --upgradable") From 9c0ccd0b4f8a6f0820165faa96bbcad3dc0c7630 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 3 May 2020 19:46:21 +0200 Subject: [PATCH 2/6] That whole thing about Specifier is completely overengineered, let's have a more simple check for version requirements... --- debian/control | 2 +- src/yunohost/utils/packages.py | 376 +++------------------------------ 2 files changed, 35 insertions(+), 343 deletions(-) diff --git a/debian/control b/debian/control index 8274197ae..5070b2050 100644 --- a/debian/control +++ b/debian/control @@ -14,7 +14,7 @@ Depends: ${python:Depends}, ${misc:Depends} , moulinette (>= 3.7), ssowat (>= 3.7) , python-psutil, python-requests, python-dnspython, python-openssl , python-miniupnpc, python-dbus, python-jinja2 - , python-toml + , python-toml, python-packaging , apt, apt-transport-https , nginx, nginx-extras (>=1.6.2) , php-fpm, php-ldap, php-intl diff --git a/src/yunohost/utils/packages.py b/src/yunohost/utils/packages.py index 23da08129..3f352f288 100644 --- a/src/yunohost/utils/packages.py +++ b/src/yunohost/utils/packages.py @@ -23,347 +23,12 @@ import os import logging from moulinette.utils.process import check_output +from packaging import version logger = logging.getLogger('yunohost.utils.packages') YUNOHOST_PACKAGES = ['yunohost', 'yunohost-admin', 'moulinette', 'ssowat'] -# Exceptions ----------------------------------------------------------------- - -class InvalidSpecifier(ValueError): - - """An invalid specifier was found.""" - - -# Version specifier ---------------------------------------------------------- -# The packaging package has been a nice inspiration for the following classes. -# See: https://github.com/pypa/packaging - -class Specifier(object): - - """Unique package version specifier - - Restrict a package version according to the `spec`. It must be a string - containing a relation from the list below followed by a version number - value. The relations allowed are, as defined by the Debian Policy Manual: - - - `<<` for strictly lower - - `<=` for lower or equal - - `=` for exactly equal - - `>=` for greater or equal - - `>>` for strictly greater - - """ - _regex_str = ( - r""" - (?P(<<|<=|=|>=|>>)) - \s* - (?P[^,;\s)]*) - """ - ) - _regex = re.compile( - r"^\s*" + _regex_str + r"\s*$", re.VERBOSE | re.IGNORECASE) - - _relations = { - "<<": "lower_than", - "<=": "lower_or_equal_than", - "=": "equal", - ">=": "greater_or_equal_than", - ">>": "greater_than", - } - - def __init__(self, spec): - if isinstance(spec, basestring): - match = self._regex.search(spec) - if not match: - raise InvalidSpecifier("Invalid specifier: '{0}'".format(spec)) - - self._spec = ( - match.group("relation").strip(), - match.group("version").strip(), - ) - elif isinstance(spec, self.__class__): - self._spec = spec._spec - else: - return NotImplemented - - def __repr__(self): - return "".format(str(self)) - - def __str__(self): - return "{0}{1}".format(*self._spec) - - def __hash__(self): - return hash(self._spec) - - def __eq__(self, other): - if isinstance(other, basestring): - try: - other = self.__class__(other) - except InvalidSpecifier: - return NotImplemented - elif not isinstance(other, self.__class__): - return NotImplemented - - return self._spec == other._spec - - def __ne__(self, other): - if isinstance(other, basestring): - try: - other = self.__class__(other) - except InvalidSpecifier: - return NotImplemented - elif not isinstance(other, self.__class__): - return NotImplemented - - return self._spec != other._spec - - def __and__(self, other): - return self.intersection(other) - - def __or__(self, other): - return self.union(other) - - def _get_relation(self, op): - return getattr(self, "_compare_{0}".format(self._relations[op])) - - def _compare_lower_than(self, version, spec): - return version_compare(version, spec) < 0 - - def _compare_lower_or_equal_than(self, version, spec): - return version_compare(version, spec) <= 0 - - def _compare_equal(self, version, spec): - return version_compare(version, spec) == 0 - - def _compare_greater_or_equal_than(self, version, spec): - return version_compare(version, spec) >= 0 - - def _compare_greater_than(self, version, spec): - return version_compare(version, spec) > 0 - - @property - def relation(self): - return self._spec[0] - - @property - def version(self): - return self._spec[1] - - def __contains__(self, item): - return self.contains(item) - - def intersection(self, other): - """Make the intersection of two specifiers - - Return a new `SpecifierSet` with version specifier(s) common to the - specifier and the other. - - Example: - >>> Specifier('>= 2.2') & '>> 2.2.1' == '>> 2.2.1' - >>> Specifier('>= 2.2') & '<< 2.3' == '>= 2.2, << 2.3' - - """ - if isinstance(other, basestring): - try: - other = self.__class__(other) - except InvalidSpecifier: - return NotImplemented - elif not isinstance(other, self.__class__): - return NotImplemented - - # store spec parts for easy access - rel1, v1 = self.relation, self.version - rel2, v2 = other.relation, other.version - result = [] - - if other == self: - result = [other] - elif rel1 == '=': - result = [self] if v1 in other else None - elif rel2 == '=': - result = [other] if v2 in self else None - elif v1 == v2: - result = [other if rel1[1] == '=' else self] - elif v2 in self or v1 in other: - is_self_greater = version_compare(v1, v2) > 0 - if rel1[0] == rel2[0]: - if rel1[0] == '>': - result = [self if is_self_greater else other] - else: - result = [other if is_self_greater else self] - else: - result = [self, other] - return SpecifierSet(result if result is not None else '') - - def union(self, other): - """Make the union of two version specifiers - - Return a new `SpecifierSet` with version specifiers from the - specifier and the other. - - Example: - >>> Specifier('>= 2.2') | '<< 2.3' == '>= 2.2, << 2.3' - - """ - if isinstance(other, basestring): - try: - other = self.__class__(other) - except InvalidSpecifier: - return NotImplemented - elif not isinstance(other, self.__class__): - return NotImplemented - - return SpecifierSet([self, other]) - - def contains(self, item): - """Check if the specifier contains an other - - Return whether the item is contained in the version specifier. - - Example: - >>> '2.2.1' in Specifier('<< 2.3') - >>> '2.4' not in Specifier('<< 2.3') - - """ - return self._get_relation(self.relation)(item, self.version) - - -class SpecifierSet(object): - - """A set of package version specifiers - - Combine several Specifier separated by a comma. It allows to restrict - more precisely a package version. Each package version specifier must be - meet. Note than an empty set of specifiers will always be meet. - - """ - - def __init__(self, specifiers): - if isinstance(specifiers, basestring): - specifiers = [s.strip() for s in specifiers.split(",") - if s.strip()] - - parsed = set() - for specifier in specifiers: - parsed.add(Specifier(specifier)) - - self._specs = frozenset(parsed) - - def __repr__(self): - return "".format(str(self)) - - def __str__(self): - return ",".join(sorted(str(s) for s in self._specs)) - - def __hash__(self): - return hash(self._specs) - - def __and__(self, other): - return self.intersection(other) - - def __or__(self, other): - return self.union(other) - - def __eq__(self, other): - if isinstance(other, basestring): - other = SpecifierSet(other) - elif isinstance(other, Specifier): - other = SpecifierSet(str(other)) - elif not isinstance(other, SpecifierSet): - return NotImplemented - - return self._specs == other._specs - - def __ne__(self, other): - if isinstance(other, basestring): - other = SpecifierSet(other) - elif isinstance(other, Specifier): - other = SpecifierSet(str(other)) - elif not isinstance(other, SpecifierSet): - return NotImplemented - - return self._specs != other._specs - - def __len__(self): - return len(self._specs) - - def __iter__(self): - return iter(self._specs) - - def __contains__(self, item): - return self.contains(item) - - def intersection(self, other): - """Make the intersection of two specifiers sets - - Return a new `SpecifierSet` with version specifier(s) common to the - set and the other. - - Example: - >>> SpecifierSet('>= 2.2') & '>> 2.2.1' == '>> 2.2.1' - >>> SpecifierSet('>= 2.2, << 2.4') & '<< 2.3' == '>= 2.2, << 2.3' - >>> SpecifierSet('>= 2.2, << 2.3') & '>= 2.4' == '' - - """ - if isinstance(other, basestring): - other = SpecifierSet(other) - elif not isinstance(other, SpecifierSet): - return NotImplemented - - specifiers = set(self._specs | other._specs) - intersection = [specifiers.pop()] if specifiers else [] - - for specifier in specifiers: - parsed = set() - for spec in intersection: - inter = spec & specifier - if not inter: - parsed.clear() - break - # TODO: validate with other specs in parsed - parsed.update(inter._specs) - intersection = parsed - if not intersection: - break - return SpecifierSet(intersection) - - def union(self, other): - """Make the union of two specifiers sets - - Return a new `SpecifierSet` with version specifiers from the set - and the other. - - Example: - >>> SpecifierSet('>= 2.2') | '<< 2.3' == '>= 2.2, << 2.3' - - """ - if isinstance(other, basestring): - other = SpecifierSet(other) - elif not isinstance(other, SpecifierSet): - return NotImplemented - - specifiers = SpecifierSet([]) - specifiers._specs = frozenset(self._specs | other._specs) - return specifiers - - def contains(self, item): - """Check if the set contains a version specifier - - Return whether the item is contained in all version specifiers. - - Example: - >>> '2.2.1' in SpecifierSet('>= 2.2, << 2.3') - >>> '2.4' not in SpecifierSet('>= 2.2, << 2.3') - - """ - return all( - s.contains(item) - for s in self._specs - ) - - -# Packages and cache helpers ------------------------------------------------- def get_ynh_package_version(package): @@ -383,14 +48,39 @@ def get_ynh_package_version(package): return {"version": out[1].strip("()"), "repo": out[2].strip(";")} -def meets_version_specifier(pkgname, specifier): - """Check if a package installed version meets specifier""" - # In practice, this function is only used to check the yunohost version installed - assert pkgname in YUNOHOST_PACKAGES - return get_ynh_package_version(pkgname) in SpecifierSet(specifier) +def meets_version_specifier(pkg_name, specifier): + """ + Check if a package installed version meets specifier + + specifier is something like ">> 1.2.3" + """ + + # In practice, this function is only used to check the yunohost version + # installed. + # We'll trim any ~foobar in the current installed version because it's not + # handled correctly by version.parse, but we don't care so much in that + # context + assert pkg_name in YUNOHOST_PACKAGES + pkg_version = get_ynh_package_version(pkg_name)["version"] + pkg_version = pkg_version.split("~")[0] + pkg_version = version.parse(pkg_version) + + # Extract operator and version specifier + op, req_version = re.search(r'(<<|<=|=|>=|>>) *([\d\.]+)', specifier).groups() + req_version = version.parse(req_version) + + # cmp is a python builtin that returns (-1, 0, 1) depending on comparison + deb_operators = { + "<<": lambda v1, v2: cmp(v1, v2) in [-1], + "<=": lambda v1, v2: cmp(v1, v2) in [-1, 0], + "=": lambda v1, v2: cmp(v1, v2) in [0], + ">=": lambda v1, v2: cmp(v1, v2) in [0, 1], + ">>": lambda v1, v2: cmp(v1, v2) in [1] + } + + return deb_operators[op](pkg_version, req_version) -# YunoHost related methods --------------------------------------------------- def ynh_packages_version(*args, **kwargs): # from cli the received arguments are: @@ -413,9 +103,11 @@ def dpkg_is_broken(): return any(re.match("^[0-9]+$", f) for f in os.listdir("/var/lib/dpkg/updates/")) + def dpkg_lock_available(): return os.system("lsof /var/lib/dpkg/lock >/dev/null") != 0 + def _list_upgradable_apt_packages(): # List upgradable packages From 5d811e2b39b987944d68732fa4a06674186440b6 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Sun, 3 May 2020 19:52:12 +0200 Subject: [PATCH 3/6] Remove stale string --- locales/en.json | 1 - 1 file changed, 1 deletion(-) diff --git a/locales/en.json b/locales/en.json index 25712e8cd..652a602f7 100644 --- a/locales/en.json +++ b/locales/en.json @@ -486,7 +486,6 @@ "no_internet_connection": "The server is not connected to the Internet", "not_enough_disk_space": "Not enough free space on '{path:s}'", "operation_interrupted": "The operation was manually interrupted?", - "package_unknown": "Unknown package '{pkgname}'", "packages_upgrade_failed": "Could not upgrade all the packages", "password_listed": "This password is among the most used passwords in the world. Please choose something more unique.", "password_too_simple_1": "The password needs to be at least 8 characters long", From 0b24aa68d504f2ab53daacff0ad982aa9ba6b650 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 7 May 2020 18:08:12 +0200 Subject: [PATCH 4/6] Ugly hack to install new deps in debian/control --- .gitlab-ci.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 7459ae982..0ce234c6b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -11,6 +11,7 @@ postinstall: image: before-postinstall stage: postinstall script: + - apt install --no-install-recommends -y $(cat debian/control | grep "^Depends" -A50 | grep "Recommends:" -B50 | grep "^ *," | grep -o -P "[\w\-]{3,}") - yunohost tools postinstall -d domain.tld -p the_password --ignore-dyndns ######################################## @@ -129,4 +130,4 @@ lint: #format-check: # extends: .lint-stage # script: -# - black --check --diff \ No newline at end of file +# - black --check --diff From 3a62d828ba9b2b3774f606f19f8c2a6ead63bfbf Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Wed, 6 May 2020 19:01:07 +0200 Subject: [PATCH 5/6] version was not defined... --- src/yunohost/app.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/yunohost/app.py b/src/yunohost/app.py index e2df6ba78..ffc1de378 100644 --- a/src/yunohost/app.py +++ b/src/yunohost/app.py @@ -2331,6 +2331,7 @@ def _check_manifest_requirements(manifest, app_instance_name): # Iterate over requirements for pkgname, spec in requirements.items(): if not packages.meets_version_specifier(pkgname, spec): + version = packages.ynh_packages_version()[pkgname]["version"] raise YunohostError('app_requirements_unmeet', pkgname=pkgname, version=version, spec=spec, app=app_instance_name) From 8bcf7530811c947093f56c8c2ad63db2537d8ca8 Mon Sep 17 00:00:00 2001 From: Alexandre Aubin Date: Thu, 7 May 2020 18:34:51 +0200 Subject: [PATCH 6/6] Also split + and - --- src/yunohost/utils/packages.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/yunohost/utils/packages.py b/src/yunohost/utils/packages.py index 3f352f288..6103206e5 100644 --- a/src/yunohost/utils/packages.py +++ b/src/yunohost/utils/packages.py @@ -63,7 +63,7 @@ def meets_version_specifier(pkg_name, specifier): # context assert pkg_name in YUNOHOST_PACKAGES pkg_version = get_ynh_package_version(pkg_name)["version"] - pkg_version = pkg_version.split("~")[0] + pkg_version = re.split(r'\~|\+|\-', pkg_version)[0] pkg_version = version.parse(pkg_version) # Extract operator and version specifier