Always allow "no-other-choice" pre-release dependencies when resolvin…

…g collection dependency tree

PR ansible#81606.

Prior to this patch, when `--pre` CLI flag was not passed, the
dependency resolver would treat concrete collection dependency
candidates (Git repositories, subdirs, tarball URLs, or local dirs or
files etc) as not meeting the requirements.

This patch makes it so pre-releases in any concrete artifact
references, and the ones being specifically pinned dependencies or
user requests, met anywhere in the dependency tree, are allowed
unconditionally.

This is achieved by moving the pre-release check from
`is_satisfied_by()` to the `find_matches()` hook, following the
Pip's example.

As a bonus, this change also fixes the situation when a collection
pre-releases weren't considered if it didn't have any stable releases.
This now works even if `--pre` wasn't requested explicitly.

Finally, this patch partially reverts commit
6f4b4c3, except for the tests. And it
also improves the `--pre` hint warning to explain that it mostly
affects Galaxy/Automation Hub-hosted collection releases.

Ref ansible#73416
Ref ansible#79112
Fixes ansible#79168
Fixes ansible#80048
Resolves ansible#81605

Co-authored-by: Sloane Hertel <[email protected]>

changelogs/fragments/81606-ansible-galaxy-collection-pre-releases.yml

@@ -0,0 +1,16 @@
+---
+
+bugfixes:
+- >-
+  ansible-galaxy - started allowing the use of pre-releases
+  for dependencies on any level of the dependency tree that
+  specifically demand exact pre-release versions of
+  collections and not version ranges.
+  (https://github.com/ansible/ansible/pull/81606)
+- >-
+  ansible-galaxy - started allowing the use of pre-releases
+  for collections that do not have any stable versions
+  published.
+  (https://github.com/ansible/ansible/pull/81606)
+
+...

lib/ansible/galaxy/collection/__init__.py

@@ -1817,15 +1817,15 @@ def _resolve_depenency_map(
         elif not req.specifier.contains(RESOLVELIB_VERSION.vstring):
             raise AnsibleError(f"ansible-galaxy requires {req.name}{req.specifier}")
 
-    if allow_pre_release:
-        pre_release_hint = ''
-    else:
-        pre_release_hint = 'Hint: Pre-releases are not installed by default unless the specific version is given. To enable pre-releases, use --pre.'
+    pre_release_hint = '' if allow_pre_release else (
+        'Hint: Pre-releases hosted on Galaxy or Automation Hub are not '
+        'installed by default unless a specific version is requested. '
+        'To enable pre-releases globally, use --pre.'
+    )
 
     collection_dep_resolver = build_collection_dependency_resolver(
         galaxy_apis=galaxy_apis,
         concrete_artifacts_manager=concrete_artifacts_manager,
-        user_requirements=requested_requirements,
         preferred_candidates=preferred_candidates,
         with_deps=not no_deps,
         with_pre_releases=allow_pre_release,

lib/ansible/galaxy/dependency_resolution/__init__.py

@@ -13,10 +13,7 @@
     from ansible.galaxy.collection.concrete_artifact_manager import (
         ConcreteArtifactsManager,
     )
-    from ansible.galaxy.dependency_resolution.dataclasses import (
-        Candidate,
-        Requirement,
-    )
+    from ansible.galaxy.dependency_resolution.dataclasses import Candidate
 
 from ansible.galaxy.collection.galaxy_api_proxy import MultiGalaxyAPIProxy
 from ansible.galaxy.dependency_resolution.providers import CollectionDependencyProvider
@@ -27,7 +24,6 @@
 def build_collection_dependency_resolver(
         galaxy_apis,  # type: t.Iterable[GalaxyAPI]
         concrete_artifacts_manager,  # type: ConcreteArtifactsManager
-        user_requirements,  # type: t.Iterable[Requirement]
         preferred_candidates=None,  # type: t.Iterable[Candidate]
         with_deps=True,  # type: bool
         with_pre_releases=False,  # type: bool
@@ -44,7 +40,6 @@ def build_collection_dependency_resolver(
         CollectionDependencyProvider(
             apis=MultiGalaxyAPIProxy(galaxy_apis, concrete_artifacts_manager, offline=offline),
             concrete_artifacts_manager=concrete_artifacts_manager,
-            user_requirements=user_requirements,
             preferred_candidates=preferred_candidates,
             with_deps=with_deps,
             with_pre_releases=with_pre_releases,

lib/ansible/galaxy/dependency_resolution/dataclasses.py

@@ -564,6 +564,27 @@ def is_concrete_artifact(self):
     def is_online_index_pointer(self):
         return not self.is_concrete_artifact
 
+    @property
+    def is_pinned(self):
+        """Indicate if the version set is considered pinned.
+
+        This essentially computes whether the version field of the current
+        requirement explicitly requests a specific version and not an allowed
+        version range.
+
+        It is then used to help the resolvelib-based dependency resolver judge
+        whether it's acceptable to consider a pre-release candidate version
+        despite pre-release installs not being requested by the end-user
+        explicitly.
+
+        See https://github.com/ansible/ansible/pull/81606 for extra context.
+        """
+        version_string = self.ver[0]
+        return version_string.isdigit() or not (
+            version_string == '*' or
+            version_string.startswith(('<', '>', '!='))
+        )
+
     @property
     def source_info(self):
         return self._source_info

lib/ansible/galaxy/dependency_resolution/providers.py

@@ -51,7 +51,6 @@ def __init__(
             self,  # type: CollectionDependencyProviderBase
             apis,  # type: MultiGalaxyAPIProxy
             concrete_artifacts_manager=None,  # type: ConcreteArtifactsManager
-            user_requirements=None,  # type: t.Iterable[Requirement]
             preferred_candidates=None,  # type: t.Iterable[Candidate]
             with_deps=True,  # type: bool
             with_pre_releases=False,  # type: bool
@@ -87,58 +86,12 @@ def __init__(
             Requirement.from_requirement_dict,
             art_mgr=concrete_artifacts_manager,
         )
-        self._pinned_candidate_requests = set(
-            # NOTE: User-provided signatures are supplemental, so signatures
-            # NOTE: are not used to determine if a candidate is user-requested
-            Candidate(req.fqcn, req.ver, req.src, req.type, None)
-            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._with_deps = with_deps
         self._with_pre_releases = with_pre_releases
         self._upgrade = upgrade
         self._include_signatures = include_signatures
 
-    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.
-            # NOTE:
-            # NOTE: User-provided signatures are supplemental, so signatures
-            # NOTE: are not used to determine if a candidate is user-requested
-            return Candidate(
-                candidate.fqcn, candidate.ver, None, candidate.type, None
-            ) in self._pinned_candidate_requests
-
-        return False
-
     def identify(self, requirement_or_candidate):
         # type: (t.Union[Candidate, Requirement]) -> str
         """Given requirement or candidate, return an identifier for it.
@@ -342,25 +295,79 @@ def _find_matches(self, requirements):
         latest_matches = []
         signatures = []
         extra_signature_sources = []  # type: list[str]
+
+        discarding_pre_releases_acceptable = any(
+            not is_pre_release(candidate_version)
+            for candidate_version, _src_server in coll_versions
+        )
+
+        # NOTE: The optimization of conditionally looping over the requirements
+        # NOTE: is used to skip having to compute the pinned status of all
+        # NOTE: requirements and apply version normalization to the found ones.
+        all_pinned_requirement_version_numbers = {
+            # NOTE: Pinned versions can start with a number, but also with an
+            # NOTE: equals sign. Stripping it at the beginning should be
+            # NOTE: enough. If there's a space after equals, the second strip
+            # NOTE: will take care of it.
+            # NOTE: Without this conversion, requirements versions like
+            # NOTE: '1.2.3-alpha.4' work, but '=1.2.3-alpha.4' don't.
+            requirement.ver.lstrip('=').strip()
+            for requirement in requirements
+            if requirement.is_pinned
+        } if discarding_pre_releases_acceptable else set()
+
         for version, src_server in coll_versions:
             tmp_candidate = Candidate(fqcn, version, src_server, 'galaxy', None)
 
-            unsatisfied = False
             for requirement in requirements:
-                unsatisfied |= not self.is_satisfied_by(requirement, tmp_candidate)
+                candidate_satisfies_requirement = self.is_satisfied_by(
+                    requirement, tmp_candidate,
+                )
+                if not candidate_satisfies_requirement:
+                    break
+
+                should_disregard_pre_release_candidate = (
+                    # NOTE: Do not discard pre-release candidates in the
+                    # NOTE: following cases:
+                    # NOTE:   * the end-user requested pre-releases explicitly;
+                    # NOTE:   * the candidate is a concrete artifact (e.g. a
+                    # NOTE:     Git repository, subdirs, a tarball URL, or a
+                    # NOTE:     local dir or file etc.);
+                    # NOTE:   * the candidate's pre-release version exactly
+                    # NOTE:     matches a version specifically requested by one
+                    # NOTE:     of the requirements in the current match
+                    # NOTE:     discovery round (i.e. matching a requirement
+                    # NOTE:     that is not a range but an explicit specific
+                    # NOTE:     version pin). This works when some requirements
+                    # NOTE:     request version ranges but others (possibly on
+                    # NOTE:     different dependency tree level depths) demand
+                    # NOTE:     pre-release dependency versions, even if those
+                    # NOTE:     dependencies are transitive.
+                    is_pre_release(tmp_candidate.ver)
+                    and discarding_pre_releases_acceptable
+                    and not (
+                        self._with_pre_releases
+                        or tmp_candidate.is_concrete_artifact
+                        or version in all_pinned_requirement_version_numbers
+                    )
+                )
+                if should_disregard_pre_release_candidate:
+                    break
+
                 # FIXME
-                # unsatisfied |= not self.is_satisfied_by(requirement, tmp_candidate) or not (
-                #    requirement.src is None or  # if this is true for some candidates but not all it will break key param - Nonetype can't be compared to str
+                # candidate_is_from_requested_source = (
+                #    requirement.src is None  # if this is true for some candidates but not all it will break key param - Nonetype can't be compared to str
                 #    or requirement.src == candidate.src
                 # )
-                if unsatisfied:
-                    break
+                # if not candidate_is_from_requested_source:
+                #     break
+
                 if not self._include_signatures:
                     continue
 
                 extra_signature_sources.extend(requirement.signature_sources or [])
 
-            if not unsatisfied:
+            else:  # candidate satisfies requirements, `break` never happened
                 if self._include_signatures:
                     for extra_source in extra_signature_sources:
                         signatures.append(get_signature_from_source(extra_source))
@@ -405,21 +412,6 @@ def is_satisfied_by(self, requirement, candidate):
         :returns: Indication whether the `candidate` is a viable \
                   solution to the `requirement`.
         """
-        # NOTE: Only allow pre-release candidates if we want pre-releases
-        # 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 (
-            requirement.ver == '*' or
-            requirement.ver.startswith('<') or
-            requirement.ver.startswith('>') or
-            requirement.ver.startswith('!=')
-        ) or self._is_user_requested(candidate)
-        if is_pre_release(candidate.ver) and not allow_pre_release:
-            return False
-
         # NOTE: This is a set of Pipenv-inspired optimizations. Ref:
         # https://github.com/sarugaku/passa/blob/2ac00f1/src/passa/models/providers.py#L58-L74
         if (

test/integration/targets/ansible-galaxy-collection/tasks/main.yml

@@ -134,6 +134,17 @@
   loop_control:
     loop_var: resolvelib_version
 
+- name: test choosing pinned pre-releases anywhere in the dependency tree
+  # This is a regression test for the case when the end-user does not
+  # explicitly allow installing pre-release collection versions, but their
+  # precise pins are still selected if met among the dependencies, either
+  # direct or transitive.
+  include_tasks: pinned_pre_releases_in_deptree.yml
+
+- name: test installing prereleases via scm direct requests
+  # In this test suite because the bug relies on the dep having versions on a Galaxy server
+  include_tasks: virtual_direct_requests.yml
+
 - name: publish collection with a dep on another server
   setup_collections:
     server: secondary

test/integration/targets/ansible-galaxy-collection/tasks/pinned_pre_releases_in_deptree.yml

@@ -0,0 +1,79 @@
+---
+
+- name: >-
+    test that the dependency resolver chooses pre-releases if they are pinned
+  environment:
+    ANSIBLE_COLLECTIONS_PATH: '{{ galaxy_dir }}'
+    ANSIBLE_CONFIG: '{{ galaxy_dir }}/ansible.cfg'
+  block:
+  - name: reset installation directory
+    file:
+      state: "{{ item }}"
+      path: "{{ galaxy_dir }}/ansible_collections"
+    loop:
+    - absent
+    - directory
+
+  - name: >-
+      install collections with pre-release versions in the dependency tree
+    command: >-
+      ansible-galaxy collection install
+      meta_ns_with_transitive_wildcard_dep.meta_name_with_transitive_wildcard_dep
+      rc_meta_ns_with_transitive_dev_dep.rc_meta_name_with_transitive_dev_dep:=2.4.5-rc5
+      {{ galaxy_verbosity }}
+    register: prioritize_direct_req
+  - assert:
+      that:
+      - >-
+        "rc_meta_ns_with_transitive_dev_dep.rc_meta_name_with_transitive_dev_dep:2.4.5-rc5 was installed successfully"
+        in prioritize_direct_req.stdout
+      - >-
+        "meta_ns_with_transitive_wildcard_dep.meta_name_with_transitive_wildcard_dep:4.5.6 was installed successfully"
+        in prioritize_direct_req.stdout
+      - >-
+        "ns_with_dev_dep.name_with_dev_dep:6.7.8 was installed successfully"
+        in prioritize_direct_req.stdout
+      - >-
+        "ns_with_wildcard_dep.name_with_wildcard_dep:5.6.7-beta.3 was installed successfully"
+        in prioritize_direct_req.stdout
+      - >-
+        "dev_and_stables_ns.dev_and_stables_name:1.2.3-dev0 was installed successfully"
+        in prioritize_direct_req.stdout
+
+  - name: cleanup
+    file:
+      state: "{{ item }}"
+      path: "{{ galaxy_dir }}/ansible_collections"
+    loop:
+    - absent
+    - directory
+
+  - name: >-
+      install collection that only has pre-release versions published
+      to the index
+    command: >-
+      ansible-galaxy collection install
+      rc_meta_ns_with_transitive_dev_dep.rc_meta_name_with_transitive_dev_dep:*
+      {{ galaxy_verbosity }}
+    register: select_pre_release_if_no_stable
+  - assert:
+      that:
+      - >-
+        "rc_meta_ns_with_transitive_dev_dep.rc_meta_name_with_transitive_dev_dep:2.4.5-rc5 was installed successfully"
+        in select_pre_release_if_no_stable.stdout
+      - >-
+        "ns_with_dev_dep.name_with_dev_dep:6.7.8 was installed successfully"
+        in select_pre_release_if_no_stable.stdout
+      - >-
+        "dev_and_stables_ns.dev_and_stables_name:1.2.3-dev0 was installed successfully"
+        in select_pre_release_if_no_stable.stdout
+  always:
+  - name: cleanup
+    file:
+      state: "{{ item }}"
+      path: "{{ galaxy_dir }}/ansible_collections"
+    loop:
+    - absent
+    - directory
+
+...