diff --git a/src/rez/data/tests/packages/py_packages/late_binding_invalid/1.0/package.py b/src/rez/data/tests/packages/py_packages/late_binding_invalid/1.0/package.py new file mode 100644 index 0000000000..60f640281d --- /dev/null +++ b/src/rez/data/tests/packages/py_packages/late_binding_invalid/1.0/package.py @@ -0,0 +1,14 @@ +name = "late_binding_invalid" + +version = "1.0" + + +@late() +def requires(): + # Returns a non-None value that will fail schema validation when a strict + # schema is applied. Used to test the re-raise path in _wrap_forwarded. + return 42 + + +def commands() -> None: + pass diff --git a/src/rez/data/tests/packages/py_packages/late_binding_none/1.0/package.py b/src/rez/data/tests/packages/py_packages/late_binding_none/1.0/package.py new file mode 100644 index 0000000000..69e2c65484 --- /dev/null +++ b/src/rez/data/tests/packages/py_packages/late_binding_none/1.0/package.py @@ -0,0 +1,22 @@ +name = "late_binding_none" + +version = "1.0" + + +@late() +def requires(): + pass + + +@late() +def build_requires(): + pass + + +@late() +def private_build_requires(): + pass + + +def commands() -> None: + pass diff --git a/src/rez/package_resources.py b/src/rez/package_resources.py index 268b8c1800..8a03cacd69 100644 --- a/src/rez/package_resources.py +++ b/src/rez/package_resources.py @@ -72,9 +72,12 @@ def late_bound(schema): # used when 'requires' is late bound -late_requires_schema = Schema([ - Or(PackageRequest, And(str, Use(PackageRequest))) -]) +late_requires_schema = Schema( + Or( + And(None, Use(lambda _: [])), + [Or(PackageRequest, And(str, Use(PackageRequest)))] + ) +) # ------------------------------------------------------------------------------ diff --git a/src/rez/packages.py b/src/rez/packages.py index e969d0b9f5..e94eb042fb 100644 --- a/src/rez/packages.py +++ b/src/rez/packages.py @@ -16,6 +16,7 @@ from rez.utils.schema import schema_keys from rez.utils.resources import ResourceHandle, ResourceWrapper from rez.exceptions import PackageFamilyNotFoundError, ResourceError +from rez.utils.logging_ import print_warning from rez.utils.typing import SupportsWrite from rez.version import Version, VersionRange from rez.version import VersionedObject @@ -168,7 +169,21 @@ def _wrap_forwarded(self, key: str, value: Any) -> Any: schema = self.late_bind_schemas.get(key) if schema is not None: - value_ = schema.validate(value_) + try: + value_ = schema.validate(value_) + except Exception: + if value_ is None: + # A late-bound function returned None. This is + # equivalent to the attribute not being set, so + # warn and treat it as a no-op rather than crashing. + # See https://github.com/AcademySoftwareFoundation/rez/issues/2153 + print_warning( + "Late-bound attribute %r on package %r returned" + " None; treating as unset.", key, self.name + ) + value_ = None + else: + raise # cache result of late bound func self._late_binding_returnvalues[key] = value_ diff --git a/src/rez/tests/test_packages.py b/src/rez/tests/test_packages.py index fb6a1bc718..6bd9836f74 100644 --- a/src/rez/tests/test_packages.py +++ b/src/rez/tests/test_packages.py @@ -19,10 +19,13 @@ from rez.tests.util import TestBase, TempdirMixin from rez.utils.formatting import PackageRequest from rez.utils.sourcecode import SourceCode +from rez.vendor.schema.schema import Schema, SchemaError import unittest +from unittest.mock import patch from rez.version import Version from rez.version import VersionError from rez.utils.filesystem import canonical_path +import logging import shutil import os.path import os @@ -55,6 +58,8 @@ 'single_unversioned', 'single_versioned-3.5', 'late_binding-1.0', + 'late_binding_none-1.0', + 'late_binding_invalid-1.0', 'timestamped-1.0.5', 'timestamped-1.0.6', 'timestamped-1.1.0', 'timestamped-1.1.1', 'timestamped-1.2.0', 'timestamped-2.0.0', 'timestamped-2.1.0', 'timestamped-2.1.5', 'multi-1.0', 'multi-1.1', 'multi-1.2', 'multi-2.0', @@ -158,6 +163,14 @@ def test_pkg_data(self) -> None: package = get_package("late_binding", "1.0") self.assertEqual(package.tools, ["util"]) + # a py-based package where late binding functions return None. + # This should be normalised to empty lists, not crash. + # See https://github.com/AcademySoftwareFoundation/rez/issues/2153 + package = get_package("late_binding_none", "1.0") + self.assertEqual(package.requires, []) + self.assertEqual(package.build_requires, []) + self.assertEqual(package.private_build_requires, []) + # a 'combined' type package package = get_package("multi", "1.0") expected_uri = canonical_path(os.path.join(self.yaml_packages_path, "multi.yaml<1.0>")) @@ -185,6 +198,43 @@ def test_pkg_data(self) -> None: expected_uri = canonical_path(os.path.join(self.py_packages_path, "multi.py<2.0>")) self.assertEqual(package.uri, expected_uri) + def test_late_binding_none_strict_schema(self) -> None: + """Test that a late-bound attribute returning None under a strict schema + warns and is treated as unset rather than crashing. + + See https://github.com/AcademySoftwareFoundation/rez/issues/2153 + """ + package = get_package("late_binding_none", "1.0") + + # The built-in late_requires_schema already accepts None. Patch it + # with a strict schema that rejects None to exercise the defensive + # fallback path in _wrap_forwarded. + strict_schema = Schema([str]) + strict_schemas = { + "requires": strict_schema, + "build_requires": strict_schema, + "private_build_requires": strict_schema, + } + + with patch.object(type(package), "late_bind_schemas", strict_schemas): + with self.assertLogs( + logger=logging.getLogger("rez.utils.logging_"), + level=logging.WARNING, + ) as cm: + result = package.requires + self.assertIsNone(result) + self.assertTrue( + any("returned None" in msg for msg in cm.output), + f"Expected warning about None return, got: {cm.output}", + ) + + # A late-bound function returning a non-None invalid value should + # still raise — the defensive fallback only applies to None. + invalid_package = get_package("late_binding_invalid", "1.0") + with patch.object(type(invalid_package), "late_bind_schemas", strict_schemas): + with self.assertRaises(SchemaError): + invalid_package.requires + def test_pkg_create(self) -> None: """test package creation.""" package_data = {