From 3a16b4de264eaff08676d90a651dba814adaf2de Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Fri, 6 Oct 2023 08:32:25 -0700 Subject: [PATCH] refactor: remove legacy copy_directory_action helper (#589) --- docs/copy_directory.md | 30 ------------ lib/copy_directory.bzl | 2 - lib/private/copy_directory.bzl | 88 ---------------------------------- 3 files changed, 120 deletions(-) diff --git a/docs/copy_directory.md b/docs/copy_directory.md index e1c3a99..407124b 100644 --- a/docs/copy_directory.md +++ b/docs/copy_directory.md @@ -37,36 +37,6 @@ for more context. | kwargs | further keyword arguments, e.g. visibility | none | - - -## copy_directory_action - -
-copy_directory_action(ctx, src, dst)
-
- -Legacy factory function that creates an action to copy a directory from src to dst. - -For improved analysis and runtime performance, it is recommended the switch -to `copy_directory_bin_action` which takes a tool binary, typically the -`@aspect_bazel_lib//tools/copy_to_directory` `go_binary` either built from -source or provided by a toolchain and creates hard links instead of performing full -file copies. - -This helper is used by copy_directory. It is exposed as a public API so it can be used within -other rule implementations. - - -**PARAMETERS** - - -| Name | Description | Default Value | -| :------------- | :------------- | :------------- | -| ctx | The rule context. | none | -| src | The directory to make a copy of. Can be a source directory or TreeArtifact. | none | -| dst | The directory to copy to. Must be a TreeArtifact. | none | - - ## copy_directory_bin_action diff --git a/lib/copy_directory.bzl b/lib/copy_directory.bzl index 26d1ef4..87f384e 100644 --- a/lib/copy_directory.bzl +++ b/lib/copy_directory.bzl @@ -7,10 +7,8 @@ on Windows (no Bash is required). load( "//lib/private:copy_directory.bzl", _copy_directory = "copy_directory", - _copy_directory_action = "copy_directory_action", _copy_directory_bin_action = "copy_directory_bin_action", ) copy_directory = _copy_directory -copy_directory_action = _copy_directory_action copy_directory_bin_action = _copy_directory_bin_action diff --git a/lib/private/copy_directory.bzl b/lib/private/copy_directory.bzl index e482a49..5d15db9 100644 --- a/lib/private/copy_directory.bzl +++ b/lib/private/copy_directory.bzl @@ -5,94 +5,6 @@ cmd.exe (on Windows). """ load(":copy_common.bzl", _COPY_EXECUTION_REQUIREMENTS = "COPY_EXECUTION_REQUIREMENTS", _progress_path = "progress_path") -load(":platform_utils.bzl", _platform_utils = "platform_utils") - -def _copy_cmd(ctx, src, 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. - # To fix that we write the command to a .bat file so no command line - # quoting or escaping is required. - # Put a hash of the file name into the name of the generated batch file to - # make it unique within the package, so that users can define multiple copy_file's. - bat = ctx.actions.declare_file("%s-%s-cmd.bat" % (ctx.label.name, hash(src.path))) - - # Flags are documented at - # 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 - cmd_tmpl = "@robocopy \"{src}\" \"{dst}\" /E >NUL & @exit 0" - mnemonic = "CopyDirectory" - progress_message = "Copying directory %s" % _progress_path(src) - - ctx.actions.write( - output = bat, - # Do not use lib/shell.bzl's shell.quote() method, because that uses - # Bash quoting syntax, which is different from cmd.exe's syntax. - content = cmd_tmpl.format( - src = src.path.replace("/", "\\"), - dst = dst.path.replace("/", "\\"), - ), - is_executable = True, - ) - ctx.actions.run( - inputs = [src], - tools = [bat], - outputs = [dst], - executable = "cmd.exe", - arguments = ["/C", bat.path.replace("/", "\\")], - mnemonic = mnemonic, - progress_message = progress_message, - use_default_shell_env = True, - execution_requirements = _COPY_EXECUTION_REQUIREMENTS, - ) - -def _copy_bash(ctx, src, dst): - cmd = "rm -Rf \"$2\" && cp -fR \"$1/\" \"$2\"" - mnemonic = "CopyDirectory" - progress_message = "Copying directory %s" % _progress_path(src) - - ctx.actions.run_shell( - tools = [src], - outputs = [dst], - command = cmd, - arguments = [src.path, dst.path], - mnemonic = mnemonic, - progress_message = progress_message, - use_default_shell_env = True, - execution_requirements = _COPY_EXECUTION_REQUIREMENTS, - ) - -# TODO(2.0): remove the legacy copy_directory_action helper -def copy_directory_action(ctx, src, dst): - """Legacy factory function that creates an action to copy a directory from src to dst. - - For improved analysis and runtime performance, it is recommended the switch - to `copy_directory_bin_action` which takes a tool binary, typically the - `@aspect_bazel_lib//tools/copy_to_directory` `go_binary` either built from - source or provided by a toolchain and creates hard links instead of performing full - file copies. - - This helper is used by copy_directory. It is exposed as a public API so it can be used within - other rule implementations. - - Args: - ctx: The rule context. - src: The directory to make a copy of. Can be a source directory or TreeArtifact. - dst: The directory to copy to. Must be a TreeArtifact. - """ - - if not src.is_source and not dst.is_directory: - fail("src must be a source directory or TreeArtifact") - if dst.is_source or not dst.is_directory: - fail("dst must be a TreeArtifact") - - # Because copy actions have "local" execution requirements, we can safely assume - # the execution is the same as the host platform and generate different actions for Windows - # and non-Windows host platforms - if _platform_utils.host_platform_is_windows(): - _copy_cmd(ctx, src, dst) - else: - _copy_bash(ctx, src, dst) def copy_directory_bin_action( ctx,