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

Do not unconditionally succeed RUSTC_WRAPPER checks when run by build scripts #13010

Merged
merged 1 commit into from Aug 13, 2022

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Aug 13, 2022

rust-analyzer's RUSTC_WRAPPER unconditionally succeeds cargo check
invocations tripping up build scripts using cargo check to probe for
successful compilations. To prevent this from happening the RUSTC_WRAPPER
now checks if it's run from a build script by looking for the
CARGO_CFG_TARGET_ARCH env var that cargo sets only when running build
scripts.

… scripts

rust-analyzer's RUSTC_WRAPPER unconditionally succeeds `cargo check`
invocations tripping up build scripts using `cargo check` to probe for
successful compilations. To prevent this from happening the RUSTC_WRAPPER
now checks if it's run from a build script by looking for the
`CARGO_CFG_TARGET_ARCH` env var that cargo sets only when running build
scripts.
@eminence
Copy link
Contributor

I tested this on two workspaces that exhibit the problem reported with anyhow, and in both cases the issue is fixed (and no obvious regressions)

@Veykril
Copy link
Member Author

Veykril commented Aug 13, 2022

Thank you for testing! There shouldn't be any regressions caused due to this (unless cargo sets this env var in other occasions which it shouldn't)
@bors r+

@bors
Copy link
Collaborator

bors commented Aug 13, 2022

📌 Commit 72ae308 has been approved by Veykril

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Aug 13, 2022

⌛ Testing commit 72ae308 with merge 5941dec...

@bors
Copy link
Collaborator

bors commented Aug 13, 2022

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing 5941dec to master...

@bors bors merged commit 5941dec into rust-lang:master Aug 13, 2022
@bors bors mentioned this pull request Aug 13, 2022
@Veykril Veykril deleted the build-script-probes branch August 13, 2022 16:19
@sdroege
Copy link

sdroege commented Aug 16, 2022

This seems to cause unnecessary rebuilds, it seems. Some crate that depends on anyhow here will always rebuild anyhow and some other crates every time I run cargo build after saving in vim/coc-rust-analyzer. That wasn't the case before.

@Veykril
Copy link
Member Author

Veykril commented Aug 16, 2022

this is only invoked on startup for rust-analyzer, so this cannot have changed the behavior for when you save

@sdroege
Copy link

sdroege commented Aug 16, 2022

Hrm, then something else must've changed between this and last Monday's release. Sorry for the noise here then.

@Veykril
Copy link
Member Author

Veykril commented Aug 16, 2022

The only recent relevant change I could think of is #12808, but that is 2 releases ago, not one

@msfjarvis
Copy link

This seems to cause unnecessary rebuilds, it seems. Some crate that depends on anyhow here will always rebuild anyhow and some other crates every time I run cargo build after saving in vim/coc-rust-analyzer. That wasn't the case before.

You might be hitting this issue.

@sdroege
Copy link

sdroege commented Aug 17, 2022

Thanks, that seems to have fixed this

bors bot added a commit to intellij-rust/intellij-rust that referenced this pull request Aug 30, 2022
9235: Do not unconditionally succeed RUSTC_WRAPPER when run by build scripts r=Undin a=vlad20012

Fixes #9198, fixes #9227.

Based on rust-lang/rust-analyzer#13010 and discussion in rust-lang/rust-analyzer#12973.

intellij-rust-native-helper in `RUSTC_WRAPPER` role unconditionally succeeds `cargo check` invocations tripping up build scripts using `cargo check` to probe for successful compilations. To prevent this from happening the `RUSTC_WRAPPER` now checks if it's run from a build script by looking for the `CARGO_CFG_TARGET_ARCH` env var that cargo sets only when running build scripts.

changelog: Fix broken `anyhow` compilation when `org.rust.cargo.evaluate.build.scripts` [experimental feature](https://plugins.jetbrains.com/plugin/8182-rust/docs/rust-faq.html#experimental-features) is enabled

Co-authored-by: vlad20012 <beskvlad@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants