From 843d8e48f763d4a9efce49d260c1e65529985586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A1szl=C3=B3=20Csomor?= Date: Tue, 9 Apr 2019 01:43:49 +0200 Subject: [PATCH] Replace genrules with copy_file/write_file rules (#658) The new rules are in bazel_skylib 0.8.0. Their advantages over genrule are: - cleaner rule interface - no Bash required on Windows - compatible with Bazel's Bash-less test execution on Windows See https://github.com/bazelbuild/bazel/issues/6622 --- internal/e2e/node/BUILD.bazel | 23 ++-- internal/npm_package/test/BUILD.bazel | 7 +- package.json | 2 +- .../bazel-skylib/rules/copy_file.bzl | 29 ++++ .../rules/private/copy_file_private.bzl | 130 ++++++++++++++++++ .../rules/private/write_file_private.bzl | 85 ++++++++++++ .../bazel-skylib/rules/write_file.bzl | 30 ++++ 7 files changed, 290 insertions(+), 16 deletions(-) create mode 100644 third_party/github.com/bazelbuild/bazel-skylib/rules/copy_file.bzl create mode 100644 third_party/github.com/bazelbuild/bazel-skylib/rules/private/copy_file_private.bzl create mode 100644 third_party/github.com/bazelbuild/bazel-skylib/rules/private/write_file_private.bzl create mode 100644 third_party/github.com/bazelbuild/bazel-skylib/rules/write_file.bzl diff --git a/internal/e2e/node/BUILD.bazel b/internal/e2e/node/BUILD.bazel index 7366443b13..db8e0f347f 100644 --- a/internal/e2e/node/BUILD.bazel +++ b/internal/e2e/node/BUILD.bazel @@ -1,4 +1,6 @@ load("@build_bazel_rules_nodejs//:defs.bzl", "jasmine_node_test") +load("//third_party/github.com/bazelbuild/bazel-skylib:rules/copy_file.bzl", "copy_file") +load("//third_party/github.com/bazelbuild/bazel-skylib:rules/write_file.bzl", "write_file") jasmine_node_test( name = "test", @@ -52,26 +54,23 @@ jasmine_node_test( ], ) -genrule( +copy_file( name = "module_resolution_lib", - srcs = ["module_resolution.spec.js"], - outs = ["module_resolution_built.spec.js"], - cmd = "cp $< $@", + src = "module_resolution.spec.js", + out = "module_resolution_built.spec.js", ) -genrule( +copy_file( name = "data_resolution_lib", - srcs = ["data_resolution.spec.js"], - outs = ["data_resolution_built.spec.js"], - cmd = "cp $< $@", + src = "data_resolution.spec.js", + out = "data_resolution_built.spec.js", ) -# This genrule creates a file that alphabetically comes before any source file in this +# This rule creates a file that alphabetically comes before any source file in this # package. This genfile can be then set up as runfile to verify that the "node_loader.js" # properly determines the local workspace path without incorrectly using the genfile as base # for the local workspace path. See: https://github.com/bazelbuild/rules_nodejs/issues/352 -genrule( +write_file( name = "genfile-runfile", - outs = ["#LEADING.txt"], - cmd = "touch $@", + out = "#LEADING.txt", ) diff --git a/internal/npm_package/test/BUILD.bazel b/internal/npm_package/test/BUILD.bazel index a48d901b15..3de7e9e414 100644 --- a/internal/npm_package/test/BUILD.bazel +++ b/internal/npm_package/test/BUILD.bazel @@ -1,10 +1,11 @@ load("//:defs.bzl", "jasmine_node_test", "npm_package") load("//internal/common:typescript_mock_lib.bzl", "mock_typescript_lib") +load("//third_party/github.com/bazelbuild/bazel-skylib:rules/write_file.bzl", "write_file") -genrule( +write_file( name = "produces_files", - outs = ["a_dep"], - cmd = "echo \"a_dep content\" > $@", + out = "a_dep", + content = ["a_dep content"], ) mock_typescript_lib( diff --git a/package.json b/package.json index b01a4d1190..9eef16a392 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ "test_legacy_e2e": "./scripts/test_legacy_e2e.sh", "test_packages_all": "./scripts/test_packages_all.sh", "test_packages": "./scripts/test_packages.sh", - "bazel:format": "find . -type f \\( -name \"*.bzl\" -or -name WORKSPACE -or -name BUILD -or -name BUILD.bazel \\) ! -path \"*/node_modules/*\" ! -path \"./dist/*\" | xargs buildifier -v --warnings=args-order,attr-cfg,attr-license,attr-non-empty,attr-output-default,attr-single-file,constant-glob,ctx-actions,ctx-args,depset-iteration,depset-union,dict-concatenation,duplicated-name,filetype,git-repository,http-archive,integer-division,load,load-on-top,native-build,native-package,out-of-order-load,output-group,package-name,package-on-top,positional-args,redefined-variable,repository-name,same-origin-load,string-iteration,unsorted-dict-items,unused-variable", + "bazel:format": "find . -type f \\( -name \"*.bzl\" -or -name WORKSPACE -or -name BUILD -or -name BUILD.bazel \\) ! -path \"*/node_modules/*\" ! -path \"./dist/*\" ! -path \"./third_party/*\" | xargs buildifier -v --warnings=args-order,attr-cfg,attr-license,attr-non-empty,attr-output-default,attr-single-file,constant-glob,ctx-actions,ctx-args,depset-iteration,depset-union,dict-concatenation,duplicated-name,filetype,git-repository,http-archive,integer-division,load,load-on-top,native-build,native-package,out-of-order-load,output-group,package-name,package-on-top,positional-args,redefined-variable,repository-name,same-origin-load,string-iteration,unsorted-dict-items,unused-variable", "bazel:lint": "yarn bazel:format --lint=warn", "bazel:lint-fix": "yarn bazel:format --lint=fix", "format": "git-clang-format", diff --git a/third_party/github.com/bazelbuild/bazel-skylib/rules/copy_file.bzl b/third_party/github.com/bazelbuild/bazel-skylib/rules/copy_file.bzl new file mode 100644 index 0000000000..f3f689c698 --- /dev/null +++ b/third_party/github.com/bazelbuild/bazel-skylib/rules/copy_file.bzl @@ -0,0 +1,29 @@ +# Copyright 2019 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""A rule that copies a file to another place. + +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). +""" + +load( + "//third_party/github.com/bazelbuild/bazel-skylib:rules/private/copy_file_private.bzl", + _copy_file = "copy_file", +) + +copy_file = _copy_file diff --git a/third_party/github.com/bazelbuild/bazel-skylib/rules/private/copy_file_private.bzl b/third_party/github.com/bazelbuild/bazel-skylib/rules/private/copy_file_private.bzl new file mode 100644 index 0000000000..4c08d400ae --- /dev/null +++ b/third_party/github.com/bazelbuild/bazel-skylib/rules/private/copy_file_private.bzl @@ -0,0 +1,130 @@ +# Copyright 2019 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""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. +""" + +def _common_impl(ctx, is_executable): + if ctx.attr.is_windows: + # 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. + bat = ctx.actions.declare_file(ctx.label.name + "-cmd.bat") + 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 = "@copy /Y \"%s\" \"%s\" >NUL" % ( + ctx.file.src.path.replace("/", "\\"), + ctx.outputs.out.path.replace("/", "\\"), + ), + is_executable = True, + ) + ctx.actions.run( + inputs = [ctx.file.src, bat], + outputs = [ctx.outputs.out], + executable = "cmd.exe", + arguments = ["/C", bat.path.replace("/", "\\")], + mnemonic = "CopyFile", + progress_message = "Copying files", + use_default_shell_env = True, + ) + else: + ctx.actions.run_shell( + inputs = [ctx.file.src], + outputs = [ctx.outputs.out], + command = "cp -f \"$1\" \"$2\"", + arguments = [ctx.file.src.path, ctx.outputs.out.path], + mnemonic = "CopyFile", + progress_message = "Copying files", + use_default_shell_env = True, + ) + + files = depset(direct = [ctx.outputs.out]) + runfiles = ctx.runfiles(files = [ctx.outputs.out]) + if 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), +} + +_copy_file = rule( + implementation = _impl, + provides = [DefaultInfo], + attrs = _ATTRS, +) + +_copy_xfile = rule( + implementation = _ximpl, + executable = True, + provides = [DefaultInfo], + attrs = _ATTRS, +) + +def copy_file(name, src, out, is_executable = 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. + + This rule uses a Bash command on Linux/macOS/non-Windows, and a cmd.exe command on Windows (no Bash is required). + + Args: + name: Name of the rule. + src: A Label. The file to make a copy of. (Can also be the label of a rule + that generates a file.) + out: Path of the output file, relative to this package. + 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. + **kwargs: further keyword arguments, e.g. `visibility` + """ + 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 + ) diff --git a/third_party/github.com/bazelbuild/bazel-skylib/rules/private/write_file_private.bzl b/third_party/github.com/bazelbuild/bazel-skylib/rules/private/write_file_private.bzl new file mode 100644 index 0000000000..26578b3394 --- /dev/null +++ b/third_party/github.com/bazelbuild/bazel-skylib/rules/private/write_file_private.bzl @@ -0,0 +1,85 @@ +# Copyright 2019 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Implementation of write_file macro and underlying rules. + +These rules write a UTF-8 encoded text file, using Bazel's FileWriteAction. +'_write_xfile' marks the resulting file executable, '_write_file' does not. +""" + +def _common_impl(ctx, is_executable): + # ctx.actions.write creates a FileWriteAction which uses UTF-8 encoding. + ctx.actions.write( + output = ctx.outputs.out, + content = "\n".join(ctx.attr.content) if ctx.attr.content else "", + is_executable = is_executable, + ) + files = depset(direct = [ctx.outputs.out]) + runfiles = ctx.runfiles(files = [ctx.outputs.out]) + if 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 = { + "content": attr.string_list(mandatory = False, allow_empty = True), + "out": attr.output(mandatory = True), +} + +_write_file = rule( + implementation = _impl, + provides = [DefaultInfo], + attrs = _ATTRS, +) + +_write_xfile = rule( + implementation = _ximpl, + executable = True, + provides = [DefaultInfo], + attrs = _ATTRS, +) + +def write_file(name, out, content = [], is_executable = False, **kwargs): + """Creates a UTF-8 encoded text file. + + Args: + name: Name of the rule. + out: Path of the output file, relative to this package. + content: A list of strings. Lines of text, the contents of the file. + Newlines are added automatically after every line except the last one. + 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. + **kwargs: further keyword arguments, e.g. `visibility` + """ + if is_executable: + _write_xfile( + name = name, + content = content, + out = out, + **kwargs + ) + else: + _write_file( + name = name, + content = content, + out = out, + **kwargs + ) diff --git a/third_party/github.com/bazelbuild/bazel-skylib/rules/write_file.bzl b/third_party/github.com/bazelbuild/bazel-skylib/rules/write_file.bzl new file mode 100644 index 0000000000..602f78d2f6 --- /dev/null +++ b/third_party/github.com/bazelbuild/bazel-skylib/rules/write_file.bzl @@ -0,0 +1,30 @@ +# Copyright 2019 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""A rule that writes a UTF-8 encoded text file from user-specified contents. + +native.genrule() is sometimes used to create a text file. The 'write_file' and +macro does this with a simpler interface than genrule. + +The rules generated by the macro do not use Bash or any other shell to write the +file. Instead they use Starlark's built-in file writing action +(ctx.actions.write). +""" + +load( + "//third_party/github.com/bazelbuild/bazel-skylib:rules/private/write_file_private.bzl", + _write_file = "write_file", +) + +write_file = _write_file