Allow sandboxing for copy_* and fix copy_directory tests (#392)

After some thought, I have to say that forcing a local strategy for
copy_file/copy_directory is inappropriate. The point of a sandbox is to
catch hermeticity bugs; disabling the sandbox may be useful for
performance, but it's up to the user to do it if they trust us - and
they can do it via flag. The default should be paranoia and safety.

And on the subject of strategies - using a genrule to create an empty
directory fails in environments where genrules run remote by default
(and thus, copy_directory tests fail). We could, of course, set local=1,
but that disables the sandbox and causes scary warnings. Instead, add a
proper empty_directory rule to test with.

3 files changed, 37 insertions, 13 deletions

diff --git a/rules/private/copy_common.bzl b/rules/private/copy_common.bzl
index 2167c3a..a8f7243 100644
--- a/rules/private/copy_common.bzl
+++ b/rules/private/copy_common.bzl
@@ -22,10 +22,6 @@ COPY_EXECUTION_REQUIREMENTS = {
     # ----------------+-----------------------------------------------------------------------------
     # no-cache        | Results in the action or test never being cached (remotely or locally)
     # ----------------+-----------------------------------------------------------------------------
-    # 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.build/reference/be/common-definitions#common-attributes
     #
     # Copying file & directories is entirely IO-bound and there is no point doing this work
@@ -44,11 +40,6 @@ COPY_EXECUTION_REQUIREMENTS = {
     # 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.
-    #
-    # 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-cache": "1",
-    "local": "1",
 }
diff --git a/tests/copy_directory/BUILD.bazel b/tests/copy_directory/BUILD.bazel
index 2c72ab9..f766e99 100644
--- a/tests/copy_directory/BUILD.bazel
+++ b/tests/copy_directory/BUILD.bazel
@@ -1,6 +1,7 @@
 # This package aids testing the 'copy_directory' rule.
 
 load("//rules:copy_directory.bzl", "copy_directory")
+load(":empty_directory.bzl", "empty_directory")
 
 licenses(["notice"])
 
@@ -13,10 +14,8 @@ copy_directory(
     out = "dir_copy",
 )
 
-genrule(
-    name = "empty_directory",
-    outs = ["empty_dir"],
-    cmd = "mkdir $@",
+empty_directory(
+    name = "empty_dir",
 )
 
 copy_directory(
diff --git a/tests/copy_directory/empty_directory.bzl b/tests/copy_directory/empty_directory.bzl
new file mode 100644
index 0000000..92c5a5f
--- /dev/null
+++ b/tests/copy_directory/empty_directory.bzl
@@ -0,0 +1,34 @@
+# Copyright 2022 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""Creates an empty directory."""
+
+def _empty_directory_impl(ctx):
+    out = ctx.actions.declare_directory(ctx.attr.name)
+    ctx.actions.run_shell(
+        outputs = [out],
+        command = "mkdir -p $@",
+        arguments = [out.path],
+        mnemonic = "EmptyDirectory",
+        progress_message = "Creating empty directory %s" % out.path,
+        use_default_shell_env = True,
+        execution_requirements = {"no-remote": "1"},  # see rules/private/copy_directory_private.bzl
+    )
+    return [DefaultInfo(files = depset(direct = [out]))]
+
+empty_directory = rule(
+    implementation = _empty_directory_impl,
+    provides = [DefaultInfo],
+    doc = "Creates an empty directory with the same name as the target",
+)