Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Sep 3, 2024
4 changes: 2 additions & 2 deletions conda_recipe_manager/grapher/recipe_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
"""
Expand Down
48 changes: 46 additions & 2 deletions conda_recipe_manager/parser/dependency.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand All @@ -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

Expand Down
25 changes: 18 additions & 7 deletions conda_recipe_manager/parser/recipe_parser_deps.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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]:
Expand Down Expand Up @@ -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():
Expand All @@ -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),
)
)
Expand Down
8 changes: 7 additions & 1 deletion conda_recipe_manager/parser/recipe_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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.
"""
Expand Down
24 changes: 22 additions & 2 deletions conda_recipe_manager/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
75 changes: 75 additions & 0 deletions tests/parser/test_pickle_integration.py
Original file line number Diff line number Diff line change
@@ -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]
Loading