From cdb1ce000a7f26c8f4f5985df26da0c49edef790 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=F0=9F=87=BA=F0=9F=87=A6=20Sviatoslav=20Sydorenko=20=28?= =?UTF-8?q?=D0=A1=D0=B2=D1=8F=D1=82=D0=BE=D1=81=D0=BB=D0=B0=D0=B2=20=D0=A1?= =?UTF-8?q?=D0=B8=D0=B4=D0=BE=D1=80=D0=B5=D0=BD=D0=BA=D0=BE=29?= Date: Sat, 8 Feb 2025 00:01:23 +0100 Subject: [PATCH] Fix `is_pinned` property of `Requirement` (#81812) Previously, requirement version specs starting with `!=` were incorrectly considered as pinned release requests because the comparison was being made against a one-char string while the operator is two-char. This patch changes the check to test against `!` which is enough to detect this case. --- ...ansible-galaxy-negative-spec-is-pinned.yml | 8 +++ .../dependency_resolution/dataclasses.py | 7 +- .../galaxy/test_collection_dataclasses.py | 64 +++++++++++++++++++ 3 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 changelogs/fragments/81812-ansible-galaxy-negative-spec-is-pinned.yml create mode 100644 test/units/galaxy/test_collection_dataclasses.py diff --git a/changelogs/fragments/81812-ansible-galaxy-negative-spec-is-pinned.yml b/changelogs/fragments/81812-ansible-galaxy-negative-spec-is-pinned.yml new file mode 100644 index 00000000000..a4997347b11 --- /dev/null +++ b/changelogs/fragments/81812-ansible-galaxy-negative-spec-is-pinned.yml @@ -0,0 +1,8 @@ +--- + +bugfixes: +- >- + ``ansible-galaxy`` — the collection dependency resolver now treats + version specifiers starting with ``!=`` as unpinned. + +... diff --git a/lib/ansible/galaxy/dependency_resolution/dataclasses.py b/lib/ansible/galaxy/dependency_resolution/dataclasses.py index ea4c875adb4..6796ad132e4 100644 --- a/lib/ansible/galaxy/dependency_resolution/dataclasses.py +++ b/lib/ansible/galaxy/dependency_resolution/dataclasses.py @@ -578,10 +578,9 @@ class _ComputedReqKindsMixin: 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(('<', '>', '!=')) + version_spec_start_char = self.ver[0] + return version_spec_start_char.isdigit() or not ( + version_spec_start_char.startswith(('<', '>', '!', '*')) ) @property diff --git a/test/units/galaxy/test_collection_dataclasses.py b/test/units/galaxy/test_collection_dataclasses.py new file mode 100644 index 00000000000..88d34304949 --- /dev/null +++ b/test/units/galaxy/test_collection_dataclasses.py @@ -0,0 +1,64 @@ +# -*- coding: utf-8 -*- +# Copyright: (c) 2023, Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) +"""Tests for depresolver dataclass objects.""" + + +from __future__ import annotations + +import pytest + +from ansible.galaxy.dependency_resolution.dataclasses import Requirement + + +NO_LEADING_WHITESPACES = pytest.mark.xfail( + reason='Does not yet support leading whitespaces', + strict=True, +) + + +@pytest.mark.parametrize( + ('collection_version_spec', 'expected_is_pinned_outcome'), + ( + ('1.2.3-dev4', True), + (' 1.2.3-dev4', True), + ('=1.2.3', True), + ('= 1.2.3', True), + (' = 1.2.3', True), + (' =1.2.3', True), + ('==1.2.3', True), + ('== 1.2.3', True), + (' == 1.2.3', True), + (' ==1.2.3', True), + ('!=1.0.0', False), + ('!= 1.0.0', False), + pytest.param(' != 1.0.0', False, marks=NO_LEADING_WHITESPACES), + pytest.param(' !=1.0.0', False, marks=NO_LEADING_WHITESPACES), + ('>1.0.0', False), + ('> 1.0.0', False), + pytest.param(' > 1.0.0', False, marks=NO_LEADING_WHITESPACES), + pytest.param(' >1.0.0', False, marks=NO_LEADING_WHITESPACES), + ('>=1.0.0', False), + ('>= 1.0.0', False), + pytest.param(' >= 1.0.0', False, marks=NO_LEADING_WHITESPACES), + pytest.param(' >=1.0.0', False, marks=NO_LEADING_WHITESPACES), + ('<1.0.0', False), + ('< 1.0.0', False), + pytest.param(' < 1.0.0', False, marks=NO_LEADING_WHITESPACES), + pytest.param(' <1.0.0', False, marks=NO_LEADING_WHITESPACES), + ('*', False), + ('* ', False), + pytest.param(' * ', False, marks=NO_LEADING_WHITESPACES), + pytest.param(' *', False, marks=NO_LEADING_WHITESPACES), + ('=1.2.3,!=1.2.3rc5', True), + ), +) +def test_requirement_is_pinned_logic( + collection_version_spec: str, + expected_is_pinned_outcome: bool, +) -> None: + """Test how Requirement's is_pinned property detects pinned spec.""" + assert Requirement( + 'namespace.collection', collection_version_spec, + None, None, None, + ).is_pinned is expected_is_pinned_outcome