refactor: remove directory functionality from copy_file (#64)

docs/copy_file.md

@@ -8,15 +8,16 @@ The 'copy_file' rule does this with a simpler interface than genrule.
 The rule uses a Bash command on Linux/macOS/non-Windows, and a cmd.exe command
 on Windows (no Bash is required).
 
-This fork of bazel-skylib's copy_file adds directory support.
+This fork of bazel-skylib's copy_file adds DirectoryPathInfo support and allows multiple
+copy_file in the same package.
 
 
 <a id="#copy_file"></a>
 
 ## copy_file
 
 <pre>
-copy_file(<a href="#copy_file-name">name</a>, <a href="#copy_file-src">src</a>, <a href="#copy_file-out">out</a>, <a href="#copy_file-is_directory">is_directory</a>, <a href="#copy_file-is_executable">is_executable</a>, <a href="#copy_file-allow_symlink">allow_symlink</a>, <a href="#copy_file-kwargs">kwargs</a>)
+copy_file(<a href="#copy_file-name">name</a>, <a href="#copy_file-src">src</a>, <a href="#copy_file-out">out</a>, <a href="#copy_file-is_executable">is_executable</a>, <a href="#copy_file-allow_symlink">allow_symlink</a>, <a href="#copy_file-kwargs">kwargs</a>)
 </pre>
 
 Copies a file or directory to another location.
@@ -38,9 +39,8 @@ for more context.
 | Name  | Description | Default Value |
 | :------------- | :------------- | :------------- |
 | <a id="copy_file-name"></a>name |  Name of the rule.   |  none |
-| <a id="copy_file-src"></a>src |  A Label. The file or directory to make a copy of. (Can also be the label of a rule that generates a file or directory.)   |  none |
+| <a id="copy_file-src"></a>src |  A Label. The file to make a copy of. (Can also be the label of a rule that generates a file.)   |  none |
 | <a id="copy_file-out"></a>out |  Path of the output file, relative to this package.   |  none |
-| <a id="copy_file-is_directory"></a>is_directory |  treat the source file as a directory Workaround for https://github.com/bazelbuild/bazel/issues/12954   |  <code>False</code> |
 | <a id="copy_file-is_executable"></a>is_executable |  A boolean. Whether to make the output file executable. When True, the rule's output can be executed using <code>bazel run</code> and can be in the srcs of binary and test rules that require executable sources. WARNING: If <code>allow_symlink</code> is True, <code>src</code> must also be executable.   |  <code>False</code> |
 | <a id="copy_file-allow_symlink"></a>allow_symlink |  A boolean. Whether to allow symlinking instead of copying. When False, the output is always a hard copy. When True, the output *can* be a symlink, but there is no guarantee that a symlink is created (i.e., at the time of writing, we don't create symlinks on Windows). Set this to True if you need fast copying and your tools can handle symlinks (which most UNIX tools can).   |  <code>False</code> |
 | <a id="copy_file-kwargs"></a>kwargs |  further keyword arguments, e.g. <code>visibility</code>   |  none |

lib/copy_file.bzl

@@ -13,10 +13,9 @@
 # limitations under the License.
 
 # LOCAL MODIFICATIONS
-# this has two PRs patched in on top of the original
+# this has a PR patched in on top of the original
 # https://github.com/bazelbuild/bazel-skylib/blob/7b859037a673db6f606661323e74c5d4751595e6/rules/private/copy_file_private.bzl
-# 1) https://github.com/bazelbuild/bazel-skylib/pull/323
-# 2) https://github.com/bazelbuild/bazel-skylib/pull/324
+# https://github.com/bazelbuild/bazel-skylib/pull/324
 
 """A rule that copies a file to another place.
 
@@ -26,7 +25,8 @@ The 'copy_file' rule does this with a simpler interface than genrule.
 The rule uses a Bash command on Linux/macOS/non-Windows, and a cmd.exe command
 on Windows (no Bash is required).
 
-This fork of bazel-skylib's copy_file adds directory support.
+This fork of bazel-skylib's copy_file adds DirectoryPathInfo support and allows multiple
+copy_file in the same package.
 """
 
 load(

lib/private/copy_file.bzl

@@ -13,14 +13,13 @@
 # limitations under the License.
 
 # LOCAL MODIFICATIONS
-# this has two PRs patched in on top of the original
+# this has a PR patched in on top of the original
 # https://github.com/bazelbuild/bazel-skylib/blob/7b859037a673db6f606661323e74c5d4751595e6/rules/private/copy_file_private.bzl
-# 1) https://github.com/bazelbuild/bazel-skylib/pull/323
-# 2) https://github.com/bazelbuild/bazel-skylib/pull/324
+# https://github.com/bazelbuild/bazel-skylib/pull/324
 
 """Implementation of copy_file macro and underlying rules.
 
-These rules copy a file or directory to another location using Bash (on Linux/macOS) or
+These rules copy a file to another location using Bash (on Linux/macOS) or
 cmd.exe (on Windows). `_copy_xfile` marks the resulting file executable,
 `_copy_file` does not.
 """
@@ -49,16 +48,9 @@ def copy_cmd(ctx, src_file, src_path, dst):
 
     # Flags are documented at
     # https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/copy
-    # https://docs.microsoft.com/en-us/windows-server/administration/windows-commands/robocopy
-    # NB: robocopy return non-zero exit codes on success so we must exit 0 after calling it
-    if dst.is_directory:
-        cmd_tmpl = "@robocopy \"{src}\" \"{dst}\" /E >NUL & @exit 0"
-        mnemonic = "CopyDirectory"
-        progress_message = "Copying directory %s" % src_path
-    else:
-        cmd_tmpl = "@copy /Y \"{src}\" \"{dst}\" >NUL"
-        mnemonic = "CopyFile"
-        progress_message = "Copying file %s" % src_path
+    cmd_tmpl = "@copy /Y \"{src}\" \"{dst}\" >NUL"
+    mnemonic = "CopyFile"
+    progress_message = "Copying file %s" % src_path
 
     ctx.actions.write(
         output = bat,
@@ -84,14 +76,9 @@ def copy_cmd(ctx, src_file, src_path, dst):
 
 # buildifier: disable=function-docstring
 def copy_bash(ctx, src_file, src_path, dst):
-    if dst.is_directory:
-        cmd_tmpl = "rm -rf \"$2\" && cp -fR \"$1/\" \"$2\""
-        mnemonic = "CopyDirectory"
-        progress_message = "Copying directory %s" % src_path
-    else:
-        cmd_tmpl = "cp -f \"$1\" \"$2\""
-        mnemonic = "CopyFile"
-        progress_message = "Copying file %s" % src_path
+    cmd_tmpl = "cp -f \"$1\" \"$2\""
+    mnemonic = "CopyFile"
+    progress_message = "Copying file %s" % src_path
 
     ctx.actions.run_shell(
         tools = [src_file],
@@ -105,19 +92,13 @@ def copy_bash(ctx, src_file, src_path, dst):
     )
 
 def _copy_file_impl(ctx):
-    # When creating a directory, declare that to Bazel so downstream rules
-    # see it as a TreeArtifact and handle correctly, e.g. for remote execution
-    if getattr(ctx.attr, "is_directory", False):
-        output = ctx.actions.declare_directory(ctx.attr.out)
-    else:
-        output = ctx.outputs.out
     if ctx.attr.allow_symlink:
         if len(ctx.files.src) != 1:
             fail("src must be a single file when allow_symlink is True")
-        if output.is_directory:
-            fail("Cannot use both is_directory and allow_symlink")
+        if ctx.files.src[0].is_directory:
+            fail("cannot use copy_file to create a symlink to a directory")
         ctx.actions.symlink(
-            output = output,
+            output = ctx.outputs.out,
             target_file = ctx.files.src[0],
             is_executable = ctx.attr.is_executable,
         )
@@ -128,17 +109,19 @@ def _copy_file_impl(ctx):
         else:
             if len(ctx.files.src) != 1:
                 fail("src must be a single file or a target that provides a DirectoryPathInfo")
+            if ctx.files.src[0].is_directory:
+                fail("cannot use copy_file on a directory; try copy_directory instead")
             src_file = ctx.files.src[0]
             src_path = src_file.path
         if ctx.attr.is_windows:
-            copy_cmd(ctx, src_file, src_path, output)
+            copy_cmd(ctx, src_file, src_path, ctx.outputs.out)
         else:
-            copy_bash(ctx, src_file, src_path, output)
+            copy_bash(ctx, src_file, src_path, ctx.outputs.out)
 
-    files = depset(direct = [output])
-    runfiles = ctx.runfiles(files = [output])
+    files = depset(direct = [ctx.outputs.out])
+    runfiles = ctx.runfiles(files = [ctx.outputs.out])
     if ctx.attr.is_executable:
-        return [DefaultInfo(files = files, runfiles = runfiles, executable = output)]
+        return [DefaultInfo(files = files, runfiles = runfiles, executable = ctx.outputs.out)]
     else:
         return [DefaultInfo(files = files, runfiles = runfiles)]
 
@@ -147,37 +130,23 @@ _ATTRS = {
     "is_windows": attr.bool(mandatory = True),
     "is_executable": attr.bool(mandatory = True),
     "allow_symlink": attr.bool(mandatory = True),
+    "out": attr.output(mandatory = True),
 }
 
-_copy_directory = rule(
-    implementation = _copy_file_impl,
-    provides = [DefaultInfo],
-    attrs = dict(_ATTRS, **{
-        "is_directory": attr.bool(default = True),
-        # Cannot declare out as an output here, because there's no API for declaring
-        # TreeArtifact outputs.
-        "out": attr.string(mandatory = True),
-    }),
-)
-
 _copy_file = rule(
     implementation = _copy_file_impl,
     provides = [DefaultInfo],
-    attrs = dict(_ATTRS, **{
-        "out": attr.output(mandatory = True),
-    }),
+    attrs = _ATTRS,
 )
 
 _copy_xfile = rule(
     implementation = _copy_file_impl,
     executable = True,
     provides = [DefaultInfo],
-    attrs = dict(_ATTRS, **{
-        "out": attr.output(mandatory = True),
-    }),
+    attrs = _ATTRS,
 )
 
-def copy_file(name, src, out, is_directory = False, is_executable = False, allow_symlink = False, **kwargs):
+def copy_file(name, src, out, is_executable = False, allow_symlink = False, **kwargs):
     """Copies a file or directory to another location.
 
     `native.genrule()` is sometimes used to copy files (often wishing to rename them). The 'copy_file' rule does this with a simpler interface than genrule.
@@ -192,11 +161,9 @@ def copy_file(name, src, out, is_directory = False, is_executable = False, allow
 
     Args:
       name: Name of the rule.
-      src: A Label. The file or directory to make a copy of.
-          (Can also be the label of a rule that generates a file or directory.)
+      src: A Label. The file to make a copy of.
+          (Can also be the label of a rule that generates a file.)
       out: Path of the output file, relative to this package.
-      is_directory: treat the source file as a directory
-          Workaround for https://github.com/bazelbuild/bazel/issues/12954
       is_executable: A boolean. Whether to make the output file executable. When
           True, the rule's output can be executed using `bazel run` and can be
           in the srcs of binary and test rules that require executable sources.
@@ -213,8 +180,6 @@ def copy_file(name, src, out, is_directory = False, is_executable = False, allow
     copy_file_impl = _copy_file
     if is_executable:
         copy_file_impl = _copy_xfile
-    elif is_directory:
-        copy_file_impl = _copy_directory
 
     copy_file_impl(
         name = name,

lib/tests/copy_to_directory/BUILD.bazel

@@ -1,16 +1,15 @@
 "tests for copy_to_directory"
 
 load("//lib:diff_test.bzl", "diff_test")
-load("//lib:copy_file.bzl", "copy_file")
+load("//lib:copy_directory.bzl", "copy_directory")
 load("//lib:copy_to_directory.bzl", "copy_to_directory")
 load("//lib:directory_path.bzl", "make_directory_paths")
 
 [
-    copy_file(
+    copy_directory(
         name = "%s" % d,
         src = "dir_%s" % d,
         out = "%s" % d,
-        is_directory = True,
     )
     for d in [
         "a",

lib/tests/external_test_repo/BUILD.bazel

@@ -1,11 +1,10 @@
-load("@aspect_bazel_lib//lib:copy_file.bzl", "copy_file")
+load("@aspect_bazel_lib//lib:copy_directory.bzl", "copy_directory")
 
 [
-    copy_file(
+    copy_directory(
         name = "%s" % d,
         src = "dir_%s" % d,
         out = "%s" % d,
-        is_directory = True,
         visibility = ["//visibility:public"],
     )
     for d in [