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.

Sign up for GitHub

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

Jump to bottom

rustc_llvm: re-run build script if config.toml changes #42429

Merged
merged 1 commit into from
Jun 7, 2017
Merged

rustc_llvm: re-run build script if config.toml changes #42429

merged 1 commit into from
Jun 7, 2017

Conversation

venkatagiri
Copy link
Contributor

closes #35199

@rust-highfive
Copy link
Collaborator

r? @pnkfelix

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

@Mark-Simulacrum
Copy link
Member

Looks good to me, but r? @alexcrichton

@venkatagiri
Copy link
Contributor Author

FYI, after this change, the config.toml file is present in the build.rs config and touching/modifying config.toml is re-compiling librustc_llvm.

$ cat build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/build/rustc_llvm-ebd551ef66e29972/output | grep rerun
cargo:rerun-if-changed=/home/vagrant/repos/rust/build/x86_64-unknown-linux-gnu/llvm/bin/llvm-config
cargo:rerun-if-changed=/home/vagrant/repos/rust/config.toml
cargo:rerun-if-changed=../rustllvm/ArchiveWrapper.cpp
cargo:rerun-if-changed=../rustllvm/llvm-rebuild-trigger
cargo:rerun-if-changed=../rustllvm/RustWrapper.cpp
cargo:rerun-if-changed=../rustllvm/rustllvm.h
cargo:rerun-if-changed=../rustllvm/README
cargo:rerun-if-changed=../rustllvm/PassWrapper.cpp

@Mark-Simulacrum
Copy link
Member

Hm, that's somewhat unfortunate -- but presumably it compiles fairly quickly? If not, we may want to reconsider this change, though I guess modifying config.toml is a somewhat rare occurence.

@alexcrichton
Copy link
Member

Thanks for the PR @venkatagiri!

@Mark-Simulacrum it's true yeah that this is a bit too coarse wrt when to rebuild things, but I think this is a good "next step" to fixing bugs in rustbuild. Want to file a follow-up issue about reducing the rebuilding granularity here?

In the meantime though this looks great for solving the issue at hand, so I'll....

@bors: r+

@bors
Copy link
Contributor

bors commented Jun 5, 2017

📌 Commit 40f8536 has been approved by alexcrichton

@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Jun 5, 2017
Closed
@Mark-Simulacrum
Copy link
Member

Filed #42444.

@bors rollup

@carols10cents carols10cents added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 5, 2017
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jun 5, 2017
@Mark-Simulacrum
Rollup merge of rust-lang#42429 - venkatagiri:llvm_config, r=alexcric…
42ec597 
…hton

rustc_llvm: re-run build script if config.toml changes

closes rust-lang#35199
@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Jun 5, 2017
Closed
@venkatagiri
Copy link
Contributor Author

@alexcrichton any ideas/suggestions on how to restrict the re-run to just llvm-config changes in config.toml. I can give it a try in fixing #42444.

@alexcrichton
Copy link
Member

@venkatagiri ideally this'll be fixed through a mechanism such as rust-lang/cargo#4125, so we may want to just wait for that PR to land and then update the build script here.

bors added a commit that referenced this pull request Jun 5, 2017
@bors
Auto merge of #42445 - Mark-Simulacrum:rollup, r=Mark-Simulacrum
80f0651 
Rollup of 4 pull requests

- Successful merges: #42304, #42415, #42429, #42438
- Failed merges:
frewsxcv added a commit to frewsxcv/rust that referenced this pull request Jun 7, 2017
@frewsxcv
Rollup merge of rust-lang#42429 - venkatagiri:llvm_config, r=alexcric…
b472b66 
…hton

rustc_llvm: re-run build script if config.toml changes

closes rust-lang#35199
@frewsxcv frewsxcv mentioned this pull request Jun 7, 2017
Merged
bors added a commit that referenced this pull request Jun 7, 2017
@bors
Auto merge of #42494 - frewsxcv:rollup, r=frewsxcv
f73f675 
Rollup of 7 pull requests

- Successful merges: #42409, #42415, #42429, #42438, #42466, #42469, #42485
- Failed merges:
@bors bors merged commit 40f8536 into rust-lang:master Jun 7, 2017
@venkatagiri venkatagiri deleted the llvm_config branch June 28, 2017 22:29
@venkatagiri venkatagiri mentioned this pull request Jun 30, 2017
Merged
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 30, 2017
@GuillaumeGomez
Rollup merge of rust-lang#42985 - venkatagiri:issue_42444, r=Mark-Sim…
f0e2f82 
…ulacrum

rustc_llvm: re-run build script when env var LLVM_CONFIG changes

This removes the changes done in rust-lang#42429 and use the newly introduced `cargo:rerun-if-env-changed` in rust-lang/cargo#4125.
As `LLVM_CONFIG` env var points to the `llvm-config` and changes when it gets configured in `config.toml` or removed from it, we can re-run the build script if this env var changes.

closes rust-lang#42444

r? @alexcrichton
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 30, 2017
@GuillaumeGomez
Rollup merge of rust-lang#42985 - venkatagiri:issue_42444, r=Mark-Sim…
209281e 
…ulacrum

rustc_llvm: re-run build script when env var LLVM_CONFIG changes

This removes the changes done in rust-lang#42429 and use the newly introduced `cargo:rerun-if-env-changed` in rust-lang/cargo#4125.
As `LLVM_CONFIG` env var points to the `llvm-config` and changes when it gets configured in `config.toml` or removed from it, we can re-run the build script if this env var changes.

closes rust-lang#42444

r? @alexcrichton
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 30, 2017
@GuillaumeGomez
Rollup merge of rust-lang#42985 - venkatagiri:issue_42444, r=Mark-Sim…
953d679 
…ulacrum

rustc_llvm: re-run build script when env var LLVM_CONFIG changes

This removes the changes done in rust-lang#42429 and use the newly introduced `cargo:rerun-if-env-changed` in rust-lang/cargo#4125.
As `LLVM_CONFIG` env var points to the `llvm-config` and changes when it gets configured in `config.toml` or removed from it, we can re-run the build script if this env var changes.

closes rust-lang#42444

r? @alexcrichton
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@alexcrichton alexcrichton

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rustbuild ignores the llvm-config setting
7 participants
@venkatagiri @rust-highfive @Mark-Simulacrum @alexcrichton @bors @pnkfelix @carols10cents