Fix update_value() (#340)

Fix `update_value()`

The implementation didn't account for a case when a macro definition needed to be commented out to succesfully set the new value but it was commented out already. This commit fixes that.

Reviewed-by: František Lachman <[email protected]>
Reviewed-by: Laura Barcziová
Reviewed-by: Nikola Forró

specfile/specfile.py

@@ -783,12 +783,16 @@ def update(value, requested_value, position):
                 and e.type == MacroDefinition
                 and not e.flip_pending
             }
-            regex, template, entities_to_flip = ValueParser.construct_regex(
-                value, modifiable_entities, flippable_entities, context=self
-            )
 
-            m = regex.match(requested_value)
-            if m:
+            # in case the value doesn't match after trying with flippable entities
+            # do a second pass without them
+            for flippable_ents in (flippable_entities, set()):
+                regex, template, entities_to_flip = ValueParser.construct_regex(
+                    value, modifiable_entities, flippable_ents, context=self
+                )
+                m = regex.match(requested_value)
+                if not m:
+                    continue
                 d = m.groupdict()
                 for grp, val in d.items():
                     if grp.startswith(SUBSTITUTION_GROUP_PREFIX):

tests/constants.py

@@ -14,6 +14,7 @@
 SPEC_INCLUDES = DATA_DIR / "spec_includes"
 SPEC_MACROS = DATA_DIR / "spec_macros"
 SPEC_PRERELEASE = DATA_DIR / "spec_prerelease"
+SPEC_PRERELEASE2 = DATA_DIR / "spec_prerelease2"
 SPEC_MULTIPLE_SOURCES = DATA_DIR / "spec_multiple_sources"
 SPEC_COMMENTED_PATCHES = DATA_DIR / "spec_commented_patches"
 SPEC_SHELL_EXPANSIONS = DATA_DIR / "spec_shell_expansions"

tests/data/spec_prerelease2/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_prerelease2/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_prerelease2/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_prerelease2/test.spec

@@ -0,0 +1,34 @@
+%global majorver 0
+%global minorver 1
+%global patchver 2
+%global basever %{majorver}.%{minorver}.%{patchver}
+
+%global prerel rc2
+
+%global pkgver %{basever}%{?prerel:~%{prerel}}
+%global upsver %{basever}%{?prerel:-%{prerel}}
+
+Name:           test
+Version:        %{pkgver}
+Release:        1%{?dist}
+Summary:        Test package
+
+License:        MIT
+
+Source0:        https://example.com/archive/%{name}/v%{majorver}.%{minorver}/%{name}-v%{upsver}.tar.xz
+Patch0:         patch0.patch
+Patch1:         patch1.patch
+Patch2:         patch2.patch
+
+
+%description
+Test package
+
+
+%prep
+%autosetup -p1 -n %{name}-%{upsver}
+
+
+%changelog
+* Thu Jun 07 2018 Nikola Forró <[email protected]> - 0.1.2~rc2-1
+- first version

tests/integration/conftest.py

@@ -17,6 +17,7 @@
     SPEC_MULTIPLE_SOURCES,
     SPEC_PATCHLIST,
     SPEC_PRERELEASE,
+    SPEC_PRERELEASE2,
     SPEC_RPMAUTOSPEC,
     SPEC_SHELL_EXPANSIONS,
     SPEC_TRADITIONAL,
@@ -87,6 +88,13 @@ def spec_prerelease(tmp_path):
     return destination / SPECFILE
 
 
+@pytest.fixture(scope="function")
+def spec_prerelease2(tmp_path):
+    destination = tmp_path / "spec_prerelease2"
+    shutil.copytree(SPEC_PRERELEASE2, destination)
+    return destination / SPECFILE
+
+
 @pytest.fixture(scope="function")
 def spec_multiple_sources(tmp_path):
     destination = tmp_path / "spec_multiple_sources"

tests/integration/test_specfile.py

@@ -575,7 +575,9 @@ 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, spec_conditionalized_version):
+def test_update_version(
+    spec_prerelease, spec_prerelease2, spec_conditionalized_version
+):
     spec = Specfile(spec_prerelease)
     prerelease_suffix_pattern = r"(-)rc\d+"
     prerelease_suffix_macro = "prerel"
@@ -609,6 +611,39 @@ def test_update_version(spec_prerelease, spec_conditionalized_version):
         assert md.prerel.body == "rc2"
         assert md.prerel.commented_out
     assert spec.version == "%{pkgver}"
+    spec = Specfile(spec_prerelease2)
+    prerelease_suffix_pattern = r"(-)rc\d+"
+    prerelease_suffix_macro = "prerel"
+    spec.update_version("0.1.2", prerelease_suffix_pattern, prerelease_suffix_macro)
+    with spec.macro_definitions() as md:
+        assert md.majorver.body == "0"
+        assert md.minorver.body == "1"
+        assert md.patchver.body == "2"
+        assert md.basever.body == "%{majorver}.%{minorver}.%{patchver}"
+        assert md.prerel.body == "rc2"
+        assert md.prerel.commented_out
+    assert spec.version == "%{pkgver}"
+    spec.update_version("0.1.3-rc1", prerelease_suffix_pattern, prerelease_suffix_macro)
+    with spec.macro_definitions() as md:
+        assert md.majorver.body == "0"
+        assert md.minorver.body == "1"
+        assert md.patchver.body == "3"
+        assert md.basever.body == "%{majorver}.%{minorver}.%{patchver}"
+        assert md.prerel.body == "rc1"
+        assert not md.prerel.commented_out
+    assert spec.version == "%{pkgver}"
+    spec = Specfile(spec_prerelease2)
+    with spec.macro_definitions() as md:
+        md.prerel.commented_out = True
+    spec.update_version("0.1.3-rc1", prerelease_suffix_pattern)
+    with spec.macro_definitions() as md:
+        assert md.majorver.body == "0"
+        assert md.minorver.body == "1"
+        assert md.patchver.body == "3"
+        assert md.basever.body == "%{majorver}.%{minorver}.%{patchver}"
+        assert md.prerel.body == "rc1"
+        assert not md.prerel.commented_out
+    assert spec.version == "%{pkgver}"
     spec = Specfile(spec_conditionalized_version)
     version = "0.1.3"
     assert spec.version == "%{upstream_version}"