Skip to content

Commit

Permalink
feat: expose a config_setting for copy execution_requirements (#606)
Browse files Browse the repository at this point in the history
* feat: expose a config_setting for copy execution_requirements

Fixes #604

* chore: add user docs

* chore: improve docs

* chore: better link to copy_file
  • Loading branch information
alexeagle committed Oct 9, 2023
1 parent f4d6fd6 commit e9f7aeb
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 19 deletions.
8 changes: 7 additions & 1 deletion docs/copy_directory.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 34 additions & 0 deletions docs/copy_file.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions docs/copy_to_directory.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 24 additions & 0 deletions lib/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
load("//lib:utils.bzl", "is_bzlmod_enabled")
load("//lib/private:copy_common.bzl", "copy_options")
load("//lib/private:stamping.bzl", "stamp_build_setting")

# Ensure that users building their own rules can dep on our bzl_library targets for their stardoc
Expand Down Expand Up @@ -31,6 +32,29 @@ bool_flag(
build_setting_default = True if is_bzlmod_enabled() else False,
)

# Users may set this flag with
bool_flag(
name = "copy_use_local_execution",
build_setting_default = True,
visibility = ["//visibility:public"],
)

config_setting(
name = "copy_use_local_execution_setting",
flag_values = {
":copy_use_local_execution": "true",
},
)

# Used in rules which spawn actions that copy files
copy_options(
name = "copy_options",
copy_use_local_execution = select({
":copy_use_local_execution_setting": True,
"//conditions:default": False,
}),
)

toolchain_type(
name = "jq_toolchain_type",
)
Expand Down
4 changes: 4 additions & 0 deletions lib/copy_directory.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
The rule uses a Bash command on Linux/macOS/non-Windows, and a cmd.exe command
on Windows (no Bash is required).
NB: See notes on [copy_file](./copy_file.md#choosing-execution-requirements)
regarding `execution_requirements` settings for remote execution.
These settings apply to the rules below as well.
"""

load(
Expand Down
34 changes: 34 additions & 0 deletions lib/copy_file.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,40 @@ on Windows (no Bash is required).
This fork of bazel-skylib's copy_file adds DirectoryPathInfo support and allows multiple
copy_file in the same package.
Choosing execution requirements
-------------------------------
Copy actions can be very numerous, especially when used on third-party dependency packages.
By default, we set the `execution_requirements` of actions we spawn to be non-sandboxed and run
locally, not reading or writing to a remote cache. For the typical user this is the fastest, because
it avoids the overhead of creating a sandbox and making network calls for every file being copied.
If you use Remote Execution and Build-without-the-bytes, then you'll want the copy action to
occur on the remote machine instead, since the inputs and outputs stay in the cloud and don't need
to be brought to the local machine where Bazel runs.
Other reasons to allow copy actions to run remotely:
- Bazel prints an annoying warning "[action] uses implicit fallback from sandbox to local, which is deprecated because it is not hermetic"
- When the host and exec platforms have different architectures, toolchain resolution runs the wrong binary locally,
see https://github.com/aspect-build/bazel-lib/issues/466
To disable our `copy_use_local_execution` flag, put this in your `.bazelrc` file:
```
# with Bazel 6.4 or greater:
# Disable default for execution_requirements of copy actions
common --@aspect_bazel_lib//lib:copy_use_local_execution=false
# with Bazel 6.3 or earlier:
# Disable default for execution_requirements of copy actions
build --@aspect_bazel_lib//lib:copy_use_local_execution=false
fetch --@aspect_bazel_lib//lib:copy_use_local_execution=false
query --@aspect_bazel_lib//lib:copy_use_local_execution=false
```
"""

load(
Expand Down
4 changes: 4 additions & 0 deletions lib/copy_to_directory.bzl
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
"""Copy files and directories to an output directory.
NB: See notes on [copy_file](./copy_file.md#choosing-execution-requirements)
regarding `execution_requirements` settings for remote execution.
These settings apply to the rules below as well.
"""

load(
Expand Down
22 changes: 20 additions & 2 deletions lib/private/copy_common.bzl
Original file line number Diff line number Diff line change
@@ -1,7 +1,25 @@
"Helpers for copy rules"

# Hints for Bazel spawn strategy
COPY_EXECUTION_REQUIREMENTS = {
CopyOptionsInfo = provider("Options for running copy actions", fields = ["execution_requirements"])

def _copy_options_impl(ctx):
return CopyOptionsInfo(
execution_requirements = COPY_EXECUTION_REQUIREMENTS_LOCAL if ctx.attr.copy_use_local_execution else {},
)

copy_options = rule(implementation = _copy_options_impl, attrs = {"copy_use_local_execution": attr.bool()})

# Helper function to be used when creating an action
def execution_requirements_for_copy(ctx):
if hasattr(ctx.attr, "_options") and CopyOptionsInfo in ctx.attr._options:
return ctx.attr._options[CopyOptionsInfo].execution_requirements

# If the rule ctx doesn't expose the CopyOptions, the default is to run locally
return COPY_EXECUTION_REQUIREMENTS_LOCAL

# When applied to execution_requirements of an action, these prevent the action from being
# sandboxed or remotely cached, for performance of builds that don't rely on RBE and build-without-bytes.
COPY_EXECUTION_REQUIREMENTS_LOCAL = {
# ----------------+-----------------------------------------------------------------------------
# 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`.
Expand Down
18 changes: 11 additions & 7 deletions lib/private/copy_directory.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ This rule copies a directory to another location using Bash (on Linux/macOS) or
cmd.exe (on Windows).
"""

load(":copy_common.bzl", _COPY_EXECUTION_REQUIREMENTS = "COPY_EXECUTION_REQUIREMENTS", _progress_path = "progress_path")
load(":copy_common.bzl", "execution_requirements_for_copy", _progress_path = "progress_path")
load(":platform_utils.bzl", _platform_utils = "platform_utils")

def _copy_cmd(ctx, src, dst):
def _copy_cmd(ctx, src, dst, override_execution_requirements = None):
# Most Windows binaries built with MSVC use a certain argument quoting
# scheme. Bazel uses that scheme too to quote arguments. However,
# cmd.exe uses different semantics, so Bazel's quoting is wrong here.
Expand Down Expand Up @@ -43,10 +43,10 @@ def _copy_cmd(ctx, src, dst):
mnemonic = mnemonic,
progress_message = progress_message,
use_default_shell_env = True,
execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
execution_requirements = override_execution_requirements or execution_requirements_for_copy(ctx),
)

def _copy_bash(ctx, src, dst):
def _copy_bash(ctx, src, dst, override_execution_requirements = None):
cmd = "rm -Rf \"$2\" && cp -fR \"$1/\" \"$2\""
mnemonic = "CopyDirectory"
progress_message = "Copying directory %s" % _progress_path(src)
Expand All @@ -59,7 +59,7 @@ def _copy_bash(ctx, src, dst):
mnemonic = mnemonic,
progress_message = progress_message,
use_default_shell_env = True,
execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
execution_requirements = override_execution_requirements or execution_requirements_for_copy(ctx),
)

# TODO(2.0): remove the legacy copy_directory_action helper
Expand Down Expand Up @@ -103,7 +103,8 @@ def copy_directory_bin_action(
dst,
copy_directory_bin,
hardlink = "auto",
verbose = False):
verbose = False,
override_execution_requirements = None):
"""Factory function that creates an action to copy a directory from src to dst using a tool binary.
The tool binary will typically be the `@aspect_bazel_lib//tools/copy_directory` `go_binary`
Expand All @@ -126,6 +127,8 @@ def copy_directory_bin_action(
See copy_directory rule documentation for more details.
verbose: If true, prints out verbose logs to stdout
override_execution_requirements: specify execution_requirements for this action
"""
args = [
src.path,
Expand All @@ -146,7 +149,7 @@ def copy_directory_bin_action(
arguments = args,
mnemonic = "CopyDirectory",
progress_message = "Copying directory %s" % _progress_path(src),
execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
execution_requirements = override_execution_requirements or execution_requirements_for_copy(ctx),
)

def _copy_directory_impl(ctx):
Expand Down Expand Up @@ -184,6 +187,7 @@ _copy_directory = rule(
default = "auto",
),
"verbose": attr.bool(),
"_options": attr.label(default = "//lib:copy_options"),
# use '_tool' attribute for development only; do not commit with this attribute active since it
# propagates a dependency on rules_go which would be breaking for users
# "_tool": attr.label(
Expand Down
11 changes: 6 additions & 5 deletions lib/private/copy_file.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ cmd.exe (on Windows). `_copy_xfile` marks the resulting file executable,
`_copy_file` does not.
"""

load(":copy_common.bzl", _COPY_EXECUTION_REQUIREMENTS = "COPY_EXECUTION_REQUIREMENTS", _progress_path = "progress_path")
load(":copy_common.bzl", "execution_requirements_for_copy", _progress_path = "progress_path")
load(":directory_path.bzl", "DirectoryPathInfo")
load(":platform_utils.bzl", _platform_utils = "platform_utils")

def _copy_cmd(ctx, src, src_path, dst):
def _copy_cmd(ctx, src, src_path, dst, override_execution_requirements = None):
# Most Windows binaries built with MSVC use a certain argument quoting
# scheme. Bazel uses that scheme too to quote arguments. However,
# cmd.exe uses different semantics, so Bazel's quoting is wrong here.
Expand Down Expand Up @@ -65,10 +65,10 @@ def _copy_cmd(ctx, src, src_path, dst):
mnemonic = mnemonic,
progress_message = progress_message,
use_default_shell_env = True,
execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
execution_requirements = override_execution_requirements or execution_requirements_for_copy(ctx),
)

def _copy_bash(ctx, src, src_path, dst):
def _copy_bash(ctx, src, src_path, dst, override_execution_requirements = None):
cmd_tmpl = "cp -f \"$1\" \"$2\""
mnemonic = "CopyFile"
progress_message = "Copying file %s" % _progress_path(src)
Expand All @@ -81,7 +81,7 @@ def _copy_bash(ctx, src, src_path, dst):
mnemonic = mnemonic,
progress_message = progress_message,
use_default_shell_env = True,
execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
execution_requirements = override_execution_requirements or execution_requirements_for_copy(ctx),
)

def copy_file_action(ctx, src, dst, dir_path = None, is_windows = None):
Expand Down Expand Up @@ -157,6 +157,7 @@ _ATTRS = {
"is_executable": attr.bool(mandatory = True),
"allow_symlink": attr.bool(mandatory = True),
"out": attr.output(mandatory = True),
"_options": attr.label(default = "//lib:copy_options"),
}

_copy_file = rule(
Expand Down
12 changes: 8 additions & 4 deletions lib/private/copy_to_directory.bzl
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"copy_to_directory implementation"

load("@bazel_skylib//lib:paths.bzl", skylib_paths = "paths")
load(":copy_common.bzl", _COPY_EXECUTION_REQUIREMENTS = "COPY_EXECUTION_REQUIREMENTS", _progress_path = "progress_path")
load(":copy_common.bzl", "execution_requirements_for_copy", _progress_path = "progress_path")
load(":directory_path.bzl", "DirectoryPathInfo")
load(":glob_match.bzl", "glob_match", "is_glob")
load(":paths.bzl", "paths")
Expand Down Expand Up @@ -277,6 +277,7 @@ _copy_to_directory_attr = {
"verbose": attr.bool(
doc = _copy_to_directory_attr_doc["verbose"],
),
"_options": attr.label(default = "//lib:copy_options"),
# use '_tool' attribute for development only; do not commit with this attribute active since it
# propagates a dependency on rules_go which would be breaking for users
# "_tool": attr.label(
Expand Down Expand Up @@ -520,7 +521,7 @@ fi
mnemonic = "CopyToDirectory",
progress_message = "Copying files to directory %s" % _progress_path(dst_dir),
use_default_shell_env = True,
execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
execution_requirements = override_execution_requirements or execution_requirements_for_copy(ctx),
)

def _copy_to_dir_cmd(ctx, copy_paths, dst_dir):
Expand Down Expand Up @@ -654,7 +655,8 @@ def copy_to_directory_bin_action(
replace_prefixes = {},
allow_overwrites = False,
hardlink = "auto",
verbose = False):
verbose = False,
override_execution_requirements = None):
"""Factory function to copy files to a directory using a tool binary.
The tool binary will typically be the `@aspect_bazel_lib//tools/copy_to_directory` `go_binary`
Expand Down Expand Up @@ -717,6 +719,8 @@ def copy_to_directory_bin_action(
See copy_to_directory rule documentation for more details.
verbose: If true, prints out verbose logs to stdout
override_execution_requirements: specify execution_requirements for this action
"""

# Replace "." in root_paths with the package name of the target
Expand Down Expand Up @@ -839,7 +843,7 @@ def copy_to_directory_bin_action(
arguments = [config_file.path],
mnemonic = "CopyToDirectory",
progress_message = "Copying files to directory %s" % _progress_path(dst),
execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
execution_requirements = override_execution_requirements or execution_requirements_for_copy(ctx),
)

# TODO(2.0): remove the legacy copy_to_directory_action helper
Expand Down

0 comments on commit e9f7aeb

Please sign in to comment.