Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: Improve copy_file.bzl's progress_message and do some cleanup #931

Merged
merged 3 commits into from
Sep 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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://github.com/bazelbuild/bazel-skylib/blob/7b859037a673db6f606661323e74c5d4751595e6/rules/private/copy_file_private.bzl
# https://github.com/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