Addressing review comments (1/2)

Author: gigaroby

rust/private/providers.bzl

@@ -50,7 +50,7 @@ DepInfo = provider(
         "transitive_build_infos": "depset[BuildInfo]",
         "transitive_crate_outputs": "depset[File]: All transitive crate outputs.",
         "transitive_crates": "depset[CrateInfo]",
-        "transitive_metadata_outputs": "depset[File]: All transitive crate metadata outputs.",
+        "transitive_metadata_outputs": "depset[File]: All transitive metadata dependencies (.rlib, for crates that provide them) and all transitive object dependencies (.rlib) for crates that don't provide metadata.",
         "transitive_noncrates": "depset[LinkerInput]: All transitive dependencies that aren't crates.",
     },
 )

rust/private/rustc.bzl

@@ -237,20 +237,16 @@ def collect_deps(
             if not crate_info.metadata:
                 depend_on = crate_info.output
 
+            # If this dependency is a proc_macro, it still can be used for lib crates
+            # that produce metadata.
+            # In that case, we don't depend on its metadata dependencies.
             transitive_metadata_outputs.append(
                 depset(
                     [depend_on],
-                    transitive = [dep_info.transitive_metadata_outputs],
+                    transitive = [] if _is_proc_macro(crate_info) else [dep_info.transitive_metadata_outputs],
                 ),
             )
 
-            # If this dependency is a proc_macro, it still can be used for lib crates
-            # that produce metadata.
-            if _is_proc_macro(crate_info):
-                transitive_metadata_outputs.append(
-                    depset([crate_info.output]),
-                )
-
             transitive_crate_outputs.append(
                 depset(
                     [crate_info.output],
@@ -922,10 +918,10 @@ def construct_arguments(
 
         _add_native_link_flags(rustc_flags, dep_info, linkstamp_outs, ambiguous_libs, crate_info.type, toolchain, cc_toolchain, feature_configuration)
 
-    link_to_metadata = _depend_on_metadata(crate_info, force_link_to_objects)
+    use_metadata = _depend_on_metadata(crate_info, force_link_to_objects)
 
     # These always need to be added, even if not linking this crate.
-    add_crate_link_flags(rustc_flags, dep_info, force_all_deps_direct, link_to_metadata = link_to_metadata)
+    add_crate_link_flags(rustc_flags, dep_info, force_all_deps_direct, use_metadata)
 
     needs_extern_proc_macro_flag = _is_proc_macro(crate_info) and crate_info.edition != "2015"
     if needs_extern_proc_macro_flag:
@@ -1448,47 +1444,26 @@ def _get_dir_names(files):
         dirs[f.dirname] = None
     return dirs.keys()
 
-def add_crate_link_flags(args, dep_info, force_all_deps_direct = False, link_to_metadata = False):
+def add_crate_link_flags(args, dep_info, force_all_deps_direct = False, use_metadata = False):
     """Adds link flags to an Args object reference
 
     Args:
         args (Args): An arguments object reference
         dep_info (DepInfo): The current target's dependency info
         force_all_deps_direct (bool, optional): Whether to pass the transitive rlibs with --extern
             to the commandline as opposed to -L.
-        link_to_metadata:
+        use_metadata (bool, optional): Build command line arugments using metadata for crates that provide it.
     """
-    # print(" link_to_metadata ", link_to_metadata)
 
-    if not link_to_metadata:
-        if force_all_deps_direct:
-            args.add_all(
-                depset(
-                    transitive = [
-                        dep_info.direct_crates,
-                        dep_info.transitive_crates,
-                    ],
-                ),
-                uniquify = True,
-                map_each = _crate_to_link_flag,
-            )
-        else:
-            # nb. Direct crates are linked via --extern regardless of their crate_type
-            args.add_all(dep_info.direct_crates, map_each = _crate_to_link_flag)
-    elif force_all_deps_direct:
-        args.add_all(
-            depset(
-                transitive = [
-                    dep_info.direct_crates,
-                    dep_info.transitive_crates,
-                ],
-            ),
-            uniquify = True,
-            map_each = _crate_to_link_flag_metadata,
-        )
-    else:
-        # nb. Direct crates are linked via --extern regardless of their crate_type
-        args.add_all(dep_info.direct_crates, map_each = _crate_to_link_flag_metadata)
+    direct_crates = depset(
+        transitive = [
+            dep_info.direct_crates,
+            dep_info.transitive_crates,
+        ],
+    ) if force_all_deps_direct else dep_info.direct_crates
+
+    crate_to_link_flags = _crate_to_link_flag_metadata if use_metadata else _crate_to_link_flag
+    args.add_all(direct_crates, uniquify = True, map_each = crate_to_link_flags)
 
     args.add_all(
         dep_info.transitive_crates,

rust/private/rustdoc.bzl

@@ -37,7 +37,7 @@ def _strip_crate_info_output(crate_info):
         aliases = crate_info.aliases,
         # This crate info should have no output
         output = None,
-        metadata = crate_info.metadata,
+        metadata = None,
         edition = crate_info.edition,
         rustc_env = crate_info.rustc_env,
         is_test = crate_info.is_test,

test/unit/pipelined_compilation/pipelined_compilation_test.bzl

@@ -101,10 +101,11 @@ def _pipelined_compilation_test():
         deps = [":second"],
     )
 
-    NOT_WINDOWS = [
-        "@platforms//os:linux",
-        "@platforms//os:macos",
-    ]
+    NOT_WINDOWS = select({
+        "@platforms//os:linux": [],
+        "@platforms//os:macos": [],
+        "//conditions:default": ["@platforms//:incompatible"],
+    })
     second_lib_test(name = "second_lib_test", target_under_test = ":second", target_compatible_with = NOT_WINDOWS)
     bin_test(name = "bin_test", target_under_test = ":bin", target_compatible_with = NOT_WINDOWS)
 

util/process_wrapper/main.rs

@@ -40,7 +40,6 @@ fn status_code(status: ExitStatus, was_killed: bool) -> i32 {
 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
-    // eprintln!("{:?} {}", status, was_killed);
     match status.code() {
         Some(code) => code,
         // If we killed the process, we expect None here