How to do metadata-only typecheck build of a rust_library? #428
Comments
|
We could change the code to react to a flag, eg. ` --compilation_mode` (
https://github.com/bazelbuild/rules_rust/blob/e64700dc9b8b3869bce4f77b78c33cb9d088cc4b/rust/private/rustc.bzl#L370-L374)
or turn `rust_library` into a macro that creates a 'check' target in
addition to the build target. I am not sure whether either approach is
'idiomatic'.
Going a little farther with the latter, rust_library could be changed to
only run check until an output artifact is actually needed by a test or
binary, but this probably requires redundant calls to rustc and more bazel
know-how than I've got :).
…On Mon, Oct 12, 2020 at 1:25 AM David Tolnay ***@***.***> wrote:
It looks like rules_rust unconditionally passes --emit=dep-info,link.
This means Bazel builds cannot benefit from the biggest rustc speedup since
1.0.0 (
https://blog.rust-lang.org/2017/03/16/Rust-1.16.html#whats-in-1160-stable).
😿
https://github.com/bazelbuild/rules_rust/blob/e64700dc9b8b3869bce4f77b78c33cb9d088cc4b/rust/private/rustc.bzl#L374
Is there any idiomatic way to perform non-code-generating builds which
type-check only? Equivalent to Cargo's cargo check or cargo rustc
--profile=check, rustc's rustc --emit=metadata, or Buck's #check flavored
builds (buck build :lib#check;
https://buck.build/concept/flavors.html#analysis_http).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#428>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAROG6A3QJJJUPR56PXFTALSKKHLHANCNFSM4SMKEBCQ>
.
|
This sounds bad if it means |
|
I don't think a `build ...` would work because any binaries wouldn't have
the necessary artifacts produced, same for `test ...`
…On Mon, Oct 12, 2020 at 1:59 PM David Tolnay ***@***.***> wrote:
or turn rust_library into a macro that creates a 'check' target in
addition to the build target.
This sounds bad if it means ... builds will build those both. Would there
be a way to not do that? I'd expect a way run a whole ... build in the
"check" compilation mode or the non-check mode.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#428 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAROG6DBTRHRM5ZMGQU2HZTSKM7XXANCNFSM4SMKEBCQ>
.
|
|
The check targets could be tagged manual to be excluded under ..., but that
doesn't let you `check ...`
…On Mon, Oct 12, 2020 at 2:12 PM Marco Farrugia ***@***.***> wrote:
I don't think a `build ...` would work because any binaries wouldn't have
the necessary artifacts produced, same for `test ...`
On Mon, Oct 12, 2020 at 1:59 PM David Tolnay ***@***.***>
wrote:
> or turn rust_library into a macro that creates a 'check' target in
> addition to the build target.
>
> This sounds bad if it means ... builds will build those both. Would
> there be a way to not do that? I'd expect a way run a whole ... build in
> the "check" compilation mode or the non-check mode.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#428 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAROG6DBTRHRM5ZMGQU2HZTSKM7XXANCNFSM4SMKEBCQ>
> .
>
|
|
For comparison, the behavior of Buck is that |
|
It sounds doable, but I don't know how off the cuff. What is the use-case
for having libraries with no dependents?
…On Mon, Oct 12, 2020 at 3:38 PM David Tolnay ***@***.***> wrote:
For comparison, the behavior of Buck is that buck build ... will do
codegen of any rust_library that is pulled in transitively by any binary
include in the build, and do check-only build of any rust_library that is
included in the ... but *not* pulled in by any binary. That seems like
the best behavior. How sure are we that there would be no way to reproduce
that in Bazel, or that we wouldn't want to?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#428 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAROG6BEL3EMBIIHRXJXU3TSKNLKXANCNFSM4SMKEBCQ>
.
|
Well if I have a library in lib/ which is used by a binary in bin/, and I am iterating on the library today, then This is why |
|
Oh sure, but I meant for the ... case. In that scenario, it seems like it
would work to have a separate check target.
…On Mon, Oct 12, 2020 at 8:44 PM David Tolnay ***@***.***> wrote:
Well if I have a library in lib/ which is used by a binary in bin/, and I
am iterating on the library today, then bazel build lib/... shouldn't be
needing to do LLVM codegen and linking.
This is why cargo check has been so important. It's not that people are
making libraries that are never to be used by anything. It's that sometimes
people work on the library itself.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#428 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAROG6A6BP2R5TLK6CP2HCLSKOPH5ANCNFSM4SMKEBCQ>
.
|
|
I think in the abstract it makes perfect sense for check to be the default
action and a link to only happen when a binary/test requires it. I believe
it's possible to do, but I'm not sure how lazily actions are evaluated --
can you comment on that @damienmg?
…On Mon, Oct 12, 2020 at 8:53 PM Marco Farrugia ***@***.***> wrote:
Oh sure, but I meant for the ... case. In that scenario, it seems like it
would work to have a separate check target.
On Mon, Oct 12, 2020 at 8:44 PM David Tolnay ***@***.***>
wrote:
> Well if I have a library in lib/ which is used by a binary in bin/, and
> I am iterating on the library today, then bazel build lib/... shouldn't
> be needing to do LLVM codegen and linking.
>
> This is why cargo check has been so important. It's not that people are
> making libraries that are never to be used by anything. It's that sometimes
> people work on the library itself.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#428 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAROG6A6BP2R5TLK6CP2HCLSKOPH5ANCNFSM4SMKEBCQ>
> .
>
|
For the |
I have a crate that uses pyo3 to create a Python extension module.
On macOS, special link flags are required:
rustc_flags = selects.with_or({
(
"@io_bazel_rules_rust//rust/platform:x86_64-apple-darwin",
): [
"-Clink-arg=-undefined",
"-Clink-arg=dynamic_lookup",
],
"//conditions:default": [],
}),
Without them, the linker on macOS fails, as the Python API is not
available until runtime.
rules_rust's clippy implementation was passing --emit=dep-info,link
to the clippy invocation, causing a clippy run to fail with linking
errors. This patch changes the invocation to use
--emit=dep-info,metadata instead, which is what cargo uses. It also
shaves a bit of time off the check, as linking no longer needs to happen.
Tangentially related to bazelbuild#428 and bazelbuild#421 - currently the clippy aspect
seems to be falling back on a non-worker compile, so it's still
noticeably slower than running cargo clippy directly when minor
changes have been made.
I have a crate that uses pyo3 to create a Python extension module.
On macOS, special link flags are required:
rustc_flags = selects.with_or({
(
"@io_bazel_rules_rust//rust/platform:x86_64-apple-darwin",
): [
"-Clink-arg=-undefined",
"-Clink-arg=dynamic_lookup",
],
"//conditions:default": [],
}),
Without them, the linker on macOS fails, as the Python API is not
available until runtime.
rules_rust's clippy implementation was passing --emit=dep-info,link
to the clippy invocation, causing a clippy run to fail with linking
errors. This patch changes the invocation to use
--emit=dep-info,metadata instead, which is what cargo uses. It also
shaves a bit of time off the check, as linking no longer needs to happen.
Tangentially related to bazelbuild#428 and bazelbuild#421 - currently the clippy aspect
seems to be falling back on a non-worker compile, so it's still
noticeably slower than running cargo clippy directly when minor
changes have been made.
I have a crate that uses pyo3 to create a Python extension module.
On macOS, special link flags are required:
rustc_flags = selects.with_or({
(
"@io_bazel_rules_rust//rust/platform:x86_64-apple-darwin",
): [
"-Clink-arg=-undefined",
"-Clink-arg=dynamic_lookup",
],
"//conditions:default": [],
}),
Without them, the linker on macOS fails, as the Python API is not
available until runtime.
rules_rust's clippy implementation was passing --emit=dep-info,link
to the clippy invocation, causing a clippy run to fail with linking
errors. This patch changes the invocation to use
--emit=dep-info,metadata instead, which is what cargo uses. It also
shaves a bit of time off the check, as linking no longer needs to happen.
Tangentially related to bazelbuild#428 and bazelbuild#421 - currently the clippy aspect
seems to be falling back on a non-worker compile, so it's still
noticeably slower than running cargo clippy directly when minor
changes have been made.
I have a crate that uses pyo3 to create a Python extension module.
On macOS, special link flags are required:
rustc_flags = selects.with_or({
(
"@io_bazel_rules_rust//rust/platform:x86_64-apple-darwin",
): [
"-Clink-arg=-undefined",
"-Clink-arg=dynamic_lookup",
],
"//conditions:default": [],
}),
Without them, the linker on macOS fails, as the Python API is not
available until runtime.
rules_rust's clippy implementation was passing --emit=dep-info,link
to the clippy invocation, causing a clippy run to fail with linking
errors. This patch changes the invocation to use
--emit=dep-info,metadata instead, which is what cargo uses. It also
shaves a bit of time off the check, as linking no longer needs to happen.
Tangentially related to bazelbuild#428 and bazelbuild#421 - currently the clippy aspect
seems to be falling back on a non-worker compile, so it's still
noticeably slower than running cargo clippy directly when minor
changes have been made.
I have a crate that uses pyo3 to create a Python extension module.
On macOS, special link flags are required:
rustc_flags = selects.with_or({
(
"@io_bazel_rules_rust//rust/platform:x86_64-apple-darwin",
): [
"-Clink-arg=-undefined",
"-Clink-arg=dynamic_lookup",
],
"//conditions:default": [],
}),
Without them, the linker on macOS fails, as the Python API is not
available until runtime.
rules_rust's clippy implementation was passing --emit=dep-info,link
to the clippy invocation, causing a clippy run to fail with linking
errors. This patch changes the invocation to use
--emit=dep-info,metadata instead, which is what cargo uses. It also
shaves a bit of time off the check, as linking no longer needs to happen.
Tangentially related to bazelbuild#428 and bazelbuild#421 - currently the clippy aspect
seems to be falling back on a non-worker compile, so it's still
noticeably slower than running cargo clippy directly when minor
changes have been made.
I have a crate that uses pyo3 to create a Python extension module.
On macOS, special link flags are required:
rustc_flags = selects.with_or({
(
"@io_bazel_rules_rust//rust/platform:x86_64-apple-darwin",
): [
"-Clink-arg=-undefined",
"-Clink-arg=dynamic_lookup",
],
"//conditions:default": [],
}),
Without them, the linker on macOS fails, as the Python API is not
available until runtime.
rules_rust's clippy implementation was passing --emit=dep-info,link
to the clippy invocation, causing a clippy run to fail with linking
errors. This patch changes the invocation to use
--emit=dep-info,metadata instead, which is what cargo uses. It also
shaves a bit of time off the check, as linking no longer needs to happen.
Tangentially related to bazelbuild#428 and bazelbuild#421 - currently the clippy aspect
seems to be falling back on a non-worker compile, so it's still
noticeably slower than running cargo clippy directly when minor
changes have been made.
I have a crate that uses pyo3 to create a Python extension module.
On macOS, special link flags are required:
rustc_flags = selects.with_or({
(
"@io_bazel_rules_rust//rust/platform:x86_64-apple-darwin",
): [
"-Clink-arg=-undefined",
"-Clink-arg=dynamic_lookup",
],
"//conditions:default": [],
}),
Without them, the linker on macOS fails, as the Python API is not
available until runtime.
rules_rust's clippy implementation was passing --emit=dep-info,link
to the clippy invocation, causing a clippy run to fail with linking
errors. This patch changes the invocation to use
--emit=dep-info,metadata instead, which is what cargo uses. It also
shaves a bit of time off the check, as linking no longer needs to happen.
Tangentially related to bazelbuild#428 and bazelbuild#421 - currently the clippy aspect
seems to be falling back on a non-worker compile, so it's still
noticeably slower than running cargo clippy directly when minor
changes have been made.
I have a crate that uses pyo3 to create a Python extension module.
On macOS, special link flags are required:
rustc_flags = selects.with_or({
(
"@io_bazel_rules_rust//rust/platform:x86_64-apple-darwin",
): [
"-Clink-arg=-undefined",
"-Clink-arg=dynamic_lookup",
],
"//conditions:default": [],
}),
Without them, the linker on macOS fails, as the Python API is not
available until runtime.
rules_rust's clippy implementation was passing --emit=dep-info,link
to the clippy invocation, causing a clippy run to fail with linking
errors. This patch changes the invocation to use
--emit=dep-info,metadata instead, which is what cargo uses. It also
shaves a bit of time off the check, as linking no longer needs to happen.
Tangentially related to bazelbuild#428 and bazelbuild#421 - currently the clippy aspect
seems to be falling back on a non-worker compile, so it's still
noticeably slower than running cargo clippy directly when minor
changes have been made.
I have a crate that uses pyo3 to create a Python extension module.
On macOS, special link flags are required:
rustc_flags = selects.with_or({
(
"@io_bazel_rules_rust//rust/platform:x86_64-apple-darwin",
): [
"-Clink-arg=-undefined",
"-Clink-arg=dynamic_lookup",
],
"//conditions:default": [],
}),
Without them, the linker on macOS fails, as the Python API is not
available until runtime.
rules_rust's clippy implementation was passing --emit=dep-info,link
to the clippy invocation, causing a clippy run to fail with linking
errors. This patch changes the invocation to use
--emit=dep-info,metadata instead, which is what cargo uses. It also
shaves a bit of time off the check, as linking no longer needs to happen.
Tangentially related to bazelbuild#428 and bazelbuild#421 - currently the clippy aspect
seems to be falling back on a non-worker compile, so it's still
noticeably slower than running cargo clippy directly when minor
changes have been made.
|
Hi all, so I believe what David suggests is doable - we'll have to register 2 actions, one doing the check and producing the throw away metadata file (and this file will be put into the DefaultInfo provider so it's built when the target is requested on the bazel command line) and other one doing the link (and the output file will be put into CrateInfo provider so it will be used from other rust rules). Related, I'll try to run some experiments with pipelining. Metadata produced by
David, since we're touching this, do you know if 2) is what Buck does (assuming Buck has pipelining enabled)? |
|
I believe it does not pipeline the Rust builds. We've talked about it and 2 is what we would do, but not implemented yet. |
|
Dependent rust_library should only need the metadata to build and/or
typecheck as well.
I think the problem with teaching bazel to pipeline is treating the 1
--emit=metadata,link action as 2 actions without doing extra work overall.
Fwiw, it's been a long time since I looked at this and the situation Monday
have improved.
I believe Cargo does option 2, without killing of course, for pipelining
(ie. start dependent builds once metadata is available):
rust-lang/cargo#6660
…On Tue, Feb 9, 2021, 03:21 David Tolnay ***@***.***> wrote:
I believe it does not pipeline the Rust builds. We've talked about it and
2 is what we would do, but not implemented yet.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#428 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAROG6HAXOWFCD3JPO3IIKDS6DV7HANCNFSM4SMKEBCQ>
.
|
Are you sure? I tried it in a tiny example where the depenent target was actually rust_binary, so maybe that's the reason, but I couldn't successfully compile using only metadata from Let's come up with simpler terminology :)
Doing what Cargo does in Bazel is borderline infeasible. I'd like to see the impact of pipelining on build speed when using remote execution and approach 2) that duplicates work. |
|
I'm a heavy user of We have a CI job on diffs that runs I've been thinking a bit about how this could be addressed (and potentially address the thin metadata typecheck use case too), but nothing seems great so far. Here's what I've considered so far:
Registering 2 actions sounds interesting, although I don't think it would help with the clippy use case (or for those libraries that happen to depend on a bunch of others in a world where their build artifacts haven't been cached yet). Has there been any progress / new thinking around this? Would love to help. |
|
This is now somewhat possible, see #1649 |
It looks like rules_rust unconditionally passes
--emit=dep-info,link. This means Bazel builds cannot benefit from the biggest rustc speedup since 1.0.0 (https://blog.rust-lang.org/2017/03/16/Rust-1.16.html#whats-in-1160-stable). 😿rules_rust/rust/private/rustc.bzl
Line 374 in e64700d
Is there any idiomatic way to perform non-code-generating builds which type-check only? Equivalent to Cargo's
cargo checkorcargo rustc --profile=check, rustc'srustc --emit=metadata, or Buck's #check flavored builds (buck build :lib#check; https://buck.build/concept/flavors.html#analysis_http).The text was updated successfully, but these errors were encountered: