Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't allow DESTDIR to influence LLVM builds #74252

Merged
merged 1 commit into from
Jul 14, 2020

Conversation

shepmaster
Copy link
Member

When running a command like DESTDIR=foo x.py install in a completely
clean build directory, this will cause LLVM to be installed into
DESTDIR, which then causes the build to fail later when it attempts
to use those LLVM files.

@rust-highfive
Copy link
Collaborator

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 11, 2020
Copy link
Member

@eddyb eddyb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may work as a workaround but I believe x.py build should not expose DESTDIR to the LLVM build, i.e. it should unset that env var for the relevant commands, which will remove the need to have RUST_DESTDIR at all.

This seems to be a combination of oversight and probably x.py install not being used to also do the build step, most of the time (think make && make DESTDIR=$PKG_OUT_DIR install).

@Mark-Simulacrum
Copy link
Member

Yes, I think @eddyb is right that the preferred approach would be to unset the env variable when invoking cmake during our LLVM installation/building, since we should be in full control of where LLVM is installed (vs. Rust itself, which I guess makes sense that DESTDIR should affect that).

When running a command like `DESTDIR=foo x.py install` in a completely
clean build directory, this will cause LLVM to be installed into
`DESTDIR`, which then causes the build to fail later when it attempts
to *use* those LLVM files.
@shepmaster shepmaster changed the title Teach bootstrap RUST_DESTDIR, a scoped alternative to DESTDIR Don't allow DESTDIR to influence LLVM builds Jul 12, 2020
@shepmaster
Copy link
Member Author

Updated to unset DESTDIR for LLVM.

@Mark-Simulacrum
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jul 12, 2020

📌 Commit 9741fbd has been approved by Mark-Simulacrum

@bors
Copy link
Contributor

bors commented Jul 12, 2020

🌲 The tree is currently closed for pull requests below priority 20, this pull request will be tested once the tree is reopened

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 12, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 14, 2020
…arth

Rollup of 11 pull requests

Successful merges:

 - rust-lang#73759 (Add missing Stdin and StdinLock examples)
 - rust-lang#74211 (Structured suggestion when not using struct pattern)
 - rust-lang#74228 (Provide structured suggestion on unsized fields and fn params)
 - rust-lang#74252 (Don't allow `DESTDIR` to influence LLVM builds)
 - rust-lang#74263 (Slight reorganization of sys/(fast_)thread_local)
 - rust-lang#74271 (process_unix: prefer i32::*_be_bytes over manually shifting bytes)
 - rust-lang#74272 (pprust: support multiline comments within lines)
 - rust-lang#74332 (Update cargo)
 - rust-lang#74334 (bootstrap: Improve wording on docs for `verbose-tests`)
 - rust-lang#74336 (typeck: use `item_name` in cross-crate packed diag)
 - rust-lang#74340 (lint: use `transparent_newtype_field` to avoid ICE)

Failed merges:

r? @ghost
@bors bors merged commit 1e74f28 into rust-lang:master Jul 14, 2020
@shepmaster shepmaster deleted the bootstrap-rust-destdir branch July 15, 2020 00:15
@cuviper cuviper added this to the 1.47.0 milestone May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants