From 9a95cab0ffbe6b655ff6dbe8860ce9815b37cfa5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Nikola=20Forr=C3=B3?= <nforro@redhat.com>
Date: Tue, 25 Jun 2024 12:54:32 +0200
Subject: [PATCH] Avoid updating entity value if the entity is referenced from
 the new value
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Consider the folowing excerpt from a spec file:

```
%global rel 1%{?dist}

Release: %{rel}
```

Previously, calling `Specfile.update_tag("Release", "%rel")` resulted
in the macro definition being updated to `%global rel %rel`, making
the spec file invalid. After this change, the macro definition stays
unchanged and the tag is updated to `Release: %rel`.

Signed-off-by: Nikola Forró <nforro@redhat.com>
---
 specfile/specfile.py               | 32 ++++++++++++++++++++++++++++++
 tests/integration/test_specfile.py |  4 ++++
 2 files changed, 36 insertions(+)

diff --git a/specfile/specfile.py b/specfile/specfile.py
index d5120ec..cffdb9a 100644
--- a/specfile/specfile.py
+++ b/specfile/specfile.py
@@ -32,6 +32,7 @@
 from specfile.tags import Tag, Tags
 from specfile.value_parser import (
     SUBSTITUTION_GROUP_PREFIX,
+    ConditionalMacroExpansion,
     EnclosedMacroSubstitution,
     MacroSubstitution,
     ValueParser,
@@ -774,7 +775,35 @@ class Entity:
             )
         entities.sort(key=lambda e: e.position)
 
+        def find_reference(entity, value):
+            def traverse(nodes):
+                for node in nodes:
+                    if isinstance(
+                        node,
+                        (
+                            MacroSubstitution,
+                            EnclosedMacroSubstitution,
+                            ConditionalMacroExpansion,
+                        ),
+                    ):
+                        if (
+                            entity.type == Tag
+                            and entity.name == node.name.lower()
+                            or entity.name == node.name
+                        ):
+                            return True
+                    if isinstance(node, ConditionalMacroExpansion):
+                        if traverse(node.body):
+                            return True
+                return False
+
+            return traverse(ValueParser.parse(value))
+
         def update(value, requested_value, position):
+            if value == requested_value:
+                # nothing to do
+                return requested_value
+
             modifiable_entities = {
                 e.name
                 for e in entities
@@ -822,6 +851,9 @@ def update(value, requested_value, position):
                     if entity.locked:
                         # avoid infinite recursion
                         return requested_value
+                    if find_reference(entity, val):
+                        # avoid updating entity value if the entity is referenced from the new value
+                        return requested_value
                     entity.locked = True
                     try:
                         entity.value = update(entity.value, val, entity.position)
diff --git a/tests/integration/test_specfile.py b/tests/integration/test_specfile.py
index bc6f2c1..10afd79 100644
--- a/tests/integration/test_specfile.py
+++ b/tests/integration/test_specfile.py
@@ -405,6 +405,10 @@ def test_update_tag(spec_macros):
     assert spec.version == "%{package_version}"
     spec.update_tag("Release", "2%{?dist}")
     assert spec.raw_release == "%{release}"
+    with spec.macro_definitions() as md:
+        assert md.release.body == "2%{?dist}"
+    spec.update_tag("Release", "%release")
+    assert spec.raw_release == "%release"
     with spec.macro_definitions() as md:
         assert md.release.body == "2%{?dist}"
     spec.update_tag(