Repository navigation
fix(packaging): ship vdsim_preset.py in the wheel - #7
Merged
Merged
Conversation
vdsim_render.py imports vdsim_preset, but the SKBUILD install list carried only the former, so `import vdsim_render` raised ModuleNotFoundError in every pip install while the source tree stayed green. Found by pre-flight C-2 (the first check run outside the source tree). test_render_presets grows a closure check: for each module the wheel installs, every first-party import of that module must be installed too. The list is read back out of python/CMakeLists.txt so the check follows the manifest rather than restating it.
python/CMakeLists.txt is on the registration surface, so the 492/492 recorded against 1fc790e no longer describes this tree and the currency gate rejects it by design. Park the count at PENDING with the reason, and re-promote after three consecutive green canonical CI runs on this commit.
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.
Rebased onto
main(bcad3db). One source line in the wheel manifest, plus a test thatpins the class of defect it belongs to.
(i) What was missing
python/vdsim_preset.py. Theif(DEFINED SKBUILD)install list inpython/CMakeLists.txtshippedvdsim_render.pybut not the module it imports, so inany clean environment installed from a wheel:
Measured 2026-09-18 in two clean environments (docker
python:3.11-slim, WindowsCPython 3.12 venv) against artifact set from wheels run 35309944365.
vdsim_preset.pyentered the tree with the render-preset work (PR #4) and was never added to the install
list; the conflict in this rebase is the same list gaining
vdsim_render3d.pyfromPR #6, resolved as a union of both entries.
(ii) Why Q14 verify did not catch it
Two independent blind spots, both of which only see what they are told to look at:
CIBW_TEST_COMMAND(.github/workflows/wheels.yml:28) importsvdsimandconstructs
Simtwice. It never importsvdsim_render, so the broken import isnever executed.
tools/verify_wheel_packaging.pyinspects the wheel's zip member list against ahand-written set (
vdsim_lab.py,vdsim_plant.py,catalog/,quickstart.py,configs,
.tirpolicy).vdsim_render.pyandvdsim_preset.pyare not in that set,so a wheel missing either one passes.
Neither gate is a closure check, so a module can be shipped while its dependency is not.
The source tree never shows it: in-tree,
python/is onsys.pathand both files arepresent. This is why the whole matrix (6 wheels x verify) was green on a broken wheel.
(iii) What the new test does and does not guarantee
tests/scripts/test_render_presets.py::test_wheel_ships_every_first_party_importGuarantees:
python/CMakeLists.txtitself, not restated, so thecheck follows the list when it changes.
.py, every top-level import that resolves to a file inpython/,cosim/orexamples/is itself in the install list. That closes the"ship A, drop A's dependency" class of defect at its source.
Does not guarantee:
CMake install rules, not of a
.whl. A scikit-build-core/CMake regression that dropsa listed file would still pass.
package_data, config trees, or thecatalog/package directory —only top-level
.pymodules named in the install list.The wheel-side counterpart (import every listed module from the installed wheel in
CIBW_TEST_COMMAND) is Q19-b and is deliberately not in this PR: it changes.github/workflows/wheels.yml, not the registration surface, and is verified by aworkflow_dispatchrun rather than by ctest.Registration surface / currency
python/CMakeLists.txtis on the registration surface, so the currency gate fails untildocs/VALIDATION.mdis re-promoted against this tree. Test count is expected to stay492/492 (this PR adds a check inside an existing ctest target, not a new target).
Promotion follows three consecutive same-commit green CI runs, per the standing
contract.
Update (13e28de): the first CI run (35349441450) confirmed the predicted gate
failure --
registration surface changed between the recorded commit 1fc790e and the built commit ...: python/CMakeLists.txt-- with ctest itself reporting 492/492 in thesame job. The currency block is now parked at
PENDINGwith the reason recorded(commit 13e28de), and is re-promoted on this branch after three consecutive green
canonical runs on the same commit.