fix: select one runfiles source by tamird · Pull Request #59 · hermeticbuild/hermetic-launcher

tamird · 2026-06-23T13:58:19Z

Select exactly one runfiles source before resolving transformed
arguments. Bazel can set both RUNFILES_DIR and
RUNFILES_MANIFEST_FILE; choosing the manifest first resolves generated
venvs into physical outputs even when their logical runfiles tree is
available, breaking layouts that rely on relative symlinks.

Give environment-provided sources precedence over adjacent discovery,
and prefer a directory within either tier. Use the selected source
consistently for lookup and child environment export, so a process never
executes from one source while inheriting the other.

Keep the logical sibling .runfiles spelling in argv[0] when an
adjacent manifest is the only source, without exporting the nonexistent
directory. On Windows, use nonexclusive existence probes and remove
inherited runfiles variables case-insensitively before exporting the
selected source.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c4c34785cf

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

malt3 · 2026-06-23T16:46:40Z

I don't think we should mix and match between manifest and directory lookup.
My suggestion is to change the high level selection preference only.

if only one source can be found, use it
if one is set via env and the other was discovered next to the executable, prefer the one set via env var
if both are present with the same precedence (both env var or both found next to executable) then we should prefer the runfiles dir

Falling back to the dir after having selected the manifest is weird.

tamird · 2026-06-23T18:03:47Z

Agreed. I rewrote this in 45d9b79 to select one source up front: environment before adjacent discovery, and directory before manifest within a tier. Resolution and child export now come from the same enum variant, so the mixed state is unrepresentable.

The conflict tests cover both environment variables, an environment manifest versus an adjacent directory, an adjacent-source tie, invalid directory variables, and the invariant that a selected directory never falls through to either an environment or adjacent manifest.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 02f88b7b50

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Bazel can provide valid directory and manifest runfiles sources at the same time. Selecting the manifest first resolves transformed paths into physical outputs even when the logical runfiles tree is available, breaking layouts that rely on relative symlinks. Select environment-provided sources before adjacent sources and prefer a directory within either tier. Represent the result as one enum so lookup and child environment export cannot disagree. Preserve logical argv[0] for an adjacent-manifest-only launch without exporting a nonexistent directory. Make Windows directory probes nonexclusive and scrub inherited runfiles variables case-insensitively before exporting the selected source.

Windows may spell equivalent paths with either separator. The runfiles precedence test compared the native path emitted by the child with the fixture path spelling, so it failed after directory selection succeeded. Parse exported environment values as paths and compare Path values. Keep the Windows test output enabled so future platform failures remain diagnosable.

malt3 · 2026-06-24T14:39:27Z

This looks great. Thank you!

jjudd · 2026-06-30T16:31:28Z

Thanks for fixing this! Any chance you could please cut a release that includes this fix? I think I'm running into an issue that will be fixed once this is released. Also sounds like #61 is dependent on this being released.

malt3 · 2026-06-30T22:02:15Z

On it: https://github.com/hermeticbuild/hermetic-launcher/actions/runs/28478666588

Although the Bazel CI is having some infra issues - expect delays.

malt3 · 2026-06-30T23:17:16Z

Updating the binary hashes in #62 uncovered a bug on Windows. HEAD is now red. We will need to address that first.

rafikk · 2026-07-16T16:00:29Z

We were also caught by this issue when trying to use hermetic_launcher with a py_binary target in aspect_rules_py 2.0 alpha.

Would be great to get this in a BCR release. For now, we're using a git_override at HEAD since a PR exists which updates the prebuilts.

jjudd · 2026-07-16T16:18:43Z

I'm pretty sure I have a fix for the Windows CI failures and that it's a legit bug. I just need to test it and put up a PR for it. I'll see if I can get that done today.

malt3 · 2026-07-16T16:28:43Z

@jjudd that would be great! Happy to review and cut a release.
My time and sanity to boot up a Windows VM is limited lately, so getting a fix is awesome.

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.

…64) 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.

malt3 · 2026-07-18T17:17:55Z

hermetic_launcher@0.0.13 is now in the BCR.

tamird mentioned this pull request Jun 23, 2026

feat: add executable-relative fallbacks #57

Closed

chatgpt-codex-connector Bot reviewed Jun 23, 2026

View reviewed changes

Comment thread runfiles-stub/src/runfiles.rs Outdated

tamird force-pushed the prefer-runfiles-directory branch from 8a118d8 to 02f88b7 Compare June 23, 2026 18:03

tamird changed the title ~~fix: prefer physical runfiles entries~~ fix: select one runfiles source Jun 23, 2026

chatgpt-codex-connector Bot reviewed Jun 23, 2026

View reviewed changes

Comment thread runfiles-stub/src/runfiles.rs Outdated

tamird force-pushed the prefer-runfiles-directory branch 2 times, most recently from e790599 to 3d78731 Compare June 23, 2026 20:56

tamird force-pushed the prefer-runfiles-directory branch from 3d78731 to 5659fd3 Compare June 24, 2026 12:20

malt3 approved these changes Jun 24, 2026

View reviewed changes

malt3 merged commit 1371ec2 into hermeticbuild:main Jun 24, 2026
6 checks passed

tamird deleted the prefer-runfiles-directory branch June 24, 2026 14:54

malt3 mentioned this pull request Jul 18, 2026

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

Merged

jjudd mentioned this pull request Jul 24, 2026

chore: upgrae hermetic_launcher to v0.0.13 aspect-build/rules_py#1273

Merged

Uh oh!

Conversation

tamird commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

malt3 commented Jun 23, 2026

Uh oh!

tamird commented Jun 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

malt3 commented Jun 24, 2026

Uh oh!

Uh oh!

jjudd commented Jun 30, 2026

Uh oh!

malt3 commented Jun 30, 2026

Uh oh!

malt3 commented Jun 30, 2026

Uh oh!

rafikk commented Jul 16, 2026

Uh oh!

jjudd commented Jul 16, 2026

Uh oh!

malt3 commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

malt3 commented Jul 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tamird commented Jun 23, 2026 •

edited

Loading

tamird commented Jun 23, 2026 •

edited

Loading

malt3 commented Jul 16, 2026 •

edited

Loading