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

Add --all-features to Rust Analyzer flags #1624

Merged
merged 7 commits into from
Mar 20, 2023

Conversation

emilk
Copy link
Member

@emilk emilk commented Mar 20, 2023

This stops rust-analyzer from building the web viewer, since crates/re_web_viewer_server/build.rs checks the __ci feature

Checklist

This stops it from building the web viewer, since
`crates/re_web_viewer_server/build.rs` checks the `__ci` feature
@emilk emilk added the 🧑‍💻 dev experience developer experience (excluding CI) label Mar 20, 2023
@Wumpf Wumpf self-requested a review March 20, 2023 16:08
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

Not the fix that I expected. Great to have it confirmed not stomping on our webviewer files!!!
And tanks for all the warning fixes!!

@@ -52,7 +52,8 @@
"--target-dir=target_ra",
"--workspace",
"--message-format=json",
"--all-targets"
"--all-targets",
"--all-features", // --all-features is critical in order to stop crates/re_web_viewer_server/build.rs from building the web viewer
Copy link
Member

Choose a reason for hiding this comment

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

This is still counter-intuitive to me. I tried reading through build.rs and even there it's not obvious why adding "all-features" here would stop the web-viewer from being built.

Copy link
Member

Choose a reason for hiding this comment

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

Ahh, piecing this comment together with the PR description I guess this is because "--all-features" implies the the CI "feature" which prevents the web-build.

Suggested change
"--all-features", // --all-features is critical in order to stop crates/re_web_viewer_server/build.rs from building the web viewer
"--all-features", // --all-features will set the `ci` feature flag, which stops crates/re_web_viewer_server/build.rs from building the web viewer

Copy link
Member Author

Choose a reason for hiding this comment

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

Also added: 5948394

@emilk
Copy link
Member Author

emilk commented Mar 20, 2023

Btw, this still works:

rm web_viewer/*.wasm
touch crates/re_viewer/src/lib.rs
cargo r -p rerun --all-features -- ../objectron.rrd --web-viewer

that's because --all-features here means all features of the rerun crate, so it won't set the __ci feature of the re_web_viewer_server crate.

Only when there is no -p/--project argument will --all-features matter (or when using it with -p re_web_viewer_server).

@emilk emilk merged commit a5c9a80 into main Mar 20, 2023
@emilk emilk deleted the emilk/rust-analyzer-doesnt-build-web-viewer branch March 20, 2023 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧑‍💻 dev experience developer experience (excluding CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants