the SDK installs the generated binding tables the shipped aot_builtin_ast.h includes - #3963
Merged
Conversation
…n.inc - the shipped aot_builtin_ast.h includes ast_gen.inc, so an SDK consumer's AOT translation unit that touched the ast module failed to compile with 'daScript/builtin/ast_gen.inc' file not found (dasProfile's AOT companion on a Debian box), while the tree compiled it from its own include directory; ci/smoke_test_bundle.sh checks that every shipped header's include the table names is in the bundle, MISSING when it is not
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The change is narrowly scoped to SDK install rules and adds a targeted smoke-test regression check without affecting runtime behavior.
Pull request overview
This PR fixes an SDK packaging gap where shipped headers (include/daScript/simulate/aot_builtin_ast.h) include generated .inc files that were not installed into the SDK, causing downstream AOT builds that touch the AST module to fail. It also strengthens the bundle smoke test to detect this class of “header includes a non-shipped file” regressions.
Changes:
- Install the generated binding tables
include/daScript/builtin/ast_gen.incandinclude/daScript/builtin/debugapi_gen.incinto the SDK. - Extend
ci/smoke_test_bundle.shwith a “Shipped headers' includes” check to ensure required included files exist in the bundle.
File summaries
| File | Description |
|---|---|
| CMakeLists.txt | Installs the generated .inc binding tables alongside other builtin headers so SDK consumers can compile AOT TUs including aot_builtin_ast.h. |
| ci/smoke_test_bundle.sh | Adds a bundle-level guard that fails if a shipped header’s required included file is missing from the installed bundle. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Behavior change: an installed SDK carries
include/daScript/builtin/ast_gen.incanddebugapi_gen.inc; an SDK consumer's AOT translation unit that touches the ast module compiles again.Why. The shipped
include/daScript/simulate/aot_builtin_ast.hincludesdaScript/builtin/ast_gen.inc, and the install rules never carried the two generated.incfiles underinclude/daScript/builtin/- onlysrc/builtin/*.hlands there. The tree compiles such a unit from its own include directory, so nothing in the repo could see it; dasProfile's AOT companion module on a Debian box did:fatal error: 'daScript/builtin/ast_gen.inc' file not found.What changes.
CMakeLists.txtinstalls the two.incfiles beside the builtin headers.ci/smoke_test_bundle.shgains a section over shipped-header includes, one line per (header, include) pair, MISSING when the bundle lacks the include.Observable behavior.
cmake --installprefix:include/daScript/builtin/gainsast_gen.incanddebugapi_gen.inc.include/daScript/builtin/ast_gen.inc OK, and MISSING against a bundle installed from master.Where to look. The install rule next to
DAS_BUILTIN_HEADERSinCMakeLists.txt; the "Shipped headers' includes" section in the smoke test.Validation, claims, ledger
Validation
cmake --installfrom this branch carries both files; the smoke check reads OK on it and MISSING withast_gen.incremoved.Not done
DASTargets.cmakeexports absolute source-tree paths for the glfw and hv static libraries (modules/dasGlfw/glfw/$<CONFIG>/lib/libglfw3.a,modules/dasHV/hv/...) - an SDK moved to another machine links those targets against paths that are not there. Seen while reading the exports, not touched here.