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

Treat rustfmt as optional #1164

Merged
merged 1 commit into from
Mar 3, 2022

Conversation

illicitonion
Copy link
Collaborator

All the code above this call treats rustfmt as optional, but then when
we make the BUILD for the rust toolchain, we unconditionally point at
the presumed rustfmt label.

At

# Rustfmt
sysroot_rustfmt = None
if rustfmt:
sysroot_rustfmt = _symlink_sysroot_bin(ctx, name, "bin", rustfmt)
direct_files.extend([sysroot_rustfmt])

we conditionally create a symlink based on whether the attribute is set,
but because we unconditionally set the attribute, this causes a hard
error failing to symlink to a non-existent file.

Instead, treat rustfmt as optional in this function too, so that the
code above and below properly convey the optionality to each other.

rust/private/repository_utils.bzl 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.

Looks good! Just one request 😄

rust/private/repository_utils.bzl Outdated Show resolved Hide resolved
@illicitonion illicitonion force-pushed the optional-rustfmt branch 2 times, most recently from 231b372 to 085ba68 Compare March 3, 2022 17:52
All the code above this call treats rustfmt as optional, but then when
we make the BUILD for the rust toolchain, we unconditionally point at
the presumed rustfmt label.

At
https://github.com/bazelbuild/rules_rust/blob/59fab4e79f62bfa13551ac851a40696c24c0c3a4/rust/toolchain.bzl#L331-L335
we conditionally create a symlink based on whether the attribute is set,
but because we unconditionally set the attribute, this causes a hard
error failing to symlink to a non-existent file.

Instead, treat rustfmt as optional in this function too, so that the
code above and below properly convey the optionality to each other.
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.

Good find, thanks!

@illicitonion illicitonion merged commit 40dce28 into bazelbuild:main Mar 3, 2022
@illicitonion illicitonion deleted the optional-rustfmt branch March 3, 2022 18:03
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