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

feat: expose a config_setting for copy execution_requirements #606

Merged
merged 4 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .bazeliskrc

This file was deleted.

6 changes: 5 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.

6 changes: 5 additions & 1 deletion 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
2 changes: 2 additions & 0 deletions lib/copy_directory.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

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) regarding execution_requirements settings for remote execution.
"""

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
2 changes: 2 additions & 0 deletions lib/copy_to_directory.bzl
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
"""Copy files and directories to an output directory.

NB: See notes on [copy_file](./copy_file.md) regarding execution_requirements settings for remote execution.
"""

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
alexeagle marked this conversation as resolved.
Show resolved Hide resolved

# 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
10 changes: 7 additions & 3 deletions lib/private/copy_directory.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,16 @@ 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")

def copy_directory_bin_action(
ctx,
src,
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 @@ -35,6 +36,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 @@ -55,7 +58,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 @@ -93,6 +96,7 @@ _copy_directory = rule(
default = "auto",
),
"verbose": attr.bool(),
"_options": attr.label(default = "//lib:copy_options"),
alexeagle marked this conversation as resolved.
Show resolved Hide resolved
# 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):
Expand Down Expand Up @@ -154,6 +154,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
10 changes: 7 additions & 3 deletions lib/private/copy_to_directory.bzl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"copy_to_directory implementation"

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(":paths.bzl", "paths")

Expand Down Expand Up @@ -254,6 +254,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 @@ -324,7 +325,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 @@ -383,6 +385,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 @@ -491,7 +495,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),
)

copy_to_directory_lib = struct(
Expand Down