Look up runfiles in the runfiles directory if it is materialized - #70
Open
fmeum wants to merge 1 commit into
Open
Look up runfiles in the runfiles directory if it is materialized#70fmeum wants to merge 1 commit into
fmeum wants to merge 1 commit into
Conversation
fmeum
force-pushed
the
prefer-materialized-runfiles-directory
branch
10 times, most recently
from
August 13, 2026 14:23
19ea6e7 to
9331a27
Compare
`runfiles_rlocation_checked` and `runfiles_current_repository` have to agree on where they look up runfiles: the output of the former is the input of the latter, and the manifest maps rlocation paths to the locations of the *original* files, so a path resolved against the runfiles directory is never the target of a manifest entry. bazel-contrib#68 made them agree on the manifest, at the cost of scanning it on every lookup and of returning paths outside the runfiles tree. Instead, have both use the runfiles directory whenever it has been materialized and the manifest otherwise, without ever falling back from one to the other. Whether the directory has been materialized is a property of how the action or test is executed rather than of the build: `--noenable_runfiles` leaves behind a directory that contains only the `MANIFEST` file and the subdirectory of the main repository, but the sandbox and remote execution materialize it in full regardless (they don't stage the manifest at all). The presence of this library in the directory distinguishes the two cases: it is an ordinary runfile, so it is materialized by exactly the mechanisms that also materialize every other runfile, and it is in the runfiles of every user of the library, which all load it via the initialization snippet that looks it up at this very path. The snippet therefore already made this determination when it sourced the library, so the library records it once on load rather than repeating it per lookup. `runfiles_export_envvars` in turn no longer passes on a runfiles directory it hasn't verified. It used to export any `RUNFILES_DIR` it could derive from the manifest path as long as that directory existed, which with `--noenable_runfiles` is the near-empty one, leaving a subprocess to resolve runfiles paths that don't exist. With both exported variables usable on their own, a subprocess needs no knowledge of which one this process picked, so that choice stays private to it. `runfiles_current_repository` now consistently returns 0 when it has to fall back to parsing the caller's execroot path; it used to do so only if the runfiles directory was used.
fmeum
force-pushed
the
prefer-materialized-runfiles-directory
branch
from
August 13, 2026 15:11
9331a27 to
5adb6de
Compare
fmeum
marked this pull request as ready for review
August 13, 2026 15:11
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
runfiles_rlocation_checkedandrunfiles_current_repositoryhave to agree on where they look up runfiles: the output of the former is the input of the latter, and a path resolved against the runfiles directory is never the target of a manifest entry. #68 made them agree on the manifest, at the cost of scanning it on every lookup and of returning paths outside the runfiles tree.Both now use the runfiles directory if it has been materialized and the manifest otherwise, never falling back from one to the other.
Whether the directory is materialized is a property of the execution rather than of the build:
--noenable_runfilesleaves behind only theMANIFESTfile and the main repository's subdirectory, while the sandbox and remote execution materialize the directory in full and don't stage the manifest at all. The presence of this library in the directory distinguishes the two cases as any other runfile would.runfiles_export_envvarsno longer passes on aRUNFILES_DIRthat fails the same check.runfiles_current_repositorynow consistently returns 0 when it falls back to parsing the caller's execroot path; it used to do so only in directory mode.