Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "refactor: reduce execution requirements for copy" #594

Merged
merged 2 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/ci.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ build:local --disk_cache=~/.cache/bazel

# Remote build execution
build:rbe --extra_execution_platforms=@aspect_bazel_lib//platforms:x86_64_linux_remote
build:rbe --genrule_strategy=remote
build:rbe --host_platform=@aspect_bazel_lib//platforms:x86_64_linux_remote
build:rbe --jobs=32

Expand Down
56 changes: 53 additions & 3 deletions lib/private/copy_common.bzl
Original file line number Diff line number Diff line change
@@ -1,11 +1,61 @@
"Helpers for copy rules"

# Hints for Bazel spawn strategy, copied from
# https://github.com/bazelbuild/bazel-skylib/blob/0171c69e5cc691e2d0cd9f3f3e4c3bf112370ca2/rules/private/copy_common.bzl
# See extensive comments there for reasoning on this execution-requirements selection.
# Hints for Bazel spawn strategy
COPY_EXECUTION_REQUIREMENTS = {
# ----------------+-----------------------------------------------------------------------------
# no-remote | Prevents the action or test from being executed remotely or cached remotely.
# | This is equivalent to using both `no-remote-cache` and `no-remote-exec`.
# ----------------+-----------------------------------------------------------------------------
# no-remote-cache | Results in the action or test never being cached remotely (but it may
# | be cached locally; it may also be executed remotely). Note: for the purposes
# | of this tag, the disk-cache is considered a local cache, whereas the http
# | and gRPC caches are considered remote. If a combined cache is specified
# | (i.e. a cache with local and remote components), it's treated as a remote
# | cache and disabled entirely unless --incompatible_remote_results_ignore_disk
# | is set in which case the local components will be used.
# ----------------+-----------------------------------------------------------------------------
# no-remote-exec | Results in the action or test never being executed remotely (but it may be
# | cached remotely).
# ----------------+-----------------------------------------------------------------------------
# no-cache | Results in the action or test never being cached (remotely or locally)
# ----------------+-----------------------------------------------------------------------------
# no-sandbox | Results in the action or test never being sandboxed; it can still be cached
# | or run remotely - use no-cache or no-remote to prevent either or both of
# | those.
# ----------------+-----------------------------------------------------------------------------
# local | Precludes the action or test from being remotely cached, remotely executed,
# | or run inside the sandbox. For genrules and tests, marking the rule with the
# | local = True attribute has the same effect.
# ----------------+-----------------------------------------------------------------------------
# See https://bazel.google.cn/reference/be/common-definitions?hl=en&authuser=0#common-attributes
#
# Copying file & directories is entirely IO-bound and there is no point doing this work
# remotely.
#
# Also, remote-execution does not allow source directory inputs, see
# https://github.com/bazelbuild/bazel/commit/c64421bc35214f0414e4f4226cc953e8c55fa0d2 So we must
# not attempt to execute remotely in that case.
#
# There is also no point pulling the output file or directory from the remote cache since the
# bytes to copy are already available locally. Conversely, no point in writing to the cache if
# no one has any reason to check it for this action.
#
# Read and writing to disk cache is disabled as well primarily to reduce disk usage on the local
# machine. A disk cache hit of a directory copy could be slghtly faster than a copy since the
# disk cache stores the directory artifact as a single entry, but the slight performance bump
# comes at the cost of heavy disk cache usage, which is an unmanaged directory that grow beyond
# the bounds of the physical disk.
# TODO: run benchmarks to measure the impact on copy_directory
#
# Sandboxing for this action is wasteful as well since there is a 1:1 mapping of input
# file/directory to output file/directory and no room for non-hermetic inputs to sneak in to the
# input.
"no-remote": "1",
"no-remote-cache": "1",
"no-remote-exec": "1",
"no-cache": "1",
"no-sandbox": "1",
"local": "1",
}

def progress_path(f):
Expand Down