Revert: feat: expose a config_setting for copy execution_requirements (#606) (#640)

2023-10-31 15:19:38 -07:00 · 2023-10-31 15:19:38 -07:00 · 602b7b8f80
parent a47eebfa65
commit 602b7b8f80
12 changed files with 17 additions and 154 deletions
--- a/.bazeliskrc
+++ b/.bazeliskrc
@ -0,0 +1,2 @@
+BAZELISK_BASE_URL=https://github.com/aspect-build/aspect-cli/releases/download
+USE_BAZEL_VERSION=aspect/5.7.2
--- a/docs/copy_directory.md
+++ b/docs/copy_directory.md
@ -5,10 +5,6 @@ A rule that copies a directory to another place.
 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.
-

 <a id="copy_directory"></a>

@ -46,8 +42,7 @@ for more context.
 ## copy_directory_bin_action

 <pre>
-copy_directory_bin_action(<a href="#copy_directory_bin_action-ctx">ctx</a>, <a href="#copy_directory_bin_action-src">src</a>, <a href="#copy_directory_bin_action-dst">dst</a>, <a href="#copy_directory_bin_action-copy_directory_bin">copy_directory_bin</a>, <a href="#copy_directory_bin_action-hardlink">hardlink</a>, <a href="#copy_directory_bin_action-verbose">verbose</a>,
-                          <a href="#copy_directory_bin_action-override_execution_requirements">override_execution_requirements</a>)
+copy_directory_bin_action(<a href="#copy_directory_bin_action-ctx">ctx</a>, <a href="#copy_directory_bin_action-src">src</a>, <a href="#copy_directory_bin_action-dst">dst</a>, <a href="#copy_directory_bin_action-copy_directory_bin">copy_directory_bin</a>, <a href="#copy_directory_bin_action-hardlink">hardlink</a>, <a href="#copy_directory_bin_action-verbose">verbose</a>)
 </pre>

 Factory function that creates an action to copy a directory from src to dst using a tool binary.
@ -70,6 +65,5 @@ within other rule implementations.
 | <a id="copy_directory_bin_action-copy_directory_bin"></a>copy_directory_bin |  Copy to directory tool binary.   |  none |
 | <a id="copy_directory_bin_action-hardlink"></a>hardlink |  Controls when to use hardlinks to files instead of making copies.<br><br>See copy_directory rule documentation for more details.   |  <code>"auto"</code> |
 | <a id="copy_directory_bin_action-verbose"></a>verbose |  If true, prints out verbose logs to stdout   |  <code>False</code> |
-| <a id="copy_directory_bin_action-override_execution_requirements"></a>override_execution_requirements |  specify execution_requirements for this action   |  <code>None</code> |


--- a/docs/copy_file.md
+++ b/docs/copy_file.md
@ -11,40 +11,6 @@ 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
-```
-

 <a id="copy_file"></a>

--- a/docs/copy_to_directory.md
+++ b/docs/copy_to_directory.md
@ -2,10 +2,6 @@

 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.
-

 <a id="copy_to_directory"></a>

@ -77,8 +73,7 @@ for more information on supported globbing patterns.
 copy_to_directory_bin_action(<a href="#copy_to_directory_bin_action-ctx">ctx</a>, <a href="#copy_to_directory_bin_action-name">name</a>, <a href="#copy_to_directory_bin_action-dst">dst</a>, <a href="#copy_to_directory_bin_action-copy_to_directory_bin">copy_to_directory_bin</a>, <a href="#copy_to_directory_bin_action-files">files</a>, <a href="#copy_to_directory_bin_action-targets">targets</a>, <a href="#copy_to_directory_bin_action-root_paths">root_paths</a>,
                             <a href="#copy_to_directory_bin_action-include_external_repositories">include_external_repositories</a>, <a href="#copy_to_directory_bin_action-include_srcs_packages">include_srcs_packages</a>,
                             <a href="#copy_to_directory_bin_action-exclude_srcs_packages">exclude_srcs_packages</a>, <a href="#copy_to_directory_bin_action-include_srcs_patterns">include_srcs_patterns</a>, <a href="#copy_to_directory_bin_action-exclude_srcs_patterns">exclude_srcs_patterns</a>,
-                             <a href="#copy_to_directory_bin_action-replace_prefixes">replace_prefixes</a>, <a href="#copy_to_directory_bin_action-allow_overwrites">allow_overwrites</a>, <a href="#copy_to_directory_bin_action-hardlink">hardlink</a>, <a href="#copy_to_directory_bin_action-verbose">verbose</a>,
-                             <a href="#copy_to_directory_bin_action-override_execution_requirements">override_execution_requirements</a>)
+                             <a href="#copy_to_directory_bin_action-replace_prefixes">replace_prefixes</a>, <a href="#copy_to_directory_bin_action-allow_overwrites">allow_overwrites</a>, <a href="#copy_to_directory_bin_action-hardlink">hardlink</a>, <a href="#copy_to_directory_bin_action-verbose">verbose</a>)
 </pre>

 Factory function to copy files to a directory using a tool binary.
@ -111,7 +106,6 @@ other rule implementations where additional_files can also be passed in.
 | <a id="copy_to_directory_bin_action-allow_overwrites"></a>allow_overwrites |  If True, allow files to be overwritten if the same output file is copied to twice.<br><br>See copy_to_directory rule documentation for more details.   |  <code>False</code> |
 | <a id="copy_to_directory_bin_action-hardlink"></a>hardlink |  Controls when to use hardlinks to files instead of making copies.<br><br>See copy_to_directory rule documentation for more details.   |  <code>"auto"</code> |
 | <a id="copy_to_directory_bin_action-verbose"></a>verbose |  If true, prints out verbose logs to stdout   |  <code>False</code> |
-| <a id="copy_to_directory_bin_action-override_execution_requirements"></a>override_execution_requirements |  specify execution_requirements for this action   |  <code>None</code> |


 <a id="copy_to_directory_lib.impl"></a>
--- a/lib/BUILD.bazel
+++ b/lib/BUILD.bazel
@ -1,7 +1,6 @@
 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
@ -32,29 +31,6 @@ 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",
 )
--- a/lib/copy_directory.bzl
+++ b/lib/copy_directory.bzl
@ -2,10 +2,6 @@

 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(
--- a/lib/copy_file.bzl
+++ b/lib/copy_file.bzl
@ -27,40 +27,6 @@ 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(
--- a/lib/copy_to_directory.bzl
+++ b/lib/copy_to_directory.bzl
@ -1,8 +1,4 @@
 """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(
--- a/lib/private/copy_common.bzl
+++ b/lib/private/copy_common.bzl
@ -1,25 +1,7 @@
 "Helpers for copy rules"

-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
-# for improved performance.
-COPY_EXECUTION_REQUIREMENTS_LOCAL = {
+# Hints for Bazel spawn strategy
+COPY_EXECUTION_REQUIREMENTS = {
    # ----------------+-----------------------------------------------------------------------------
    # no-sandbox      | Results in the action or test never being sandboxed; it can still be cached
    #                 | or run remotely.
--- a/lib/private/copy_directory.bzl
+++ b/lib/private/copy_directory.bzl
@ -4,7 +4,7 @@ This rule copies a directory to another location using Bash (on Linux/macOS) or
 cmd.exe (on Windows).
 """

-load(":copy_common.bzl", "execution_requirements_for_copy", _progress_path = "progress_path")
+load(":copy_common.bzl", _COPY_EXECUTION_REQUIREMENTS = "COPY_EXECUTION_REQUIREMENTS", _progress_path = "progress_path")

 def copy_directory_bin_action(
        ctx,
@ -12,8 +12,7 @@ def copy_directory_bin_action(
        dst,
        copy_directory_bin,
        hardlink = "auto",
-        verbose = False,
-        override_execution_requirements = None):
+        verbose = False):
    """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`
@ -36,8 +35,6 @@ 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,
@ -58,7 +55,7 @@ def copy_directory_bin_action(
        arguments = args,
        mnemonic = "CopyDirectory",
        progress_message = "Copying directory %s" % _progress_path(src),
-        execution_requirements = override_execution_requirements or execution_requirements_for_copy(ctx),
+        execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
    )

 def _copy_directory_impl(ctx):
@ -96,7 +93,6 @@ _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(
--- a/lib/private/copy_file.bzl
+++ b/lib/private/copy_file.bzl
@ -24,11 +24,11 @@ cmd.exe (on Windows). `_copy_xfile` marks the resulting file executable,
 `_copy_file` does not.
 """

-load(":copy_common.bzl", "execution_requirements_for_copy", _progress_path = "progress_path")
+load(":copy_common.bzl", _COPY_EXECUTION_REQUIREMENTS = "COPY_EXECUTION_REQUIREMENTS", _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, override_execution_requirements = None):
+def _copy_cmd(ctx, src, src_path, dst):
    # 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.
@ -65,10 +65,10 @@ def _copy_cmd(ctx, src, src_path, dst, override_execution_requirements = None):
        mnemonic = mnemonic,
        progress_message = progress_message,
        use_default_shell_env = True,
-        execution_requirements = override_execution_requirements or execution_requirements_for_copy(ctx),
+        execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
    )

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

 def copy_file_action(ctx, src, dst, dir_path = None):
@ -154,7 +154,6 @@ _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(
--- a/lib/private/copy_to_directory.bzl
+++ b/lib/private/copy_to_directory.bzl
@ -1,6 +1,6 @@
 "copy_to_directory implementation"

-load(":copy_common.bzl", "execution_requirements_for_copy", _progress_path = "progress_path")
+load(":copy_common.bzl", _COPY_EXECUTION_REQUIREMENTS = "COPY_EXECUTION_REQUIREMENTS", _progress_path = "progress_path")
 load(":directory_path.bzl", "DirectoryPathInfo")
 load(":paths.bzl", "paths")

@ -254,7 +254,6 @@ _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(
@ -325,8 +324,7 @@ def copy_to_directory_bin_action(
        replace_prefixes = {},
        allow_overwrites = False,
        hardlink = "auto",
-        verbose = False,
-        override_execution_requirements = None):
+        verbose = False):
    """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`
@ -385,8 +383,6 @@ 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
@ -495,7 +491,7 @@ def copy_to_directory_bin_action(
        arguments = [config_file.path, ctx.label.workspace_name],
        mnemonic = "CopyToDirectory",
        progress_message = "Copying files to directory %s" % _progress_path(dst),
-        execution_requirements = override_execution_requirements or execution_requirements_for_copy(ctx),
+        execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
    )

 copy_to_directory_lib = struct(