fix(windows): prefer the manifest over the sparse runfiles directory

PR #59 changed runfiles source selection to prefer a directory over a
manifest at equal precedence. On Windows, Bazel does not materialize the
runfiles symlink tree by default, so a sibling `<exe>.runfiles` / `RUNFILES_DIR`
exists but is sparse and only the manifest maps runfiles to real paths.
Directory-first selection there resolves rlocations to files that do not
exist, so the launcher fails to start its target (and drops
RUNFILES_MANIFEST_FILE from the child environment). On Linux/macOS the tree
is materialized, so the directory is fully populated and directory-first is
correct.

Gate the within-tier directory-vs-manifest preference on a per-backend
`PREFER_DIRECTORY_SOURCE` constant (true on Linux/macOS, false on Windows),
consumed by a new `select_source` helper in runfiles.rs. The
environment-over-adjacent tiering is unchanged; only Windows reverts to the
pre-#59 manifest-first order.

Update `test_runfiles_source_precedence` to assert the platform-correct
winner and to cover a sparse-directory no-fallthrough case per platform, and
document the Windows exception in the README.

Author: malt3

README.md

@@ -152,7 +152,9 @@ At startup the finalized launcher selects one runfiles source from:
 2. `<executable>.runfiles/` and `<executable>.runfiles_manifest`
 
 Valid environment-provided sources take precedence over adjacent sources. When both
-sources exist at the same precedence, the runfiles directory wins. Every argument
+sources exist at the same precedence, the runfiles directory wins on platforms that
+materialize the runfiles tree (Linux, macOS); on Windows the tree is not materialized
+by default and the directory is sparse, so the manifest wins there. Every argument
 marked `--transform` resolves only through the selected source (manifest lookup or
 directory join; tree-artifact prefixes supported), and only that source is exported to
 the child. Absolute paths (leading `/`) pass through unchanged. The launcher then

integration-tests/src/main.rs

@@ -444,6 +444,11 @@ fn test_add_numbers_runtime_args(config: &TestConfig) -> Result<(), String> {
 fn test_runfiles_source_precedence(config: &TestConfig) -> Result<(), String> {
     println!("  Running test: runfiles_source_precedence");
 
+    // The launcher prefers a directory at equal precedence where Bazel
+    // materializes the runfiles tree; on Windows the tree is sparse, so the
+    // manifest wins. Mirror that choice when asserting the selected source.
+    let prefer_directory = !cfg!(windows);
+
     let test_dir = config.work_dir.join("test_runfiles_source_precedence");
     fs::create_dir_all(&test_dir).map_err(|e| format!("Failed to create test dir: {}", e))?;
 
@@ -475,8 +480,9 @@ fn test_runfiles_source_precedence(config: &TestConfig) -> Result<(), String> {
         &[0],
     )?;
 
-    // Both environment sources have the same precedence, so the directory owns
-    // both resolution and the environment exported to the child.
+    // Both environment sources have the same precedence. Where the runfiles tree
+    // is materialized the directory owns both resolution and the exported
+    // environment; on Windows the tree is sparse, so the manifest wins.
     let mut command = Command::new(&stub_path);
     command
         .env("RUNFILES_DIR", &runfiles.runfiles_dir)
@@ -487,14 +493,21 @@ fn test_runfiles_source_precedence(config: &TestConfig) -> Result<(), String> {
         .map_err(|e| format!("Failed to run stub with both runfiles variables: {}", e))?;
     let stdout = String::from_utf8_lossy(&output.stdout);
     let stderr = String::from_utf8_lossy(&output.stderr);
-    if !output.status.success()
-        || !stdout.contains("ARGC:3")
-        || environment_path(&stdout, "RUNFILES_DIR") != Some(runfiles.runfiles_dir.as_path())
-        || environment_path(&stdout, "JAVA_RUNFILES") != Some(runfiles.runfiles_dir.as_path())
-        || !stdout.contains("ENV:RUNFILES_MANIFEST_FILE=<unset>")
-    {
+    // The directory maps `tool` to print-env (prints ARGC/ENV), the manifest to
+    // add-numbers (prints SUM), so the output identifies which source was chosen.
+    let selected_expected = if prefer_directory {
+        output.status.success()
+            && stdout.contains("ARGC:3")
+            && environment_path(&stdout, "RUNFILES_DIR") == Some(runfiles.runfiles_dir.as_path())
+            && environment_path(&stdout, "JAVA_RUNFILES") == Some(runfiles.runfiles_dir.as_path())
+            && stdout.contains("ENV:RUNFILES_MANIFEST_FILE=<unset>")
+    } else {
+        output.status.success() && stdout.contains("SUM:15") && !stdout.contains("ARGC:3")
+    };
+    if !selected_expected {
         return Err(format!(
-            "Directory environment source did not win the tie.\nstdout: {}\nstderr: {}",
+            "Same-precedence environment sources did not select the {} source.\nstdout: {}\nstderr: {}",
+            if prefer_directory { "directory" } else { "manifest" },
             stdout, stderr
         ));
     }
@@ -561,7 +574,8 @@ fn test_runfiles_source_precedence(config: &TestConfig) -> Result<(), String> {
     }
 
     // With no environment source, the adjacent directory and manifest have equal
-    // precedence, so the directory wins and is exported consistently.
+    // precedence: the directory wins where the tree is materialized, the manifest
+    // on Windows, and the winner is exported consistently.
     let mut command = Command::new(&stub_path);
     command
         .env_remove("RUNFILES_DIR")
@@ -572,65 +586,112 @@ fn test_runfiles_source_precedence(config: &TestConfig) -> Result<(), String> {
         .map_err(|e| format!("Failed to run stub with adjacent sources: {}", e))?;
     let stdout = String::from_utf8_lossy(&output.stdout);
     let stderr = String::from_utf8_lossy(&output.stderr);
-    if !output.status.success()
-        || !stdout.contains("ARGC:3")
-        || environment_path(&stdout, "RUNFILES_DIR") != Some(runfiles.runfiles_dir.as_path())
-        || environment_path(&stdout, "JAVA_RUNFILES") != Some(runfiles.runfiles_dir.as_path())
-        || !stdout.contains("ENV:RUNFILES_MANIFEST_FILE=<unset>")
-    {
+    let selected_expected = if prefer_directory {
+        output.status.success()
+            && stdout.contains("ARGC:3")
+            && environment_path(&stdout, "RUNFILES_DIR") == Some(runfiles.runfiles_dir.as_path())
+            && environment_path(&stdout, "JAVA_RUNFILES") == Some(runfiles.runfiles_dir.as_path())
+            && stdout.contains("ENV:RUNFILES_MANIFEST_FILE=<unset>")
+    } else {
+        output.status.success() && stdout.contains("SUM:15") && !stdout.contains("ARGC:3")
+    };
+    if !selected_expected {
         return Err(format!(
-            "Adjacent directory did not win the tie.\nstdout: {}\nstderr: {}",
+            "Adjacent {} source did not win the tie.\nstdout: {}\nstderr: {}",
+            if prefer_directory { "directory" } else { "manifest" },
             stdout, stderr,
         ));
     }
 
-    // Source selection is global, not per key. A missing directory entry must
-    // not fall through to either an environment or adjacent manifest.
-    let missing_rlocation = format!("{}/bin/missing{}", WORKSPACE_NAME, EXE_EXT);
-    let missing_stub_name = format!("missing_stub{}", EXE_EXT);
-    let missing_stub = test_dir.join(&missing_stub_name);
-    let missing_manifest = test_dir.join(format!("{}.runfiles_manifest", missing_stub_name));
-    let add_target = add_binary.to_string_lossy();
-    #[cfg(windows)]
-    let add_target = add_target.replace('\\', "/");
-    fs::write(
-        &missing_manifest,
-        format!("{} {}\n", missing_rlocation, add_target),
-    )
-    .map_err(|e| format!("Failed to write no-fallback fixture: {}", e))?;
-    finalize_stub(
-        config,
-        &missing_stub,
-        &[&missing_rlocation, "7", "8"],
-        &[0],
-    )?;
-    for (case, manifest_env) in [
-        ("environment manifest", Some(&missing_manifest)),
-        ("adjacent manifest", None),
-    ] {
-        let mut command = Command::new(&missing_stub);
+    // Source selection is global, not per key: a key that only the NON-selected
+    // source can resolve must not fall through to it.
+    //
+    // Case A: both sources come from the environment, so the platform preference
+    // picks the winner. Embed a key that lives ONLY in the loser — any
+    // fall-through would resolve it and betray the mixing.
+    {
+        let a_stub_name = format!("no_fallthrough_env_stub{}", EXE_EXT);
+        let a_stub = test_dir.join(&a_stub_name);
+        let a_manifest = test_dir.join("no_fallthrough_env.runfiles_manifest");
+        // Manifest RHS values use forward slashes (Bazel's Windows convention);
+        // harmless on Unix, where paths carry no backslashes.
+        let add_target = add_binary.to_string_lossy().replace('\\', "/");
+
+        let embedded_key = if prefer_directory {
+            // Directory wins; put the embedded key only in the manifest.
+            let key = format!("{}/bin/only_in_manifest{}", WORKSPACE_NAME, EXE_EXT);
+            fs::write(&a_manifest, format!("{} {}\n", key, add_target))
+                .map_err(|e| format!("Failed to write no-fallthrough manifest: {}", e))?;
+            key
+        } else {
+            // Manifest wins; reuse `tool` (present only in the directory, as
+            // print-env) and give the manifest an unrelated entry so it loads but
+            // cannot resolve the key.
+            let decoy = format!("{}/bin/decoy{}", WORKSPACE_NAME, EXE_EXT);
+            fs::write(&a_manifest, format!("{} {}\n", decoy, add_target))
+                .map_err(|e| format!("Failed to write no-fallthrough manifest: {}", e))?;
+            executable_rlocation.clone()
+        };
+        finalize_stub(config, &a_stub, &[&embedded_key, "7", "8"], &[0])?;
+
+        let mut command = Command::new(&a_stub);
         command
             .env("RUNFILES_DIR", &runfiles.runfiles_dir)
+            .env("RUNFILES_MANIFEST_FILE", &a_manifest)
             .env_remove("JAVA_RUNFILES");
-        if let Some(manifest) = manifest_env {
-            command.env("RUNFILES_MANIFEST_FILE", manifest);
+        let output = command
+            .output()
+            .map_err(|e| format!("Failed to test no-fallthrough environment sources: {}", e))?;
+        let stdout = String::from_utf8_lossy(&output.stdout);
+        let stderr = String::from_utf8_lossy(&output.stderr);
+        // The loser's happy-path signature must never appear: SUM from the
+        // manifest's add-numbers, or ARGC from the directory's print-env.
+        let loser_ran = if prefer_directory {
+            stdout.contains("SUM:15")
         } else {
-            command.env_remove("RUNFILES_MANIFEST_FILE");
+            stdout.contains("ARGC")
+        };
+        if output.status.success() || loser_ran {
+            return Err(format!(
+                "Selected source fell through to the other (environment sources).\nstdout: {}\nstderr: {}",
+                stdout, stderr
+            ));
         }
+    }
+
+    // Case B: RUNFILES_DIR is the only environment source (the manifest is merely
+    // adjacent), so the environment directory wins on every platform; a key only
+    // the adjacent manifest holds must not fall through to it.
+    {
+        let missing_rlocation = format!("{}/bin/missing{}", WORKSPACE_NAME, EXE_EXT);
+        let missing_stub_name = format!("missing_stub{}", EXE_EXT);
+        let missing_stub = test_dir.join(&missing_stub_name);
+        let missing_manifest =
+            test_dir.join(format!("{}.runfiles_manifest", missing_stub_name));
+        let add_target = add_binary.to_string_lossy().replace('\\', "/");
+        fs::write(&missing_manifest, format!("{} {}\n", missing_rlocation, add_target))
+            .map_err(|e| format!("Failed to write no-fallthrough adjacent fixture: {}", e))?;
+        finalize_stub(config, &missing_stub, &[&missing_rlocation, "7", "8"], &[0])?;
+
+        let mut command = Command::new(&missing_stub);
+        command
+            .env("RUNFILES_DIR", &runfiles.runfiles_dir)
+            .env_remove("RUNFILES_MANIFEST_FILE")
+            .env_remove("JAVA_RUNFILES");
         let output = command
             .output()
-            .map_err(|e| format!("Failed to test no-fallback {}: {}", case, e))?;
+            .map_err(|e| format!("Failed to test no-fallthrough adjacent manifest: {}", e))?;
         let stdout = String::from_utf8_lossy(&output.stdout);
         let stderr = String::from_utf8_lossy(&output.stderr);
         if output.status.success() || stdout.contains("SUM:15") {
             return Err(format!(
-                "Directory selection fell through to {}.\nstdout: {}\nstderr: {}",
-                case, stdout, stderr
+                "Environment directory fell through to the adjacent manifest.\nstdout: {}\nstderr: {}",
+                stdout, stderr
             ));
         }
     }
 
-    println!("    PASS (environment precedence, then directory preference)");
+    println!("    PASS (environment precedence, then platform source preference)");
     Ok(())
 }
 

runfiles-stub/src/linux.rs

@@ -116,6 +116,11 @@ const AT_EXECFN: usize = 31;
 pub const SEP: char = '/';
 pub const NEWLINE: &[u8] = b"\n";
 
+// Linux materializes the runfiles symlink tree, so a runfiles directory is fully
+// populated and preferred over an equal-precedence manifest. See runfiles.rs
+// `select_source`.
+pub const PREFER_DIRECTORY_SOURCE: bool = true;
+
 pub fn is_absolute(path: &str) -> bool {
     path.starts_with('/')
 }

runfiles-stub/src/macos.rs

@@ -95,6 +95,11 @@ const MAXPATHLEN: usize = 1024;
 pub const SEP: char = '/';
 pub const NEWLINE: &[u8] = b"\n";
 
+// macOS materializes the runfiles symlink tree, so a runfiles directory is fully
+// populated and preferred over an equal-precedence manifest. See runfiles.rs
+// `select_source`.
+pub const PREFER_DIRECTORY_SOURCE: bool = true;
+
 pub fn is_absolute(path: &str) -> bool {
     path.starts_with('/')
 }

runfiles-stub/src/runfiles.rs

@@ -24,47 +24,59 @@ pub enum Runfiles {
 impl Runfiles {
     pub fn create(rt: &platform::RuntimeArgs) -> Option<Self> {
         // Environment-provided sources take precedence over sources discovered
-        // next to the executable. At the same precedence, prefer a directory.
-        if let Some(runfiles_dir) = platform::get_env_var(b"RUNFILES_DIR")
-            .filter(|path| !path.is_empty() && dir_exists(path))
-        {
-            return Some(Self::Directory { path: runfiles_dir });
-        }
-        if let Some(manifest_path) = platform::get_env_var(b"RUNFILES_MANIFEST_FILE")
-            .filter(|path| !path.is_empty())
-        {
-            if let Some(manifest) = load_manifest(&manifest_path) {
-                return Some(Self::Manifest {
-                    manifest,
-                    path: manifest_path,
-                    logical_dir: None,
-                });
-            }
+        // next to the executable. Within a tier, `select_source` applies the
+        // platform's directory-vs-manifest preference.
+        if let Some(rf) = select_source(
+            || {
+                platform::get_env_var(b"RUNFILES_DIR")
+                    .filter(|path| !path.is_empty() && dir_exists(path))
+                    .map(|path| Self::Directory { path })
+            },
+            || {
+                platform::get_env_var(b"RUNFILES_MANIFEST_FILE")
+                    .filter(|path| !path.is_empty())
+                    .and_then(|path| {
+                        load_manifest(&path).map(|manifest| Self::Manifest {
+                            manifest,
+                            path,
+                            logical_dir: None,
+                        })
+                    })
+            },
+        ) {
+            return Some(rf);
         }
 
-        // Locate runfiles next to the launching executable:
-        // <executable>.runfiles directory first, then
-        // <executable>.runfiles_manifest file. The executable path comes from the
-        // OS (an absolute, non-symlink-resolved launch path), not from argv[0].
+        // Locate runfiles next to the launching executable: the
+        // <executable>.runfiles directory and the <executable>.runfiles_manifest
+        // file, in the platform's preferred order. The executable path comes from
+        // the OS (an absolute, non-symlink-resolved launch path), not from argv[0].
         if let Some(exe_path) = rt.executable_path() {
             let exe_len = cstr_len(&exe_path);
             if exe_len > 0 {
                 // Convert the executable path to a string (if valid UTF-8).
                 let exe_str = core::str::from_utf8(&exe_path[..exe_len]).ok()?;
                 let runfiles_dir = String::from(exe_str) + ".runfiles";
-                if dir_exists(&runfiles_dir) {
-                    return Some(Self::Directory { path: runfiles_dir });
-                }
-
                 let manifest_path = String::from(exe_str) + ".runfiles_manifest";
-                if let Some(manifest) = load_manifest(&manifest_path) {
-                    return Some(Self::Manifest {
-                        manifest,
-                        path: manifest_path,
-                        // Preserve the logical path even though the sibling tree
-                        // is absent; the manifest selected the actual executable.
-                        logical_dir: Some(runfiles_dir),
-                    });
+
+                if let Some(rf) = select_source(
+                    || {
+                        dir_exists(&runfiles_dir).then(|| Self::Directory {
+                            path: runfiles_dir.clone(),
+                        })
+                    },
+                    || {
+                        load_manifest(&manifest_path).map(|manifest| Self::Manifest {
+                            manifest,
+                            path: manifest_path.clone(),
+                            // Preserve the logical path even though the sibling
+                            // tree is absent; the manifest selected the actual
+                            // executable.
+                            logical_dir: Some(runfiles_dir.clone()),
+                        })
+                    },
+                ) {
+                    return Some(rf);
                 }
             }
         }
@@ -107,6 +119,25 @@ impl Runfiles {
     }
 }
 
+/// Choose between a directory and a manifest source at equal precedence.
+///
+/// Both are evaluated lazily — we never probe a directory or open a manifest we
+/// do not end up selecting. The directory is preferred where the platform
+/// materializes the runfiles tree; on Windows the tree is not materialized by
+/// default, so the sibling directory is sparse and the manifest must win
+/// (otherwise `rlocation`s resolve to files that do not exist). See
+/// `platform::PREFER_DIRECTORY_SOURCE`.
+fn select_source(
+    directory: impl FnOnce() -> Option<Runfiles>,
+    manifest: impl FnOnce() -> Option<Runfiles>,
+) -> Option<Runfiles> {
+    if platform::PREFER_DIRECTORY_SOURCE {
+        directory().or_else(manifest)
+    } else {
+        manifest().or_else(directory)
+    }
+}
+
 fn dir_exists(path: &str) -> bool {
     // A trailing separator makes the existing-path probe directory-specific on
     // Unix and Windows: regular files cannot be traversed as directories.

runfiles-stub/src/windows.rs

@@ -135,6 +135,13 @@ extern "system" {
 pub const SEP: char = '\\';
 pub const NEWLINE: &[u8] = b"\r\n";
 
+// Windows does not materialize the runfiles symlink tree by default (creating
+// symlinks requires privileges), so a sibling `<exe>.runfiles` / `RUNFILES_DIR`
+// is sparse — only the manifest maps runfiles to real paths. Prefer the manifest
+// at equal precedence; a directory-first choice would resolve to files that do
+// not exist. See runfiles.rs `select_source`.
+pub const PREFER_DIRECTORY_SOURCE: bool = false;
+
 pub fn is_absolute(path: &str) -> bool {
     let b = path.as_bytes();
     b.len() >= 2