Skip to content

Remove calls to resolve_tools - #2446

Open
iteratee wants to merge 1 commit into
masterfrom
kb/bazel86
Open

Remove calls to resolve_tools#2446
iteratee wants to merge 1 commit into
masterfrom
kb/bazel86

Conversation

@iteratee

Copy link
Copy Markdown
Collaborator

Replace them mostly with FilesToRunProviders. For cc_wrapper, where repl's also need the runfiles, use a struct that has the executable, the FilesToRunProvider and the runfiles. The executable could be retrieved from the FilesToRunProvider, but it is more convenient to just have it in the struct.

FilesToRunProvider also makes sure that when passed as a tool, all the necessary runfiles are available for that tool. Use this for protoc and cc_wrapper.

cc_wrapper cannot rely on the RUNFILES_* environment variables. Pass cc_wrapper either its path or the path to its manifest directly.

Make sure that we correctly pass around env variables at various points so that tools we depend are in the PATH and have the environment they expect.

Remove outdated comment about re-enabling resolve_tools for bazel 8

Replace them mostly with `FilesToRunProvider`s. For cc_wrapper, where
repl's also need the runfiles, use a struct that has the executable, the
`FilesToRunProvider` and the runfiles. The executable could be retrieved
from the `FilesToRunProvider`, but it is more convenient to just have it
in the struct.

FilesToRunProvider also makes sure that when passed as a tool, all the
necessary runfiles are available for that tool. Use this for protoc and
cc_wrapper.

cc_wrapper cannot rely on the RUNFILES_* environment variables. Pass
cc_wrapper either its path or the path to its manifest directly.

Make sure that we correctly pass around env variables at various points
so that tools we depend are in the `PATH` and have the environment they
expect.

Remove outdated comment about re-enabling resolve_tools for bazel 8
@iteratee

Copy link
Copy Markdown
Collaborator Author

Making a duplicate to compare the CI situation with forked PR vs local PR.

@iteratee
iteratee marked this pull request as ready for review August 27, 2026 01:28
@iteratee
iteratee requested a review from avdv as a code owner August 27, 2026 01:28
@iteratee

Copy link
Copy Markdown
Collaborator Author

An important note: Things still work with these changes, but the way that they work is subtly different. Before, all of the runfiles were aggregated together as the runfiles of whatever tool was being executed. Now, each tool lives next to its own runfiles and possibly its own runfiles manifest.

This shows up with cc_wrapper. When cc_wrapper runs it needs to know where its runfiles are located. Previously, they were located with the big pile of runfiles. Now they aren't there. We solve this by making sure that cc_wrapper knows where it lives. All the other programs can figure it out without being told (provided we clean out the environment variables that might mislead them).

Another thing to note is PATH manipulation. Previously, when the tools were piled into the runfiles, the runfiles directories were already added to the search path and could be found. Now when an external tool is required, we need to make sure that the directory where the tool lives is available in the PATH.

Once you get the approach, almost all of the PR is easy to follow:

  • Take things that were previously runfiles, and pass them as tools instead.
  • Use FilesToRunProvider instead of the raw executable to make this happen.
  • Make sure that the tools are available in the PATH.
  • Clear out the environment in a few more places so that tools can find their own runfiles.
  • cc_wrapper is a special case. It needs to know where its runfiles are, and so we use a different environment variable just for it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant