Treat ctlgeom-types.h and geom-ctl-io.c as checked-in, not BUILT_SOURCES#87
Closed
Luochenghuang wants to merge 2 commits into
Closed
Treat ctlgeom-types.h and geom-ctl-io.c as checked-in, not BUILT_SOURCES#87Luochenghuang wants to merge 2 commits into
Luochenghuang wants to merge 2 commits into
Conversation
PR NanoComp#74 (mesh geometry import) hand-extended utils/ctlgeom-types.h with C-only mesh internals (mesh_bvh_node, face_normals, bvh, is_closed, etc.) that don't have Scheme analogs in geom.scm, and committed the matching deep-copy code in utils/geom-ctl-io.c. But Makefile.am still classified both files as BUILT_SOURCES with regen rules that run when configure detects Guile. The regen rewrites both files from geom.scm, which knows nothing about the rich mesh fields — result: compilation fails with 'unknown type name mesh_bvh_node'. Travis didn't catch this because its Guile detection was already broken on modern distros; the new GitHub Actions CI installs guile-3.0-dev and trips it immediately. This commit formalizes PR NanoComp#74's actual intent: both files are hand-maintained, not generated. Removes them from BUILT_SOURCES, deletes their regen rules, moves ctlgeom-types.h to include_HEADERS, adds both to EXTRA_DIST, drops them from clean-local. Trade-off: future additions to public scheme classes in geom.scm now need manual mirroring in ctlgeom-types.h and geom-ctl-io.c. Matches the de facto practice since PR NanoComp#74. Verified locally: build clean, 'make check' passes with 0 failures in test-mesh (including test_cube_vs_block, test_cube_segments_vs_block, test_copy_destroy, and all closure-validation cases) and test-prism.
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
1 participant
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.
Fixes the
unknown type name 'mesh_bvh_node'build failure when CI (or any from-git build with Guile installed) clobbers the committedctlgeom-types.hvia the BUILT_SOURCES regen rule.PR #74 hand-extended
ctlgeom-types.handgeom-ctl-io.cwith mesh internals that don't have Scheme analogs (mesh_bvh_node,face_normals,bvh,is_closed, ...), butMakefile.amstill treats both files as generated. Any build with Guile detected runs the regen rule, rewrites the files fromgeom.scm, and loses the internals.This commit formalizes what PR #74 actually intended, as both files are hand-maintained: dropped from
BUILT_SOURCES, regen rules deleted,ctlgeom-types.hmoved toinclude_HEADERS, both added toEXTRA_DIST, removed fromclean-local. 5 meaningful lines ofMakefile.am, zero C source changes.Trade-off: future additions to public scheme classes in
geom.scmnow need manual mirroring inctlgeom-types.handgeom-ctl-io.c.The architecturally clean long-term fix is a PIMPL split, public
meshstruct exposes scheme-visible fields plus an opaquevoid *internal, withmesh_bvh_nodeand the cache in a separate private header. See this branch.Closes #86