Project-local + relocated-bundle support: resolver, JIT rpath, .app bundle#2588
Merged
Project-local + relocated-bundle support: resolver, JIT rpath, .app bundle#2588
Conversation
Three coordinated changes that together let a daspkg-shipped Mac/Linux
bundle Just Work after being moved off the build machine:
1. Tier-3 fallback in builtin_resolve_this_module_dir
src/builtin/module_builtin_runtime.cpp + include/daScript/simulate/aot_builtin.h
When a script has no `/modules/` segment in its baked path (i.e. user
project-local code like `main.das`) AND the dev-time baked dir is gone
on the running machine (relocated bundle), tier 3 now returns
`<exe_dir>` instead of the dead path. Tier 3 unchanged for
`/modules/`-rooted call sites, so a missing module isn't masked.
Header now exports the function as DAS_API so the daslang-side test
can call it via `__builtin_resolve_this_module_dir`.
Test: tests/fio/get_this_module_dir_resolver.das (4 cases — real path,
dead project-local, dead /modules/ path, empty input).
2. JIT linker emits self-locating rpath
src/builtin/module_jit.cpp
Adds `@executable_path` (Mach-O) / `$ORIGIN` (ELF) as the *primary*
rpath, build-tree path as fallback. Without this a shipped exe
dyld-loads via the build-machine fallback and crashes once the bundle
moves. The format-string `\" \"` insert splits the outer quotes so
the linker sees two distinct -Wl,-rpath flags.
3. daspkg release: macOS .app bundle + runtime dylib shipping
utils/daspkg/commands.das + daslib/daspkg.das + utils/daspkg/package_runner.das
On Darwin `cmd_release` now produces `<name>.app/Contents/MacOS/{exe
+ dylibs + modules}` with a minimal `Contents/Info.plist`
(CFBundleIdentifier from `release_bundle_id()` or derived
`com.<author>.<name>`). The Mac exe drops the `.exe` extension to
match Mac convention. Linux/Windows layout unchanged.
New step 2.5 ships the daslang runtime (`libDaScriptDyn*` from
`<das_root>/lib`) next to the exe — without these the bundle's
primary `@executable_path` / `$ORIGIN` rpath doesn't resolve and the
exe falls through to the build-machine path.
`release_bundle_id()` is a new ReleaseSpec builder; mirrored as
PackageReleaseInfo.bundle_id in package_runner.das.
Skill updates (filesystem.md, daspkg.md): drop the "two workarounds
until the resolver learns to fall back" guidance — the resolver does
fall back now. State the new tier-3 behavior plainly: ship project-local
assets next to the exe and `path_join(get_this_module_dir(), ...)` works
in both dev and shipped bundles.
Comment polish in modules/dasLLVM/daslib/llvm_exe.das (no behavior
change, just clarifies what the post-main shutdown drain does).
Test updates (utils/daspkg/test_daspkg.das): release tests now use
platform-aware path helpers (`bundle_root_dir`, `bundle_artifacts_dir`,
`bundle_exe_path`, `moved_bundle_exe_path`) to assert the right layout
on Darwin vs flat-dir on Linux/Windows. The "moved bundle prints
expected sqlite output" test now exercises the full Mac .app-relocation
path, which is exactly the smoke test we want for this feature.
Local: 7,873 interpreter tests pass; 7,267 AOT tests pass; 199 daspkg
tests pass on macOS.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves relocatability of daslang/daspkg-produced bundles on macOS/Linux by (1) making project-local get_this_module_dir() resilient to relocated builds, (2) emitting self-relative rpaths for JIT-linked artifacts, and (3) extending daspkg release to produce a macOS .app bundle layout and ship required runtime libraries alongside the executable.
Changes:
- Add tier-3 fallback to
<exe_dir>for project-local baked paths whose original dev directory no longer exists (relocated-bundle case), plus a new targeted test. - Update JIT linker command generation to prepend
@executable_path(macOS) /$ORIGIN(ELF) rpaths ahead of the build-tree rpath. - Extend
daspkg releaseto emit a macOS.appstructure, shiplibDaScriptDyn*runtime libraries next to the exe, and add a configurablerelease_bundle_id().
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/daspkg/test_daspkg.das | Updates release tests to be layout-aware on macOS .app vs flat bundles. |
| utils/daspkg/package_runner.das | Adds bundle_id to release info extraction from .das_package. |
| utils/daspkg/commands.das | Implements .app bundle output, writes Info.plist, and ships runtime dylibs next to the exe. |
| tests/fio/get_this_module_dir_resolver.das | Adds coverage for project-local resolver fallback behavior and edge cases. |
| src/builtin/module_jit.cpp | Emits self-locating rpaths (@executable_path / $ORIGIN) before build-tree fallback. |
| src/builtin/module_builtin_runtime.cpp | Adds resolver fallback to <exe_dir> when project-local baked dir is missing. |
| skills/filesystem.md | Updates guidance to reflect new project-local fallback behavior. |
| skills/daspkg.md | Updates bundling guidance and removes obsolete workarounds. |
| modules/dasLLVM/daslib/llvm_exe.das | Comment-only clarification around teardown behavior. |
| include/daScript/simulate/aot_builtin.h | Exports builtin_resolve_this_module_dir for AOT/test access. |
| daslib/module_path.das | Documentation update to reflect the new tier-3 fallback behavior. |
| daslib/daspkg.das | Adds ReleaseSpec.bundle_id and release_bundle_id() API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…d + Windows DLL path Three Copilot review fixes on PR #2588: 1. build_info_plist now uses dasPUGIXML's with_doc + tag/attr EDSL instead of a hand-rolled string template. XML metacharacters in any plist value (`& < > " '`) are escaped automatically — previously the code interpolated `bundle_id` and `pkg.author`-derived strings raw, so a `&` or `<` in the author name (or in user-supplied release_bundle_id) would produce an invalid Info.plist. Verified via `plutil -lint` + `plutil -extract` round-trip on a value containing all five XML metas. Adds a hard `require pugixml/PUGIXML_boost` to commands.das. CI release configs already enable the module via release_modules.txt; builds with `DAS_PUGIXML_DISABLED=ON` will fail to load daspkg release, which is acceptable given the dependency is the right tool for this job. The DOCTYPE is omitted — modern CFPropertyList accepts plist without it, and pugixml's daslib EDSL doesn't expose doctype-node creation. 2. Derived CFBundleIdentifier is sanitized. Apple requires `[A-Za-z0-9.-]` only; `pkg.author` can contain spaces (the existing `daspkg-test-versions` fixture has author "Test Author"), which previously yielded an invalid `com.Test Author.<name>` id. New private helper `sanitize_bundle_id_part(s)`: lowercases, replaces every byte outside `[a-z0-9]` with `-`, collapses runs, strips leading/trailing dashes, falls back to "x" if nothing salvageable. Applied only to the derived default (`com.<sanitized-author>.<sanitized-name>`); user-supplied `release_bundle_id()` passes through untouched on the reasonable-trust assumption that the user knows the format and Apple's own validator catches mistakes downstream. 3. Windows DLL search path now points at `<das_root>/bin` instead of `<das_root>/lib`. CMake's RUNTIME artifact (the .dll) goes to bin/ while the LIBRARY artifact goes to lib/, so step 2.5 was finding nothing on Windows and shipping a bundle with no daslang runtime. Mac/Linux unchanged (.dylib / .so still in lib/). Refactored the 3-platform select into a single `let` ternary chain so it doesn't trip LINT003. 199/199 daspkg tests still pass on macOS. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…irs, generic_string in resolver Three Copilot review fixes on PR #2588 (round 2): 1. Step 2.5 now propagates `copy_file` failures and refuses to ship a bundle that found zero runtime libs. Previously the `return` inside the glob block exited only the callback — `cmd_release` continued and returned 0 even if every dylib failed to copy. Tracks `runtime_copy_failed` and `runtime_copies` and fails the whole release on either. 2. Glob patterns prefixed with `**/` so the recursive walk actually matches files in config subdirs. CMake multi-config generators (Visual Studio, Xcode) put the artifact in `bin/Release/...`, not `bin/...`; the previous `libDaScriptDyn*.dll` pattern only matched top-level entries since `*` doesn't cross `/`. Mac/Linux single-config layouts are unchanged (nothing nested = `**` is a no-op). 3. `builtin_resolve_this_module_dir` switches every `.string()` conversion on `std::filesystem::path` to `.generic_string()`. `path::string()` emits native separators on Windows (`\`), but `getDasRoot()` and the rest of daslang's path conventions are `/`. Without the fix, `get_this_module_dir()` would return `C:\...\foo` on Windows while sibling string-formed paths use `/`, breaking string compares and any path arithmetic that mixes the two. Pure no-op on Mac/Linux (where native already is `/`). 199/199 daspkg tests pass; 171/171 tests/fio/ pass. Windows verification relies on the CI matrix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #2588 added a hard `require pugixml/PUGIXML_boost` to utils/daspkg/commands.das, so daspkg can use the real XML library for Info.plist emission instead of hand-rolled string templates. With the previous default (`DAS_PUGIXML_DISABLED ON`), that broke every CI matrix entry whose cmake configure didn't pass `-DDAS_PUGIXML_DISABLED=OFF` — build.yml (sanitizer matrix on linux/mac), msvc.yml, wasm_build.yml, build_eastl.yml — at the `Building daspkg executable (daslang -exe)` step: error[30901]: missing prerequisite 'pugixml'; file not found from modules/dasPUGIXML/daslib/PUGIXML_boost.das required from utils/daspkg/commands.das required from utils/daspkg/main.das Flipping the default to OFF (= module enabled) brings every CI entry in line with extended_checks.yml, release.yml, doc.yml, pages.yml — which already enabled it explicitly. pugixml is small, mature, has no external deps, and is the right primitive for any tool that needs to emit/parse XML. No CI workflow currently sets `DAS_PUGIXML_DISABLED=ON`, so flipping the default doesn't break any matrix entry. The redundant `=OFF` overrides in pages.yml / doc.yml / ci/release_modules.txt stay (harmless after the default flip, deleting them would mix scope). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Summary
Three coordinated changes that together let a daspkg-shipped Mac/Linux bundle Just Work after being moved off the build machine. Closes the gap left by PR #2579 — exe-relative resolution worked for
/modules/-rooted code, but project-localmain.dasstill returned the dev-time baked dir on a relocated machine.1. Tier-3 fallback in
builtin_resolve_this_module_dirsrc/builtin/module_builtin_runtime.cpp, include/daScript/simulate/aot_builtin.h, daslib/module_path.das
When a script has no
/modules/segment in its baked path AND the dev-time baked dir is gone on the running machine (relocated bundle), tier 3 now returns<exe_dir>instead of the dead path. Tier 3 unchanged for/modules/-rooted call sites — a missing module still surfaces as a missing dir rather than getting silently masked by the exe dir.The header now exports the function as
DAS_APIso the daslang-side test can reach it via__builtin_resolve_this_module_dir.Test: tests/fio/get_this_module_dir_resolver.das — 4 cases (real path, dead project-local, dead
/modules/path, empty input).2. JIT linker emits self-locating rpath
src/builtin/module_jit.cpp
Adds
@executable_path(Mach-O) /$ORIGIN(ELF) as the primary rpath, build-tree path as fallback. Without this a shipped exe dyld-loads via the build-machine fallback and crashes once the bundle moves. The format-string\" \"insert splits the outer quotes so the linker sees two distinct-Wl,-rpathflags rather than one with an embedded space.3. daspkg release: macOS .app bundle + runtime dylib shipping
utils/daspkg/commands.das, daslib/daspkg.das, utils/daspkg/package_runner.das
cmd_releasenow emits<name>.app/Contents/MacOS/{exe + dylibs + modules}with a minimalContents/Info.plist(CFBundleIdentifier fromrelease_bundle_id()or derivedcom.<author>.<name>). Mac exe drops the.exeextension to match convention. Linux/Windows layout unchanged.libDaScriptDyn*from<das_root>/lib) next to the exe so the bundle's primary@executable_path/$ORIGINrpath actually resolves. Without this the exe falls through to the build-machine path.release_bundle_id(): newReleaseSpecbuilder for the CFBundleIdentifier; mirrored asPackageReleaseInfo.bundle_idinpackage_runner.das.Skill updates
skills/filesystem.md, skills/daspkg.md: drop the "two workarounds until the resolver learns to fall back" guidance — the resolver does fall back now. State the new tier-3 behavior plainly: ship project-local assets next to the exe and
path_join(get_this_module_dir(), ...)works in both dev and shipped bundles.Test updates
utils/daspkg/test_daspkg.das: release tests now use platform-aware path helpers (
bundle_root_dir,bundle_artifacts_dir,bundle_exe_path,moved_bundle_exe_path) to assert the right layout on Darwin vs flat-dir on Linux/Windows. The "moved bundle prints expected sqlite output" test now exercises the full Mac.app-relocation path — which is the smoke test we wanted for this whole feature.Misc
Comment polish in modules/dasLLVM/daslib/llvm_exe.das (no behavior change).
Test plan
format_file+linton changed.das— already formatted, 0 issuesdaslang+test_aot(resolver C++ change touches a builtin)dastest -- --test tests/fio/— 171/171dastest -- --test tests/— 7,873/7,873 (matches master)dastest -- --test utils/daspkg/test_daspkg.das— 199/199 (after the platform-aware helper rewrite)dastest -- --test utils/daspkg/test_daspkg_git.das— 70/70test_aot -use-aot dastest -- --use-aot --test tests/— 7,267/7,267, 0 fail/0 error/6 skipped (master parity)test_cmd_release_native_dep): build a sqlite-using bundle,renameit to a new path, run the moved exe — sqlite dylib resolves via the new@executable_pathrpath through the .app/Contents/MacOS layout. Prints expected output, exits 0.🤖 Generated with Claude Code