-
Notifications
You must be signed in to change notification settings - Fork 447
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
Mitigate & document problems with mixed generated sources #3127
base: main
Are you sure you want to change the base?
Conversation
- deterministically prefer rust_library's crate spec over rust_test's. This means root_module gets remapped to a workspace path more often, and fixes flakiness of generated_srcs_test. - document the reasons and tradeoffs for this remapping. - stop adding `include_dirs` when remapping, it doesn't do anything to help with this problem, and is confusing. - fix test to strictly assert the path, which I broke in 74f164b Partially fixes bazelbuild#3126
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me, thanks!
Is there something useful we can assert on here, perhaps in https://github.com/bazelbuild/rules_rust/blob/main/test/rust_analyzer/rust_analyzer_test_runner.sh?
Sorry about the delay here. There is already a test for this in |
(it looks like there are unrelated failures on buildkite, so I might need help getting this merged) |
if info.crate.root.short_path in src_map: | ||
crate["root_module"] = _WORKSPACE_TEMPLATE + src_map[info.crate.root.short_path].path | ||
crate["source"]["include_dirs"].append(path_prefix + info.crate.root.dirname) | ||
# 'root' is a transformed source, likely a symlink. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be if is_generated and not is_external
?
Or this logic also applies to external repositories?
This means root_module gets remapped to a workspace path more often,
and fixes flakiness of generated_srcs_test.
include_dirs
when remapping, it doesn't do anything tohelp with this problem, and is confusing.
74f164b
Partially fixes #3126