Fix require-runtime-dependencies being ignored for build hooks#2257
Open
filbranden wants to merge 3 commits into
Open
Fix require-runtime-dependencies being ignored for build hooks#2257filbranden wants to merge 3 commits into
filbranden wants to merge 3 commits into
Conversation
Author
Author
|
Reopening after @cobaltt7's comment |
…2110) PR pypa#2073 (workspace support) introduced a separate `additional_dependencies` list on `EnvironmentInterface` and changed `prepare_build_environment` to extend that list with the dynamic build-hook dependencies discovered by `get_required_build_deps` (e.g. project runtime deps when a build hook sets `require-runtime-dependencies = true`). Bug: by the time `prepare_build_environment` extends `additional_dependencies`, the cached property chain that consumes it: `dependencies_complex` -> `all_dependencies_complex` -> `missing_dependencies` has already been populated by the preceding `prepare_environment` call (via `dependency_hash()` and `dependencies_in_sync()`) with the *empty* `additional_dependencies` snapshot. The second `sync_dependencies()` call then early-returns because the cached `missing_dependencies` reflects the pre-mutation deps which have just been installed — so the runtime deps never get installed and the build hook fails with `ModuleNotFoundError`. Before PR pypa#2073, this worked because the old code mutated the cached `dependencies` list in place and the old `sync_dependencies` reinstalled the entire list via pip — there was no cached `missing_dependencies` short-circuit. Fix: add `EnvironmentInterface.add_additional_dependencies()` which extends `additional_dependencies` and invalidates the dependent cached properties (`dependencies_complex`, `dependencies`, `all_dependencies_complex`, `all_dependencies`, `missing_dependencies`). Update `prepare_build_environment` to use it. Add a regression test that primes the cache before extending and asserts the new dep is visible.
a9f33c5 to
3154db3
Compare
|
Still an issue with Hatch 1.17.0 |
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.
PR #2073 (workspace support) introduced a separate
additional_dependencieslist onEnvironmentInterfaceand changedprepare_build_environmentto extend that list with the dynamic build-hook dependencies discovered byget_required_build_deps(e.g. project runtime deps when a build hook setsrequire-runtime-dependencies = true).Bug: by the time
prepare_build_environmentextendsadditional_dependencies, the cached property chain that consumes it:dependencies_complex->all_dependencies_complex->missing_dependencieshas already been populated by the precedingprepare_environmentcall (viadependency_hash()anddependencies_in_sync()) with the emptyadditional_dependenciessnapshot.The second
sync_dependencies()call then early-returns because the cachedmissing_dependenciesreflects the pre-mutation deps which have just been installed — so the runtime deps never get installed and the build hook fails withModuleNotFoundError.Before PR #2073, this worked because the old code mutated the cached
dependencieslist in place and the oldsync_dependenciesreinstalled the entire list via pip — there was no cachedmissing_dependenciesshort-circuit.Fix: add
EnvironmentInterface.add_additional_dependencies()which extendsadditional_dependenciesand invalidates the dependent cached properties (dependencies_complex,dependencies,all_dependencies_complex,all_dependencies,missing_dependencies). Updateprepare_build_environmentto use it.Add a regression test that primes the cache before extending and asserts the new dep is visible.
Fixes: #2110