feat: allow customizing the command for running build scripts by fee1-dead · Pul...
source link: https://github.com/rust-lang/rust-analyzer/pull/11956
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.
New issue
feat: allow customizing the command for running build scripts #11956
Conversation
I have tested this locally and it fixed #9201 with some small changes on the compiler side with suggestions from #9201 (comment).
I have also added an environment variable IS_RA_BUILDSCRIPT_CHECK for crates to detect that it is a check for buildscripts, and allows defaulting to bogus values for expected environment variables.
@Veykril this helps with properly expanding procedural macros in rust-lang/rust. Would you like to review this?
I have also added an environment variable IS_RA_BUILDSCRIPT_CHECK for crates to detect that it is a check for buildscripts, and allows defaulting to bogus values for expected environment variables.
Is this required? I am a bit reluctant to setting that env var since we don't really want crates to specialize for the case where r-a builds them.
Is this required?
Not really. rust-lang/rust requires a few env vars to be set as can be seen with eddyb's comment: CFG_RELEASE= CFG_RELEASE_CHANNEL= RUSTC_INSTALL_BINDIR=. I'm not sure which is better: adding a configuration for rust-analyzer setting env vars when checking for buildscripts, or just use that env var. Not doing anything needs rustc to always default to bogus values if those variables are not set, which should really be treated as errors unless r-a is building them.
I wonder if this is the right solution. Usually, if the project is a normal cargo workspace there should be no reason why we can't cargo check everything (especially with the rustc wrapper). If the project is not a normal cargo workspace, cargo check is most likely to not be the right thing to call at all. So to me it would make more sense to make the "run build scripts and compile proc macros" command completely customizable?
So to me it would make more sense to make the "run build scripts and compile proc macros" command completely customizable?
cargo check -p rustc_driver works with the rustc wrapper. But x.py check does not. So the best way is still using cargo check.
Sure, but making it fully configurable wouldn't mean that it can't be configured to still use cargo check though -- or to use some x.py variant that either doesn't use the rustc wrapper or does work with it.
(I don't want to let the perfect be the enemy of the good though.)
Yeah. That is why I pushed the fully configurable version.
crates/rust-analyzer/src/config.rs
Outdated
/// Advanced option, fully override the command rust-analyzer uses for
/// checking buildscripts and procedural macros. The command should
/// include `--message-format=json` or similar option.
checkBuildScriptOnSave_overrideCommand: Option<Vec<String>> = "null",
I suggest we call this rust-analyzer.cargo.runBuildScriptsCommand (maybe runBuildScripts.command would be ideal if we didn't already have runBuildScripts as a boolean option). (The point of running cargo check here is not to check the build scripts, it's to have their results and the proc macros available.) For the description how about something like this:
Advanced option, fully override the command rust-analyzer uses to run build scripts and build procedural macros. The command should include `--message-format=json` or a similar option.
done.
Maybe update the PR title and commit message?
changed the title feat: allow selecting which packages to check for buildscripts
feat: allow customizing the command for running build scripts
bors r+
bors doesn't work right now because of the repo move
Are you working on the PRs to migrate to homu? I would like to help.
Ye I was about to look into that, help with would be appreciated :)
@bors ping
@bors r=flodiebold
Commit 73a033e has been approved by flodiebold
Testing commit 
Test successful - checks-actions
Approved by: flodiebold
Pushing 15844bf to master...
Can we submit a change to the rustc contributor docs to say how to enable this feature?
Can we submit a change to the rustc contributor docs to say how to enable this feature?
It shouldn't be changed until another nightly is released, rust-analyzer bumped in rust-lang/rust. But in the mean time I will write the instructions up and prepare a PR to rustc-dev-guide.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
No one assigned
None yet
No milestone
Successfully merging this pull request may close these issues.
Recommend
About Joyk
Aggregate valuable and interesting links.
Joyk means Joy of geeK