Avoid updating entity value if the entity is referenced from the new value (#388)

Avoid updating entity value if the entity is referenced from the new value

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.

Reviewed-by: Laura Barcziová

Author: softwarefactory-project-zuul[bot]

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)

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(