Fix late-bound requires attributes returning None (#2153) - #2158
Open
maxnbk wants to merge 2 commits into
Open
Conversation
…wareFoundation#2153) In 3.4.0, PR AcademySoftwareFoundation#2004 added build_requires and private_build_requires to late_bind_schemas to fix a rez-search crash. However, late_requires_schema only accepted lists, so a @late() function returning None (e.g. when there are no private build requirements) would raise SchemaError instead of being treated as an empty list. Fix in two layers: - Modify late_requires_schema to accept None and normalize to []. - Add a defensive check in _wrap_forwarded: if any late-bound attribute's schema validation fails and the value is None, warn and treat as unset rather than crashing. This protects attributes beyond the requires-family. Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2158 +/- ##
==========================================
+ Coverage 60.65% 61.32% +0.66%
==========================================
Files 164 164
Lines 20584 20575 -9
Branches 3579 3577 -2
==========================================
+ Hits 12485 12617 +132
+ Misses 7224 7087 -137
+ Partials 875 871 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
maxnbk
force-pushed
the
fix/2153-late-binding-empty-none-schema-interaction
branch
from
July 20, 2026 19:30
0bd39d2 to
e5c1274
Compare
…wareFoundation#2153) Add a test fixture package (late_binding_none) with @late() functions that return None for requires, build_requires, and private_build_requires. Assert that each attribute normalizes to an empty list via the updated late_requires_schema. Add a second test that patches late_bind_schemas with a strict schema (rejecting None) to exercise the defensive fallback path in _wrap_forwarded, asserting that a warning is logged and the value is treated as unset (None) rather than crashing. Signed-off-by: Stephen Mackenzie <maxnbk@users.noreply.github.com>
maxnbk
force-pushed
the
fix/2153-late-binding-empty-none-schema-interaction
branch
from
July 20, 2026 19:47
e5c1274 to
16afd3a
Compare
Member
|
@maxnbk That's an oversight from me, thanks for fixing! I never thought someone would return None in a late bound function. Do you think this is something we should discourage this practice? If so, we should add a note in the docs and also in the warning about that I think as to make it more actionable. Thoughts? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2153
3.4.0: PR #2004 added
build_requiresandprivate_build_requirestolate_bind_schemasto fix a bug inrez-searchcausing it to crash. However,late_requires_schemaonly accepted lists. So, a@late()function returningNone, for example, when there are no private build requirements, would raiseSchemaError: None should be instance of <class 'list'>instead of being treated as an empty list. In 3.3.0, the value passed through unchecked and theor []inget_requires()handled it.The fix was performed in two layers. Only one is necessary, but the second is a safety-guard for other schemas to produce warnings rather than crash.
Layer one:
late_requires_schema(package_resources.py) - modified in place to acceptNoneand normalize to[].Layer two:
_wrap_forwarded(packages.py) - The defensive check: if any late-bound attribute's schema validation fails, and the value isNone, warn, and treat as unset, rather than crashing. Protects attributes beyond the requires-family (tools,cachable, etc.) that have their own schemas.To test, a new fixture
late_binding_nonewith@late()functions returningNoneforrequires,build_requires, andprivate_build_requires. All three assert that they are normalized to[].An additional test exercises the Layer two fix by patching the schema to intentionally hit the otherwise invalid path.
Disclosure: GLM-5.2 was used to assist in diagnostics, docstring, and test-writing.