Bazel: when to create a runfiles tree

Created on 7 Nov 2018  路  11Comments  路  Source: bazelbuild/bazel

I happened to stumble upon runfiles tree creation in Bazel today and found the current behavior to be as follows:

  • Runfiles trees live in bazel info output_path and are as permanent as the build outputs that use them, as running a binary requires its runfiles tree to be in place (if any). IIUC there is no such contract for tests, but these runfiles still don't get deleted after a test finishes.
  • Runfiles trees are created by the SymlinkTreeAction using a C++ program build-runfiles.cc which gets its information by reading the MANIFEST file which is created by the SourceManifestAction. Such runfile trees are created for binaries and tests that have runfiles and they are created independently of the spawn and test strategy.
  • There are three flags governing runfiles tree/manifest creation:

    • --build_runfile_manifests: If set to false, no MANIFEST file and no runfiles tree are created and Bazel refuses to run any test, independent of the execution strategy. It's enabled by default.

    • --build_runfile_links: If set to false, no runfiles tree is created when building a target but when running tests the runfiles tree is still created. It's enabled by default.

    • --experimental_enable_runfiles: Like --build_runfile_links but also disables runfiles tree creation for tests (MANIFEST only). It's disabled by default on Windows and enabled on all other platforms.

  • Tests run inside a sandbox also need access to their runfiles, but there they are just treated as normal inputs and created via calls to vfs.FileSystem.createSymlink(...) and not by using the build-runfiles.cc tool.

I take from the above that the state of runfiles tree/manifest creation can clearly be improved:

  • For a test that is remotely executed/cached or that uses any form of sandboxing, creating a runfiles tree and manifest file is pure overhead as its never used.
  • For a binary that is remotely executed/cached or that uses any form of sandboxing, creating a runfiles tree _may_ be overhead namely so if the binary is just built but never run. We should document that using experimental_enable_runfiles is the way to go here.
  • --build_runfile_links seems obsolete as it was replaced by experimental_enable_runfiles.
  • --build_runfile_manifests doesn't seem useful in a world where the prerequisite for a runfiles tree is a MANIFEST.
  • We should benchmark build-runfiles.cc and vfs.FileSystem.createSymlink(...) and then go for one or the other. In particular so as when creating a runfiles tree, we only seem to create the MANIFEST file because build-runfiles.cc requires it.

@lberki @ulfjack @laszlocsomor @philwo @meisterT

P3 team-Local-Exec

Most helpful comment

IIUC I have fixed this a while ago in Bazel [1]. It's enabled by default when running with --remote_download_minimal [2]

[1] https://github.com/bazelbuild/bazel/commit/03246077f948f2790a83520e7dccc2625650e6df
[2] https://github.com/bazelbuild/bazel/commit/b51f956dbd814967abd81dce202518bd3990856b

All 11 comments

would also be interested in @janakdr and @ericfelly thoughts on this :-)

Some thoughts:

  • IIUC there is no such contract for tests, but these runfiles still don't get deleted after a test finishes.

Seems to be useful when re-running tests.

  • For a binary that is remotely executed/cached or that uses any form of sandboxing, creating a runfiles tree _may_ be overhead namely so if the binary is just built but never run.

FTR, the binary may be executed post-build, needing its runfiles.

We should document that using experimental_enable_runfiles is the way to go here.

Why?

  • We should benchmark build-runfiles.cc and vfs.FileSystem.createSymlink(...) and then go for one or the other. In particular so as when creating a runfiles tree, we only seem to create the MANIFEST file because build-runfiles.cc requires it.

For local execution, one or the other is enough. But don't we use build-runfiles for remote execution? If we do, we may need to maintain both tools (build-runfiles and the Java code).

Seems to be useful when re-running tests.

Only for local non-sandboxed execution. remote/sandboxed tests don't use them.

Why?

--experimental_enable_runfiles disables runfiles for tests and binaries, while --build_runfile_links only disables them for tests which (at least to me) seems unexpected.

For local execution, one or the other is enough. But don't we use build-runfiles for remote execution? If we do, we may need to maintain both tools (build-runfiles and the Java code).

Remote execution doesn't use runfiles. It's like the local sandbox.

Makes sense, thanks!

also @benjaminp

--build_runfile_links behavior is different in Blaze vs. Bazel; in Blaze, it is used to defer local creation of runfiles trees until needed, but still have creation of runfiles trees for remote execution, as well as for local execution (including when falling back from remote execution). Blaze doesn't use SpawnStrategy to execute tests, and therefore the test strategy knows whether it's executing locally or remotely.

This was/is on my todo list to simplify and unify test execution between Blaze and Bazel, but there are still other blockers around environment variables that seem like a bigger problem.

Are there any plans to have the behavior that --build_runfile_links provides in Blaze, also in Bazel? On my end, I see a lot of remotely cached tests that still take up a lot of time due to unnecessary creation of the runfile trees.

AFAIK, nobody is working even remotely on this area right now. Personally I have left Google since my last post. I agree that having a feature like this would be beneficial. However, I probably won't be able to work on it in the near future.

IIUC I have fixed this a while ago in Bazel [1]. It's enabled by default when running with --remote_download_minimal [2]

[1] https://github.com/bazelbuild/bazel/commit/03246077f948f2790a83520e7dccc2625650e6df
[2] https://github.com/bazelbuild/bazel/commit/b51f956dbd814967abd81dce202518bd3990856b

Closing per the last reply from @buchgr , which claims that this was already fixed. Thanks!

Thanks @buchgr. I can confirm that this works.

Was this page helpful?
0 / 5 - 0 ratings