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

Build rust_test targets using a crate name different from the underlying lib #2828

Conversation

felipeamp
Copy link
Contributor

This is a rollforward of
#2803, but behind the incompatible_change_rust_test_compilation_output_directory incompatible flag.

This PR also makes rust_test put its compilation outputs in the same directory as the rust_library rule (i.e. not in a test-{hash} subdirectory anymore).

After this change both the rust_library and rust_test rules will put all its compilation outputs in the same directory, but there won't be any name collisions in non-sandboxed environments (see #1427 for more context).

Issue with context: #2827

@felipeamp felipeamp force-pushed the add-feature-flag-and-change-rust-test-compilation-output-dir branch from 65363ab to 25d9133 Compare September 2, 2024 13:16
underlying lib

This is a rollforward of
bazelbuild#2803, but behind a feature
flag.

This PR also makes `rust_test` put its compilation outputs in the same
directory as the `rust_library` rule (i.e. not in a `test-{hash}`
subdirectory anymore).

After this change both the `rust_library` and `rust_test` rules will put
all its compilation outputs in the same directory, but there won't be
any name collisions in non-sandboxed environments (see
bazelbuild#1427 for more context).

Issue: bazelbuild#2827
@felipeamp felipeamp force-pushed the add-feature-flag-and-change-rust-test-compilation-output-dir branch from 25d9133 to a99f026 Compare September 2, 2024 13:20
@krasimirgg krasimirgg self-requested a review September 5, 2024 14:10
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.

Thanks! Had a few questions.

test/rust/src/lib.rs Outdated Show resolved Hide resolved
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.

Thanks! Could I also get you to make (or modify an existing) issue ticket to track the new incompatibility flag? We have some guidance on https://github.com/bazelbuild/rules_rust/blob/0.49.3/COMPATIBILITY.md#how-to-make-a-backwards-incompatible-change but since this could be a breaking change we try to give users a chance to raise concerns during the rollout. I've been writing mine to look like #2082

@felipeamp
Copy link
Contributor Author

I have filed an issue for this, see #2827. Is there anything specific that's missing there?

@felipeamp felipeamp force-pushed the add-feature-flag-and-change-rust-test-compilation-output-dir branch from 61662e9 to 7e498f3 Compare September 10, 2024 16:26
@felipeamp felipeamp force-pushed the add-feature-flag-and-change-rust-test-compilation-output-dir branch from 7e498f3 to 0656abd Compare September 11, 2024 09:01
@felipeamp
Copy link
Contributor Author

I've also fixed the windows tests (that weren't working due to the toolchain binary extension) and added an assert on the directory name as well.

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.

Nice! Thank you!

@UebelAndre
Copy link
Collaborator

Just gotta resolve the conflicts!

@felipeamp felipeamp force-pushed the add-feature-flag-and-change-rust-test-compilation-output-dir branch from 5c98d7a to 69e493c Compare September 12, 2024 08:47
@felipeamp felipeamp force-pushed the add-feature-flag-and-change-rust-test-compilation-output-dir branch from 69e493c to 3986b3b Compare September 12, 2024 08:50
@felipeamp
Copy link
Contributor Author

All done.

@krasimirgg krasimirgg added this pull request to the merge queue Sep 12, 2024
Merged via the queue into bazelbuild:main with commit bad49c4 Sep 12, 2024
4 checks passed
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