Fix unresolved ${out_dir} token in dep env files#4124
Open
basvandijk wants to merge 1 commit into
Open
Conversation
`outputs_to_dep_env` redacted the producer's out_dir to the generic
`${out_dir}` substitution token, the same way `outputs_to_env` does.
But dep env (`DEP_*`) files are consumed by *downstream* crates' build
scripts, whose runner only substitutes `${pwd}` and whose own out_dir
points to a different directory, so the token was left unresolved (or
would resolve to the wrong directory). For example, libssh2-sys failed
to find zlib.h from libz-sys's DEP_Z_INCLUDE.
Only substitute the exec root in dep env files and keep the real
out_dir path, which is a declared input of downstream build script
actions.
Assisted-by: GitHub Copilot
Author
|
EDIT: False alarm. The bug was caused by something else and fixed in dfinity/ic@082d8ae. |
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.
The bug
Since the
OUT_DIRsanitization work (#4050 / #4011, first released in 0.71.x),outputs_to_dep_envredacts the producer'sout_dirto the generic${out_dir}substitution token, the same wayoutputs_to_envdoes.That redaction is only correct for
_bs.envfiles, which are consumed by the target that directly owns the build script (whereprocess_wrapper's--out-dirresolves the token to the right directory). Dep env (DEP_*) files, however, are consumed by downstream crates' build scripts: their runner only substitutes${pwd}, and their ownout_dirpoints to a different directory. The token is therefore left unresolved, or would resolve to the wrong directory.Real-world failure:
libssh2-sys's build script fails to findzlib.hbecauselibz-sys'sDEP_Z_INCLUDEcontains a literal${out_dir}path component. Found while upgradingrules_rustto 0.71.3 in dfinity/ic (see dfinity/ic#10632, where this fix is currently carried as a patch).The fix
Only substitute the exec root in dep env files and keep the real
out_dirpath. That path is valid for consumers: the producer'sout_diris a declared input of downstream build script actions.Tests
out_dir_in_dep_env_value_is_not_redacted_to_substitution_tokenincargo/private/cargo_build_script_runner/lib.rs.//cargo/tests/dep_env:build_read_out_dirmirroring the libz-sys → libssh2-sys scenario: a producer build script advertisescargo:include=$OUT_DIR/includeand the consumer build script assertsDEP_Z_INCLUDEpoints at an existing directory. Fails without the fix, passes with it.Assisted-by: GitHub Copilot