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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 3 commits into from
Sep 10, 2024
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
594926b
Cleanup copy_file.bzl
dzbarsky Sep 7, 2024
4010cfa
Fix the directory copies as well
dzbarsky Sep 7, 2024
456d4f1
Update docs
dzbarsky Sep 10, 2024
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
12 changes: 0 additions & 12 deletions lib/private/copy_common.bzl
View file
Open in desktop
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("../")
4 changes: 2 additions & 2 deletions lib/private/copy_directory.bzl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ This rule copies a directory to another location using Bash (on Linux/macOS) or
cmd.exe (on Windows).
"""

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 +60,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
20 changes: 6 additions & 14 deletions lib/private/copy_file.bzl
View file
Open in desktop
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
View file
Open in desktop
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
Loading