Repository navigation
Q21: reject a shadowed vdsim core at import time - #13
Merged
Merged
Conversation
Another PyPI distribution ships a top-level vdsim/ package. Installed beside our wheel it wins the import over vdsim.cpython-*.so with no warning from pip, and the failure only surfaces later as an AttributeError on an unrelated line. python/vdsim_guard.py holds the decision once: a module bound to the name vdsim must be a compiled extension (importlib EXTENSION_SUFFIXES) and expose the core symbols. load_core() also retries an explicit build directory, so a repo checkout keeps working in an environment that has the shadowing package. vdsim_lab, vdsim_plant and vdsim_rl now go through load_core(); the other shipped modules never import the core, so they are untouched. The error names the file that was actually loaded plus a one-line cause and remedy; the check itself is structural and never tests for a distribution name. docs/VALIDATION.md drops to PENDING: python/CMakeLists.txt is part of the registration surface, so the canonical count is re-measured on this tree.
Measured tree 7b7947f; three identical CI attempts on the same head. Numbers are in the PR body and the currency block. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
What
import vdsimcan silently resolve to somebody else's package. Another PyPIdistribution ships a top-level
vdsim/package; installed in the sameenvironment as our wheel, that directory wins over
vdsim.cpython-*.so. pipneither warns nor overwrites, so the failure surfaces much later as an
AttributeErroron an unrelated line.The import name is unchanged. This PR only makes the shadowing detectable at
the point of import.
Shared check — one place
python/vdsim_guard.py(new, shipped in the wheel):diagnose(mod)Noneifmodis the core, else a one-line reasoncheck_core(mod)modor raisesCoreShadowedErrorload_core(fallback_paths=())vdsim, validates, retries explicit build dirsTwo structural properties, both must hold:
vdsim.__file__ends with one ofimportlib.machinery.EXTENSION_SUFFIXESSimSession,VehicleParams,TireParams,make_sim_sessionThe check never looks at a distribution name — the offending package is named
only inside the remedy sentence, as an example.
Failure is an
ImportErrorsubclass, not a warning.Wiring
vdsim_lab,vdsim_plantandvdsim_rlare the only shipped modules thatimport the core; all three now call
load_core().vdsim_render,vdsim_render3d,vdsim_preset,vdsim_traceandvdsim_campaignneverimport
vdsim, so shadowing cannot affect them and they are untouched.CLI entry points are covered transitively:
vdsim-quickstart→vdsim_lab,vdsim-campaign→ child processes that importvdsim_lab.vdsim-render/vdsim-render3doperate on.vdtracefiles only.load_corealso retries the explicit build directory when the ambient import isshadowed, so a repo checkout keeps working in an environment that has the other
package installed — strictly better than the old
try/except ImportError.Error message
Tests
Added to
tests/scripts/test_experiment_api.py(existingexperiment_apitarget — no new ctest target):
test_shadowed_core_is_rejected— a fakevdsim/package first onPYTHONPATH,load_core([])must raise and the message must name the realload path plus
Cause:/Fix:test_shadowed_core_recovers_from_build_dir— same shadow, but with the builddirectory as a fallback: the core wins
test_real_core_passes_the_check— the shipped.sopassesdiagnoseNegative verification: forcing
check_coreto skip the check makesexperiment_apiFailed; restoring it makes it Passed.Packaging
python/vdsim_guard.pyadded to the SKBUILD install list.tools/wheel_import_smoke.pyreads that list as its single source, so the newmodule is covered by the pre-flight C-1 import closure automatically
(
modules to import (15): vdsim, vdsim_guard, ...).Counts
python/CMakeLists.txt+1 line → the 3× identical-countcontract applies.
docs/VALIDATION.mdcurrency is dropped toPENDINGwith areason line; no number is written until CI measures this tree three times.