rustc_compile_action: fix output_hash in filename

[adrianimboden]

I'm experimenting with experimental_use_cc_common_link and found this to be wrong under certain circumstances (I suspect that this codepath could never be taken, since currently only rust_binary seems to support experimental_use_cc_common_link.

When I try to enable experimental_use_cc_common_link for rust_library and companions (adrianimboden@08cb690), the o file gets generated with the hash file in it, generating this error:

$ bazel build @rrra__unicode-ident-1.0.10//:unicode_ident --@rules_rust//rust/settings:experimental_use_cc_common_link
--- snip ---
WARNING: Remote Cache: Expected output external/rrra__unicode-ident-1.0.10/unicode_ident.o was not created locally.
ERROR: /home/thingdust/.cache/bazel/_bazel_thingdust/690491f57e04da302046e3d5841bd6e3/external/rrra__unicode-ident-1.0.10/BUILD.bazel:13:13: output 'external/rrra__unicode-ident-1.0.10/unicode_ident.o' was not created
ERROR: /home/thingdust/.cache/bazel/_bazel_thingdust/690491f57e04da302046e3d5841bd6e3/external/rrra__unicode-ident-1.0.10/BUILD.bazel:13:13: Compiling Rust rlib unicode_ident v1.0.10 (11 files) failed: not all outputs were created or valid
--- snip ---

when we look inside the sandbox:

$ find -name *.o
./bazel-out/k8-fastbuild/bin/external/rrra__unicode-ident-1.0.10/unicode_ident-1105039059.o

this MR fixes that

[krasimirgg]

When I try to enable experimental_use_cc_common_link for rust_library...

What is the underlying issue that triggered your experiments?

It's not the case that "experimental_use_cc_common_link" is not supported for rust_library-es; rather this feature naturally doesn't apply to them; this is already supported without having to do anything today (modulo various bugs and incompatibilities and stuff).

When building individual rust_library-es, no linking happens in general (modulo dynamic libraries, etc., which let's ignore). The experimental_use_cc_common_link is only for rules where the "final linking" step happens, generally binaries such as rust_binary and rust_test. There we have an option to use rustc as the linker (the default), or use the cc_common.link infrastructure (whatever the underlying C++ toolchain uses for linking).

The outputs of the rust_library actions (.rlib-s) are compatible with both linking modes in general, hence we don't need to have the cc_commmon.link attributes on such rules.

[adrianimboden]

Long: https://bazelbuild.slack.com/archives/CSV56UT0F/p1709026282260219

In Short: Running bazel run @rules_rust//tools/rust_analyzer:gen_rust_project does not work when the system compiler does not support -lgcc_s, which automatically gets added by rustc. The error for gen_rust_project comes from a rust_proc_macro of a dependency (e.g. @rrra__clap_derive-4.3.2//:clap_derive).

I tried to add non-rustc linking to rust_proc_macro, but I don't understand the whole picture probably, so this may not make sense at all.

Nevertheless I saw that the declared object file name does not depend on the output_hash, which only by accident does not get triggered. When sometimes later the function gets used with output_hash!=None, it will fail because the generated name is not the same as the declared name.

It is just a minor fix/improvement that does not change anything right now.

[krasimirgg]

Long: https://bazelbuild.slack.com/archives/CSV56UT0F/p1709026282260219

In Short: Running bazel run @rules_rust//tools/rust_analyzer:gen_rust_project does not work when the system compiler does not support -lgcc_s, which automatically gets added by rustc. The error for gen_rust_project comes from a rust_proc_macro of a dependency (e.g. @rrra__clap_derive-4.3.2//:clap_derive).

I tried to add non-rustc linking to rust_proc_macro, but I don't understand the whole picture probably, so this may not make sense at all.

Nevertheless I saw that the declared object file name does not depend on the output_hash, which only by accident does not get triggered. When sometimes later the function gets used with output_hash!=None, it will fail because the generated name is not the same as the declared name.

It is just a minor fix/improvement that does not change anything right now.

Thanks!

non-rustc linking to rust_proc_macro...

Unfortunately this is a dead end right now: rustc adds custom load-bearing sections to proc-macros that we cannot simulate via via cc_common.link, see #2458 (comment).

Nevertheless I saw that the declared object file name does not depend on the output_hash, which only by accident does not get triggered.

Could you elaborate / provide code pointers?
For context:

• It's intentional that this .o file name doesn't depend on the output_hash in the context of building rust_binary with cc_common_link (in that case it matches the output that rustc uses to emit this .o file, effectively it matches the expected name of the final binary).
• The usage of this .o file should be exclusively guarded by experimental_use_cc_common_link in the code (naively if that's not the case, it's probably a bug).

[adrianimboden]

The "bug" is this (links are to the current main branch):

These two code pieces should correspond to each other:

rules_rust/rust/private/rustc.bzl

Line 1271 in 33860ab

output_o = ctx.actions.declare_file(crate_info.name + obj_ext, sibling = crate_info.output)

rules_rust/rust/private/rustc.bzl

Line 954 in 33860ab

rustc_flags.add(output_hash, format = "--codegen=extra-filename=-%s")

but currently, the rustc is being told to add the hash to the filename, while at the same time bazel is told that the file will get generated by rustc without the hash in the filename.

The bug only does not get triggered, because when experimental_use_cc_common_link is True, output_hash always is None.

I argue that this is pure conincidence and this precondition should either be asserted or this combination should be supported.
Currently, none of those things are done, which results in the bazel build error which states that the expected file was not generated.

This MR makes this a supported case.

[krasimirgg]

The bug only does not get triggered, because when experimental_use_cc_common_link is True, output_hash always is None.

I argue that this is pure conincidence and this precondition should either be asserted or this combination should be supported.

Thank you!

There the comment above the second instance that explains this situation, which suggests that's not coincidental:

rules_rust/rust/private/rustc.bzl

Lines 945 to 951 in 33860ab

	# Mangle symbols to disambiguate crates with the same name. This could
	# happen only for non-final artifacts where we compute an output_hash,
	# e.g., rust_library.
	#
	# For "final" artifacts and ones intended for distribution outside of
	# Bazel, such as rust_binary, rust_static_library and rust_shared_library,
	# where output_hash is None we don't need to add these flags.

However it's quite convolved... so +1, adding an assert for this precondition would be great!