Skip to content

Tweak import macro bootstrap to trick rust analyzer aspect #1179

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

Merged
merged 7 commits into from
Mar 16, 2022

Conversation

hlopko
Copy link
Member

@hlopko hlopko commented Mar 7, 2022

In #1166 we discovered that rust analyzer aspect doesn't understand the _with_import_macro_bootstrapping_mode rule. This PR changes the rule to pretend to be more like a normal Rust rule.

CC @cfredric

@hlopko hlopko requested a review from UebelAndre March 7, 2022 22:17
@hlopko
Copy link
Member Author

hlopko commented Mar 7, 2022

The alias_with_import_macro_bootstrapping_mode is not expected to be used anywhere else. I see it as a private rule, and implementation detail. Vast majority of users should not have to know about it, and if somebody is looking there, it's likely because they are investigating something and they are trying to understand what's going on.

We intentionally omitted the docs attributes so this rule doesn't pollute our docs pages. But now that I'm thinking about it it's not the presence of docs, but explicit configuration in bzl files in //docs that matters for what is shown on docs pages. Therefore I'll add docs in the morning :)

@UebelAndre
Copy link
Collaborator

Sweet! I'll await docs

@UebelAndre
Copy link
Collaborator

@hlopko is there anything more than docs this is waiting on?

@hlopko
Copy link
Member Author

hlopko commented Mar 10, 2022

That, tests, and also I think I'll re-do the PR to add support for aliases into rust analyzer tooling (currently alias is not traversed), and then I'm change the transitioning rules to mimic alias (name of the attribute will be actual).

@hlopko hlopko force-pushed the fix_import_macro_bootstrap branch from d8d2f12 to 4c279b0 Compare March 15, 2022 16:55
@hlopko
Copy link
Member Author

hlopko commented Mar 15, 2022

This is now ready for another review, please take a look :)

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

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

Looking good! Just some nits 😅

@UebelAndre
Copy link
Collaborator

UebelAndre commented Mar 15, 2022

I'm not entirely sure what happened here: https://buildkite.com/bazel/rules-rust-rustlang/builds/5705#1fa56c67-7d6d-407a-b50c-aa78c5327cf1
There was a timeout but as far as I can tell the timeout should be 15 minutes and the build only ran for 11... I'll dig more into this but I highly suspect a rebuild will fix it (since that's what happened to me).

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you!

@hlopko hlopko merged commit 86d47a1 into bazelbuild:main Mar 16, 2022
@hlopko hlopko deleted the fix_import_macro_bootstrap branch March 16, 2022 15:47
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.

3 participants