Merge pull request #153 from packit/context_management

Make context managers better

This commit introduces a new ContextManager decorator that builds on top of contextlib.contextmanager and effectively makes context managers a singleton. Any nested context of the same kind will be the same as the outermost context of that particular kind.
This makes it possible, for example, to set properties that internally use context managers inside an existing context, or in general mix and nest context managers almost freely.
There are still instances of conflicting context managers, for example macro_definitions() and sections(), that, when combined, will overwrite each other's underlying data. All such instances will be documented.
Now it is also possible to access data directly, avoiding the with statement, by using GeneratorContextManager.content property (I'm open to suggestions how to name it better):
for tag in spec.tags().content:
    print(f"{tag.name}: {tag.expanded_value}")
No modifications will be preserved though.

RELEASE NOTES BEGIN
Context managers (Specfile.sections(), Specfile.tags() etc.) can now be nested and combined together (with one exception - Specfile.macro_definitions()), and it is also possible to use tag properties (e.g. Specfile.version, Specfile.license) inside them. It is also possible to access the data directly, avoiding the with statement, by using the content property (e.g. Specfile.tags().content), but be aware that no modifications done to such data will be preserved. You must use with to make changes.
RELEASE NOTES END

Reviewed-by: František Nečas <[email protected]>
Reviewed-by: Nikola Forró <None>

Author: softwarefactory-project-zuul[bot]

specfile/context_management.py

@@ -0,0 +1,141 @@
+# Copyright Contributors to the Packit project.
+# SPDX-License-Identifier: MIT
+
+import collections
+import contextlib
+import io
+import os
+import pickle
+import sys
+import tempfile
+import types
+from typing import Any, Callable, Dict, Generator, List, Optional, overload
+
+
+@contextlib.contextmanager
+def capture_stderr() -> Generator[List[bytes], None, None]:
+    """
+    Context manager for capturing output to stderr. A stderr output of anything run
+    in its context will be captured in the target variable of the with statement.
+
+    Yields:
+        List of captured lines.
+    """
+    fileno = sys.__stderr__.fileno()
+    with tempfile.TemporaryFile() as stderr, os.fdopen(os.dup(fileno)) as backup:
+        sys.stderr.flush()
+        os.dup2(stderr.fileno(), fileno)
+        data: List[bytes] = []
+        try:
+            yield data
+        finally:
+            sys.stderr.flush()
+            os.dup2(backup.fileno(), fileno)
+            stderr.flush()
+            stderr.seek(0, io.SEEK_SET)
+            data.extend(stderr.readlines())
+
+
+class GeneratorContextManager(contextlib._GeneratorContextManager):
+    """
+    Extended contextlib._GeneratorContextManager that provides get() method.
+    """
+
+    def __init__(self, function: Callable) -> None:
+        super().__init__(function, tuple(), {})
+
+    def __del__(self) -> None:
+        # make sure the generator is fully consumed, as it is possible
+        # that neither __enter__() nor content() have been called
+        collections.deque(self.gen, maxlen=0)
+
+    @property
+    def content(self) -> Any:
+        """
+        Fully consumes the underlying generator and returns the yielded value.
+
+        Returns:
+            Value that would normally be the target variable of an associated with statement.
+
+        Raises:
+            StopIteration if the underlying generator is already exhausted.
+        """
+        result = next(self.gen)
+        next(self.gen, None)
+        return result
+
+
+class ContextManager:
+    """
+    Class for decorating generator functions that should act as a context manager.
+
+    Just like with contextlib.contextmanager, the generator returned from the decorated function
+    must yield exactly one value that will be used as the target variable of the with statement.
+    If the same function with the same arguments is called again from within previously generated
+    context, the generator will be ignored and the target variable will be reused.
+
+    Attributes:
+        function: Decorated generator function.
+        generators: Mapping of serialized function arguments to generators.
+        values: Mapping of serialized function arguments to yielded values.
+    """
+
+    def __init__(self, function: Callable) -> None:
+        self.function = function
+        self.is_bound = False
+        self.generators: Dict[bytes, Generator[Any, None, None]] = {}
+        self.values: Dict[bytes, Any] = {}
+
+    @overload
+    def __get__(self, obj: None, objtype: Optional[type] = None) -> "ContextManager":
+        pass
+
+    @overload
+    def __get__(self, obj: object, objtype: Optional[type] = None) -> types.MethodType:
+        pass
+
+    # implementing __get__() makes the class a non-data descriptor,
+    # so it can be used as method decorator
+    def __get__(self, obj, objtype=None):
+        if obj is None:
+            return self
+        self.is_bound = True
+        return types.MethodType(self, obj)
+
+    def __call__(self, *args: Any, **kwargs: Any) -> GeneratorContextManager:
+        # serialize the passed arguments, excluding cls/self if present
+        key = pickle.dumps(
+            (args[1:] if self.is_bound else args, sorted(kwargs.items())),
+            protocol=pickle.HIGHEST_PROTOCOL,
+        )
+        if (
+            key in self.generators
+            # gi_frame is None only in case generator is exhausted
+            and self.generators[key].gi_frame is not None  # type: ignore[attr-defined]
+        ):
+            # generator is suspended, use existing value
+            def existing_value():
+                try:
+                    yield self.values[key]
+                except KeyError:
+                    # if the generator is being consumed in GeneratorContextManager destructor,
+                    # self.values[key] could have already been deleted
+                    pass
+
+            return GeneratorContextManager(existing_value)
+        # create the generator
+        self.generators[key] = self.function(*args, **kwargs)
+        # first iteration yields the value
+        self.values[key] = next(self.generators[key])
+
+        def new_value():
+            try:
+                yield self.values[key]
+            finally:
+                # second iteration wraps things up
+                next(self.generators[key], None)
+                # the generator is now exhausted and the value is no longer valid
+                del self.generators[key]
+                del self.values[key]
+
+        return GeneratorContextManager(new_value)

specfile/macros.py

@@ -8,8 +8,8 @@
 
 import rpm
 
+from specfile.context_management import capture_stderr
 from specfile.exceptions import MacroRemovalException, RPMException
-from specfile.utils import capture_stderr
 
 MAX_REMOVAL_RETRIES = 20
 

specfile/spec_parser.py

@@ -11,11 +11,12 @@
 
 import rpm
 
+from specfile.context_management import capture_stderr
 from specfile.exceptions import RPMException
 from specfile.macros import Macros
 from specfile.sections import Section
 from specfile.tags import Tags
-from specfile.utils import capture_stderr, get_filename_from_location
+from specfile.utils import get_filename_from_location
 from specfile.value_parser import ConditionalMacroExpansion, ShellExpansion, ValueParser
 
 logger = logging.getLogger(__name__)

specfile/specfile.py

@@ -1,18 +1,18 @@
 # Copyright Contributors to the Packit project.
 # SPDX-License-Identifier: MIT
 
-import contextlib
 import datetime
 import re
 import subprocess
 import types
 from dataclasses import dataclass
 from pathlib import Path
-from typing import Iterator, List, Optional, Tuple, Type, Union
+from typing import Generator, List, Optional, Tuple, Type, Union
 
 import rpm
 
 from specfile.changelog import Changelog, ChangelogEntry
+from specfile.context_management import ContextManager
 from specfile.exceptions import SourceNumberException, SpecfileException
 from specfile.macro_definitions import MacroDefinition, MacroDefinitions
 from specfile.macros import Macro, Macros
@@ -148,8 +148,8 @@ def get_active_macros(self) -> List[Macro]:
         self._parser.parse(str(self))
         return Macros.dump()
 
-    @contextlib.contextmanager
-    def lines(self) -> Iterator[List[str]]:
+    @ContextManager
+    def lines(self) -> Generator[List[str], None, None]:
         """
         Context manager for accessing spec file lines.
 
@@ -163,8 +163,8 @@ def lines(self) -> Iterator[List[str]]:
             if self.autosave:
                 self.save()
 
-    @contextlib.contextmanager
-    def macro_definitions(self) -> Iterator[MacroDefinitions]:
+    @ContextManager
+    def macro_definitions(self) -> Generator[MacroDefinitions, None, None]:
         """
         Context manager for accessing macro definitions.
 
@@ -178,8 +178,8 @@ def macro_definitions(self) -> Iterator[MacroDefinitions]:
             finally:
                 lines[:] = macro_definitions.get_raw_data()
 
-    @contextlib.contextmanager
-    def sections(self) -> Iterator[Sections]:
+    @ContextManager
+    def sections(self) -> Generator[Sections, None, None]:
         """
         Context manager for accessing spec file sections.
 
@@ -200,8 +200,10 @@ def parsed_sections(self) -> Optional[Sections]:
             return None
         return Sections.parse(self._parser.spec.parsed.splitlines())
 
-    @contextlib.contextmanager
-    def tags(self, section: Union[str, Section] = "package") -> Iterator[Tags]:
+    @ContextManager
+    def tags(
+        self, section: Union[str, Section] = "package"
+    ) -> Generator[Tags, None, None]:
         """
         Context manager for accessing tags in a specified section.
 
@@ -212,26 +214,21 @@ def tags(self, section: Union[str, Section] = "package") -> Iterator[Tags]:
         Yields:
             Tags in the section as `Tags` object.
         """
-        if isinstance(section, Section):
-            raw_section = section
-            parsed_section = getattr(self.parsed_sections, section.name, None)
+        with self.sections() as sections:
+            if isinstance(section, Section):
+                raw_section = section
+                parsed_section = getattr(self.parsed_sections, section.name, None)
+            else:
+                raw_section = getattr(sections, section)
+                parsed_section = getattr(self.parsed_sections, section, None)
             tags = Tags.parse(raw_section, parsed_section)
             try:
                 yield tags
             finally:
                 raw_section.data = tags.get_raw_section_data()
-        else:
-            with self.sections() as sections:
-                raw_section = getattr(sections, section)
-                parsed_section = getattr(self.parsed_sections, section, None)
-                tags = Tags.parse(raw_section, parsed_section)
-                try:
-                    yield tags
-                finally:
-                    raw_section.data = tags.get_raw_section_data()
 
-    @contextlib.contextmanager
-    def changelog(self) -> Iterator[Optional[Changelog]]:
+    @ContextManager
+    def changelog(self) -> Generator[Optional[Changelog], None, None]:
         """
         Context manager for accessing changelog.
 
@@ -250,8 +247,8 @@ def changelog(self) -> Iterator[Optional[Changelog]]:
                 finally:
                     section.data = changelog.get_raw_section_data()
 
-    @contextlib.contextmanager
-    def prep(self) -> Iterator[Optional[Prep]]:
+    @ContextManager
+    def prep(self) -> Generator[Optional[Prep], None, None]:
         """
         Context manager for accessing %prep section.
 
@@ -270,13 +267,13 @@ def prep(self) -> Iterator[Optional[Prep]]:
                 finally:
                     section.data = prep.get_raw_section_data()
 
-    @contextlib.contextmanager
+    @ContextManager
     def sources(
         self,
         allow_duplicates: bool = False,
         default_to_implicit_numbering: bool = False,
         default_source_number_digits: int = 1,
-    ) -> Iterator[Sources]:
+    ) -> Generator[Sources, None, None]:
         """
         Context manager for accessing sources.
 
@@ -288,7 +285,7 @@ def sources(
         Yields:
             Spec file sources as `Sources` object.
         """
-        with self.sections() as sections, self.tags(sections.package) as tags:
+        with self.sections() as sections, self.tags() as tags:
             sourcelists = [
                 (s, Sourcelist.parse(s, context=self))
                 for s in sections
@@ -307,13 +304,13 @@ def sources(
                 for section, sourcelist in sourcelists:
                     section.data = sourcelist.get_raw_section_data()
 
-    @contextlib.contextmanager
+    @ContextManager
     def patches(
         self,
         allow_duplicates: bool = False,
         default_to_implicit_numbering: bool = False,
         default_source_number_digits: int = 1,
-    ) -> Iterator[Patches]:
+    ) -> Generator[Patches, None, None]:
         """
         Context manager for accessing patches.
 
@@ -325,7 +322,7 @@ def patches(
         Yields:
             Spec file patches as `Patches` object.
         """
-        with self.sections() as sections, self.tags(sections.package) as tags:
+        with self.sections() as sections, self.tags() as tags:
             patchlists = [
                 (s, Sourcelist.parse(s, context=self))
                 for s in sections

specfile/utils.py

@@ -2,13 +2,7 @@
 # SPDX-License-Identifier: MIT
 
 import collections
-import contextlib
-import io
-import os
 import re
-import sys
-import tempfile
-from typing import Iterator, List
 
 from specfile.constants import ARCH_NAMES
 from specfile.exceptions import SpecfileException
@@ -120,30 +114,6 @@ def from_string(cls, nevra: str) -> "NEVRA":
         return cls(name=n, epoch=int(e) if e else 0, version=v, release=r, arch=a)
 
 
-@contextlib.contextmanager
-def capture_stderr() -> Iterator[List[bytes]]:
-    """
-    Context manager for capturing output to stderr. A stderr output of anything run
-    in its context will be captured in the target variable of the with statement.
-
-    Yields:
-        List of captured lines.
-    """
-    fileno = sys.__stderr__.fileno()
-    with tempfile.TemporaryFile() as stderr, os.fdopen(os.dup(fileno)) as backup:
-        sys.stderr.flush()
-        os.dup2(stderr.fileno(), fileno)
-        data: List[bytes] = []
-        try:
-            yield data
-        finally:
-            sys.stderr.flush()
-            os.dup2(backup.fileno(), fileno)
-            stderr.flush()
-            stderr.seek(0, io.SEEK_SET)
-            data.extend(stderr.readlines())
-
-
 def get_filename_from_location(location: str) -> str:
     """
     Extracts filename from given source location.

tests/integration/test_specfile.py

@@ -391,3 +391,18 @@ def test_shell_expansions(spec_shell_expansions):
     spec = Specfile(spec_shell_expansions)
     assert spec.expanded_version == "1035.4200"
     assert "C.UTF-8" in spec.expand("%numeric_locale")
+
+
+def test_context_management(spec_autosetup):
+    spec = Specfile(spec_autosetup)
+    with spec.tags() as tags:
+        tags.license.value = "BSD"
+        assert spec.license == "BSD"
+        spec.license = "BSD-3-Clause"
+        tags.patch0.value = "first_patch.patch"
+        with spec.patches() as patches:
+            assert patches[0].location == "first_patch.patch"
+            patches[0].location = "patch_0.patch"
+    assert spec.license == "BSD-3-Clause"
+    with spec.patches() as patches:
+        assert patches[0].location == "patch_0.patch"