make find_libilo_a honour CARGO_TARGET_DIR and .cargo/config.toml#514
Merged
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
The AOT linker helper hard-coded $CARGO_MANIFEST_DIR/target/{release,debug}
as the libilo.a search path. Fix worktrees that redirect cargo's target dir
out of the tree (typical: [build] target-dir = "/tmp/ilo-targets/<name>")
ended up with the staticlib at the redirected path while the lookup kept
probing target/, so every aot_compile_* test failed with "cannot find
libilo.a" and each PR carried a ln -sf symlink workaround.
Probe order now:
1. CARGO_TARGET_DIR env var
2. build.target-dir in $CARGO_MANIFEST_DIR/.cargo/config.toml
3. $CARGO_MANIFEST_DIR/target
4. workspace parent target/
Each step tries release then debug. Ad-hoc TOML parser, no new dep:
target-dir = "..." inside [build] is the only line we care about.
Unit tests cover the parser (double quotes, single quotes, ignoring other
sections, missing file) and the probe helper (release-over-debug
preference, debug fallback, no-match). The end-to-end integration is the
existing aot_compile_* suite itself: this very worktree redirects its
target dir, so a passing `cargo test --release --features cranelift`
without a symlink is the regression test.
1c8fc33 to
bbc36df
Compare
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
The AOT linker helper
find_libilo_a(insrc/vm/compile_cranelift.rs) hard-coded$CARGO_MANIFEST_DIR/target/{release,debug}/libilo.aas its only search path. Fix worktrees that redirect cargo's target dir out of the tree, which is the convention we adopted for isolated parallel builds (.cargo/config.tomlcontaining[build]\ntarget-dir = "/tmp/ilo-targets/<worktree-name>"), ended up with the staticlib at the redirected path while the lookup kept probingtarget/. Everyaot_compile_*test failed withcannot find libilo.a, and each fix PR carried aln -sf /tmp/ilo-targets/<name>/release/libilo.a target/release/libilo.aworkaround.That workaround was load-bearing in at least #499, #511, #506, and #512. It's fragile (litters the worktree, can race the build), and every future fix agent had to re-discover it. This PR makes the helper match where cargo actually puts the staticlib.
Repro before / after
In any worktree with
.cargo/config.tomlredirectingbuild.target-dir:Before:
After (this worktree's
target/release/libilo.adoes not exist):No symlink. The lookup reads
.cargo/config.toml, findstarget-dir = "/tmp/ilo-targets/ilo-fix-aot-lookup", and links from there.What is in the diff
One commit (
1c8fc33d):find_libilo_aprobes in order:CARGO_TARGET_DIRenv var,build.target-dirfrom$CARGO_MANIFEST_DIR/.cargo/config.toml,$CARGO_MANIFEST_DIR/target, workspace parenttarget/. Each step triesreleasethendebug.cargo_config_target_dir(ad-hoc TOML parser, no new dep, looks only fortarget-dir = "..."inside[build]),libilo_a_in(probe one target dir).target-diroutside[build], missing-file behaviour, release-over-debug preference, debug fallback, no-match returnsNone.The end-to-end integration test is the existing
aot_compile_*suite plustests/coverage_cranelift_compile.rs. This very worktree exercises the new lookup path: it redirects target-dir to/tmp/ilo-targets/ilo-fix-aot-lookupand links successfully without any symlink. I deliberately did not write a unit test that mutatesCARGO_TARGET_DIRdirectly, because that would race other tests sharing the process; the helper-level tests plus the end-to-end suite cover it cleanly.Test plan
cargo build --release --features craneliftcleancargo test --release --features cranelift --lib compile_craneliftpasses (163 tests, includes the new helper tests and all AOT tests)cargo test --release --features craneliftfull suite passes (50 test binaries, 0 failed)cargo fmt --checkcleancargo clippy --release --features cranelift --all-targets -- -D warningscleantarget/release/libilo.adoes not exist in this worktree, AOT tests still link from/tmp/ilo-targets/ilo-fix-aot-lookup/release/libilo.aFollow-ups