From 63ca3762f8a8b978c0bf1133c3b553cc330da492 Mon Sep 17 00:00:00 2001 From: Walter Gray Date: Fri, 10 Apr 2026 09:49:01 -0700 Subject: [PATCH] Switch pipelined compilation from --rustc-quit-on-rmeta to -Zno-codegen Replace the process wrapper's rmeta-interception approach (kill rustc after metadata emission) with rustc's -Zno-codegen flag, which produces a hollow rlib containing metadata and MIR but no object code. This is the same approach used by Buck2. Key changes: - Metadata action uses `rustc -Zno-codegen --emit=link=` to produce a hollow rlib (_meta.rlib) instead of raw .rmeta files - Remove --rustc-quit-on-rmeta flag, LineOutput::Terminate, and all associated kill logic from the process wrapper - Full action emits only --emit=link (no longer includes metadata) - Set RUSTC_BOOTSTRAP=1 on both metadata and full actions for SVH compatibility (required for the unstable -Zno-codegen flag) --- cargo/Cargo.lock | 74 ++++++++++-- extensions/prost/private/prost.bzl | 4 +- rust/private/clippy.bzl | 2 +- rust/private/rust.bzl | 8 +- rust/private/rustc.bzl | 37 +++--- rust/private/unpretty.bzl | 2 +- rust/settings/settings.bzl | 11 +- test/process_wrapper/BUILD.bazel | 4 +- ...uit_on_rmeta.rs => rustc_output_format.rs} | 58 +++------ .../metadata_output_groups.bzl | 4 +- .../pipelined_compilation_test.bzl | 112 ++++++++++-------- test/unit/pipelined_compilation/wrap.bzl | 2 +- .../codegen_disambiguation_test.bzl | 2 +- util/process_wrapper/main.rs | 110 ++--------------- util/process_wrapper/options.rs | 15 +-- util/process_wrapper/output.rs | 5 - util/process_wrapper/rustc.rs | 31 ----- 17 files changed, 202 insertions(+), 279 deletions(-) rename test/process_wrapper/{rustc_quit_on_rmeta.rs => rustc_output_format.rs} (60%) diff --git a/cargo/Cargo.lock b/cargo/Cargo.lock index a87f55badf..0e7b096968 100644 --- a/cargo/Cargo.lock +++ b/cargo/Cargo.lock @@ -19,19 +19,19 @@ dependencies = [ "serde-untagged", "serde-value", "thiserror", - "toml", + "toml 0.8.23", "unicode-xid", "url", ] [[package]] name = "cargo_toml" -version = "0.20.5" +version = "0.22.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "88da5a13c620b4ca0078845707ea9c3faf11edbc3ffd8497d11d686211cd1ac0" +checksum = "374b7c592d9c00c1f4972ea58390ac6b18cbb6ab79011f3bdc90a0b82ca06b77" dependencies = [ "serde", - "toml", + "toml 0.9.12+spec-1.1.0", ] [[package]] @@ -48,7 +48,7 @@ dependencies = [ "cargo-util-schemas", "pathdiff", "semver", - "toml", + "toml 0.8.23", ] [[package]] @@ -346,6 +346,15 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_spanned" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6662b5879511e06e8999a8a235d848113e942c9124f211511b16466ee2995f26" +dependencies = [ + "serde_core", +] + [[package]] name = "smallvec" version = "1.15.1" @@ -417,11 +426,26 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dc1beb996b9d83529a9e75c17a1686767d148d70663143c7854d8b4a09ced362" dependencies = [ "serde", - "serde_spanned", - "toml_datetime", + "serde_spanned 0.6.9", + "toml_datetime 0.6.11", "toml_edit", ] +[[package]] +name = "toml" +version = "0.9.12+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cf92845e79fc2e2def6a5d828f0801e29a2f8acc037becc5ab08595c7d5e9863" +dependencies = [ + "indexmap", + "serde_core", + "serde_spanned 1.1.1", + "toml_datetime 0.7.5+spec-1.1.0", + "toml_parser", + "toml_writer", + "winnow 0.7.13", +] + [[package]] name = "toml_datetime" version = "0.6.11" @@ -431,6 +455,15 @@ dependencies = [ "serde", ] +[[package]] +name = "toml_datetime" +version = "0.7.5+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92e1cfed4a3038bc5a127e35a2d360f145e1f4b971b551a2ba5fd7aedf7e1347" +dependencies = [ + "serde_core", +] + [[package]] name = "toml_edit" version = "0.22.27" @@ -439,10 +472,19 @@ checksum = "41fe8c660ae4257887cf66394862d21dbca4a6ddd26f04a3560410406a2f819a" dependencies = [ "indexmap", "serde", - "serde_spanned", - "toml_datetime", + "serde_spanned 0.6.9", + "toml_datetime 0.6.11", "toml_write", - "winnow", + "winnow 0.7.13", +] + +[[package]] +name = "toml_parser" +version = "1.1.2+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a2abe9b86193656635d2411dc43050282ca48aa31c2451210f4202550afb7526" +dependencies = [ + "winnow 1.0.1", ] [[package]] @@ -451,6 +493,12 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5d99f8c9a7727884afe522e9bd5edbfc91a3312b36a77b5fb8926e4c31a41801" +[[package]] +name = "toml_writer" +version = "1.1.1+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "756daf9b1013ebe47a8776667b466417e2d4c5679d441c26230efd9ef78692db" + [[package]] name = "typeid" version = "1.0.3" @@ -496,6 +544,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "winnow" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09dac053f1cd375980747450bfc7250c264eaae0583872e845c0c7cd578872b5" + [[package]] name = "writeable" version = "0.6.2" diff --git a/extensions/prost/private/prost.bzl b/extensions/prost/private/prost.bzl index 4eecf85a06..6a8d36e10d 100644 --- a/extensions/prost/private/prost.bzl +++ b/extensions/prost/private/prost.bzl @@ -180,7 +180,7 @@ def _compile_rust( prefix = "lib", name = crate_name, lib_hash = output_hash, - extension = ".rmeta", + extension = "_meta.rlib", ) lib = ctx.actions.declare_file(lib_name) @@ -193,7 +193,7 @@ def _compile_rust( prefix = "lib", name = crate_name, lib_hash = output_hash, - extension = ".rmeta", + extension = "_meta.rlib", ) rmeta = ctx.actions.declare_file(rmeta_name) rustc_rmeta_output = generate_output_diagnostics(ctx, rmeta) diff --git a/rust/private/clippy.bzl b/rust/private/clippy.bzl index 318c05af8d..16285e7987 100644 --- a/rust/private/clippy.bzl +++ b/rust/private/clippy.bzl @@ -177,7 +177,7 @@ def rust_clippy_action(ctx, clippy_executable, process_wrapper, crate_info, conf out_dir = out_dir, build_env_files = build_env_files, build_flags_files = build_flags_files, - emit = ["dep-info", "metadata"], + emit = ["metadata"], skip_expanding_rustc_env = True, use_json_output = bool(clippy_diagnostics_file), error_format = error_format, diff --git a/rust/private/rust.bzl b/rust/private/rust.bzl index d50abbf180..2b98e2fee2 100644 --- a/rust/private/rust.bzl +++ b/rust/private/rust.bzl @@ -206,7 +206,7 @@ def _rust_library_common(ctx, crate_type): disable_pipelining = getattr(ctx.attr, "disable_pipelining", False), ): rust_metadata = ctx.actions.declare_file( - paths.replace_extension(rust_lib_name, ".rmeta"), + paths.replace_extension(rust_lib_name, "_meta.rlib"), sibling = rust_lib, ) rustc_rmeta_output = generate_output_diagnostics(ctx, rust_metadata) @@ -279,7 +279,7 @@ def _rust_binary_impl(ctx): rustc_rmeta_output = None if can_build_metadata(toolchain, ctx, ctx.attr.crate_type): rust_metadata = ctx.actions.declare_file( - paths.replace_extension("lib" + crate_name, ".rmeta"), + paths.replace_extension("lib" + crate_name, "_meta.rlib"), sibling = output, ) rustc_rmeta_output = generate_output_diagnostics(ctx, rust_metadata) @@ -381,7 +381,7 @@ def _rust_test_impl(ctx): rustc_rmeta_output = None if can_build_metadata(toolchain, ctx, crate_type): rust_metadata = ctx.actions.declare_file( - paths.replace_extension("lib" + crate_name, ".rmeta"), + paths.replace_extension("lib" + crate_name, "_meta.rlib"), sibling = output, ) rustc_rmeta_output = generate_output_diagnostics(ctx, rust_metadata) @@ -451,7 +451,7 @@ def _rust_test_impl(ctx): rustc_rmeta_output = None if can_build_metadata(toolchain, ctx, crate_type): rust_metadata = ctx.actions.declare_file( - paths.replace_extension("lib" + crate_name, ".rmeta"), + paths.replace_extension("lib" + crate_name, "_meta.rlib"), sibling = output, ) rustc_rmeta_output = generate_output_diagnostics(ctx, rust_metadata) diff --git a/rust/private/rustc.bzl b/rust/private/rustc.bzl index 6dc895198b..d94f22a0c6 100644 --- a/rust/private/rustc.bzl +++ b/rust/private/rustc.bzl @@ -901,7 +901,7 @@ def construct_arguments( out_dir, build_env_files, build_flags_files, - emit = ["dep-info", "link"], + emit = ["link"], force_all_deps_direct = False, add_flags_for_binary = False, include_link_flags = True, @@ -1045,8 +1045,11 @@ def construct_arguments( error_format = "json" if build_metadata: - # Configure process_wrapper to terminate rustc when metadata are emitted - process_wrapper_flags.add("--rustc-quit-on-rmeta", "true") + # Build a hollow rlib (metadata-full, Buck2 equivalent) using -Zno-codegen. + # This produces an rlib with metadata but no object code, allowing downstream + # crates to start compiling without waiting for codegen. + # RUSTC_BOOTSTRAP=1 must be set in the action env for this unstable flag. + rustc_flags.add("-Zno-codegen") if crate_info.rustc_rmeta_output: process_wrapper_flags.add("--output-file", crate_info.rustc_rmeta_output.path) elif crate_info.rustc_output: @@ -1079,7 +1082,12 @@ def construct_arguments( emit_without_paths = [] for kind in emit: - if kind == "link" and crate_info.type == "bin" and crate_info.output != None: + if kind == "link" and build_metadata and crate_info.metadata != None: + # Redirect hollow rlib output to the declared metadata file path, + # since -Zno-codegen --emit=link would otherwise write lib.rlib + # which collides with the full action's output. + rustc_flags.add(crate_info.metadata, format = "--emit=link=%s") + elif kind == "link" and crate_info.type == "bin" and crate_info.output != None: rustc_flags.add(crate_info.output, format = "--emit=link=%s") else: emit_without_paths.append(kind) @@ -1377,17 +1385,12 @@ def rustc_compile_action( experimental_use_cc_common_link = experimental_use_cc_common_link, ) + compile_inputs_metadata = compile_inputs + # The types of rustc outputs to emit. - # If we build metadata, we need to keep the command line of the two invocations - # (rlib and rmeta) as similar as possible, otherwise rustc rejects the rmeta as - # a candidate. - # Because of that we need to add emit=metadata to both the rlib and rmeta invocation. - # # When cc_common linking is enabled, emit a `.o` file, which is later # passed to the cc_common.link action. - emit = ["dep-info", "link"] - if build_metadata: - emit.append("metadata") + emit = ["link"] if experimental_use_cc_common_link: emit = ["obj"] @@ -1435,7 +1438,7 @@ def rustc_compile_action( toolchain = toolchain, tool_path = toolchain.rustc.path, cc_toolchain = cc_toolchain, - emit = emit, + emit = ["link"], feature_configuration = feature_configuration, crate_info = crate_info, dep_info = dep_info, @@ -1458,6 +1461,12 @@ def rustc_compile_action( # this is the final list of env vars env.update(env_from_args) + if build_metadata: + # RUSTC_BOOTSTRAP=1 is required for -Zno-codegen on stable rustc, and must + # be set on both the metadata and full actions for SVH compatibility (since + # RUSTC_BOOTSTRAP affects the crate hash). + env["RUSTC_BOOTSTRAP"] = "1" + if hasattr(attr, "version") and attr.version != "0.0.0": formatted_version = " v{}".format(attr.version) else: @@ -1529,7 +1538,7 @@ def rustc_compile_action( if args_metadata: ctx.actions.run( executable = ctx.executable._process_wrapper, - inputs = compile_inputs, + inputs = compile_inputs_metadata, outputs = [build_metadata] + [x for x in [rustc_rmeta_output] if x], env = env, arguments = args_metadata.all, diff --git a/rust/private/unpretty.bzl b/rust/private/unpretty.bzl index be111c83a3..dbb2d28a85 100644 --- a/rust/private/unpretty.bzl +++ b/rust/private/unpretty.bzl @@ -202,7 +202,7 @@ def _rust_unpretty_aspect_impl(target, ctx): out_dir = out_dir, build_env_files = build_env_files, build_flags_files = build_flags_files, - emit = ["dep-info", "metadata"], + emit = ["metadata"], skip_expanding_rustc_env = True, ) diff --git a/rust/settings/settings.bzl b/rust/settings/settings.bzl index 2769272dc5..8ad40deb8e 100644 --- a/rust/settings/settings.bzl +++ b/rust/settings/settings.bzl @@ -112,10 +112,15 @@ def use_real_import_macro(): ) def pipelined_compilation(): - """When set, this flag causes rustc to emit `*.rmeta` files and use them for `rlib -> rlib` dependencies. + """When set, this flag enables pipelined compilation for `rlib -> rlib` dependencies. - While this involves one extra (short) rustc invocation to build the rmeta file, - it allows library dependencies to be unlocked much sooner, increasing parallelism during compilation. + For each library target, a second RustcMetadata action is created that runs rustc with + `-Zno-codegen --emit=link` to produce a hollow rlib (metadata & MIR, no object code). + Downstream library compilations can start as soon as this hollow rlib is available, + increasing parallelism during compilation. + + Requires RUSTC_BOOTSTRAP=1, which is set automatically on both the metadata and full + actions for pipelined targets. """ bool_flag( name = "pipelined_compilation", diff --git a/test/process_wrapper/BUILD.bazel b/test/process_wrapper/BUILD.bazel index 43e2420062..1ab6510841 100644 --- a/test/process_wrapper/BUILD.bazel +++ b/test/process_wrapper/BUILD.bazel @@ -157,8 +157,8 @@ rust_binary( ) rust_test( - name = "rustc_quit_on_rmeta", - srcs = ["rustc_quit_on_rmeta.rs"], + name = "rustc_output_format", + srcs = ["rustc_output_format.rs"], data = [ ":fake_rustc", "//util/process_wrapper", diff --git a/test/process_wrapper/rustc_quit_on_rmeta.rs b/test/process_wrapper/rustc_output_format.rs similarity index 60% rename from test/process_wrapper/rustc_quit_on_rmeta.rs rename to test/process_wrapper/rustc_output_format.rs index b804ba1ebb..1fa64e7dc9 100644 --- a/test/process_wrapper/rustc_quit_on_rmeta.rs +++ b/test/process_wrapper/rustc_output_format.rs @@ -40,54 +40,34 @@ mod test { } #[test] - fn test_rustc_quit_on_rmeta_quits() { - let out_content = fake_rustc( - &[ - "--rustc-quit-on-rmeta", - "true", - "--rustc-output-format", - "rendered", - ], - &[], - true, + fn test_rustc_output_format_rendered() { + let out_content = fake_rustc(&["--rustc-output-format", "rendered"], &[], true); + assert!( + out_content.contains("should be\nin output"), + "output should contain the first rendered message", + ); + assert!( + out_content.contains("should not be in output"), + "output should contain the second rendered message", ); assert!( - !out_content.contains("should not be in output"), - "output should not contain 'should not be in output' but did", + !out_content.contains(r#""rendered""#), + "rendered mode should not print raw json", ); } #[test] - fn test_rustc_quit_on_rmeta_output_json() { - let json_content = fake_rustc( - &[ - "--rustc-quit-on-rmeta", - "true", - "--rustc-output-format", - "json", - ], - &[], - true, - ); + fn test_rustc_output_format_json() { + let json_content = fake_rustc(&["--rustc-output-format", "json"], &[], true); assert_eq!( json_content, - concat!(r#"{"rendered": "should be\nin output"}"#, "\n") - ); - } - - #[test] - fn test_rustc_quit_on_rmeta_output_rendered() { - let rendered_content = fake_rustc( - &[ - "--rustc-quit-on-rmeta", - "true", - "--rustc-output-format", - "rendered", - ], - &[], - true, + concat!( + r#"{"rendered": "should be\nin output"}"#, + "\n", + r#"{"rendered": "should not be in output"}"#, + "\n" + ) ); - assert_eq!(rendered_content, "should be\nin output"); } #[test] diff --git a/test/unit/metadata_output_groups/metadata_output_groups.bzl b/test/unit/metadata_output_groups/metadata_output_groups.bzl index ec89e0f132..a20c9ac5b2 100644 --- a/test/unit/metadata_output_groups/metadata_output_groups.bzl +++ b/test/unit/metadata_output_groups/metadata_output_groups.bzl @@ -13,8 +13,8 @@ def _metadata_output_groups_present_test_impl(ctx): asserts.equals(env, 1, len(build_metadata), "Expected 1 build_metadata file") asserts.true( - build_metadata[0].basename.endswith(".rmeta"), - "Expected %s to end with .rmeta" % build_metadata[0], + build_metadata[0].basename.endswith("_meta.rlib"), + "Expected %s to end with _meta.rlib" % build_metadata[0], ) asserts.equals(env, 1, len(rustc_rmeta_output), "Expected 1 rustc_rmeta_output file") diff --git a/test/unit/pipelined_compilation/pipelined_compilation_test.bzl b/test/unit/pipelined_compilation/pipelined_compilation_test.bzl index 36a3de891b..a10dad44e2 100644 --- a/test/unit/pipelined_compilation/pipelined_compilation_test.bzl +++ b/test/unit/pipelined_compilation/pipelined_compilation_test.bzl @@ -2,7 +2,7 @@ load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") load("//rust:defs.bzl", "rust_binary", "rust_library", "rust_proc_macro") -load("//test/unit:common.bzl", "assert_argv_contains", "assert_list_contains_adjacent_elements", "assert_list_contains_adjacent_elements_not") +load("//test/unit:common.bzl", "assert_argv_contains") load(":wrap.bzl", "wrap") ENABLE_PIPELINING = { @@ -22,49 +22,56 @@ def _second_lib_test_impl(ctx): rlib_action = [act for act in tut.actions if act.mnemonic == "Rustc"][0] metadata_action = [act for act in tut.actions if act.mnemonic == "RustcMetadata"][0] - # Both actions should use the same --emit= - assert_argv_contains(env, rlib_action, "--emit=dep-info,link,metadata") - assert_argv_contains(env, metadata_action, "--emit=dep-info,link,metadata") + # The full action emits link; the metadata action emits only + # link with an explicit path (--emit=link=) and uses -Zno-codegen to + # produce a hollow rlib (metadata-full). + assert_argv_contains(env, rlib_action, "--emit=link") - # The metadata action should have a .rmeta as output and the rlib action a .rlib + # RustcMetadata uses --emit=link= to redirect the hollow rlib to the + # declared .rmeta output. Check that it contains the flag with a path. + metadata_emit = [arg for arg in metadata_action.argv if arg.startswith("--emit=link=")] + asserts.true( + env, + len(metadata_emit) == 1, + "expected RustcMetadata to have --emit=link=, got " + str(metadata_emit), + ) + + # RustcMetadata must use -Zno-codegen to produce a hollow rlib + assert_argv_contains(env, metadata_action, "-Zno-codegen") + + # The metadata action outputs a hollow .rlib (_meta.rlib), the full action a normal .rlib path = rlib_action.outputs.to_list()[0].path asserts.true( env, - path.endswith(".rlib"), - "expected Rustc to output .rlib, got " + path, + path.endswith(".rlib") and not path.endswith("_meta.rlib"), + "expected Rustc to output .rlib (not _meta.rlib), got " + path, ) path = metadata_action.outputs.to_list()[0].path asserts.true( env, - path.endswith(".rmeta"), - "expected RustcMetadata to output .rmeta, got " + path, + path.endswith("_meta.rlib"), + "expected RustcMetadata to output _meta.rlib, got " + path, ) - # Only the action building metadata should contain --rustc-quit-on-rmeta - assert_list_contains_adjacent_elements_not(env, rlib_action.argv, ["--rustc-quit-on-rmeta", "true"]) - assert_list_contains_adjacent_elements(env, metadata_action.argv, ["--rustc-quit-on-rmeta", "true"]) - - # Check that both actions refer to the metadata of :first, not the rlib - extern_metadata = [arg for arg in metadata_action.argv if arg.startswith("--extern=first=") and "libfirst" in arg and arg.endswith(".rmeta")] + # Both actions should refer to the metadata artifact of :first. + extern_metadata = [arg for arg in metadata_action.argv if arg.startswith("--extern=first=") and "libfirst" in arg and arg.endswith("_meta.rlib")] asserts.true( env, len(extern_metadata) == 1, - "did not find a --extern=first=*.rmeta but expected one", + "expected RustcMetadata --extern=first=*_meta.rlib, got " + str([a for a in metadata_action.argv if "--extern=first=" in a]), ) - extern_rlib = [arg for arg in rlib_action.argv if arg.startswith("--extern=first=") and "libfirst" in arg and arg.endswith(".rmeta")] + extern_rlib = [arg for arg in rlib_action.argv if arg.startswith("--extern=first=") and "libfirst" in arg and arg.endswith("_meta.rlib")] asserts.true( env, len(extern_rlib) == 1, - "did not find a --extern=first=*.rlib but expected one", + "expected Rustc --extern=first=*_meta.rlib, got " + str([a for a in rlib_action.argv if "--extern=first=" in a]), ) - # Check that the input to both actions is the metadata of :first - input_metadata = [i for i in metadata_action.inputs.to_list() if i.basename.startswith("libfirst")] - asserts.true(env, len(input_metadata) == 1, "expected only one libfirst input, found " + str([i.path for i in input_metadata])) - asserts.true(env, input_metadata[0].extension == "rmeta", "expected libfirst dependency to be rmeta, found " + input_metadata[0].path) - input_rlib = [i for i in rlib_action.inputs.to_list() if i.basename.startswith("libfirst")] - asserts.true(env, len(input_rlib) == 1, "expected only one libfirst input, found " + str([i.path for i in input_rlib])) - asserts.true(env, input_rlib[0].extension == "rmeta", "expected libfirst dependency to be rmeta, found " + input_rlib[0].path) + # Both actions should take the metadata artifact as input. + input_metadata = [i for i in metadata_action.inputs.to_list() if i.basename.startswith("libfirst") and i.basename.endswith("_meta.rlib")] + asserts.true(env, len(input_metadata) == 1, "expected one libfirst _meta.rlib input to RustcMetadata, found " + str([i.path for i in metadata_action.inputs.to_list() if i.basename.startswith("libfirst")])) + input_rlib = [i for i in rlib_action.inputs.to_list() if i.basename.startswith("libfirst") and i.basename.endswith("_meta.rlib")] + asserts.true(env, len(input_rlib) == 1, "expected one libfirst _meta.rlib input to Rustc, found " + str([i.path for i in rlib_action.inputs.to_list() if i.basename.startswith("libfirst")])) return analysistest.end(env) @@ -73,10 +80,10 @@ def _bin_test_impl(ctx): tut = analysistest.target_under_test(env) bin_action = [act for act in tut.actions if act.mnemonic == "Rustc"][0] - # Check that no inputs to this binary are .rmeta files. - metadata_inputs = [i.path for i in bin_action.inputs.to_list() if i.path.endswith(".rmeta")] + # Check that no inputs to this binary are hollow rlib (_meta.rlib) files. + metadata_inputs = [i.path for i in bin_action.inputs.to_list() if i.path.endswith("_meta.rlib")] - # Filter out toolchain targets. This test intends to only check for rmeta files of `deps`. + # Filter out toolchain targets. This test intends to only check for metadata files of `deps`. metadata_inputs = [i for i in metadata_inputs if "/lib/rustlib" not in i] asserts.false(env, metadata_inputs, "expected no metadata inputs, found " + json.encode_indent(metadata_inputs, indent = " " * 4)) @@ -130,6 +137,14 @@ def _pipelined_compilation_test(): ":bin_test", ] +def _is_metadata_file(path): + """Returns True if the path is a hollow rlib (metadata-full) file.""" + return path.endswith("_meta.rlib") + +def _is_full_rlib(path): + """Returns True if the path is a full rlib (not a hollow rlib).""" + return path.endswith(".rlib") and not path.endswith("_meta.rlib") + def _rmeta_is_propagated_through_custom_rule_test_impl(ctx): env = analysistest.begin(ctx) tut = analysistest.target_under_test(env) @@ -138,24 +153,21 @@ def _rmeta_is_propagated_through_custom_rule_test_impl(ctx): # also depend on metadata for 'wrapper'. rust_action = [act for act in tut.actions if act.mnemonic == "RustcMetadata"][0] - metadata_inputs = [i for i in rust_action.inputs.to_list() if i.path.endswith(".rmeta")] - rlib_inputs = [i for i in rust_action.inputs.to_list() if i.path.endswith(".rlib")] - seen_wrapper_metadata = False seen_to_wrap_metadata = False - for mi in metadata_inputs: - if "libwrapper" in mi.path: - seen_wrapper_metadata = True - if "libto_wrap" in mi.path: - seen_to_wrap_metadata = True - seen_wrapper_rlib = False seen_to_wrap_rlib = False - for ri in rlib_inputs: - if "libwrapper" in ri.path: - seen_wrapper_rlib = True - if "libto_wrap" in ri.path: - seen_to_wrap_rlib = True + for i in rust_action.inputs.to_list(): + if "libwrapper" in i.path: + if _is_metadata_file(i.path): + seen_wrapper_metadata = True + elif _is_full_rlib(i.path): + seen_wrapper_rlib = True + if "libto_wrap" in i.path: + if _is_metadata_file(i.path): + seen_to_wrap_metadata = True + elif _is_full_rlib(i.path): + seen_to_wrap_rlib = True if ctx.attr.generate_metadata: asserts.true(env, seen_wrapper_metadata, "expected dependency on metadata for 'wrapper' but not found") @@ -176,22 +188,23 @@ def _rmeta_is_used_when_building_custom_rule_test_impl(ctx): # This is the custom rule invocation of rustc. rust_action = [act for act in tut.actions if act.mnemonic == "Rustc"][0] - # We want to check that the action depends on metadata, regardless of ctx.attr.generate_metadata + # The custom rule invocation should depend on metadata, regardless of whether + # the wrapper itself generates metadata. seen_to_wrap_rlib = False - seen_to_wrap_rmeta = False + seen_to_wrap_metadata = False for act in rust_action.inputs.to_list(): - if "libto_wrap" in act.path and act.path.endswith(".rlib"): + if "libto_wrap" in act.path and _is_full_rlib(act.path): seen_to_wrap_rlib = True - elif "libto_wrap" in act.path and act.path.endswith(".rmeta"): - seen_to_wrap_rmeta = True + elif "libto_wrap" in act.path and _is_metadata_file(act.path): + seen_to_wrap_metadata = True - asserts.true(env, seen_to_wrap_rmeta, "expected dependency on metadata for 'to_wrap' but not found") + asserts.true(env, seen_to_wrap_metadata, "expected dependency on metadata for 'to_wrap' but not found") asserts.false(env, seen_to_wrap_rlib, "expected no dependency on object for 'to_wrap' but it was found") return analysistest.end(env) rmeta_is_propagated_through_custom_rule_test = analysistest.make(_rmeta_is_propagated_through_custom_rule_test_impl, attrs = {"generate_metadata": attr.bool()}, config_settings = ENABLE_PIPELINING) -rmeta_is_used_when_building_custom_rule_test = analysistest.make(_rmeta_is_used_when_building_custom_rule_test_impl, config_settings = ENABLE_PIPELINING) +rmeta_is_used_when_building_custom_rule_test = analysistest.make(_rmeta_is_used_when_building_custom_rule_test_impl, attrs = {"generate_metadata": attr.bool()}, config_settings = ENABLE_PIPELINING) def _rmeta_not_produced_if_pipelining_disabled_test_impl(ctx): env = analysistest.begin(ctx) @@ -249,6 +262,7 @@ def _custom_rule_test(generate_metadata, suffix): rmeta_is_used_when_building_custom_rule_test( name = "rmeta_is_used_when_building_custom_rule_test" + suffix, + generate_metadata = generate_metadata, target_under_test = ":wrapper" + suffix, target_compatible_with = _NO_WINDOWS, ) diff --git a/test/unit/pipelined_compilation/wrap.bzl b/test/unit/pipelined_compilation/wrap.bzl index f24a0e421a..89b75b1850 100644 --- a/test/unit/pipelined_compilation/wrap.bzl +++ b/test/unit/pipelined_compilation/wrap.bzl @@ -44,7 +44,7 @@ def _wrap_impl(ctx): prefix = "lib", name = crate_name, lib_hash = output_hash, - extension = ".rmeta", + extension = "_meta.rlib", ) tgt = ctx.attr.target diff --git a/test/unit/rust_test_codegen_disambiguation/codegen_disambiguation_test.bzl b/test/unit/rust_test_codegen_disambiguation/codegen_disambiguation_test.bzl index 1b0c353f61..f263a4538c 100644 --- a/test/unit/rust_test_codegen_disambiguation/codegen_disambiguation_test.bzl +++ b/test/unit/rust_test_codegen_disambiguation/codegen_disambiguation_test.bzl @@ -1,7 +1,7 @@ """Tests that rust_test targets receive codegen disambiguation flags. rust_test targets pass --codegen=metadata and --codegen=extra-filename to rustc -so that intermediate compilation artifacts (.o, .d files) get unique names. This +so that intermediate compilation artifacts (such as .o files) get unique names. This prevents collisions with rust_binary or rust_library targets that share the same crate name, which would otherwise cause link failures on non-sandboxed builds (e.g. Windows or --spawn_strategy=standalone). See diff --git a/util/process_wrapper/main.rs b/util/process_wrapper/main.rs index 39a6d6db16..5df20a814c 100644 --- a/util/process_wrapper/main.rs +++ b/util/process_wrapper/main.rs @@ -31,27 +31,13 @@ use crate::output::{process_output, LineOutput}; use crate::rustc::ErrorFormat; #[cfg(windows)] -fn status_code(status: ExitStatus, was_killed: bool) -> i32 { - // On windows, there's no good way to know if the process was killed by a signal. - // If we killed the process, we override the code to signal success. - if was_killed { - 0 - } else { - status.code().unwrap_or(1) - } +fn status_code(status: ExitStatus) -> i32 { + status.code().unwrap_or(1) } #[cfg(not(windows))] -fn status_code(status: ExitStatus, was_killed: bool) -> i32 { - // On unix, if code is None it means that the process was killed by a signal. - // https://doc.rust-lang.org/std/process/struct.ExitStatus.html#method.success - match status.code() { - Some(code) => code, - // If we killed the process, we expect None here - None if was_killed => 0, - // Otherwise it's some unexpected signal - None => 1, - } +fn status_code(status: ExitStatus) -> i32 { + status.code().unwrap_or(1) } #[derive(Debug)] @@ -91,12 +77,7 @@ fn json_warning(line: &str) -> JsonValue { ])) } -fn process_line( - mut line: String, - quit_on_rmeta: bool, - format: ErrorFormat, - metadata_emitted: &mut bool, -) -> Result { +fn process_line(mut line: String, format: ErrorFormat) -> Result { // LLVM can emit lines that look like the following, and these will be interspersed // with the regular JSON output. Arguably, rustc should be fixed not to emit lines // like these (or to convert them to JSON), but for now we convert them to JSON @@ -110,11 +91,7 @@ fn process_line( return Ok(LineOutput::Skip); } } - if quit_on_rmeta { - rustc::stop_on_rmeta_completion(line, format, metadata_emitted) - } else { - rustc::process_json(line, format) - } + rustc::process_json(line, format) } fn main() -> Result<(), ProcessWrapperError> { @@ -172,25 +149,13 @@ fn main() -> Result<(), ProcessWrapperError> { None }; - let mut was_killed = false; let result = if let Some(format) = opts.rustc_output_format { - let quit_on_rmeta = opts.rustc_quit_on_rmeta; - // Process json rustc output and kill the subprocess when we get a signal - // that we emitted a metadata file. - let mut me = false; - let metadata_emitted = &mut me; let result = process_output( &mut child_stderr, stderr.as_mut(), output_file.as_mut(), - move |line| process_line(line, quit_on_rmeta, format, metadata_emitted), + move |line| process_line(line, format), ); - if me { - // If recv returns Ok(), a signal was sent in this channel so we should terminate the child process. - // We can safely ignore the Result from kill() as we don't care if the process already terminated. - let _ = child.kill(); - was_killed = true; - } result } else { // Process output normally by forwarding stderr @@ -206,8 +171,7 @@ fn main() -> Result<(), ProcessWrapperError> { let status = child .wait() .map_err(|e| ProcessWrapperError(format!("failed to wait for child process: {}", e)))?; - // If the child process is rustc and is killed after metadata generation, that's also a success. - let code = status_code(status, was_killed); + let code = status_code(status); let success = code == 0; if success { if let Some(tf) = opts.touch_file { @@ -241,7 +205,6 @@ mod test { #[test] fn test_process_line_diagnostic_json() -> Result<(), String> { - let mut metadata_emitted = false; let LineOutput::Message(msg) = process_line( r#" { @@ -250,9 +213,7 @@ mod test { } "# .to_string(), - false, ErrorFormat::Json, - &mut metadata_emitted, )? else { return Err("Expected a LineOutput::Message".to_string()); @@ -273,7 +234,6 @@ mod test { #[test] fn test_process_line_diagnostic_rendered() -> Result<(), String> { - let mut metadata_emitted = false; let LineOutput::Message(msg) = process_line( r#" { @@ -282,9 +242,7 @@ mod test { } "# .to_string(), - /*quit_on_rmeta=*/ false, ErrorFormat::Rendered, - &mut metadata_emitted, )? else { return Err("Expected a LineOutput::Message".to_string()); @@ -295,17 +253,11 @@ mod test { #[test] fn test_process_line_noise() -> Result<(), String> { - let mut metadata_emitted = false; for text in [ "'+zaamo' is not a recognized feature for this target (ignoring feature)", " WARN rustc_errors::emitter Invalid span...", ] { - let LineOutput::Message(msg) = process_line( - text.to_string(), - /*quit_on_rmeta=*/ false, - ErrorFormat::Json, - &mut metadata_emitted, - )? + let LineOutput::Message(msg) = process_line(text.to_string(), ErrorFormat::Json)? else { return Err("Expected a LineOutput::Message".to_string()); }; @@ -327,48 +279,4 @@ mod test { } Ok(()) } - - #[test] - fn test_process_line_emit_link() -> Result<(), String> { - let mut metadata_emitted = false; - assert!(matches!( - process_line( - r#" - { - "$message_type": "artifact", - "emit": "link" - } - "# - .to_string(), - /*quit_on_rmeta=*/ true, - ErrorFormat::Rendered, - &mut metadata_emitted, - )?, - LineOutput::Skip - )); - assert!(!metadata_emitted); - Ok(()) - } - - #[test] - fn test_process_line_emit_metadata() -> Result<(), String> { - let mut metadata_emitted = false; - assert!(matches!( - process_line( - r#" - { - "$message_type": "artifact", - "emit": "metadata" - } - "# - .to_string(), - /*quit_on_rmeta=*/ true, - ErrorFormat::Rendered, - &mut metadata_emitted, - )?, - LineOutput::Terminate - )); - assert!(metadata_emitted); - Ok(()) - } } diff --git a/util/process_wrapper/options.rs b/util/process_wrapper/options.rs index 2f252cadc7..bf4056fce0 100644 --- a/util/process_wrapper/options.rs +++ b/util/process_wrapper/options.rs @@ -44,9 +44,6 @@ pub(crate) struct Options { // If set, also logs all unprocessed output from the rustc output to this file. // Meant to be used to get json output out of rustc for tooling usage. pub(crate) output_file: Option, - // If set, it configures rustc to emit an rmeta file and then - // quit. - pub(crate) rustc_quit_on_rmeta: bool, // This controls the output format of rustc messages. pub(crate) rustc_output_format: Option, } @@ -64,7 +61,6 @@ pub(crate) fn options() -> Result { let mut stdout_file = None; let mut stderr_file = None; let mut output_file = None; - let mut rustc_quit_on_rmeta_raw = None; let mut rustc_output_format_raw = None; let mut flags = Flags::new(); let mut require_explicit_unstable_features = None; @@ -102,15 +98,10 @@ pub(crate) fn options() -> Result { "Log all unprocessed subprocess stderr in this file.", &mut output_file, ); - flags.define_flag( - "--rustc-quit-on-rmeta", - "If enabled, this wrapper will terminate rustc after rmeta has been emitted.", - &mut rustc_quit_on_rmeta_raw, - ); flags.define_flag( "--rustc-output-format", - "Controls the rustc output format if --rustc-quit-on-rmeta is set.\n\ - 'json' will cause the json output to be output, \ + "Controls how rustc JSON messages are forwarded.\n\ + 'json' will pass through JSON messages, \ 'rendered' will extract the rendered message and print that.\n\ Default: `rendered`", &mut rustc_output_format_raw, @@ -179,7 +170,6 @@ pub(crate) fn options() -> Result { }) .transpose()?; - let rustc_quit_on_rmeta = rustc_quit_on_rmeta_raw.is_some_and(|s| s == "true"); let rustc_output_format = rustc_output_format_raw .map(|v| match v.as_str() { "json" => Ok(rustc::ErrorFormat::Json), @@ -227,7 +217,6 @@ pub(crate) fn options() -> Result { stdout_file, stderr_file, output_file, - rustc_quit_on_rmeta, rustc_output_format, }) } diff --git a/util/process_wrapper/output.rs b/util/process_wrapper/output.rs index 4b3604b18d..6ab48a2214 100644 --- a/util/process_wrapper/output.rs +++ b/util/process_wrapper/output.rs @@ -19,14 +19,10 @@ use std::io::{self, prelude::*}; /// LineOutput tells process_output what to do when a line is processed. /// If a Message is returned, it will be written to write_end, if /// Skip is returned nothing will be printed and execution continues, -/// if Terminate is returned, process_output returns immediately. -/// Terminate is used to stop processing when we see an emit metadata -/// message. #[derive(Debug)] pub(crate) enum LineOutput { Message(String), Skip, - Terminate, } #[derive(Debug)] @@ -95,7 +91,6 @@ where match process_line(line.clone()) { Ok(LineOutput::Message(to_write)) => output_writer.write_all(to_write.as_bytes())?, Ok(LineOutput::Skip) => {} - Ok(LineOutput::Terminate) => return Ok(()), Err(msg) => { failed_on = Some((line, msg)); break; diff --git a/util/process_wrapper/rustc.rs b/util/process_wrapper/rustc.rs index 97ee466337..e250c2857f 100644 --- a/util/process_wrapper/rustc.rs +++ b/util/process_wrapper/rustc.rs @@ -39,16 +39,12 @@ fn get_key(value: &JsonValue, key: &str) -> Option { #[derive(Debug)] enum RustcMessage { - Emit(String), Message(String), } impl TryFrom for RustcMessage { type Error = (); fn try_from(val: JsonValue) -> Result { - if let Some(emit) = get_key(&val, "emit") { - return Ok(Self::Emit(emit)); - } if let Some(rendered) = get_key(&val, "rendered") { return Ok(Self::Message(rendered)); } @@ -73,33 +69,6 @@ pub(crate) fn process_json(line: String, error_format: ErrorFormat) -> LineResul }) } -/// stop_on_rmeta_completion parses the json output of rustc in the same way -/// process_rustc_json does. In addition, it will signal to stop when metadata -/// is emitted so the compiler can be terminated. -/// This is used to implement pipelining in rules_rust, please see -/// https://internals.rust-lang.org/t/evaluating-pipelined-rustc-compilation/10199 -/// Returns an error if parsing json fails. -/// TODO: pass a function to handle the emit event and merge with process_json -pub(crate) fn stop_on_rmeta_completion( - line: String, - error_format: ErrorFormat, - kill: &mut bool, -) -> LineResult { - let parsed: JsonValue = line - .parse() - .map_err(|_| "error parsing rustc output as json".to_owned())?; - Ok(match parsed.try_into() { - Ok(RustcMessage::Emit(emit)) if emit == "metadata" => { - *kill = true; - LineOutput::Terminate - } - Ok(RustcMessage::Message(rendered)) => { - output_based_on_error_format(line, rendered, error_format) - } - _ => LineOutput::Skip, - }) -} - fn output_based_on_error_format( line: String, rendered: String,