Match user-requested transitive pre-releases in collection dependency resolver (#73416)

* Add direct+transitive pre-release regression test

* Match user-requested transitive pre-releases

This change makes sure that in scenario when a user requests
collectionA-pre and collectionB that depends on collectionA,
collectionA-pre would actually match that collectionA requirement
while `--pre` is unset.

Co-authored-by: Jordan Borean <jborean93@gmail.com>
This commit is contained in:
Sviatoslav Sydorenko 2021-02-01 05:26:49 +01:00 committed by GitHub
parent c7cb944315
commit 6f4b4c345b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 97 additions and 4 deletions

View file

@ -1286,6 +1286,7 @@ def _resolve_depenency_map(
collection_dep_resolver = build_collection_dependency_resolver( collection_dep_resolver = build_collection_dependency_resolver(
galaxy_apis=galaxy_apis, galaxy_apis=galaxy_apis,
concrete_artifacts_manager=concrete_artifacts_manager, concrete_artifacts_manager=concrete_artifacts_manager,
user_requirements=requested_requirements,
preferred_candidates=preferred_candidates, preferred_candidates=preferred_candidates,
with_deps=not no_deps, with_deps=not no_deps,
with_pre_releases=allow_pre_release, with_pre_releases=allow_pre_release,

View file

@ -17,7 +17,10 @@ if TYPE_CHECKING:
from ansible.galaxy.collection.concrete_artifact_manager import ( from ansible.galaxy.collection.concrete_artifact_manager import (
ConcreteArtifactsManager, ConcreteArtifactsManager,
) )
from ansible.galaxy.dependency_resolution.dataclasses import Candidate from ansible.galaxy.dependency_resolution.dataclasses import (
Candidate,
Requirement,
)
from ansible.galaxy.collection.galaxy_api_proxy import MultiGalaxyAPIProxy from ansible.galaxy.collection.galaxy_api_proxy import MultiGalaxyAPIProxy
from ansible.galaxy.dependency_resolution.providers import CollectionDependencyProvider from ansible.galaxy.dependency_resolution.providers import CollectionDependencyProvider
@ -28,6 +31,7 @@ from ansible.galaxy.dependency_resolution.resolvers import CollectionDependencyR
def build_collection_dependency_resolver( def build_collection_dependency_resolver(
galaxy_apis, # type: Iterable[GalaxyAPI] galaxy_apis, # type: Iterable[GalaxyAPI]
concrete_artifacts_manager, # type: ConcreteArtifactsManager concrete_artifacts_manager, # type: ConcreteArtifactsManager
user_requirements, # type: Iterable[Requirement]
preferred_candidates=None, # type: Iterable[Candidate] preferred_candidates=None, # type: Iterable[Candidate]
with_deps=True, # type: bool with_deps=True, # type: bool
with_pre_releases=False, # type: bool with_pre_releases=False, # type: bool
@ -41,6 +45,7 @@ def build_collection_dependency_resolver(
CollectionDependencyProvider( CollectionDependencyProvider(
apis=MultiGalaxyAPIProxy(galaxy_apis, concrete_artifacts_manager), apis=MultiGalaxyAPIProxy(galaxy_apis, concrete_artifacts_manager),
concrete_artifacts_manager=concrete_artifacts_manager, concrete_artifacts_manager=concrete_artifacts_manager,
user_requirements=user_requirements,
preferred_candidates=preferred_candidates, preferred_candidates=preferred_candidates,
with_deps=with_deps, with_deps=with_deps,
with_pre_releases=with_pre_releases, with_pre_releases=with_pre_releases,

View file

@ -40,6 +40,7 @@ class CollectionDependencyProvider(AbstractProvider):
self, # type: CollectionDependencyProvider self, # type: CollectionDependencyProvider
apis, # type: MultiGalaxyAPIProxy apis, # type: MultiGalaxyAPIProxy
concrete_artifacts_manager=None, # type: ConcreteArtifactsManager concrete_artifacts_manager=None, # type: ConcreteArtifactsManager
user_requirements=None, # type: Iterable[Requirement]
preferred_candidates=None, # type: Iterable[Candidate] preferred_candidates=None, # type: Iterable[Candidate]
with_deps=True, # type: bool with_deps=True, # type: bool
with_pre_releases=False, # type: bool with_pre_releases=False, # type: bool
@ -64,10 +65,51 @@ class CollectionDependencyProvider(AbstractProvider):
Requirement.from_requirement_dict, Requirement.from_requirement_dict,
art_mgr=concrete_artifacts_manager, art_mgr=concrete_artifacts_manager,
) )
self._pinned_candidate_requests = set(
Candidate(req.fqcn, req.ver, req.src, req.type)
for req in (user_requirements or ())
if req.is_concrete_artifact or (
req.ver != '*' and
not req.ver.startswith(('<', '>', '!='))
)
)
self._preferred_candidates = set(preferred_candidates or ()) self._preferred_candidates = set(preferred_candidates or ())
self._with_deps = with_deps self._with_deps = with_deps
self._with_pre_releases = with_pre_releases self._with_pre_releases = with_pre_releases
def _is_user_requested(self, candidate): # type: (Candidate) -> bool
"""Check if the candidate is requested by the user."""
if candidate in self._pinned_candidate_requests:
return True
if candidate.is_online_index_pointer and candidate.src is not None:
# NOTE: Candidate is a namedtuple, it has a source server set
# NOTE: to a specific GalaxyAPI instance or `None`. When the
# NOTE: user runs
# NOTE:
# NOTE: $ ansible-galaxy collection install ns.coll
# NOTE:
# NOTE: then it's saved in `self._pinned_candidate_requests`
# NOTE: as `('ns.coll', '*', None, 'galaxy')` but then
# NOTE: `self.find_matches()` calls `self.is_satisfied_by()`
# NOTE: with Candidate instances bound to each specific
# NOTE: server available, those look like
# NOTE: `('ns.coll', '*', GalaxyAPI(...), 'galaxy')` and
# NOTE: wouldn't match the user requests saved in
# NOTE: `self._pinned_candidate_requests`. This is why we
# NOTE: normalize the collection to have `src=None` and try
# NOTE: again.
# NOTE:
# NOTE: When the user request comes from `requirements.yml`
# NOTE: with the `source:` set, it'll match the first check
# NOTE: but it still can have entries with `src=None` so this
# NOTE: normalized check is still necessary.
return Candidate(
candidate.fqcn, candidate.ver, None, candidate.type,
) in self._pinned_candidate_requests
return False
def identify(self, requirement_or_candidate): def identify(self, requirement_or_candidate):
# type: (Union[Candidate, Requirement]) -> str # type: (Union[Candidate, Requirement]) -> str
"""Given requirement or candidate, return an identifier for it. """Given requirement or candidate, return an identifier for it.
@ -214,14 +256,18 @@ class CollectionDependencyProvider(AbstractProvider):
:returns: Indication whether the `candidate` is a viable \ :returns: Indication whether the `candidate` is a viable \
solution to the `requirement`. solution to the `requirement`.
""" """
# NOTE: Only allow pre-release candidates if we want pre-releases or # NOTE: Only allow pre-release candidates if we want pre-releases
# the req ver was an exact match with the pre-release version. # NOTE: or the req ver was an exact match with the pre-release
# NOTE: version. Another case where we'd want to allow
# NOTE: pre-releases is when there are several user requirements
# NOTE: and one of them is a pre-release that also matches a
# NOTE: transitive dependency of another requirement.
allow_pre_release = self._with_pre_releases or not ( allow_pre_release = self._with_pre_releases or not (
requirement.ver == '*' or requirement.ver == '*' or
requirement.ver.startswith('<') or requirement.ver.startswith('<') or
requirement.ver.startswith('>') or requirement.ver.startswith('>') or
requirement.ver.startswith('!=') requirement.ver.startswith('!=')
) ) or self._is_user_requested(candidate)
if is_pre_release(candidate.ver) and not allow_pre_release: if is_pre_release(candidate.ver) and not allow_pre_release:
return False return False

View file

@ -420,3 +420,38 @@
file: file:
path: '{{ galaxy_dir }}/ansible_collections' path: '{{ galaxy_dir }}/ansible_collections'
state: absent state: absent
- name: download collections with pre-release dep - {{ test_name }}
command: ansible-galaxy collection download dep_with_beta.parent namespace1.name1:1.1.0-beta.1 -p '{{ galaxy_dir }}/scratch'
- name: install collection with concrete pre-release dep - {{ test_name }}
command: ansible-galaxy collection install -r '{{ galaxy_dir }}/scratch/requirements.yml'
args:
chdir: '{{ galaxy_dir }}/scratch'
environment:
ANSIBLE_COLLECTIONS_PATHS: '{{ galaxy_dir }}/ansible_collections'
register: install_concrete_pre
- name: get result of install collections with concrete pre-release dep - {{ test_name }}
slurp:
path: '{{ galaxy_dir }}/ansible_collections/{{ collection }}/MANIFEST.json'
register: install_concrete_pre_actual
loop_control:
loop_var: collection
loop:
- namespace1/name1
- dep_with_beta/parent
- name: assert install collections with ansible-galaxy install - {{ test_name }}
assert:
that:
- '"Installing ''namespace1.name1:1.1.0-beta.1'' to" in install_concrete_pre.stdout'
- '"Installing ''dep_with_beta.parent:1.0.0'' to" in install_concrete_pre.stdout'
- (install_concrete_pre_actual.results[0].content | b64decode | from_json).collection_info.version == '1.1.0-beta.1'
- (install_concrete_pre_actual.results[1].content | b64decode | from_json).collection_info.version == '1.0.0'
- name: remove collection dir after round of testing - {{ test_name }}
file:
path: '{{ galaxy_dir }}/ansible_collections'
state: absent

View file

@ -123,3 +123,9 @@ collection_list:
- namespace: cache - namespace: cache
name: cache name: cache
version: 1.0.0 version: 1.0.0
# Dep with beta version
- namespace: dep_with_beta
name: parent
dependencies:
namespace1.name1: '*'