Add the ability to mark an action as always needing to run

[ismell]

Description of the feature request:

In ChromeOS, we are using bazel to build packages. We also want to also use bazel to install these packages into an external (to bazel) sysroot. We previously used to have a bazel run action that would perform the installation of all packages in one go. This method required waiting for all packages to be built before installation could take place. Installing all the packages can take a while. To speed this up, we want to install each individual package as soon as possible. We ended up creating a installation build action that installs the package into the external sysroot as soon as the package's dependencies are installed into the sysroot, and the package is available.

The problem we are running into is that this action only runs once if none of the inputs change. It's possible for the external sysroot to be modified or even cleared between bazel invocations. We need a mechanism to force the action to always run.

This is what our current invocation looks like:

BOARD=amd64-generic bazel build @portage//target/virtual/target-os:installed

This will install all the OS packages into the sysroot at /build/amd64-generic.

Here is a simplified example of our current "install" rule:

ebuild_install_action(
    name = "1.0-r2803_installed",
    board = "amd64-generic",
    package = ":1.0-r2803",
    requires = [
        "//internal/packages/stage2/target/board/chromiumos/chromeos-base/crosid:0.0.1-r227_installed",
        "//internal/packages/stage2/target/board/chromiumos/dev-libs/nss:3.68.2-r5_installed",
        "//internal/packages/stage2/target/board/chromiumos/sys-apps/coreboot-utils:0.0.1-r5575_installed",
        "//internal/packages/stage2/target/board/chromiumos/sys-apps/flashrom:0.9.9-r1697_installed",
        "//internal/packages/stage2/target/board/chromiumos/sys-apps/util-linux:2.38.1-r3_installed",
        "//internal/packages/stage2/target/board/portage-stable/app-arch/libarchive:3.7.1_installed",
        "//internal/packages/stage2/target/board/portage-stable/dev-libs/libzip:1.10.0-r1_installed",
        "//internal/packages/stage2/target/board/portage-stable/dev-libs/openssl:3.2.1-r1_installed",
    ],
    visibility = ["//:__subpackages__"],
)

The input to the rule is the package that we want installed, and the _installed action of all its dependencies.

One thought we had was to add a repository rule that analyzed the external sysroot and output a "digest" file that we consumed from the install action. This kind of works, but the repository rule is computed before the rule that modifies the sysroot, so on the next invocation the digest has changed and the install action runs again.

Put another way, we need a way to execute various bazel run actions according to the dep graph. At some point we will start running ebuild unit tests, and it would also be ideal if we could make the tests also run as part of this single invocation so that everything happens in parallel.

Which category does this issue belong to?

Core

What underlying problem are you trying to solve with this feature?

No response

Which operating system are you running Bazel on?

No response

What is the output of bazel info release?

No response

If bazel info release returns development version or (@non-git), tell us how you built Bazel.

No response

What's the output of git remote get-url origin; git rev-parse HEAD ?

No response

Have you found anything relevant by searching the web?

The interwebs suggest using --workspace_status_command and the version_file.

I naively tried the following patch, but the action only gets executed once:

diff --git a/portage/build_defs/ebuild.bzl b/portage/build_defs/ebuild.bzl
index 189655c8..6e3f7e8b 100644
--- a/portage/build_defs/ebuild.bzl
+++ b/portage/build_defs/ebuild.bzl
@@ -844,6 +844,10 @@ def _ebuild_install_action_impl(ctx):
         args.add("-s")
         args.add(dep[_EbuildInstalledInfo].checksum)

+    # We force the action to run every time by depending on the version_file
+    # which changes every invocation.
+    inputs.append(ctx.version_file)
+
     ctx.actions.run(
         executable = ctx.executable._action_wrapper,
         inputs = inputs,

I suspect the version_file is special cased to not cause rebuilds every time? I didn't try adding the --stamp flag since I don't actually want a stamp.

Another option we could try is to have a repository rule that writes the CACHE_BUST_DATE to a file and have the install action depend on that. I tried using --action_env=$CACHE_BUST_DATE, but I wasn't able to get that to work.

Any other information, logs, or outputs that you want to share?

No response

[lberki]

@ahumesky @ted-xie how are you planning to deal with the same "action needs to run unconditionally because the state of the world might have changed" issue with the Starlark version bazel mobile-install?

Currently, such things are special-cased in Bazel and we in fact have the "run action unconditionally" functionality internally, but it's a bit of a risky feature to add for two reasons

1. It'd be yet another step towards turning Bazel into a generic "workflow management system" (and Bazel is not that)
2. It would be a very convenient way for people to paper over not declaring their dependencies properly, thus, ultimately probably harmful

For these reasons, I'm really reluctant to add such a flag, but I don't really see any other option.

re: $CACHE_BUST_DATE, IIRC you are passing it by --repo_env, which does not get to actions. What you can do is to write the value of that environment variable into a file in the repository rule, then depend on that file in the action you want to always invalidate.

[meisterT]

Is this a dupe of https://github.com/bazelbuild/bazel/issues/3041?

In particular this could be partially worked around with a wrapper as described in https://github.com/bazelbuild/bazel/issues/3041#issuecomment-341491297

[lberki]

Nope, #3041 is for repo rules, this one is for regular actions.

[tjgq]

Can we consider this to be the same as https://github.com/bazelbuild/bazel/issues/15516, then? (i.e., make the no-cache tag also apply to the persistent action cache, or introduce a different tag with that effect)

[lberki]

Maybe? I'm not sure if the hypothetical "this action should always be executed" and the "this action should not be cached on RBE / disk cache" knobs should be the same one.

@ismell : would it be feasible to use a repository rule to determine the current state of the sysroot, write that into a file then depend on that from the action that updates it? That would sidestep the requirement for "always run" actions (or rather, it would re-use the "always re-run repository rule" hack that you already have)

[tbaing]

Using the "always run" repo rule workaround is definitely viable as a workaround, and even if #3041 is implemented we could still use it to trigger a repo rule that creates a single-file repo that we can depend on in our regular actions. Some further discussion since @ismell submitted this has landed us on the idea that we'd do this work unconditionally rather than conditionally based on the state of the external sysroot, so there isn't really a need for a repo rule to do any analysis. So if eventually there was a way to trigger this regular action unconditionally without relying on a repo rule, that would be preferred, but it's a viable and not-too-awkward workaround until such a feature exists.

[ismell]

A @tbaing pointed out, we have decided to wipe the sysroot every invocation. Otherwise we need to implement a diffing algorithm.

I'm also hoping to remove the "CACHE_BUST_DATE" hack at some point since bazel needs to re-hash all of a repository's watched files (if the repository has env variable inputs) when an environment variable changes.

[ismell]

Just FYI, I implemented a CACHE_BUST_DATE hack for now: https://chromium-review.googlesource.com/c/chromiumos/bazel/+/5386329/1

Unfortunately this means we need to keep depending on CACHE_BUST_DATE and invalidating cache for all the repo rules that read environment variables.

[ismell]

So we are going to try switching over to a file that always changes instead: https://chromium-review.googlesource.com/c/chromiumos/bazel/+/5420211

This should hopefully allow us to drop CACHE_BUST_DATE.