Re-apply JULIA_LOAD_PATH leakage fix when running as a Pkg app#145
Open
gbaraldi wants to merge 1 commit into
Open
Re-apply JULIA_LOAD_PATH leakage fix when running as a Pkg app#145gbaraldi wants to merge 1 commit into
gbaraldi wants to merge 1 commit into
Conversation
Re-applies the fix from #127 (commit c3716ef, reverted in edd1e25) and addresses the test failure that prompted the revert. When JuliaC is installed as a Pkg app, the generated shim exports JULIA_LOAD_PATH=<JuliaC project dir>. The compile subprocesses JuliaC spawns (Pkg.instantiate/Pkg.precompile and juliac-buildscript.jl) inherit that env, locking the load path to JuliaC's project and breaking precompilation of the user's project ("Missing source file for Base.PkgId(...)" — #106, #124). Fix: always clear JULIA_LOAD_PATH for the subprocesses (restoring 3a17f5c's behavior). Inject HostCPUFeatures trim preferences directly into the temp project copy via LocalPreferences.toml so trim mode no longer needs JULIA_LOAD_PATH manipulation. Tests: * New programmatic regression tests (Project with dependencies, with and without --trim) covering #124. * New end-to-end test using Pkg.Apps.develop to install JuliaC and invoke its shim against a user project (Unix-only). * The simulated-shim test (julia -m JuliaC with JULIA_LOAD_PATH set manually) is gated on JULIAC_TEST_PKG_APP_SHIM_SIM=1 — required because it depends on \$ROOT/Manifest.toml, which only exists in a dev checkout (Manifest.toml is gitignored). JuliaC's CI sets the env var; downstream consumers like Julia's CI that install JuliaC as a package will skip it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
Author
|
Checked on upstream CI JuliaLang/julia#61840. And it no longer breaks |
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
Re-applies the fix from #127 (commit c3716ef, reverted in edd1e25) and fixes the test failure that prompted the revert. Closes #124, fixes #106.
When JuliaC is installed as a Pkg app, the generated shim exports
JULIA_LOAD_PATH=<JuliaC project dir>. The compile subprocesses JuliaC spawns (Pkg.instantiate/Pkg.precompileandjuliac-buildscript.jl) inherit it, locking the load path to JuliaC's own project and breaking the user project's precompilation:Missing source file for Base.PkgId(...).3a17f5c (#59) originally fixed this with an unconditional
JULIA_LOAD_PATH => nothingoverride; 6940a31 (#96) inadvertently regressed it by making the override conditional onis_trim_enabled.Fix
JULIA_LOAD_PATHfor the spawned subprocesses (restores 3a17f5c).--trim, injectHostCPUFeatures.freeze_cpu_target=truedirectly into the temp project copy'sLocalPreferences.tomlinstead of constructing an ad-hoc env onJULIA_LOAD_PATH— separates environment isolation from preference injection.TOML(stdlib) to[deps]/[compat]for the preferences injection.Why #127 was reverted, and what's different here
#127's regression test (
Pkg app JULIA_LOAD_PATH isolation) simulated the shim by manually settingJULIA_LOAD_PATH=$ROOT/. That works in a dev checkout because the dev tree has a resolvedManifest.toml, but*Manifest*.tomlis in this repo's.gitignore. When Julia's CI installs JuliaC viaPkg.add, noManifest.tomlends up under$ROOT, so the simulation couldn't even load JuliaC's own deps in the subprocess and failed before exercising the fix.This PR keeps both regression tests but reorganises them:
Project with dependencies (no trim) (#124)/(trim) (#124)— programmatic API tests, always on, would have caught "Missing source file" for direct dependencies introduced in v0.3.1 #124 before merge.Pkg app end-to-end (#106)— usesPkg.Apps.developto install JuliaC and run its real shim against a user project (Unix only). Always on.Pkg app JULIA_LOAD_PATH isolation (#106)— the simulated-shim variant, gated onJULIAC_TEST_PKG_APP_SHIM_SIM=1with a hard precondition check on$ROOT/Manifest.toml. JuliaC's own CI sets the env var in.github/workflows/ci.yml; downstream consumers (like Julia's CI) won't run it.Test plan
mainwith a minimalSHA-using project underJULIA_LOAD_PATH=$JC/Pkg.Apps.develop+ invoke the generated shim → executable runs and prints expected outputPkg.test()passes locally withJULIAC_TEST_PKG_APP_SHIM_SIM=1🤖 Generated with Claude Code