Merge pull request #117 from packit/macros

Provide context for macro expansions

When using multiple instances of Specfile, macros have to be expanded in the right context (because RPM uses a single global macro context, so this global context has to be emptied and repopulated on every context switch).
RELEASE NOTES BEGIN
Fixed an issue with multiple instances of Specfile not expanding macros in the right context.
RELEASE NOTES END

Reviewed-by: Tomas Tomecek <[email protected]>
Reviewed-by: Hunor Csomortáni <[email protected]>
Reviewed-by: Nikola Forró <None>

Author: softwarefactory-project-zuul[bot]

specfile/sourcelist.py

@@ -2,12 +2,15 @@
 # SPDX-License-Identifier: MIT
 
 import collections
-from typing import List, Optional, SupportsIndex, overload
+from typing import TYPE_CHECKING, List, Optional, SupportsIndex, overload
 
 from specfile.rpm import Macros
 from specfile.sections import Section
 from specfile.tags import Comments
 
+if TYPE_CHECKING:
+    from specfile.specfile import Specfile
+
 
 class SourcelistEntry:
     """
@@ -18,9 +21,23 @@ class SourcelistEntry:
         comments: List of comments associated with the source/patch.
     """
 
-    def __init__(self, location: str, comments: Comments) -> None:
+    def __init__(
+        self, location: str, comments: Comments, context: Optional["Specfile"] = None
+    ) -> None:
+        """
+        Constructs a `SourceListEntry` object.
+
+        Args:
+            location: Literal location of the source/patch as stored in the spec file.
+            comments: List of comments associated with the source/patch.
+            context: `Specfile` instance that defines the context for macro expansions.
+
+        Returns:
+            Constructed instance of `SourceListEntry` class.
+        """
         self.location = location
         self.comments = comments.copy()
+        self._context = context
 
     def __eq__(self, other: object) -> bool:
         if not isinstance(other, SourcelistEntry):
@@ -34,6 +51,8 @@ def __repr__(self) -> str:
     @property
     def expanded_location(self) -> str:
         """URL of the source/patch after expanding macros."""
+        if self._context:
+            return self._context.expand(self.location)
         return Macros.expand(self.location)
 
 
@@ -88,12 +107,15 @@ def copy(self) -> "Sourcelist":
         return Sourcelist(self.data, self._remainder)
 
     @classmethod
-    def parse(cls, section: Section) -> "Sourcelist":
+    def parse(
+        cls, section: Section, context: Optional["Specfile"] = None
+    ) -> "Sourcelist":
         """
         Parses a section into sources/patches.
 
         Args:
             section: %sourcelist/%patchlist section.
+            context: `Specfile` instance that defines the context for macro expansions.
 
         Returns:
             Constructed instance of `Sourcelist` class.
@@ -102,7 +124,7 @@ def parse(cls, section: Section) -> "Sourcelist":
         buffer: List[str] = []
         for line in section:
             if line and not line.lstrip().startswith("#"):
-                data.append(SourcelistEntry(line, Comments.parse(buffer)))
+                data.append(SourcelistEntry(line, Comments.parse(buffer), context))
                 buffer = []
             else:
                 buffer.append(line)

specfile/sources.py

@@ -5,14 +5,17 @@
 import re
 import urllib.parse
 from abc import ABC, abstractmethod
-from typing import Iterable, List, Optional, Tuple, Union, cast, overload
+from typing import TYPE_CHECKING, Iterable, List, Optional, Tuple, Union, cast, overload
 
 from specfile.exceptions import DuplicateSourceException
 from specfile.rpm import Macros
 from specfile.sourcelist import Sourcelist, SourcelistEntry
 from specfile.tags import Comments, Tag, Tags
 from specfile.utils import get_filename_from_location
 
+if TYPE_CHECKING:
+    from specfile.specfile import Specfile
+
 
 class Source(ABC):
     """Class that represents a source."""
@@ -218,6 +221,7 @@ def __init__(
         allow_duplicates: bool = False,
         default_to_implicit_numbering: bool = False,
         default_source_number_digits: int = 1,
+        context: Optional["Specfile"] = None,
     ) -> None:
         """
         Constructs a `Sources` object.
@@ -228,6 +232,7 @@ def __init__(
             allow_duplicates: Whether to allow duplicate entries when adding new sources.
             default_to_implicit_numbering: Use implicit numbering (no source numbers) by default.
             default_source_number_digits: Default number of digits in a source number.
+            context: `Specfile` instance that defines the context for macro expansions.
 
         Returns:
             Constructed instance of `Sources` class.
@@ -237,6 +242,7 @@ def __init__(
         self._allow_duplicates = allow_duplicates
         self._default_to_implicit_numbering = default_to_implicit_numbering
         self._default_source_number_digits = default_source_number_digits
+        self._context = context
 
     def __repr__(self) -> str:
         tags = repr(self._tags)
@@ -306,6 +312,11 @@ def __delitem__(self, i: Union[int, slice]) -> None:
             _, container, index = items[i]
             del container[index]
 
+    def _expand(self, s: str) -> str:
+        if self._context:
+            return self._context.expand(s)
+        return Macros.expand(s)
+
     def _get_tags(self) -> List[Tuple[TagSource, Tags, int]]:
         """
         Gets all tag sources.
@@ -474,7 +485,7 @@ def insert(self, i: int, location: str) -> None:
                 name, separator = self._get_tag_format(source, number)
                 container.insert(
                     index,
-                    Tag(name, location, Macros.expand(location), separator, Comments()),
+                    Tag(name, location, self._expand(location), separator, Comments()),
                 )
                 self._deduplicate_tag_names(i)
             else:
@@ -488,7 +499,7 @@ def insert(self, i: int, location: str) -> None:
             index, name, separator = self._get_initial_tag_setup()
             self._tags.insert(
                 index,
-                Tag(name, location, Macros.expand(location), separator, Comments()),
+                Tag(name, location, self._expand(location), separator, Comments()),
             )
 
     def insert_numbered(self, number: int, location: str) -> int:
@@ -522,7 +533,7 @@ def insert_numbered(self, number: int, location: str) -> int:
             i = 0
             index, name, separator = self._get_initial_tag_setup(number)
         self._tags.insert(
-            index, Tag(name, location, Macros.expand(location), separator, Comments())
+            index, Tag(name, location, self._expand(location), separator, Comments())
         )
         self._deduplicate_tag_names(i)
         return i

specfile/specfile.py

@@ -233,7 +233,9 @@ def sources(
         """
         with self.sections() as sections, self.tags(sections.package) as tags:
             sourcelists = [
-                (s, Sourcelist.parse(s)) for s in sections if s.name == "sourcelist"
+                (s, Sourcelist.parse(s, context=self))
+                for s in sections
+                if s.name == "sourcelist"
             ]
             try:
                 yield Sources(
@@ -242,6 +244,7 @@ def sources(
                     allow_duplicates,
                     default_to_implicit_numbering,
                     default_source_number_digits,
+                    context=self,
                 )
             finally:
                 for section, sourcelist in sourcelists:
@@ -267,7 +270,9 @@ def patches(
         """
         with self.sections() as sections, self.tags(sections.package) as tags:
             patchlists = [
-                (s, Sourcelist.parse(s)) for s in sections if s.name == "patchlist"
+                (s, Sourcelist.parse(s, context=self))
+                for s in sections
+                if s.name == "patchlist"
             ]
             try:
                 yield Patches(
@@ -276,6 +281,7 @@ def patches(
                     allow_duplicates,
                     default_to_implicit_numbering,
                     default_source_number_digits,
+                    context=self,
                 )
             finally:
                 for section, patchlist in patchlists:
@@ -572,7 +578,9 @@ class Entity:
         entities.update({k.upper(): v for k, v in entities.items() if v.type == Tag})
 
         def update(value, requested_value):
-            regex, template = ValueParser.construct_regex(value, entities.keys())
+            regex, template = ValueParser.construct_regex(
+                value, entities.keys(), context=self
+            )
             m = regex.match(requested_value)
             if m:
                 d = m.groupdict()

specfile/value_parser.py

@@ -5,12 +5,15 @@
 import re
 from abc import ABC
 from string import Template
-from typing import List, Tuple
+from typing import TYPE_CHECKING, List, Optional, Tuple
 from typing.re import Pattern
 
 from specfile.exceptions import UnterminatedMacroException
 from specfile.rpm import Macros
 
+if TYPE_CHECKING:
+    from specfile.specfile import Specfile
+
 SUBSTITUTION_GROUP_PREFIX = "sub_"
 
 
@@ -227,7 +230,10 @@ def find_macro_end(index):
 
     @classmethod
     def construct_regex(
-        cls, value: str, modifiable_entities: List[str]
+        cls,
+        value: str,
+        modifiable_entities: List[str],
+        context: Optional["Specfile"] = None,
     ) -> Tuple[Pattern, Template]:
         """
         Parses the given value and constructs a regex that allows matching
@@ -250,18 +256,24 @@ def construct_regex(
             value: Value string to parse.
             modifiable_entities: Names of modifiable entities, i.e. local macro definitions
               and tags.
+            context: `Specfile` instance that defines the context for macro expansions.
 
         Returns:
             Tuple in the form of (constructed regex, corresponding template).
         """
 
+        def expand(s):
+            if context:
+                return context.expand(s)
+            return Macros.expand(s)
+
         def flatten(nodes):
             # get rid of conditional macro expansions
             result = []
             for node in nodes:
                 if isinstance(node, ConditionalMacroExpansion):
                     # evaluate the condition
-                    if Macros.expand(f"%{node.prefix}{node.name}"):
+                    if expand(f"%{node.prefix}{node.name}"):
                         result.append(f"%{{{node.prefix}{node.name}:")
                         result.extend(flatten(node.body))
                         result.append("}")
@@ -281,13 +293,13 @@ def flatten(nodes):
             elif isinstance(node, StringLiteral):
                 tokens.append(("v", node.value, ""))
             elif isinstance(node, (ShellExpansion, ExpressionExpansion)):
-                const = Macros.expand(str(node))
+                const = expand(str(node))
                 tokens.append(("c", const, str(node)))
             elif isinstance(node, MacroSubstitution):
                 if node.prefix.count("!") % 2 == 0 and node.name in modifiable_entities:
                     tokens.append(("g", node.name, str(node)))
                 else:
-                    const = Macros.expand(str(node))
+                    const = expand(str(node))
                     tokens.append(("c", const, str(node)))
             elif isinstance(node, EnclosedMacroSubstitution):
                 if (
@@ -297,7 +309,7 @@ def flatten(nodes):
                 ):
                     tokens.append(("g", node.name, str(node)))
                 else:
-                    const = Macros.expand(str(node))
+                    const = expand(str(node))
                     tokens.append(("c", const, str(node)))
 
         def escape(s):

tests/integration/test_specfile.py

@@ -333,3 +333,14 @@ def test_update_tag(spec_macros):
         assert md.minorver.body == "2"
     with spec.sources() as sources:
         assert sources[1].location == "tests-86.tar.xz"
+
+
+def test_multiple_instances(spec_minimal, spec_autosetup):
+    spec1 = Specfile(spec_minimal)
+    spec2 = Specfile(spec_autosetup)
+    spec1.version = "14.2"
+    assert spec2.expanded_version == "0.1"
+    with spec2.sources() as sources:
+        assert sources[0].expanded_location == "test-0.1.tar.xz"
+        sources.append("tests-%{version}.tar.xz")
+        assert sources[1].expanded_location == "tests-0.1.tar.xz"