Skip to content

Commit e5869a8

Browse files
authored
Fix cargo_build_script tree artifact failures (#3945)
After `drain_runfiles_dir` removes symlinks and retains only files matching `filename_suffixes_to_retain` (default: `.lib`, `.so`), targets with no matching files were left with a directory tree of empty subdirectories. The existing `.empty` file guard used a shallow `read_dir().count() == 0` check, which returned non-zero because empty nested directories (e.g. `workspace_name/pkg/...`) still existed -- even though remote execution tree artifacts only track files, not directories. This adds a recursive `dir_contains_files` helper, moves the `.empty` guard into the shared `drain_runfiles_dir` method (covering both Unix and Windows paths), and recursively cleans up empty ancestor directories during drain. The same `.empty` guard is also applied to `OUT_DIR` in `bin.rs`.
1 parent 67b6938 commit e5869a8

5 files changed

Lines changed: 132 additions & 4 deletions

File tree

cargo/private/cargo_build_script_runner/bin.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,26 @@ fn run_buildrs() -> Result<(), String> {
237237
.drain_runfiles_dir(&out_dir_abs)
238238
.unwrap();
239239
}
240+
241+
// If out_dir is empty add an empty file to the directory to avoid an upstream Bazel bug
242+
// https://github.com/bazelbuild/bazel/issues/28286
243+
if out_dir_abs.read_dir().map(|read| read.count()).unwrap_or(0) == 0 {
244+
create_dir_all(&out_dir_abs).unwrap_or_else(|e| {
245+
panic!(
246+
"Failed to create OUT_DIR `{}`\n{:?}",
247+
out_dir_abs.display(),
248+
e
249+
)
250+
});
251+
std::fs::write(out_dir_abs.join(".empty"), "").unwrap_or_else(|e| {
252+
panic!(
253+
"Failed to write empty file to OUT_DIR `{}`\n{:?}",
254+
out_dir_abs.display(),
255+
e
256+
)
257+
})
258+
}
259+
240260
Ok(())
241261
}
242262

cargo/private/cargo_build_script_runner/cargo_manifest_dir.rs

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,32 @@ fn is_dir_empty(path: &Path) -> Result<bool, String> {
7878
Ok(entries.next().is_none())
7979
}
8080

81+
/// Recursively checks whether a directory tree contains any regular files.
82+
///
83+
/// Returns `false` if the directory only contains empty subdirectories,
84+
/// which is important because remote execution tree artifacts only track
85+
/// files, not directories.
86+
fn dir_contains_files(path: &Path) -> bool {
87+
let entries = match std::fs::read_dir(path) {
88+
Ok(entries) => entries,
89+
Err(_) => return false,
90+
};
91+
for entry in entries.flatten() {
92+
let file_type = match entry.file_type() {
93+
Ok(ft) => ft,
94+
Err(_) => continue,
95+
};
96+
if file_type.is_dir() {
97+
if dir_contains_files(&entry.path()) {
98+
return true;
99+
}
100+
} else {
101+
return true;
102+
}
103+
}
104+
false
105+
}
106+
81107
/// A struct for generating runfiles directories to use when running Cargo build scripts.
82108
pub struct RunfilesMaker {
83109
/// The output where a runfiles-like directory should be written.
@@ -248,17 +274,24 @@ impl RunfilesMaker {
248274
.iter()
249275
.any(|suffix| dest.ends_with(suffix))
250276
{
251-
if let Some(parent) = abs_dest.parent() {
252-
if is_dir_empty(parent).map_err(|e| {
277+
let mut dir = abs_dest.parent().map(Path::to_path_buf);
278+
while let Some(parent) = dir {
279+
if parent == self.output_dir {
280+
break;
281+
}
282+
if is_dir_empty(&parent).map_err(|e| {
253283
format!("Failed to determine if directory was empty with: {:?}", e)
254284
})? {
255-
std::fs::remove_dir(parent).map_err(|e| {
285+
std::fs::remove_dir(&parent).map_err(|e| {
256286
format!(
257287
"Failed to delete directory {} with {:?}",
258288
parent.display(),
259289
e
260290
)
261291
})?;
292+
dir = parent.parent().map(Path::to_path_buf);
293+
} else {
294+
break;
262295
}
263296
}
264297
continue;
@@ -273,6 +306,7 @@ impl RunfilesMaker {
273306
)
274307
})?;
275308
}
309+
276310
Ok(())
277311
}
278312

@@ -312,6 +346,20 @@ impl RunfilesMaker {
312346
self.drain_runfiles_dir_unix()?;
313347
}
314348

349+
// If the runfiles dir contains no files, add an empty file to avoid
350+
// an upstream Bazel bug where tree artifacts with only empty
351+
// subdirectories are considered "not created" in remote execution.
352+
// https://github.com/bazelbuild/bazel/issues/28286
353+
if !dir_contains_files(&self.output_dir) {
354+
std::fs::write(self.output_dir.join(".empty"), "").unwrap_or_else(|e| {
355+
panic!(
356+
"Failed to write empty file to runfiles dir `{}`\n{:?}",
357+
self.output_dir.display(),
358+
e
359+
)
360+
})
361+
}
362+
315363
// Due to the symlinks in `CARGO_MANIFEST_DIR`, some build scripts
316364
// may have placed symlinks over real files in `OUT_DIR`. To counter
317365
// this, all non-relative symlinks are resolved.

extensions/wasm_bindgen/private/BUILD.bazel

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@ bzl_library(
1515
],
1616
)
1717

18+
rust_binary(
19+
name = "wasm_bindgen_wrapper",
20+
srcs = ["wasm_bindgen_wrapper.rs"],
21+
edition = "2021",
22+
visibility = ["//visibility:public"],
23+
)
24+
1825
rust_binary(
1926
name = "wasm_bindgen_test_runner",
2027
srcs = ["wasm_bindgen_test_runner.rs"],

extensions/wasm_bindgen/private/wasm_bindgen.bzl

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,18 @@ def rust_wasm_bindgen_action(*, ctx, toolchain, wasm_file, target_output, flags
7777
outputs = [bindgen_wasm_module, snippets] + js_out + ts_out
7878

7979
args = ctx.actions.args()
80+
args.add(bindgen_bin.path)
81+
args.add(snippets.path)
82+
args.add("--")
8083
args.add("--target", target_output)
8184
args.add("--out-dir", bindgen_wasm_module.dirname)
8285
args.add("--out-name", out_name)
8386
args.add_all(flags)
8487
args.add(input_file)
8588

8689
ctx.actions.run(
87-
executable = bindgen_bin,
90+
executable = ctx.executable._process_wrapper,
91+
tools = [bindgen_bin],
8892
inputs = [input_file],
8993
outputs = outputs,
9094
mnemonic = "RustWasmBindgen",
@@ -178,6 +182,12 @@ WASM_BINDGEN_ATTR = {
178182
"_allowlist_function_transition": attr.label(
179183
default = Label("@bazel_tools//tools/allowlists/function_transition_allowlist"),
180184
),
185+
"_process_wrapper": attr.label(
186+
doc = "A process wrapper for ensuring tree artifacts are non-empty.",
187+
default = Label("//private:wasm_bindgen_wrapper"),
188+
executable = True,
189+
cfg = "exec",
190+
),
181191
}
182192

183193
rust_wasm_bindgen = rule(
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
//! A process wrapper for `wasm-bindgen` that ensures the `snippets` tree
2+
//! artifact directory is never empty after the tool runs.
3+
//!
4+
//! Bazel does not track empty directories in remote execution, so an empty
5+
//! `snippets` directory can cause action cache misses or "output not created"
6+
//! errors. See <https://github.com/bazelbuild/bazel/issues/28286>.
7+
8+
use std::env;
9+
use std::fs;
10+
use std::path::Path;
11+
use std::process::{self, Command};
12+
13+
fn main() {
14+
let mut args = env::args().skip(1);
15+
16+
let bindgen = args
17+
.next()
18+
.expect("expected wasm-bindgen binary path as first argument");
19+
let snippets_dir = args
20+
.next()
21+
.expect("expected snippets dir as second argument");
22+
23+
let remaining: Vec<String> = args.skip_while(|a| a == "--").collect();
24+
25+
let status = Command::new(&bindgen)
26+
.args(&remaining)
27+
.status()
28+
.unwrap_or_else(|e| panic!("failed to spawn {}: {}", bindgen, e));
29+
30+
if !status.success() {
31+
process::exit(status.code().unwrap_or(1));
32+
}
33+
34+
let is_empty = fs::read_dir(&snippets_dir)
35+
.map(|mut entries| entries.next().is_none())
36+
.unwrap_or(true);
37+
38+
if is_empty {
39+
let sentinel = Path::new(&snippets_dir).join(".empty");
40+
fs::File::create(&sentinel)
41+
.unwrap_or_else(|e| panic!("failed to create {}: {}", sentinel.display(), e));
42+
}
43+
}

0 commit comments

Comments
 (0)