Take validity into account when accessing tags and macro definitions by name

specfile/macro_definitions.py

@@ -219,11 +219,36 @@ def get(self, name: str, position: Optional[int] = None) -> MacroDefinition:
         return self.data[self.find(name, position)]
 
     def find(self, name: str, position: Optional[int] = None) -> int:
+        """
+        Finds a macro definition with the specified name. If position is not specified,
+        returns the first valid matching macro definiton. If there is no such macro
+        definition, returns the first match, if any. If position is specified and there is
+        a matching macro definition at that position, it is returned, otherwise
+        ValueError is raised.
+
+        Args:
+            name: Name of the tag to find.
+            position: Optional position in the spec file.
+
+        Returns:
+            Index of the matching tag.
+
+        Raises:
+            ValueError if there is no match.
+        """
+        first_match = None
         for i, macro_definition in enumerate(self.data):
             if macro_definition.name == name:
-                if position is None or macro_definition.get_position(self) == position:
+                if position is None:
+                    if first_match is None:
+                        first_match = i
+                    if macro_definition.valid:
+                        return i
+                elif macro_definition.get_position(self) == position:
                     return i
-        raise ValueError
+        if first_match is None or position is not None:
+            raise ValueError
+        return first_match
 
     @classmethod
     def _parse(

specfile/tags.py

@@ -441,11 +441,35 @@ def get(self, name: str, position: Optional[int] = None) -> Tag:
         return self.data[self.find(name, position)]
 
     def find(self, name: str, position: Optional[int] = None) -> int:
+        """
+        Finds a tag with the specified name. If position is not specified,
+        returns the first valid matching tag. If there is no such tag, returns
+        the first match, if any. If position is specified and there is a matching
+        tag at that position, it is returned, otherwise ValueError is raised.
+
+        Args:
+            name: Name of the tag to find.
+            position: Optional position in the spec file.
+
+        Returns:
+            Index of the matching tag.
+
+        Raises:
+            ValueError if there is no match.
+        """
+        first_match = None
         for i, tag in enumerate(self.data):
             if tag.name.capitalize() == name.capitalize():
-                if position is None or tag.get_position(self) == position:
+                if position is None:
+                    if first_match is None:
+                        first_match = i
+                    if tag.valid:
+                        return i
+                elif tag.get_position(self) == position:
                     return i
-        raise ValueError
+        if first_match is None or position is not None:
+            raise ValueError
+        return first_match
 
     def insert(self, i: int, item: Tag) -> None:
         if i > len(self.data):

tests/constants.py

@@ -18,5 +18,6 @@
 SPEC_COMMENTED_PATCHES = DATA_DIR / "spec_commented_patches"
 SPEC_SHELL_EXPANSIONS = DATA_DIR / "spec_shell_expansions"
 SPEC_CONDITIONALIZED_CHANGELOG = DATA_DIR / "spec_conditionalized_changelog"
+SPEC_CONDITIONALIZED_VERSION = DATA_DIR / "spec_conditionalized_version"
 
 SPECFILE = "test.spec"

tests/data/spec_conditionalized_version/patch0.patch

@@ -0,0 +1,18 @@
+From b7af0b9194585c6d208de3a0e9978d5ad9c5d97b Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Nikola=20Forr=C3=B3?= <[email protected]>
+Date: Wed, 16 Mar 2022 10:29:59 +0100
+Subject: [PATCH 1/3] patch0
+
+---
+ test.txt | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/test.txt b/test.txt
+index 9daeafb..dec2cbe 100644
+--- a/test.txt
++++ b/test.txt
+@@ -1 +1,2 @@
+ test
++test
+--
+2.35.1

tests/data/spec_conditionalized_version/patch1.patch

@@ -0,0 +1,19 @@
+From 6d5d1561b3ccf2df9d001a7af011144acc352361 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Nikola=20Forr=C3=B3?= <[email protected]>
+Date: Wed, 16 Mar 2022 10:30:15 +0100
+Subject: [PATCH 2/3] patch1
+
+---
+ test.txt | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/test.txt b/test.txt
+index dec2cbe..0867e73 100644
+--- a/test.txt
++++ b/test.txt
+@@ -1,2 +1,3 @@
+ test
+ test
++test
+--
+2.35.1

tests/data/spec_conditionalized_version/patch2.patch

@@ -0,0 +1,20 @@
+From ae1d3bbca0caf1cce1842ceab4c6d7252c0a7bd8 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Nikola=20Forr=C3=B3?= <[email protected]>
+Date: Wed, 16 Mar 2022 10:30:29 +0100
+Subject: [PATCH 3/3] patch2
+
+---
+ test.txt | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/test.txt b/test.txt
+index 0867e73..d0c7fbe 100644
+--- a/test.txt
++++ b/test.txt
+@@ -1,3 +1,4 @@
+ test
+ test
+ test
++test
+--
+2.35.1

tests/data/spec_conditionalized_version/test.spec

@@ -0,0 +1,41 @@
+%dnl %global commit 202ab7e698a34154129bb9ded589db58996eeb53
+%global shortcommit %(c=%{commit}; echo ${c:0:7})
+
+%global upstream_version 0.1.2
+
+Name:           test
+%if 0%{?commit:1}
+Version:        %{upstream_version}^git%{shortcommit}
+%else
+Version:        %{upstream_version}
+%endif
+Release:        1%{?dist}
+Summary:        Test package
+
+License:        MIT
+
+%if 0%{?commit:1}
+Source0:        https://example.com/archive/%{name}/%{commit}/%{name}-%{shortcommit}.tar.xz
+%else
+Source0:        https://example.com/archive/%{name}/v%{version}/%{name}-%{version}.tar.xz
+%endif
+Patch0:         patch0.patch
+Patch1:         patch1.patch
+Patch2:         patch2.patch
+
+
+%description
+Test package
+
+
+%prep
+%if 0%{?commit:1}
+%autosetup -p1 -n %{name}-%{shortcommit}
+%else
+%autosetup -p1 -n %{name}-%{version}
+%endif
+
+
+%changelog
+* Thu Jun 07 2018 Nikola Forró <[email protected]> - 0.1.2-1
+- first version

tests/integration/conftest.py

@@ -10,6 +10,7 @@
     SPEC_AUTOSETUP,
     SPEC_COMMENTED_PATCHES,
     SPEC_CONDITIONALIZED_CHANGELOG,
+    SPEC_CONDITIONALIZED_VERSION,
     SPEC_INCLUDES,
     SPEC_MACROS,
     SPEC_MINIMAL,
@@ -112,3 +113,10 @@ def spec_conditionalized_changelog(tmp_path):
     specfile_path = tmp_path / SPECFILE
     shutil.copyfile(SPEC_CONDITIONALIZED_CHANGELOG / SPECFILE, specfile_path)
     return specfile_path
+
+
+@pytest.fixture(scope="function")
+def spec_conditionalized_version(tmp_path):
+    specfile_path = tmp_path / SPECFILE
+    shutil.copyfile(SPEC_CONDITIONALIZED_VERSION / SPECFILE, specfile_path)
+    return specfile_path

tests/integration/test_specfile.py

@@ -575,7 +575,7 @@ def test_parse_if_necessary(spec_macros):
     rpm.__version__ < "4.16",
     reason="condition expression evaluation requires rpm 4.16 or higher",
 )
-def test_update_version(spec_prerelease):
+def test_update_version(spec_prerelease, spec_conditionalized_version):
     spec = Specfile(spec_prerelease)
     prerelease_suffix_pattern = r"(-)rc\d+"
     prerelease_suffix_macro = "prerel"
@@ -609,3 +609,20 @@ def test_update_version(spec_prerelease):
         assert md.prerel.body == "rc2"
         assert md.prerel.commented_out
     assert spec.version == "%{pkgver}"
+    spec = Specfile(spec_conditionalized_version)
+    version = "0.1.3"
+    assert spec.version == "%{upstream_version}"
+    spec.update_version(version, prerelease_suffix_pattern)
+    with spec.macro_definitions() as md:
+        assert md.upstream_version.body == version
+    assert spec.version == "%{upstream_version}"
+    assert spec.expanded_version == version
+    spec = Specfile(spec_conditionalized_version)
+    with spec.macro_definitions() as md:
+        md.commit.commented_out = False
+    assert spec.version == "%{upstream_version}^git%{shortcommit}"
+    spec.update_version(version, prerelease_suffix_pattern)
+    with spec.macro_definitions() as md:
+        assert md.upstream_version.body != version
+    assert spec.version == version
+    assert spec.expanded_version == version