From 8f3151fb4a91d5f2ae4cad5901ea72fe30a2aba0 Mon Sep 17 00:00:00 2001 From: Yannic Date: Fri, 10 Jul 2020 20:08:02 +0200 Subject: [PATCH] copy_file: Add parameter to allow symlinks (#252) * copy_file: Add parameter to allow symlinks This change adds a new parameter `allow_symlinks` to `copy_file` that allows the action to create a symlink instead of doing an expensive copy if the execution platform (host) allows it. Updates #248 * Update docs * Refactor `is_executable` into attribute * Fix typo * s/_impl/_copy_file_impl/ --- docs/copy_file_doc.md | 21 +++++++- rules/private/copy_file_private.bzl | 75 +++++++++++++++-------------- tests/copy_file/BUILD | 56 ++++++++++++++++++++- tests/copy_file/a_with_exec_bit.txt | 2 + tests/copy_file/copy_file_tests.sh | 22 +++++++++ 5 files changed, 138 insertions(+), 38 deletions(-) create mode 100755 tests/copy_file/a_with_exec_bit.txt diff --git a/docs/copy_file_doc.md b/docs/copy_file_doc.md index e2b6025..67148bc 100755 --- a/docs/copy_file_doc.md +++ b/docs/copy_file_doc.md @@ -1,7 +1,11 @@ + + + + ## copy_file 
-copy_file(name, src, out, is_executable, kwargs)
+copy_file(name, src, out, is_executable, allow_symlink, kwargs)
Copies a file to another location. @@ -55,6 +59,21 @@ This rule uses a Bash command on Linux/macOS/non-Windows, and a cmd.exe command 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. + WARNING: If `allow_symlink` is True, `src` must also be executable. +

+ + + + allow_symlink + + optional. default is False +

+ 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).

diff --git a/rules/private/copy_file_private.bzl b/rules/private/copy_file_private.bzl index 7549585..75cb027 100644 --- a/rules/private/copy_file_private.bzl +++ b/rules/private/copy_file_private.bzl @@ -58,45 +58,48 @@ def copy_bash(ctx, src, dst): use_default_shell_env = True, ) -def _common_impl(ctx, is_executable): - if ctx.attr.is_windows: - copy_cmd(ctx, ctx.file.src, ctx.outputs.out) +def _copy_file_impl(ctx): + if ctx.attr.allow_symlink: + ctx.actions.symlink( + output = ctx.outputs.out, + target_file = ctx.file.src, + is_executable = ctx.attr.is_executable, + ) else: - copy_bash(ctx, ctx.file.src, ctx.outputs.out) + if ctx.attr.is_windows: + copy_cmd(ctx, ctx.file.src, ctx.outputs.out) + else: + copy_bash(ctx, ctx.file.src, ctx.outputs.out) files = depset(direct = [ctx.outputs.out]) runfiles = ctx.runfiles(files = [ctx.outputs.out]) - if is_executable: + if ctx.attr.is_executable: return [DefaultInfo(files = files, runfiles = runfiles, executable = ctx.outputs.out)] else: return [DefaultInfo(files = files, runfiles = runfiles)] -def _impl(ctx): - return _common_impl(ctx, False) - -def _ximpl(ctx): - return _common_impl(ctx, True) - _ATTRS = { "src": attr.label(mandatory = True, allow_single_file = True), "out": attr.output(mandatory = True), "is_windows": attr.bool(mandatory = True), + "is_executable": attr.bool(mandatory = True), + "allow_symlink": attr.bool(mandatory = True), } _copy_file = rule( - implementation = _impl, + implementation = _copy_file_impl, provides = [DefaultInfo], attrs = _ATTRS, ) _copy_xfile = rule( - implementation = _ximpl, + implementation = _copy_file_impl, executable = True, provides = [DefaultInfo], attrs = _ATTRS, ) -def copy_file(name, src, out, is_executable = False, **kwargs): +def copy_file(name, src, out, is_executable = False, allow_symlink = False, **kwargs): """Copies a file 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. @@ -111,27 +114,29 @@ def copy_file(name, src, out, is_executable = False, **kwargs): 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. + WARNING: If `allow_symlink` is True, `src` must also be executable. + 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). **kwargs: further keyword arguments, e.g. `visibility` """ + + copy_file_impl = _copy_file if is_executable: - _copy_xfile( - name = name, - src = src, - out = out, - is_windows = select({ - "@bazel_tools//src/conditions:host_windows": True, - "//conditions:default": False, - }), - **kwargs - ) - else: - _copy_file( - name = name, - src = src, - out = out, - is_windows = select({ - "@bazel_tools//src/conditions:host_windows": True, - "//conditions:default": False, - }), - **kwargs - ) + copy_file_impl = _copy_xfile + + copy_file_impl( + name = name, + src = src, + out = out, + is_windows = select({ + "@bazel_tools//src/conditions:host_windows": True, + "//conditions:default": False, + }), + is_executable = is_executable, + allow_symlink = allow_symlink, + **kwargs + ) diff --git a/tests/copy_file/BUILD b/tests/copy_file/BUILD index 7c8a4c3..79ab7ba 100644 --- a/tests/copy_file/BUILD +++ b/tests/copy_file/BUILD @@ -46,6 +46,7 @@ sh_test( ":file_deps", # Use DefaultInfo.runfiles from 'copy_gen'. ":copy_gen", + ":copy_gen_symlink", "//tests:unittest.bash", ], deps = ["@bazel_tools//tools/bash/runfiles"], @@ -56,6 +57,7 @@ filegroup( # Use DefaultInfo.files from 'copy_src'. srcs = [ ":copy_src", + ":copy_src_symlink", ], ) @@ -64,15 +66,23 @@ filegroup( genrule( name = "run_executables", outs = [ + "xsrc-out-symlink.txt", + "xgen-out-symlink.txt", "xsrc-out.txt", "xgen-out.txt", ], - cmd = ("$(location :bin_src) > $(location xsrc-out.txt) && " + - "$(location :bin_gen) > $(location xgen-out.txt)"), + cmd = " && ".join([ + "$(location :bin_src_symlink) > $(location xsrc-out-symlink.txt)", + "$(location :bin_gen_symlink) > $(location xgen-out-symlink.txt)", + "$(location :bin_src) > $(location xsrc-out.txt)", + "$(location :bin_gen) > $(location xgen-out.txt)", + ]), output_to_bindir = 1, tools = [ ":bin_gen", ":bin_src", + ":bin_gen_symlink", + ":bin_src_symlink", ], ) @@ -82,22 +92,48 @@ sh_binary( srcs = [":copy_xsrc"], ) +# If 'bin_src' is built, then 'copy_xsrc' made its output executable. +sh_binary( + name = "bin_src_symlink", + srcs = [":copy_xsrc_symlink"], +) + # If 'bin_gen' is built, then 'copy_xgen' made its output executable. sh_binary( name = "bin_gen", srcs = [":copy_xgen"], ) +# If 'bin_gen' is built, then 'copy_xgen' made its output executable. +sh_binary( + name = "bin_gen_symlink", + srcs = [":copy_xgen_symlink"], +) + copy_file( name = "copy_src", src = "a.txt", out = "out/a-out.txt", ) +copy_file( + name = "copy_src_symlink", + src = "a.txt", + out = "out/a-out-symlink.txt", + allow_symlink = True, +) + copy_file( name = "copy_gen", src = ":gen", out = "out/gen-out.txt", + allow_symlink = True, +) + +copy_file( + name = "copy_gen_symlink", + src = ":gen", + out = "out/gen-out-symlink.txt", ) copy_file( @@ -107,6 +143,14 @@ copy_file( is_executable = True, ) +copy_file( + name = "copy_xsrc_symlink", + src = "a_with_exec_bit.txt", + out = "xout/a-out-symlink.sh", + is_executable = True, + allow_symlink = True, +) + copy_file( name = "copy_xgen", src = ":gen", @@ -114,6 +158,14 @@ copy_file( is_executable = True, ) +copy_file( + name = "copy_xgen_symlink", + src = ":gen", + out = "xout/gen-out-symlink.sh", + is_executable = True, + allow_symlink = True, +) + genrule( name = "gen", outs = ["b.txt"], diff --git a/tests/copy_file/a_with_exec_bit.txt b/tests/copy_file/a_with_exec_bit.txt new file mode 100755 index 0000000..acd332a --- /dev/null +++ b/tests/copy_file/a_with_exec_bit.txt @@ -0,0 +1,2 @@ +#!/bin/bash +echo aaa diff --git a/tests/copy_file/copy_file_tests.sh b/tests/copy_file/copy_file_tests.sh index 23f45d5..dfee635 100755 --- a/tests/copy_file/copy_file_tests.sh +++ b/tests/copy_file/copy_file_tests.sh @@ -44,20 +44,42 @@ function test_copy_src() { expect_log '^echo aaa$' } +function test_copy_src_symlink() { + cat "$(rlocation bazel_skylib/tests/copy_file/out/a-out-symlink.txt)" >"$TEST_log" + expect_log '^#!/bin/bash$' + expect_log '^echo aaa$' +} + function test_copy_gen() { cat "$(rlocation bazel_skylib/tests/copy_file/out/gen-out.txt)" >"$TEST_log" expect_log '^#!/bin/bash$' expect_log '^echo potato$' } +function test_copy_gen_symlink() { + cat "$(rlocation bazel_skylib/tests/copy_file/out/gen-out-symlink.txt)" >"$TEST_log" + expect_log '^#!/bin/bash$' + expect_log '^echo potato$' +} + function test_copy_xsrc() { cat "$(rlocation bazel_skylib/tests/copy_file/xsrc-out.txt)" >"$TEST_log" expect_log '^aaa$' } +function test_copy_xsrc_symlink() { + cat "$(rlocation bazel_skylib/tests/copy_file/xsrc-out-symlink.txt)" >"$TEST_log" + expect_log '^aaa$' +} + function test_copy_xgen() { cat "$(rlocation bazel_skylib/tests/copy_file/xgen-out.txt)" >"$TEST_log" expect_log '^potato$' } +function test_copy_xgen_symlink() { + cat "$(rlocation bazel_skylib/tests/copy_file/xgen-out-symlink.txt)" >"$TEST_log" + expect_log '^potato$' +} + run_suite "copy_file_tests test suite"