Update crate_universe to run Buildifier on stdin (#3912)

closes #2972

Author: UebelAndre

cargo/tests/cargo_build_script/run_from_exec_root/build.rs

@@ -1,5 +1,6 @@
 fn main() {
-    let contents = std::fs::read_to_string("cargo/tests/cargo_build_script/run_from_exec_root/data.txt")
-        .expect("Failed to read data file");
+    let contents =
+        std::fs::read_to_string("cargo/tests/cargo_build_script/run_from_exec_root/data.txt")
+            .expect("Failed to read data file");
     println!("cargo:rustc-env=DATA={}", contents);
 }

crate_universe/private/vendor_utils.bzl

@@ -3,14 +3,14 @@
 load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_file")
 load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")
 
-_BUILDIFIER_VERSION = "8.0.3"
+_BUILDIFIER_VERSION = "8.5.1"
 _BUILDIFIER_URL_TEMPLATE = "https://github.com/bazelbuild/buildtools/releases/download/v{version}/{bin}"
 _BUILDIFIER_INTEGRITY = {
-    "buildifier-darwin-amd64": "sha256-t6MVLN4LOXGxEH8idK/neMXBVNzfbJxmmiMePABPBH4=",
-    "buildifier-darwin-arm64": "sha256-Z0xmP3tc0DwAL4yoNKjBwAjMtSegoqEy0Ip6NViDsi0=",
-    "buildifier-linux-amd64": "sha256-yWlIfBr4XnCFdsjf3Qu0aB6uWKrXnmiuSIgscIcYQbc=",
-    "buildifier-linux-arm64": "sha256-vdm5Lixl1Gr/7srvtU5o00wnLR9KjFtUkpo+kqt4ggo=",
-    "buildifier-windows-amd64.exe": "sha256-Y6JC9X4lPv57lXPXOcCKPQ5ijv2EAVyNrRfYe2Qp5EM=",
+    "buildifier-darwin-amd64": "sha256-Md4Ynho/5Tqp6Mj3SgMJwyUnStGXkzk5GeHKZRY8oaQ=",
+    "buildifier-darwin-arm64": "sha256-YoNqlmf6DbMJsNkehA8KPygTqcjqPkS5zVgYfJC8iLo=",
+    "buildifier-linux-amd64": "sha256-iHN3/GTSOoUPTRigd7XbBbGZE/S5mycNGT88czS1qac=",
+    "buildifier-linux-arm64": "sha256-lHv2cA1wgCayBXsJvqCau8PK/BXZ7Oo1uziFxLCczQQ=",
+    "buildifier-windows-amd64.exe": "sha256-9Oy5xz3ivDi4RdTuJ2aPYkjEgTpmR9tLSTGnVWBS5OE=",
 }
 
 def crates_vendor_deps():

crate_universe/src/cli/vendor.rs

@@ -1,8 +1,9 @@
 //! The cli entrypoint for the `vendor` subcommand
 
-use std::collections::{BTreeSet, HashMap};
+use std::collections::{BTreeMap, HashMap};
 use std::env;
 use std::fs;
+use std::io::Write;
 use std::path::{Path, PathBuf};
 use std::process::{self, ExitStatus};
 use std::sync::Arc;
@@ -89,19 +90,41 @@ pub struct VendorOptions {
     pub nonhermetic_root_bazel_workspace_dir: Utf8PathBuf,
 }
 
-/// Run buildifier on a given file.
-fn buildifier_format(bin: &Path, file: &Path) -> anyhow::Result<ExitStatus> {
-    let status = process::Command::new(bin)
+/// Format content via buildifier's stdin/stdout, avoiding the need to write
+/// the file to disk before formatting. The `path` argument is passed as
+/// `--path` so buildifier can infer the file type (BUILD vs .bzl).
+///
+/// See <https://github.com/bazelbuild/rules_rust/issues/2972>.
+fn buildifier_format(bin: &Path, content: &str, path: &Path) -> anyhow::Result<String> {
+    let mut child = process::Command::new(bin)
         .args(["-lint=fix", "-mode=fix", "-warnings=all"])
-        .arg(file)
-        .status()
-        .context("Failed to apply buildifier fixes")?;
-
-    if !status.success() {
-        bail!(status)
+        .arg(format!("--path={}", path.display()))
+        .stdin(process::Stdio::piped())
+        .stdout(process::Stdio::piped())
+        .stderr(process::Stdio::piped())
+        .spawn()
+        .context("Failed to spawn buildifier")?;
+
+    child
+        .stdin
+        .take()
+        .unwrap()
+        .write_all(content.as_bytes())
+        .context("Failed to write to buildifier stdin")?;
+
+    let output = child
+        .wait_with_output()
+        .context("Failed to wait for buildifier")?;
+
+    if !output.status.success() {
+        bail!(
+            "buildifier failed on {}: {}",
+            path.display(),
+            String::from_utf8_lossy(&output.stderr)
+        );
     }
 
-    Ok(status)
+    String::from_utf8(output.stdout).context("buildifier produced invalid UTF-8")
 }
 
 /// Run `bazel mod tidy` in a workspace.
@@ -295,21 +318,25 @@ pub fn vendor(opt: VendorOptions) -> anyhow::Result<()> {
     // make cargo versioned crates compatible with bazel labels
     let normalized_outputs = normalize_cargo_file_paths(outputs, &opt.workspace_dir);
 
-    // buildifier files to check
-    let file_names: BTreeSet<PathBuf> = normalized_outputs.keys().cloned().collect();
+    // Optionally format outputs through buildifier before writing to disk.
+    // Piping via stdin avoids a race where a freshly-written file may not yet
+    // be visible to the buildifier subprocess.
+    let normalized_outputs = if let Some(ref buildifier_bin) = opt.buildifier {
+        normalized_outputs
+            .into_iter()
+            .map(|(path, content)| {
+                let formatted = buildifier_format(buildifier_bin, &content, &path)
+                    .with_context(|| format!("Failed to run buildifier on {}", path.display()))?;
+                Ok((path, formatted))
+            })
+            .collect::<anyhow::Result<BTreeMap<_, _>>>()?
+    } else {
+        normalized_outputs
+    };
 
     // Write outputs
     write_outputs(normalized_outputs, opt.dry_run).context("Failed writing output files")?;
 
-    // Optionally apply buildifier fixes
-    if let Some(buildifier_bin) = opt.buildifier {
-        for file in file_names {
-            let file_path = opt.workspace_dir.join(file);
-            buildifier_format(&buildifier_bin, &file_path)
-                .with_context(|| format!("Failed to run buildifier on {}", file_path.display()))?;
-        }
-    }
-
     // Optionally perform bazel mod tidy to update the MODULE.bazel file
     if bazel_info.release >= semver::Version::new(7, 0, 0) {
         let module_bazel = opt.workspace_dir.join("MODULE.bazel");

crate_universe/tools/urls_generator/src/main.rs

@@ -1,9 +1,9 @@
 //! A helper tool for generating urls and sha256 checksums of cargo-bazel binaries and writing them to a module.
 
 use std::collections::BTreeMap;
-use std::io::{BufRead, BufReader};
+use std::io::{BufRead, BufReader, Write};
 use std::path::{Path, PathBuf};
-use std::process::Command;
+use std::process::{Command, Stdio};
 use std::{env, fs};
 
 use clap::Parser;
@@ -151,25 +151,34 @@ fn render_module(artifacts: &[Artifact]) -> String {
         )
 }
 
-fn write_module(content: &str) -> PathBuf {
-    let dest = PathBuf::from(
-        env::var("BUILD_WORKSPACE_DIRECTORY").expect("This binary is required to run under Bazel"),
-    )
-    .join(env!("MODULE_ROOT_PATH"));
+fn run_buildifier(buildifier_path: &Path, content: &str, path: &Path) -> String {
+    let mut child = Command::new(buildifier_path)
+        .args(["-lint=fix", "-mode=fix", "-warnings=all"])
+        .arg(format!("--path={}", path.display()))
+        .stdin(Stdio::piped())
+        .stdout(Stdio::piped())
+        .stderr(Stdio::piped())
+        .spawn()
+        .unwrap();
 
-    fs::write(&dest, content).unwrap();
+    child
+        .stdin
+        .take()
+        .unwrap()
+        .write_all(content.as_bytes())
+        .unwrap();
 
-    dest
-}
+    let output = child.wait_with_output().unwrap();
 
-fn run_buildifier(buildifier_path: &Path, module: &Path) {
-    Command::new(buildifier_path)
-        .arg("-lint=fix")
-        .arg("-mode=fix")
-        .arg("-warnings=all")
-        .arg(module)
-        .output()
-        .unwrap();
+    if !output.status.success() {
+        panic!(
+            "buildifier failed on {}: {}",
+            path.display(),
+            String::from_utf8_lossy(&output.stderr)
+        );
+    }
+
+    String::from_utf8(output.stdout).unwrap()
 }
 
 fn main() {
@@ -179,9 +188,16 @@ fn main() {
 
     let content = render_module(&artifacts);
 
-    let path = write_module(&content);
+    let dest = PathBuf::from(
+        env::var("BUILD_WORKSPACE_DIRECTORY").expect("This binary is required to run under Bazel"),
+    )
+    .join(env!("MODULE_ROOT_PATH"));
 
-    if let Some(buildifier_path) = opt.buildifier {
-        run_buildifier(&buildifier_path, &path);
-    }
+    let content = if let Some(ref buildifier_path) = opt.buildifier {
+        run_buildifier(buildifier_path, &content, &dest)
+    } else {
+        content
+    };
+
+    fs::write(&dest, content).unwrap();
 }

test/unit/cc_toolchain_runtime_lib/lib.rs

@@ -0,0 +1 @@
+