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

cargo-bazel now supports alias_rule to customize the aliases generated #2312

Merged
merged 2 commits into from
Dec 15, 2023

Conversation

rickvanprim
Copy link
Contributor

@rickvanprim rickvanprim commented Dec 9, 2023

Motivator is to be able to set default_alias_rule = "opt" so that all Cargo fetched 3rd party dependencies are built with compilation_mode=opt regardless of what compilation_mode the local code is being built with.

Per my conversation with @illicitonion. Please bikeshed on naming 🙂

Copy link
Collaborator

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

This looks great, thanks!

I'll leave this one open for a day or two in case anyone else wants to jump in, otherwise I left a few comments :))

crate_universe/test_data/serialized_configs/BUILD.bazel Outdated Show resolved Hide resolved
crate_universe/3rdparty/crates/transition_opt_alias.bzl Outdated Show resolved Hide resolved
crate_universe/private/crates_repository.bzl Outdated Show resolved Hide resolved
Copy link
Collaborator

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

Generally LGTM modulo the comments :)

crate_universe/src/rendering.rs Outdated Show resolved Hide resolved
crate_universe/src/rendering.rs Outdated Show resolved Hide resolved
crate_universe/src/rendering.rs Outdated Show resolved Hide resolved
illicitonion added a commit to illicitonion/rules_rust that referenced this pull request Dec 13, 2023
I intend to publish this to crates.io after review.

This will be used to test
bazelbuild#2312.
illicitonion added a commit to illicitonion/rules_rust that referenced this pull request Dec 13, 2023
I intend to publish this to crates.io after review.

This will be used to test
bazelbuild#2312.
illicitonion added a commit that referenced this pull request Dec 13, 2023
I intend to publish this to crates.io after review.

This will be used to test
#2312.
@rickvanprim rickvanprim force-pushed the compilation_mode branch 4 times, most recently from fb9c17f to 326eed7 Compare December 14, 2023 20:26
@rickvanprim rickvanprim changed the title cargo-bazel generated targets can optionally always use transition_opt_alias to transition to compilation_mode=opt. cargo-bazel now supports alias_rule to customize the aliases generated Dec 14, 2023
…` to apply transitions, including builtin ones for `compilation_mode=dbg|fastbuild|opt`.
Copy link
Collaborator

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

Surprisingly clean! Nicely put together, thanks!

Just one bikeshed to paint :)

#[derive(Debug, Default, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, Clone)]
pub enum AliasRule {
#[default]
#[serde(rename = "default")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would be tempted to call this alias or native.alias rather than default? WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do!

@UebelAndre UebelAndre self-requested a review December 15, 2023 13:58
@illicitonion illicitonion merged commit 3803401 into bazelbuild:main Dec 15, 2023
3 checks passed
@rickvanprim rickvanprim deleted the compilation_mode branch December 15, 2023 17:13
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.

None yet

2 participants