From 8018ecc4c96d08aba253dc7750fa1e61063195b8 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Mon, 16 Mar 2020 13:32:19 -0500 Subject: [PATCH] Support pre-releases via new SemanticVersion. Fixes #64905 --- changelogs/fragments/64905-semver.yml | 4 + lib/ansible/cli/galaxy.py | 9 +- lib/ansible/galaxy/collection.py | 42 +++--- lib/ansible/utils/version.py | 210 ++++++++++++++++++++++++++ test/units/utils/test_version.py | 170 +++++++++++++++++++++ 5 files changed, 415 insertions(+), 20 deletions(-) create mode 100644 changelogs/fragments/64905-semver.yml create mode 100644 lib/ansible/utils/version.py create mode 100644 test/units/utils/test_version.py diff --git a/changelogs/fragments/64905-semver.yml b/changelogs/fragments/64905-semver.yml new file mode 100644 index 00000000000000..ecafeb435d2dcd --- /dev/null +++ b/changelogs/fragments/64905-semver.yml @@ -0,0 +1,4 @@ +minor_changes: +- Add ``--pre``/``--pre-release`` flags to ``ansible-galaxy collection install/verify`` + to allow pulling in the most recent pre-release version of a collection + (https://github.com/ansible/ansible/issues/64905) diff --git a/lib/ansible/cli/galaxy.py b/lib/ansible/cli/galaxy.py index a5558e278459a0..b8ee5cc6fec1a7 100644 --- a/lib/ansible/cli/galaxy.py +++ b/lib/ansible/cli/galaxy.py @@ -303,6 +303,8 @@ def add_verify_options(self, parser, parents=None): help='Ignore errors during verification and continue with the next specified collection.') verify_parser.add_argument('-r', '--requirements-file', dest='requirements', help='A file containing a list of collections to be verified.') + verify_parser.add_argument('--pre', '--pre-release', dest='pre_release', action='store_true', + help='Allow pre-releases') def add_install_options(self, parser, parents=None): galaxy_type = 'collection' if parser.metavar == 'COLLECTION_ACTION' else 'role' @@ -339,6 +341,8 @@ def add_install_options(self, parser, parents=None): help='The path to the directory containing your collections.') install_parser.add_argument('-r', '--requirements-file', dest='requirements', help='A file containing a list of collections to be installed.') + install_parser.add_argument('--pre', '--pre-release', dest='pre_release', action='store_true', + help='Allow pre-releases') else: install_parser.add_argument('-r', '--role-file', dest='role_file', help='A file containing a list of roles to be imported.') @@ -897,7 +901,8 @@ def execute_verify(self): resolved_paths = [validate_collection_path(GalaxyCLI._resolve_path(path)) for path in search_paths] - verify_collections(requirements, resolved_paths, self.api_servers, (not ignore_certs), ignore_errors) + verify_collections(requirements, resolved_paths, self.api_servers, (not ignore_certs), ignore_errors, + allow_pre_release=context.CLIARGS['pre_release']) return 0 @@ -941,7 +946,7 @@ def execute_install(self): os.makedirs(b_output_path) install_collections(requirements, output_path, self.api_servers, (not ignore_certs), ignore_errors, - no_deps, force, force_deps) + no_deps, force, force_deps, context.CLIARGS['pre_release']) return 0 diff --git a/lib/ansible/galaxy/collection.py b/lib/ansible/galaxy/collection.py index 76312c1fb672c9..d2827521f16b3f 100644 --- a/lib/ansible/galaxy/collection.py +++ b/lib/ansible/galaxy/collection.py @@ -18,7 +18,6 @@ from collections import namedtuple from contextlib import contextmanager -from distutils.version import LooseVersion, StrictVersion from hashlib import sha256 from io import BytesIO from yaml.error import YAMLError @@ -38,6 +37,7 @@ from ansible.utils.collection_loader import AnsibleCollectionRef from ansible.utils.display import Display from ansible.utils.hashing import secure_hash, secure_hash_s +from ansible.utils.version import SemanticVersion from ansible.module_utils.urls import open_url urlparse = six.moves.urllib.parse.urlparse @@ -104,7 +104,7 @@ def metadata(self): @property def latest_version(self): try: - return max([v for v in self.versions if v != '*'], key=LooseVersion) + return max([v for v in self.versions if v != '*'], key=SemanticVersion) except ValueError: # ValueError: max() arg is an empty sequence return '*' @@ -144,7 +144,7 @@ def add_requirement(self, parent, requirement): for p, r in self.required_by ) - versions = ", ".join(sorted(self.versions, key=LooseVersion)) + versions = ", ".join(sorted(self.versions, key=SemanticVersion)) raise AnsibleError( "%s from source '%s'. Available versions before last requirement added: %s\nRequirements from:\n%s" % (msg, collection_source, versions, req_by) @@ -298,7 +298,7 @@ def _meets_requirements(self, version, requirements, parent): elif requirement == '*' or version == '*': continue - if not op(LooseVersion(version), LooseVersion(requirement)): + if not op(SemanticVersion(version), SemanticVersion(requirement)): break else: return True @@ -360,7 +360,9 @@ def from_path(b_path, force, parent=None): name = manifest['name'] version = to_text(manifest['version'], errors='surrogate_or_strict') - if not hasattr(LooseVersion(version), 'version'): + try: + SemanticVersion().parse(version) + except ValueError: display.warning("Collection at '%s' does not have a valid version set, falling back to '*'. Found " "version: '%s'" % (to_text(b_path), version)) version = '*' @@ -383,7 +385,7 @@ def from_path(b_path, force, parent=None): metadata=meta, files=files, skip=True) @staticmethod - def from_name(collection, apis, requirement, force, parent=None): + def from_name(collection, apis, requirement, force, parent=None, allow_pre_release=False): namespace, name = collection.split('.', 1) galaxy_meta = None @@ -401,9 +403,10 @@ def from_name(collection, apis, requirement, force, parent=None): else: resp = api.get_collection_versions(namespace, name) - # Galaxy supports semver but ansible-galaxy does not. We ignore any versions that don't match - # StrictVersion (x.y.z) and only support pre-releases if an explicit version was set (done above). - versions = [v for v in resp if StrictVersion.version_re.match(v)] + if allow_pre_release: + versions = resp + else: + versions = [v for v in resp if not SemanticVersion(v).is_prerelease] except GalaxyError as err: if err.http_code == 404: display.vvv("Collection '%s' is not available from server %s %s" @@ -493,7 +496,8 @@ def publish_collection(collection_path, api, wait, timeout): % (api.name, api.api_server, import_uri)) -def install_collections(collections, output_path, apis, validate_certs, ignore_errors, no_deps, force, force_deps): +def install_collections(collections, output_path, apis, validate_certs, ignore_errors, no_deps, force, force_deps, + allow_pre_release=False): """ Install Ansible collections to the path specified. @@ -512,7 +516,8 @@ def install_collections(collections, output_path, apis, validate_certs, ignore_e display.display("Process install dependency map") with _display_progress(): dependency_map = _build_dependency_map(collections, existing_collections, b_temp_path, apis, - validate_certs, force, force_deps, no_deps) + validate_certs, force, force_deps, no_deps, + allow_pre_release=allow_pre_release) display.display("Starting collection install process") with _display_progress(): @@ -557,7 +562,7 @@ def validate_collection_path(collection_path): return collection_path -def verify_collections(collections, search_paths, apis, validate_certs, ignore_errors): +def verify_collections(collections, search_paths, apis, validate_certs, ignore_errors, allow_pre_release=False): with _display_progress(): with _tempdir() as b_temp_path: @@ -585,7 +590,7 @@ def verify_collections(collections, search_paths, apis, validate_certs, ignore_e # Download collection on a galaxy server for comparison try: - remote_collection = CollectionRequirement.from_name(collection_name, apis, collection_version, False, parent=None) + remote_collection = CollectionRequirement.from_name(collection_name, apis, collection_version, False, parent=None, allow_pre_release=allow_pre_release) except AnsibleError as e: if e.message == 'Failed to find collection %s:%s' % (collection[0], collection[1]): raise AnsibleError('Failed to find remote collection %s:%s on any of the galaxy servers' % (collection[0], collection[1])) @@ -921,13 +926,13 @@ def find_existing_collections(path): def _build_dependency_map(collections, existing_collections, b_temp_path, apis, validate_certs, force, force_deps, - no_deps): + no_deps, allow_pre_release=False): dependency_map = {} # First build the dependency map on the actual requirements for name, version, source in collections: _get_collection_info(dependency_map, existing_collections, name, version, source, b_temp_path, apis, - validate_certs, (force or force_deps)) + validate_certs, (force or force_deps), allow_pre_release=allow_pre_release) checked_parents = set([to_text(c) for c in dependency_map.values() if c.skip]) while len(dependency_map) != len(checked_parents): @@ -943,7 +948,7 @@ def _build_dependency_map(collections, existing_collections, b_temp_path, apis, for dep_name, dep_requirement in parent_info.dependencies.items(): _get_collection_info(dependency_map, existing_collections, dep_name, dep_requirement, parent_info.api, b_temp_path, apis, validate_certs, force_deps, - parent=parent) + parent=parent, allow_pre_release=allow_pre_release) checked_parents.add(parent) @@ -963,7 +968,7 @@ def _build_dependency_map(collections, existing_collections, b_temp_path, apis, def _get_collection_info(dep_map, existing_collections, collection, requirement, source, b_temp_path, apis, - validate_certs, force, parent=None): + validate_certs, force, parent=None, allow_pre_release=False): dep_msg = "" if parent: dep_msg = " - as dependency of %s" % parent @@ -999,7 +1004,8 @@ def _get_collection_info(dep_map, existing_collections, collection, requirement, collection_info.add_requirement(parent, requirement) else: apis = [source] if source else apis - collection_info = CollectionRequirement.from_name(collection, apis, requirement, force, parent=parent) + collection_info = CollectionRequirement.from_name(collection, apis, requirement, force, parent=parent, + allow_pre_release=allow_pre_release) existing = [c for c in existing_collections if to_text(c) == to_text(collection_info)] if existing and not collection_info.force: diff --git a/lib/ansible/utils/version.py b/lib/ansible/utils/version.py new file mode 100644 index 00000000000000..e9540f7b6eb1ae --- /dev/null +++ b/lib/ansible/utils/version.py @@ -0,0 +1,210 @@ +# Copyright (c) 2020 Matt Martz +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +# Make coding more python3-ish +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import re + +from distutils.version import Version + +from ansible.module_utils.six import text_type + + +# Regular expression taken from +# https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string +SEMVER_RE = re.compile( + r''' + ^ + (?P0|[1-9]\d*) + \. + (?P0|[1-9]\d*) + \. + (?P0|[1-9]\d*) + (?: + - + (?P + (?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*) + (?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))* + ) + )? + (?: + \+ + (?P[0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*) + )? + $ + ''', + flags=re.X +) + + +class _Alpha(text_type): + """Class to easily allow comparing strings""" + def __init__(self, specifier): + self.specifier = specifier + + def __eq__(self, other): + if isinstance(other, _Alpha): + return self.specifier == other.specifier + elif isinstance(other, str): + return self.specifier == other + + return False + + def __ne__(self, other): + return not self.__eq__(other) + + def __lt__(self, other): + if isinstance(other, _Alpha): + return self.specifier < other.specifier + elif isinstance(other, str): + return self.specifier < other + elif isinstance(other, _Numeric): + return False + + raise ValueError + + def __gt__(self, other): + return not self.__lt__(other) + + def __le__(self, other): + return self.__lt__(other) or self.__eq__(other) + + def __ge__(self, other): + return self.__gt__(other) or self.__eq__(other) + + +class _Numeric(int): + """Class to easily allow comparing numbers""" + def __init__(self, specifier): + self.specifier = int(specifier) + + def __eq__(self, other): + if isinstance(other, _Numeric): + return self.specifier == other.specifier + elif isinstance(other, int): + return self.specifier == other + + return False + + def __ne__(self, other): + return not self.__eq__(other) + + def __lt__(self, other): + if isinstance(other, _Numeric): + return self.specifier < other.specifier + elif isinstance(other, int): + return self.specifier < other + elif isinstance(other, _Alpha): + return True + + raise ValueError + + def __gt__(self, other): + return not self.__lt__(other) + + def __le__(self, other): + return self.__lt__(other) or self.__eq__(other) + + def __ge__(self, other): + return self.__gt__(other) or self.__eq__(other) + + +class SemanticVersion(Version): + version_re = SEMVER_RE + + def __init__(self, vstring=None): + self.vstring = vstring + self.major = None + self.minor = None + self.patch = None + self.prerelease = () + self.buildmetadata = () + + if vstring: + self.parse(vstring) + + def __repr__(self): + return 'SemanticVersion(%r)' % self.vstring + + def parse(self, vstring): + match = SEMVER_RE.match(vstring) + if not match: + raise ValueError("invalid semantic version '%s'" % vstring) + + (major, minor, patch, prerelease, buildmetadata) = match.group(1, 2, 3, 4, 5) + self.major = int(major) + self.minor = int(minor) + self.patch = int(patch) + + if prerelease: + self.prerelease = tuple(_Numeric(x) if x.isdigit() else _Alpha(x) for x in prerelease.split('.')) + if buildmetadata: + self.buildmetadata = tuple(_Numeric(x) if x.isdigit() else _Alpha(x) for x in buildmetadata.split('.')) + + @property + def core(self): + return self.major, self.minor, self.patch + + @property + def is_prerelease(self): + return self.major == 0 or any((self.prerelease, self.buildmetadata)) + + def _cmp(self, other): + if isinstance(other, str): + other = SemanticVersion(other) + + if self.core != other.core: + if self.core < other.core: + return -1 + else: + return 1 + + if not any((self.prerelease, other.prerelease, self.buildmetadata, other.buildmetadata)): + return 0 + + val = 0 + + if self.prerelease and not other.prerelease: + val -= 1 + elif not self.prerelease and other.prerelease: + val += 1 + elif self.prerelease and other.prerelease: + if self.prerelease < other.prerelease: + val -= 1 + elif self.prerelease > other.prerelease: + val += 1 + + if self.buildmetadata and not other.buildmetadata: + val += 1 + elif not self.buildmetadata and other.buildmetadata: + val -= 1 + elif self.buildmetadata and other.buildmetadata: + if self.buildmetadata < other.buildmetadata: + val -= 1 + elif self.buildmetadata > other.buildmetadata: + val += 1 + + return val + + # The Py2 and Py3 implementations of distutils.version.Version + # are quite different, this makes the Py2 and Py3 implementations + # the same + def __eq__(self, other): + return self._cmp(other) == 0 + + def __ne__(self, other): + return not self.__eq__(other) + + def __lt__(self, other): + return self._cmp(other) < 0 + + def __le__(self, other): + return self._cmp(other) <= 0 + + def __gt__(self, other): + return self._cmp(other) > 0 + + def __ge__(self, other): + return self._cmp(other) >= 0 diff --git a/test/units/utils/test_version.py b/test/units/utils/test_version.py new file mode 100644 index 00000000000000..14419507780c48 --- /dev/null +++ b/test/units/utils/test_version.py @@ -0,0 +1,170 @@ +# -*- coding: utf-8 -*- +# (c) 2020 Matt Martz +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +import pytest + +from ansible.utils.version import SemanticVersion + + +EQ = [ + ('1.0.0', '1.0.0', True), + ('1.0.0', '1.0.0-beta', False), + ('1.0.0-beta2+build1', '1.0.0-beta.2+build.1', False), + ('1.0.0-beta+build', '1.0.0-beta+build', True), +] + +NE = [ + ('1.0.0', '1.0.0', False), + ('1.0.0', '1.0.0-beta', True), + ('1.0.0-beta2+build1', '1.0.0-beta.2+build.1', True), + ('1.0.0-beta+build', '1.0.0-beta+build', False), +] + +LT = [ + ('1.0.0', '2.0.0', True), + ('1.0.0-beta', '2.0.0-alpha', True), + ('1.0.0-alpha', '1.0.0', True), + ('1.0.0-beta', '1.0.0-alpha3', False), + ('1.0.0+foo', '1.0.0-alpha', False), + ('1.0.0-beta.1', '1.0.0-beta.a', True), +] + +GT = [ + ('1.0.0', '2.0.0', False), + ('1.0.0-beta', '2.0.0-alpha', False), + ('1.0.0-alpha', '1.0.0', False), + ('1.0.0-beta', '1.0.0-alpha3', True), + ('1.0.0+foo', '1.0.0-alpha', True), + ('1.0.0-beta.1', '1.0.0-beta.a', False), +] + +LE = [ + ('1.0.0', '1.0.0', True), + ('1.0.0', '2.0.0', True), +] + +GE = [ + ('1.0.0', '1.0.0', True), + ('1.0.0', '2.0.0', False), +] + + +VALID = [ + "0.0.4", + "1.2.3", + "10.20.30", + "1.1.2-prerelease+meta", + "1.1.2+meta", + "1.1.2+meta-valid", + "1.0.0-alpha", + "1.0.0-beta", + "1.0.0-alpha.beta", + "1.0.0-alpha.beta.1", + "1.0.0-alpha.1", + "1.0.0-alpha0.valid", + "1.0.0-alpha.0valid", + "1.0.0-alpha-a.b-c-somethinglong+build.1-aef.1-its-okay", + "1.0.0-rc.1+build.1", + "2.0.0-rc.1+build.123", + "1.2.3-beta", + "10.2.3-DEV-SNAPSHOT", + "1.2.3-SNAPSHOT-123", + "1.0.0", + "2.0.0", + "1.1.7", + "2.0.0+build.1848", + "2.0.1-alpha.1227", + "1.0.0-alpha+beta", + "1.2.3----RC-SNAPSHOT.12.9.1--.12+788", + "1.2.3----R-S.12.9.1--.12+meta", + "1.2.3----RC-SNAPSHOT.12.9.1--.12", + "1.0.0+0.build.1-rc.10000aaa-kk-0.1", + "99999999999999999999999.999999999999999999.99999999999999999", + "1.0.0-0A.is.legal", +] + +INVALID = [ + "1", + "1.2", + "1.2.3-0123", + "1.2.3-0123.0123", + "1.1.2+.123", + "+invalid", + "-invalid", + "-invalid+invalid", + "-invalid.01", + "alpha", + "alpha.beta", + "alpha.beta.1", + "alpha.1", + "alpha+beta", + "alpha_beta", + "alpha.", + "alpha..", + "beta", + "1.0.0-alpha_beta", + "-alpha.", + "1.0.0-alpha..", + "1.0.0-alpha..1", + "1.0.0-alpha...1", + "1.0.0-alpha....1", + "1.0.0-alpha.....1", + "1.0.0-alpha......1", + "1.0.0-alpha.......1", + "01.1.1", + "1.01.1", + "1.1.01", + "1.2", + "1.2.3.DEV", + "1.2-SNAPSHOT", + "1.2.31.2.3----RC-SNAPSHOT.12.09.1--..12+788", + "1.2-RC-SNAPSHOT", + "-1.0.3-gamma+b7718", + "+justmeta", + "9.8.7+meta+meta", + "9.8.7-whatever+meta+meta", +] + + +@pytest.mark.parametrize('left,right,expected', EQ) +def test_eq(left, right, expected): + assert (SemanticVersion(left) == SemanticVersion(right)) is expected + + +@pytest.mark.parametrize('left,right,expected', NE) +def test_ne(left, right, expected): + assert (SemanticVersion(left) != SemanticVersion(right)) is expected + + +@pytest.mark.parametrize('left,right,expected', LT) +def test_lt(left, right, expected): + assert (SemanticVersion(left) < SemanticVersion(right)) is expected + + +@pytest.mark.parametrize('left,right,expected', LE) +def test_le(left, right, expected): + assert (SemanticVersion(left) <= SemanticVersion(right)) is expected + + +@pytest.mark.parametrize('left,right,expected', GT) +def test_gt(left, right, expected): + assert (SemanticVersion(left) > SemanticVersion(right)) is expected + + +@pytest.mark.parametrize('left,right,expected', GE) +def test_ge(left, right, expected): + assert (SemanticVersion(left) >= SemanticVersion(right)) is expected + + +@pytest.mark.parametrize('value', VALID) +def test_valid(value): + SemanticVersion(value) + + +@pytest.mark.parametrize('value', INVALID) +def test_invalid(value): + pytest.raises(ValueError, SemanticVersion, value)