Skip to content

Commit

Permalink
perf: improve copy_file.bzl progress_message (#931)
Browse files Browse the repository at this point in the history
  • Loading branch information
dzbarsky authored Sep 10, 2024
1 parent de9fd59 commit 4c1267f
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 43 deletions.
5 changes: 2 additions & 3 deletions docs/copy_directory.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions docs/copy_file.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions lib/copy_directory.bzl
Original file line number Diff line number Diff line change
@@ -1,7 +1,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).
The rule uses a precompiled binary to perform the copy, so no shell is required.
"""

load(
Expand Down
3 changes: 1 addition & 2 deletions lib/copy_file.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@
`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.
The rule uses a Bash command on Linux/macOS/non-Windows, and a `cmd.exe` command
on Windows (no Bash is required).
This rule uses a hermetic uutils/coreutils `cp` binary, no shell is required.
This fork of bazel-skylib's copy_file adds `DirectoryPathInfo` support and allows multiple
`copy_file` rules in the same package.
Expand Down
12 changes: 0 additions & 12 deletions lib/private/copy_common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,3 @@ COPY_EXECUTION_REQUIREMENTS = {
# output file/directory so little room for non-hermetic inputs to sneak in to the execution.
"no-sandbox": "1",
}

def progress_path(f):
"""
Convert a file to an appropriate string to display in an action progress message.
Args:
f: a file to show as a path in a progress message
Returns:
The path formatted for use in a progress message
"""
return f.short_path.removeprefix("../")
9 changes: 4 additions & 5 deletions lib/private/copy_directory.bzl
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
"""Implementation of copy_directory macro and underlying rules.
This rule copies a directory to another location using Bash (on Linux/macOS) or
cmd.exe (on Windows).
This rule copies a directory to another location using a precompiled binary.
"""

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

def copy_directory_bin_action(
ctx,
Expand Down Expand Up @@ -60,7 +59,7 @@ def copy_directory_bin_action(
executable = copy_directory_bin,
arguments = args,
mnemonic = "CopyDirectory",
progress_message = "Copying directory %s" % _progress_path(src),
progress_message = "Copying directory %{input}",
execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
)

Expand Down Expand Up @@ -123,7 +122,7 @@ def copy_directory(
**kwargs):
"""Copies a directory to another location.
This rule uses a Bash command on Linux/macOS/non-Windows, and a cmd.exe command on Windows (no Bash is required).
This rule uses a precompiled binary to perform the copy, so no shell is required.
If using this rule with source directories, it is recommended that you use the
`--host_jvm_args=-DBAZEL_TRACK_SOURCE_DIRECTORIES=1` startup option so that changes
Expand Down
20 changes: 6 additions & 14 deletions lib/private/copy_file.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,13 @@
# See the License for the specific language governing permissions and
# limitations under the License.

# LOCAL MODIFICATIONS
# this has a PR patched in on top of the original
# https:/bazelbuild/bazel-skylib/blob/7b859037a673db6f606661323e74c5d4751595e6/rules/private/copy_file_private.bzl
# https:/bazelbuild/bazel-skylib/pull/324

"""Implementation of copy_file macro and underlying rules.
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.
These rules copy a file to another location using hermetic uutils/coreutils `cp`.
`_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", _COPY_EXECUTION_REQUIREMENTS = "COPY_EXECUTION_REQUIREMENTS")
load(":directory_path.bzl", "DirectoryPathInfo")

_COREUTILS_TOOLCHAIN = "@aspect_bazel_lib//lib:coreutils_toolchain_type"
Expand Down Expand Up @@ -89,7 +83,7 @@ def copy_file_action(ctx, src, dst, dir_path = None):
inputs = [src],
outputs = [dst],
mnemonic = "CopyFile",
progress_message = "Copying file %s" % _progress_path(src),
progress_message = "Copying file %{input}",
execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
)

Expand Down Expand Up @@ -152,7 +146,7 @@ def copy_file(name, src, out, is_executable = False, allow_symlink = False, **kw
`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.
This rule uses a Bash command on Linux/macOS/non-Windows, and a cmd.exe command on Windows (no Bash is required).
This rule uses a hermetic uutils/coreutils `cp` binary, no shell is required.
If using this rule with source directories, it is recommended that you use the
`--host_jvm_args=-DBAZEL_TRACK_SOURCE_DIRECTORIES=1` startup option so that changes
Expand All @@ -178,9 +172,7 @@ def copy_file(name, src, out, is_executable = False, allow_symlink = False, **kw
**kwargs: further keyword arguments, e.g. `visibility`
"""

copy_file_impl = _copy_file
if is_executable:
copy_file_impl = _copy_xfile
copy_file_impl = _copy_xfile if is_executable else _copy_file

copy_file_impl(
name = name,
Expand Down
4 changes: 2 additions & 2 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", _COPY_EXECUTION_REQUIREMENTS = "COPY_EXECUTION_REQUIREMENTS")
load(":directory_path.bzl", "DirectoryPathInfo")
load(":paths.bzl", "paths")

Expand Down Expand Up @@ -497,7 +497,7 @@ def copy_to_directory_bin_action(
executable = copy_to_directory_bin,
arguments = [config_file.path, ctx.label.workspace_name],
mnemonic = "CopyToDirectory",
progress_message = "Copying files to directory %s" % _progress_path(dst),
progress_message = "Copying files to directory %{output}",
execution_requirements = _COPY_EXECUTION_REQUIREMENTS,
)

Expand Down

0 comments on commit 4c1267f

Please sign in to comment.