Parse only if necessary

Parsing before every macro expansion etc. is unnecessary and can cause
performance issues, because parsing is a really slow operation.

Parse only after a context switch (necessary because all `Specfile`
instances share a single global macro context) or if something that
affects parsing has actually changed.

Signed-off-by: Nikola Forró <[email protected]>

Author: nforro

specfile/sections.py

@@ -231,12 +231,7 @@ def parse(
 
         def expand(s):
             if context:
-                result = context.expand(
-                    s, skip_parsing=getattr(expand, "skip_parsing", False)
-                )
-                # parse only once
-                expand.skip_parsing = True
-                return result
+                return context.expand(s)
             return Macros.expand(s)
 
         def split_id(line):

specfile/spec_parser.py

@@ -3,8 +3,10 @@
 
 import contextlib
 import copy
+import hashlib
 import logging
 import os
+import pickle
 import re
 import tempfile
 from pathlib import Path
@@ -41,6 +43,9 @@ class SpecParser:
           and were replaced with dummy files.
     """
 
+    # hash of input parameters to the last parse performed
+    _last_parse_hash = None
+
     def __init__(
         self,
         sourcedir: Path,
@@ -325,11 +330,32 @@ def parse(
         Raises:
             RPMException, if parsing error occurs.
         """
+        # calculate hash of all input parameters
+        payload = (
+            id(self),
+            self.sourcedir,
+            self.macros,
+            self.force_parse,
+            content,
+            extra_macros,
+        )
+        parse_hash = hashlib.sha256(
+            pickle.dumps(payload, protocol=pickle.HIGHEST_PROTOCOL)
+        ).digest()
+        if parse_hash == SpecParser._last_parse_hash:
+            # none of the input parameters has changed, no need to parse again
+            return
         if self.spec:
             # workaround RPM lua tables feature/bug, see above for details
             del self.spec
         try:
-            self.spec, self.tainted = self._do_parse(content, extra_macros)
+            try:
+                self.spec, self.tainted = self._do_parse(content, extra_macros)
+            except Exception:
+                SpecParser._last_parse_hash = None
+                raise
+            else:
+                SpecParser._last_parse_hash = parse_hash
         except RPMException:
             self.spec = None
             self.tainted = False

specfile/specfile.py

@@ -64,7 +64,6 @@ def __init__(
         self._parser = SpecParser(
             Path(sourcedir or self.path.parent), macros, force_parse
         )
-        # parse here to fail early on parsing errors
         self._parser.parse(str(self))
 
     def __eq__(self, other: object) -> bool:
@@ -161,23 +160,18 @@ def expand(
         self,
         expression: str,
         extra_macros: Optional[List[Tuple[str, str]]] = None,
-        skip_parsing: bool = False,
     ) -> str:
         """
         Expands an expression in the context of the spec file.
 
         Args:
             expression: Expression to expand.
             extra_macros: Extra macros to be defined before expansion is performed.
-            skip_parsing: Do not parse the spec file before expansion is performed.
-              Defaults to False. Mutually exclusive with extra_macros. Set this to True
-              only if you are certain that the global macro context is up-to-date.
 
         Returns:
             Expanded expression.
         """
-        if not skip_parsing:
-            self._parser.parse(str(self), extra_macros)
+        self._parser.parse(str(self), extra_macros)
         return Macros.expand(expression)
 
     def get_active_macros(self) -> List[Macro]:

specfile/value_parser.py

@@ -312,12 +312,7 @@ def construct_regex(
 
         def expand(s):
             if context:
-                result = context.expand(
-                    s, skip_parsing=getattr(expand, "skip_parsing", False)
-                )
-                # parse only once
-                expand.skip_parsing = True
-                return result
+                return context.expand(s)
             return Macros.expand(s)
 
         def flatten(nodes):

tests/integration/test_specfile.py

@@ -12,7 +12,7 @@
 from specfile.exceptions import RPMException, SpecfileException
 from specfile.prep import AutopatchMacro, AutosetupMacro, PatchMacro, SetupMacro
 from specfile.sections import Section
-from specfile.specfile import Specfile
+from specfile.specfile import Specfile, SpecParser
 
 
 def test_parse(spec_multiple_sources):
@@ -481,3 +481,21 @@ def test_copy(spec_autosetup):
     assert deep_copy is not spec
     assert deep_copy._lines is not spec._lines
     assert deep_copy._parser is not spec._parser
+
+
+def test_parse_if_necessary(spec_macros):
+    flexmock(SpecParser).should_call("_do_parse").once()
+    spec1 = Specfile(spec_macros)
+    spec2 = copy.deepcopy(spec1)
+    flexmock(SpecParser).should_call("_do_parse").never()
+    assert spec1.expanded_name == "test"
+    flexmock(SpecParser).should_call("_do_parse").once()
+    assert spec2.expanded_name == "test"
+    assert spec2.expanded_version == "0.1.2~rc2"
+    flexmock(SpecParser).should_call("_do_parse").once()
+    assert spec1.expanded_version == "0.1.2~rc2"
+    with spec1.macro_definitions() as md:
+        md[0].body = "28"
+    flexmock(SpecParser).should_call("_do_parse").once()
+    assert spec1.expanded_name == "test"
+    assert spec1.expanded_version == "28.1.2~rc2"