Fixes several issues found by the new Grapher module work (#105)

* Addresses issue where variables that can't be rendered are 569XZilmssign escaped when get_value() is called with the sub_vars flag

* Fixes pickling SentinelTypes and guards against bad dependency data

* Adds integration test to detect pickling regressions

* Fixes default initialization type in get_all_dependencies()

* Adds SentinelType pickle integration test.

* Implements @dholth's approach for the SentinelType

conda_recipe_manager/grapher/recipe_graph.py

@@ -101,12 +101,12 @@ def __init__(
                     match dep.type:
                         # Build graph
                         case DependencySection.BUILD | DependencySection.HOST:
-                            self._build_graph.add_edge(package_name, dep.match_spec.name)  # type: ignore[misc]
+                            self._build_graph.add_edge(package_name, dep.data.name)  # type: ignore[misc]
 
                         # Test graph
                         # TODO does this include run constraints?
                         case DependencySection.RUN | DependencySection.TESTS:
-                            self._test_graph.add_edge(package_name, dep.match_spec.name)  # type: ignore[misc]
+                            self._test_graph.add_edge(package_name, dep.data.name)  # type: ignore[misc]
 
     def __bool__(self) -> bool:
         """

conda_recipe_manager/parser/dependency.py

@@ -9,6 +9,7 @@
 
 from conda.models.match_spec import MatchSpec
 
+from conda_recipe_manager.parser._types import Regex
 from conda_recipe_manager.parser.selector_parser import SelectorParser
 from conda_recipe_manager.parser.types import SchemaVersion
 
@@ -97,6 +98,49 @@ def str_to_dependency_section(s: str) -> Optional[DependencySection]:
             return None
 
 
+class DependencyVariable:
+    """
+    Represents a dependency that contains a JINJA variable that is unable to be resolved by the recipe's variable table.
+    """
+
+    def __init__(self, s: str):
+        """
+        Constructs a DependencyVariable instance.
+
+        :param s: String to initialize the instance with.
+        """
+        # Using `name` allows this class to be used trivially with MatchSpec without type guards.
+        # TODO normalize common JINJA functions for quote usage
+        self.name = s
+
+    def __eq__(self, o: object) -> bool:
+        """
+        Checks to see if two objects are equivalent.
+
+        :param o: Other instance to check.
+        :returns: True if two DependencyVariable instances are equivalent. False otherwise.
+        """
+        if not isinstance(o, DependencyVariable):
+            return False
+        return self.name == o.name
+
+
+# Type alias for types allowed in a Dependency's `data` field.
+DependencyData = MatchSpec | DependencyVariable
+
+
+def dependency_data_from_string(s: str) -> DependencyData:
+    """
+    Constructs a DataDependency object from a dependency string in a recipe file.
+
+    :param s: String to process.
+    :returns: A DataDependency instance.
+    """
+    if Regex.JINJA_V1_SUB.search(s):
+        return DependencyVariable(s)
+    return MatchSpec(s)
+
+
 class Dependency(NamedTuple):
     """
     Structure that contains metadata about a dependency found in the recipe. This is immutable by design.
@@ -108,8 +152,8 @@ class Dependency(NamedTuple):
     path: str
     # Identifies what kind of dependency this is
     type: DependencySection
-    # Parses the dependency's name and version constraints
-    match_spec: MatchSpec
+    # Parsed dependency. Identifies a name and version constraints
+    data: DependencyData
     # The selector applied to this dependency, if applicable
     selector: Optional[SelectorParser] = None
 

conda_recipe_manager/parser/recipe_parser_deps.py

@@ -6,10 +6,13 @@
 
 from typing import Final, Optional, cast
 
-from conda.models.match_spec import MatchSpec
-
 from conda_recipe_manager.parser._types import ROOT_NODE_VALUE
-from conda_recipe_manager.parser.dependency import Dependency, DependencyMap, str_to_dependency_section
+from conda_recipe_manager.parser.dependency import (
+    Dependency,
+    DependencyMap,
+    dependency_data_from_string,
+    str_to_dependency_section,
+)
 from conda_recipe_manager.parser.recipe_reader import RecipeReader
 from conda_recipe_manager.parser.selector_parser import SelectorParser
 from conda_recipe_manager.parser.types import SchemaVersion
@@ -34,7 +37,7 @@ def _add_top_level_dependencies(root_package: str, dep_map: DependencyMap) -> No
                 continue
             # Change the "required_by" package name to the current package, not the root package name.
             dep_map[package].extend(
-                [Dependency(package, d.path, d.type, d.match_spec, d.selector) for d in root_dependencies]
+                [Dependency(package, d.path, d.type, d.data, d.selector) for d in root_dependencies]
             )
 
     def _fetch_optional_selector(self, path: str) -> Optional[SelectorParser]:
@@ -98,8 +101,8 @@ def get_all_dependencies(self) -> DependencyMap:
                 root_package = package
 
             requirements = cast(
-                dict[str, list[str]],
-                self.get_value(RecipeReader.append_to_path(path, "/requirements"), default=[], sub_vars=True),
+                dict[str, list[str | None]],
+                self.get_value(RecipeReader.append_to_path(path, "/requirements"), default={}, sub_vars=True),
             )
             dep_map[package] = []
             for section_str, deps in requirements.items():
@@ -109,14 +112,22 @@ def get_all_dependencies(self) -> DependencyMap:
                     continue
 
                 for i, dep in enumerate(deps):
+                    # Sanitize and ignore empty data. In theory, recipes shouldn't contain `Null`s or empty strings in
+                    # their dependency lists, but we will guard against it out of an abundance of caution.
+                    if dep is None:
+                        continue
+                    dep = dep.strip()
+                    if not dep:
+                        continue
+
                     # NOTE: `get_dependency_paths()` uses the same approach for calculating dependency paths.
                     dep_path = RecipeReader.append_to_path(path, f"/requirements/{section_str}/{i}")
                     dep_map[package].append(
                         Dependency(
                             required_by=package,
                             path=dep_path,
                             type=section,
-                            match_spec=MatchSpec(dep),
+                            data=dependency_data_from_string(dep),
                             selector=self._fetch_optional_selector(dep_path),
                         )
                     )

conda_recipe_manager/parser/recipe_reader.py

@@ -253,6 +253,12 @@ def _set_on_schema_version() -> tuple[int, re.Pattern[str]]:
                 if lower_match:
                     value = value.lower()
                 s = s.replace(match, value)
+            # $-Escaping the unresolved variable does a few things:
+            #   - Clearly identifies the value as an unresolved variable
+            #   - Normalizes the substitution syntax with V1
+            #   - Ensures the returned value is YAML-parsable
+            elif self._schema_version == SchemaVersion.V0:
+                s = f"${s}"
         return cast(JsonType, yaml.safe_load(s))
 
     def _init_vars_tbl(self) -> None:
@@ -737,7 +743,7 @@ def get_value(self, path: str, default: JsonType | SentinelType = _sentinel, sub
         :param path: JSON patch (RFC 6902)-style path to a value.
         :param default: (Optional) If the value is not found, return this value instead.
         :param sub_vars: (Optional) If set to True and the value contains a Jinja template variable, the Jinja value
-            will be "rendered".
+            will be "rendered". Any variables that can't be resolved will be escaped with `${{ }}`.
         :raises KeyError: If the value is not found AND no default is specified
         :returns: If found, the value in the recipe at that path. Otherwise, the caller-specified default value.
         """

conda_recipe_manager/types.py

@@ -34,10 +34,30 @@
 # Generic, hashable type
 H = TypeVar("H", bound=Hashable)
 
+# Bootstraps global singleton used by `SentinelType`
+_schema_type_singleton: SentinelType
+
 
-# All sentinel values used in this module should be constructed with this class, for typing purposes.
 class SentinelType:
-    pass
+    """
+    A single sentinel class to be used in this project, as an alternative to `None` when `None` cannot be used.
+    It is defined in a way such that SentinelType instances survive pickling and allocations in different memory
+    spaces.
+    """
+
+    def __new__(cls) -> SentinelType:
+        """
+        Constructs a global singleton SentinelType instance, once.
+
+        :returns: The SentinelType instance
+        """
+        # Credit to @dholth for suggesting this approach in PR #105.
+        global _schema_type_singleton
+        try:
+            return _schema_type_singleton
+        except NameError:
+            _schema_type_singleton = super().__new__(cls)
+            return _schema_type_singleton
 
 
 class MessageCategory(StrEnum):

tests/parser/test_pickle_integration.py

@@ -0,0 +1,75 @@
+"""
+:Description: Integration tests for the RecipeParserConvert class
+"""
+
+from __future__ import annotations
+
+import pickle
+from typing import Callable
+
+import pytest
+
+from conda_recipe_manager.parser.recipe_parser import RecipeParser
+from conda_recipe_manager.parser.recipe_parser_convert import RecipeParserConvert
+from conda_recipe_manager.parser.recipe_parser_deps import RecipeParserDeps
+from conda_recipe_manager.parser.selector_parser import SelectorParser
+from conda_recipe_manager.parser.types import SchemaVersion
+from conda_recipe_manager.types import SentinelType
+from tests.file_loading import TEST_FILES_PATH, load_file
+
+
+def test_pickle_integration_sentinel_type() -> None:
+    """
+    Verifies that the `SentinelType` is pickle-able. All `SentinelType` instances act as a second `None`-type.
+    """
+    assert pickle.loads(pickle.dumps(SentinelType())) == SentinelType()  # type: ignore[misc]
+
+
+@pytest.mark.parametrize(
+    "file,constructor",
+    [
+        ("types-toml.yaml", RecipeParser),
+        ("cctools-ld64.yaml", RecipeParser),
+        ("v1_format/v1_types-toml.yaml", RecipeParser),
+        ("v1_format/v1_cctools-ld64.yaml", RecipeParser),
+        ("types-toml.yaml", RecipeParserConvert),
+        ("cctools-ld64.yaml", RecipeParserConvert),
+        ("v1_format/v1_types-toml.yaml", RecipeParserConvert),
+        ("v1_format/v1_cctools-ld64.yaml", RecipeParserConvert),
+        ("types-toml.yaml", RecipeParserDeps),
+        ("cctools-ld64.yaml", RecipeParserDeps),
+        ("v1_format/v1_types-toml.yaml", RecipeParserDeps),
+        ("v1_format/v1_cctools-ld64.yaml", RecipeParserDeps),
+    ],
+)
+def test_pickle_integration_recipe_parsers(file: str, constructor: Callable[[str], RecipeParser]) -> None:
+    """
+    Verifies that recipe parsers can be round-tripped with `pickle`'s serialization system. This ensures RecipeParser
+    classes can be used with standard libraries that utilize `pickle`, like `multiprocessing`.
+
+    Prior to this test, `RecipeParser` classes could not be reliably pickled due to the original implementation of the
+    `SentinelType`. This caused some data corruption issues when `RecipeParser` instances were being generated by
+    the thread pool tooling available in `multiprocessing`.
+
+    More details about this problem can be found in this PR:
+      https://github.com/conda-incubator/conda-recipe-manager/pull/105
+    """
+    file_text = load_file(TEST_FILES_PATH / file)
+    parser = constructor(file_text)
+    assert pickle.loads(pickle.dumps(parser)).render() == parser.render()  # type: ignore[misc]
+
+
+@pytest.mark.parametrize(
+    "selector,schema",
+    [
+        # TODO: Add V1 when V1 selectors are supported
+        ("[osx]", SchemaVersion.V0),
+        ("[osx and win-32]", SchemaVersion.V0),
+    ],
+)
+def test_pickle_integration_selector_parser(selector: str, schema: SchemaVersion) -> None:
+    """
+    This test ensures that the SelectorParser class is `pickle`-able.
+    """
+    parser = SelectorParser(selector, schema)
+    assert str(pickle.loads(pickle.dumps(parser))) == str(parser)  # type: ignore[misc]