Rebase a generated path handed over through an env var - #56
Merged
Conversation
A build script can give the compiler a path through a custom env var
rather than through OUT_DIR, and it records that path where it ran. The
build script's directory is gone by the time the crate compiles, so the
crate does not build at all:
error: couldn't read .../_mime_guess_build_script._build/mime_guess_out/mime_types_generated.rs
--> src/impl_bin_search.rs:4:1
|
4 | include!(env!("MIME_TYPES_GENERATED_PATH"));
mime_guess is the crate that found it, and it sits under a good deal of
the web ecosystem. Nothing in this repo declared it, so nothing here
failed.
The machinery already existed: rebase_build_path rewrites a recorded
build-script path to where the output actually landed, and was applied
to link search paths and not to env values. It only rewrites a value
that starts with the recorded out dir, so an env var that is not a path
passes through untouched.
test/buildscript_env is a crate doing the same thing deliberately: its
build script writes into OUT_DIR, hands the path over as an env var, and
its lib includes it. Red before this with the same error.
test/buildscript_env proves a fix in the tool, so a release made before that fix fails it. That is what the test is for, and not a regression in the released tool. The pinned-tool job asks whether a released tool still works with these rules. A change it predates is outside that question, so the run skips tests labelled requires_unreleased_tool, on both linux and macos. rust_test grows a labels argument to make that sayable. It had none, so a test could not be selected or skipped at all, which is what labels are for everywhere else in plz.
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.
Found while building
wasm-bindgen-clifor #26. Independent of that work, so it goes on its own.What was broken
mime_guesscould not compile at all:Its build script writes a generated source file and hands the compiler its path through a custom env var rather than through
OUT_DIR:The out dir is recorded relative and survives. The env var records the absolute path where the build script ran, and that sandbox is gone by compile time.
This is the same class as #19: a path that was true where it was recorded and is not true where it is used. #19 covered
OUT_DIRitself andinclude!(concat!(env!("OUT_DIR"), ..)). This is the other idiom, where the build script names the file directly.The fix
The machinery already existed.
rebase_build_pathrewrites a recorded build-script path to where the output actually landed, and was applied to link search paths but not to env values. It rewrites only a value that starts with the recorded out dir, so an env var that is not a path passes through untouched.mime_guessbuilds now.Why nothing here caught it
Nothing in this repo declared
mime_guess, and rust-corpus does not either, so its 1,112 declarations would not have found this. It took building an unrelated tool to surface it.test/buildscript_envnow covers the idiom deliberately: a crate whose build script writes intoOUT_DIR, hands the path over as an env var, and whose lib doesinclude!(env!(..))on it. Verified red before the fix with the identicalcouldn't readerror, and it also asserts that an env var which is not a path survives untouched.192 tests pass, clippy clean, fmt clean, no dep left outside a lock.