From 3b93ee0baa464bfc0898bb84e87e71da0b596f11 Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Mon, 14 Mar 2022 17:33:52 -0700 Subject: [PATCH] Add DefaultOutputPathInfo provider and update write_source_files to accept it (#48) Also update write_source_files to accept DirectoryPathInfo --- .prettierignore | 6 +- docs/BUILD.bazel | 7 +- docs/default_info_files.md | 49 +++++ docs/directory_path.md | 2 +- docs/write_source_files.md | 4 +- lib/BUILD.bazel | 18 +- lib/default_info_files.bzl | 11 ++ lib/directory_path.bzl | 14 -- lib/private/BUILD.bazel | 24 ++- lib/private/copy_file.bzl | 2 +- lib/private/default_info_files.bzl | 70 +++++++ lib/private/diff_test.bzl | 34 +++- lib/private/directory_path.bzl | 3 +- lib/private/write_source_file.bzl | 186 ++++++++++++++++++ lib/private/write_source_files.bzl | 161 --------------- lib/tests/default_output_gen.bzl | 30 +++ lib/tests/write_source_files/BUILD.bazel | 81 ++++++-- lib/tests/write_source_files/a2.js | 2 +- lib/tests/write_source_files/b2.js | 2 +- .../write_source_files/e2_dir/e-contained.js | 1 + .../write_source_files/e_dir/e-contained.js | 1 + lib/tests/write_source_files/e_dir/e.js | 1 - lib/tests/write_source_files/f.js | 1 + lib/tests/write_source_files/f2.js | 1 + .../write_source_files/subdir/BUILD.bazel | 8 +- .../subdir/subsubdir/BUILD.bazel | 2 +- ...es_test.bzl => write_source_file_test.bzl} | 133 +++++++------ lib/write_source_files.bzl | 87 ++++---- 28 files changed, 622 insertions(+), 319 deletions(-) create mode 100644 docs/default_info_files.md create mode 100644 lib/default_info_files.bzl create mode 100644 lib/private/default_info_files.bzl create mode 100644 lib/private/write_source_file.bzl delete mode 100644 lib/private/write_source_files.bzl create mode 100644 lib/tests/default_output_gen.bzl create mode 100644 lib/tests/write_source_files/e2_dir/e-contained.js create mode 100644 lib/tests/write_source_files/e_dir/e-contained.js delete mode 100644 lib/tests/write_source_files/e_dir/e.js create mode 100644 lib/tests/write_source_files/f.js create mode 100644 lib/tests/write_source_files/f2.js rename lib/tests/write_source_files/{write_source_files_test.bzl => write_source_file_test.bzl} (57%) diff --git a/.prettierignore b/.prettierignore index 5daed7cec..e0dc6af75 100644 --- a/.prettierignore +++ b/.prettierignore @@ -1,5 +1,5 @@ docs/*.md lib/tests/jq/*.json -lib/tests/write_source_files/a2.js -lib/tests/write_source_files/b2.js -lib/tests/write_source_files/e_dir/e.js \ No newline at end of file +lib/lib/tests/write_source_files/*.js +lib/lib/tests/write_source_files/subdir/*.js +lib/lib/tests/write_source_files/subdir/subsubdir/*.js \ No newline at end of file diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel index 16f17002e..dca1bd950 100644 --- a/docs/BUILD.bazel +++ b/docs/BUILD.bazel @@ -48,6 +48,9 @@ stardoc_with_diff_test( bzl_library_target = "//lib:directory_path", ) -update_docs( - name = "update", +stardoc_with_diff_test( + name = "default_info_files", + bzl_library_target = "//lib:default_info_files", ) + +update_docs() diff --git a/docs/default_info_files.md b/docs/default_info_files.md new file mode 100644 index 000000000..20d30814a --- /dev/null +++ b/docs/default_info_files.md @@ -0,0 +1,49 @@ + + +A rule that provides file(s) from a given target's DefaultInfo + + + + +## default_info_files + +
+default_info_files(name, paths, target)
+
+ +A rule that provides file(s) from a given target's DefaultInfo + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | +| paths | the paths of the files to provide in the DefaultInfo of the target relative to its root | List of strings | required | | +| target | the target to look in for requested paths in its' DefaultInfo | Label | required | | + + + + +## make_default_info_files + +
+make_default_info_files(name, target, paths)
+
+ +Helper function to generate a default_info_files target and return its label. + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| name | unique name for the generated default_info_files target. | none | +| target | the target to look in for requested paths in its' DefaultInfo | none | +| paths | the paths of the files to provide in the DefaultInfo of the target relative to its root | none | + +**RETURNS** + +The label `name` + + diff --git a/docs/directory_path.md b/docs/directory_path.md index 9ffbb66ad..6e6b484a2 100644 --- a/docs/directory_path.md +++ b/docs/directory_path.md @@ -53,7 +53,7 @@ Joins a label pointing to a TreeArtifact with a path nested within that director make_directory_path(name, directory, path) -Helper function to convert generate a directory_path target and return its label. +Helper function to generate a directory_path target and return its label. **PARAMETERS** diff --git a/docs/write_source_files.md b/docs/write_source_files.md index 79df273f6..0ee9d7c1c 100644 --- a/docs/write_source_files.md +++ b/docs/write_source_files.md @@ -87,8 +87,8 @@ If you have many sources that you want to update as a group, we recommend wrappi | :------------- | :------------- | :------------- | | name | Name of the executable target that creates or updates the source file | none | | files | A dict where the keys are source files or folders to write to and the values are labels pointing to the desired content. Sources must be within the same bazel package as the target. | {} | -| additional_update_targets | (Optional) List of other write_source_files targets to update in the same run | [] | -| suggested_update_target | (Optional) Label of the write_source_files target to suggest running when files are out of date | None | +| additional_update_targets | (Optional) List of other write_source_file or other executable updater targets to call in the same run | [] | +| suggested_update_target | (Optional) Label of the write_source_file target to suggest running when files are out of date | None | | kwargs | Other common named parameters such as tags or visibility | none | diff --git a/lib/BUILD.bazel b/lib/BUILD.bazel index 946880ca8..150115dbf 100644 --- a/lib/BUILD.bazel +++ b/lib/BUILD.bazel @@ -63,6 +63,13 @@ bzl_library( deps = ["//lib/private:directory_path"], ) +bzl_library( + name = "default_info_files", + srcs = ["default_info_files.bzl"], + visibility = ["//visibility:public"], + deps = ["//lib/private:default_info_files"], +) + bzl_library( name = "copy_to_directory", srcs = ["copy_to_directory.bzl"], @@ -80,9 +87,16 @@ bzl_library( srcs = ["write_source_files.bzl"], visibility = ["//visibility:public"], deps = [ + ":diff_test", ":utils", "//lib/private:fail_with_message_test", - "//lib/private:write_source_files", - "@bazel_skylib//rules:diff_test", + "//lib/private:write_source_file", ], ) + +bzl_library( + name = "diff_test", + srcs = ["diff_test.bzl"], + visibility = ["//visibility:public"], + deps = ["//lib/private:diff_test"], +) diff --git a/lib/default_info_files.bzl b/lib/default_info_files.bzl new file mode 100644 index 000000000..5eb9a1ad8 --- /dev/null +++ b/lib/default_info_files.bzl @@ -0,0 +1,11 @@ +"""A rule that provides file(s) from a given target's DefaultInfo +""" + +load( + "//lib/private:default_info_files.bzl", + _default_info_files = "default_info_files", + _make_default_info_files = "make_default_info_files", +) + +default_info_files = _default_info_files +make_default_info_files = _make_default_info_files diff --git a/lib/directory_path.bzl b/lib/directory_path.bzl index be137afe8..8487e9440 100644 --- a/lib/directory_path.bzl +++ b/lib/directory_path.bzl @@ -1,17 +1,3 @@ -# 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. - """Rule and corresponding provider that joins a label pointing to a TreeArtifact with a path nested within that directory """ diff --git a/lib/private/BUILD.bazel b/lib/private/BUILD.bazel index 8c9f6aa07..66436f097 100644 --- a/lib/private/BUILD.bazel +++ b/lib/private/BUILD.bazel @@ -10,6 +10,7 @@ bzl_library( srcs = ["copy_to_directory.bzl"], visibility = ["//lib:__subpackages__"], deps = [ + ":default_info_files", ":directory_path", ":paths", "@bazel_skylib//lib:paths", @@ -61,10 +62,14 @@ bzl_library( ) bzl_library( - name = "write_source_files", - srcs = ["write_source_files.bzl"], + name = "write_source_file", + srcs = ["write_source_file.bzl"], visibility = ["//lib:__subpackages__"], - deps = ["//lib:utils"], + deps = [ + ":default_info_files", + ":directory_path", + "//lib:utils", + ], ) bzl_library( @@ -79,3 +84,16 @@ bzl_library( visibility = ["//lib:__subpackages__"], deps = ["//lib:utils"], ) + +bzl_library( + name = "default_info_files", + srcs = ["default_info_files.bzl"], + visibility = ["//lib:__subpackages__"], + deps = ["//lib:utils"], +) + +bzl_library( + name = "diff_test", + srcs = ["diff_test.bzl"], + visibility = ["//lib:__subpackages__"], +) diff --git a/lib/private/copy_file.bzl b/lib/private/copy_file.bzl index b098fade6..886e80d2b 100644 --- a/lib/private/copy_file.bzl +++ b/lib/private/copy_file.bzl @@ -127,7 +127,7 @@ def _copy_file_impl(ctx): src_path = "/".join([src_file.path, ctx.attr.src[DirectoryPathInfo].path]) else: if len(ctx.files.src) != 1: - fail("src must be a single file or a target with a DirectoryPathInfo provider") + fail("src must be a single file or a target that provides a DirectoryPathInfo") src_file = ctx.files.src[0] src_path = src_file.path if ctx.attr.is_windows: diff --git a/lib/private/default_info_files.bzl b/lib/private/default_info_files.bzl new file mode 100644 index 000000000..849804d27 --- /dev/null +++ b/lib/private/default_info_files.bzl @@ -0,0 +1,70 @@ +"""default_info_files implementation +""" + +load("//lib:utils.bzl", _to_label = "to_label") + +def _default_info_files(ctx): + files = [] + for path in ctx.attr.paths: + file = find_short_path_in_default_info( + ctx.attr.target, + path, + ) + if not file: + fail("%s file not found within the DefaultInfo of %s" % (ctx.attr.path, ctx.attr.target)) + files.append(file) + return [DefaultInfo( + files = depset(direct = files), + runfiles = ctx.runfiles(files = files), + )] + +default_info_files = rule( + doc = "A rule that provides file(s) from a given target's DefaultInfo", + implementation = _default_info_files, + attrs = { + "target": attr.label( + doc = "the target to look in for requested paths in its' DefaultInfo", + mandatory = True, + ), + "paths": attr.string_list( + doc = "the paths of the files to provide in the DefaultInfo of the target relative to its root", + mandatory = True, + allow_empty = False, + ), + }, + provides = [DefaultInfo], +) + +def make_default_info_files(name, target, paths): + """Helper function to generate a default_info_files target and return its label. + + Args: + name: unique name for the generated `default_info_files` target. + target: the target to look in for requested paths in its' DefaultInfo + paths: the paths of the files to provide in the DefaultInfo of the target relative to its root + + Returns: + The label `name` + """ + default_info_files( + name = name, + target = target, + paths = paths, + ) + return _to_label(name) + +def find_short_path_in_default_info(default_info, short_path): + """Helper function find a file in a DefaultInfo by short path + + Args: + default_info: a DefaultInfo + short_path: the short path (path relative to root) to search for + + Returns: + The File if found else None + """ + if default_info.files: + for file in default_info.files.to_list(): + if file.short_path == short_path: + return file + return None diff --git a/lib/private/diff_test.bzl b/lib/private/diff_test.bzl index 0e5410f5c..e239607e0 100644 --- a/lib/private/diff_test.bzl +++ b/lib/private/diff_test.bzl @@ -18,6 +18,8 @@ The rule uses a Bash command (diff) on Linux/macOS/non-Windows, and a cmd.exe command (fc.exe) on Windows (no Bash is required). """ +load(":directory_path.bzl", "DirectoryPathInfo") + def _runfiles_path(f): if f.root.path: return f.path[len(f.root.path) + 1:] # generated file @@ -25,6 +27,24 @@ def _runfiles_path(f): return f.path # source file def _diff_test_impl(ctx): + if DirectoryPathInfo in ctx.attr.file1: + file1 = ctx.attr.file1[DirectoryPathInfo].directory + file1_path = "/".join([_runfiles_path(file1), ctx.attr.file1[DirectoryPathInfo].path]) + else: + if len(ctx.files.file1) != 1: + fail("file1 must be a single file or a target that provides a DirectoryPathInfo") + file1 = ctx.files.file1[0] + file1_path = _runfiles_path(file1) + + if DirectoryPathInfo in ctx.attr.file2: + file2 = ctx.attr.file2[DirectoryPathInfo].directory + file2_path = "/".join([_runfiles_path(file2), ctx.attr.file2[DirectoryPathInfo].path]) + else: + if len(ctx.files.file2) != 1: + fail("file2 must be a single file or a target that provides a DirectoryPathInfo") + file2 = ctx.files.file2[0] + file2_path = _runfiles_path(file2) + if ctx.attr.is_windows: test_bin = ctx.actions.declare_file(ctx.label.name + "-test.bat") ctx.actions.write( @@ -138,8 +158,8 @@ exit /b 0 exit /b 1 """.format( fail_msg = ctx.attr.failure_message, - file1 = _runfiles_path(ctx.file.file1), - file2 = _runfiles_path(ctx.file.file2), + file1 = file1_path, + file2 = file2_path, ), is_executable = True, ) @@ -191,26 +211,26 @@ else fi """.format( fail_msg = ctx.attr.failure_message, - file1 = _runfiles_path(ctx.file.file1), - file2 = _runfiles_path(ctx.file.file2), + file1 = file1_path, + file2 = file2_path, ), is_executable = True, ) return DefaultInfo( executable = test_bin, files = depset(direct = [test_bin]), - runfiles = ctx.runfiles(files = [test_bin, ctx.file.file1, ctx.file.file2]), + runfiles = ctx.runfiles(files = [test_bin, file1, file2]), ) _diff_test = rule( attrs = { "failure_message": attr.string(), "file1": attr.label( - allow_single_file = True, + allow_files = True, mandatory = True, ), "file2": attr.label( - allow_single_file = True, + allow_files = True, mandatory = True, ), "is_windows": attr.bool(mandatory = True), diff --git a/lib/private/directory_path.bzl b/lib/private/directory_path.bzl index 1712c0f71..6ba7c9b8d 100644 --- a/lib/private/directory_path.bzl +++ b/lib/private/directory_path.bzl @@ -33,10 +33,11 @@ Otherwise there is no way to give a Bazel label for it.""", mandatory = True, ), }, + provides = [DirectoryPathInfo], ) def make_directory_path(name, directory, path): - """Helper function to convert generate a directory_path target and return its label. + """Helper function to generate a directory_path target and return its label. Args: name: Unique name for the generated `directory_path` target. diff --git a/lib/private/write_source_file.bzl b/lib/private/write_source_file.bzl new file mode 100644 index 000000000..58ac2c3f4 --- /dev/null +++ b/lib/private/write_source_file.bzl @@ -0,0 +1,186 @@ +"write_source_file implementation" + +load("//lib:utils.bzl", "is_external_label") +load(":directory_path.bzl", "DirectoryPathInfo") + +_write_source_file_attrs = { + "in_file": attr.label(allow_files = True, mandatory = False), + "out_file": attr.label(allow_files = True, mandatory = False), + "additional_update_targets": attr.label_list(cfg = "host", mandatory = False), + "is_windows": attr.bool(mandatory = True), +} + +def _write_source_file_sh(ctx, paths): + updater = ctx.actions.declare_file( + ctx.label.name + "_update.sh", + ) + + additional_update_scripts = [] + for target in ctx.attr.additional_update_targets: + if target[DefaultInfo].files_to_run and target[DefaultInfo].files_to_run.executable: + additional_update_scripts.append(target[DefaultInfo].files_to_run.executable) + else: + fail("additional_update_targets target %s does not provide an executable") + + contents = ["""#!/usr/bin/env bash +set -o errexit -o nounset -o pipefail +runfiles_dir=$PWD +# BUILD_WORKSPACE_DIRECTORY not set when running as a test, uses the sandbox instead +if [[ ! -z "${BUILD_WORKSPACE_DIRECTORY:-}" ]]; then + cd "$BUILD_WORKSPACE_DIRECTORY" +fi"""] + + for in_path, out_path in paths: + contents.append(""" +in=$runfiles_dir/{in_path} +out={out_path} + +mkdir -p "$(dirname "$out")" +echo "Copying $in to $out in $PWD" + +if [[ -f "$in" ]]; then + cp -f "$in" "$out" + chmod 664 "$out" +else + mkdir -p "$out" + cp -rf "$in"/* "$out" + chmod 664 "$out"/* +fi +""".format(in_path = in_path, out_path = out_path)) + + contents.extend([ + "cd \"$runfiles_dir\"", + "# Run the update scripts for all write_source_file deps", + ]) + for update_script in additional_update_scripts: + contents.append("\"{update_script}\"".format(update_script = update_script.short_path)) + + ctx.actions.write( + output = updater, + is_executable = True, + content = "\n".join(contents), + ) + + return updater + +def _write_source_file_bat(ctx, paths): + updater = ctx.actions.declare_file( + ctx.label.name + "_update.bat", + ) + + additional_update_scripts = [] + for target in ctx.attr.additional_update_targets: + if target[DefaultInfo].files_to_run and target[DefaultInfo].files_to_run.executable: + additional_update_scripts.append(target[DefaultInfo].files_to_run.executable) + else: + fail("additional_update_targets target %s does not provide an executable") + + contents = ["""@rem Generated by write_source_file.bzl, do not edit. +@echo off +set runfiles_dir=%cd% +if defined BUILD_WORKSPACE_DIRECTORY ( + cd %BUILD_WORKSPACE_DIRECTORY% +)"""] + + for in_path, out_path in paths: + contents.append(""" +set in=%runfiles_dir%\\{in_path} +set out={out_path} + +if not defined BUILD_WORKSPACE_DIRECTORY ( + @rem Because there's no sandboxing in windows, if we copy over the target + @rem file's symlink it will get copied back into the source directory + @rem during tests. Work around this in tests by deleting the target file + @rem symlink before copying over it. + del %out% +) + +echo Copying %in% to %out% in %cd% + +if exist "%in%\\*" ( + mkdir "%out%" >NUL 2>NUL + robocopy "%in%" "%out%" /E >NUL +) else ( + copy %in% %out% >NUL +) +""".format(in_path = in_path.replace("/", "\\"), out_path = out_path.replace("/", "\\"))) + + contents.extend([ + "cd %runfiles_dir%", + "@rem Run the update scripts for all write_source_file deps", + ]) + for update_script in additional_update_scripts: + contents.append("call {update_script".format(update_script = update_script.short_path)) + + ctx.actions.write( + output = updater, + is_executable = True, + context = "\n".join(contents).replace("\n", "\r\n"), + ) + return updater + +def _write_source_file_impl(ctx): + if ctx.attr.out_file: + if not ctx.attr.in_file: + fail("in_file must be specified if out_file is set") + if is_external_label(ctx.attr.out_file.label): + fail("out file %s must be in the user workspace" % ctx.attr.out_file.label) + if ctx.attr.out_file.label.package != ctx.label.package: + fail("out file %s (in package '%s') must be a source file within the target's package: '%s'" % (ctx.attr.out_file.label, ctx.attr.out_file.label.package, ctx.label.package)) + + if ctx.attr.in_file and not ctx.attr.out_file: + if not ctx.attr.in_file: + fail("out_file must be specified if in_file is set") + + paths = [] + runfiles = [] + + if ctx.attr.in_file and ctx.attr.out_file: + if DirectoryPathInfo in ctx.attr.in_file: + in_path = "/".join([ + ctx.attr.in_file[DirectoryPathInfo].directory.short_path, + ctx.attr.in_file[DirectoryPathInfo].path, + ]) + runfiles.append(ctx.attr.in_file[DirectoryPathInfo].directory) + elif len(ctx.files.in_file) == 0: + fail("in file %s must provide files" % ctx.attr.in_file.label) + elif len(ctx.files.in_file) == 1: + in_path = ctx.files.in_file[0].short_path + else: + fail("in file %s must be a single file or a target that provides DefaultOutputPathInfo or DirectoryPathInfo" % ctx.attr.in_file.label) + + if len(ctx.files.out_file) != 1: + fail("out file %s must be a single file or directory" % ctx.attr.out_file.label) + elif not ctx.files.out_file[0].is_source: + fail("out file %s must be a source file or directory, not a generated file" % ctx.attr.out_file.label) + + out_path = ctx.files.out_file[0].short_path + paths.append((in_path, out_path)) + + if ctx.attr.is_windows: + updater = _write_source_file_bat(ctx, paths) + else: + updater = _write_source_file_sh(ctx, paths) + + runfiles = ctx.runfiles( + files = runfiles, + transitive_files = ctx.attr.in_file.files if ctx.attr.in_file else None, + ) + deps_runfiles = [dep[DefaultInfo].default_runfiles for dep in ctx.attr.additional_update_targets] + if "merge_all" in dir(runfiles): + runfiles = runfiles.merge_all(deps_runfiles) + else: + for dep in deps_runfiles: + runfiles = runfiles.merge(dep) + + return [ + DefaultInfo( + executable = updater, + runfiles = runfiles, + ), + ] + +write_source_file_lib = struct( + attrs = _write_source_file_attrs, + implementation = _write_source_file_impl, +) diff --git a/lib/private/write_source_files.bzl b/lib/private/write_source_files.bzl deleted file mode 100644 index bdc19a2ee..000000000 --- a/lib/private/write_source_files.bzl +++ /dev/null @@ -1,161 +0,0 @@ -"write_source_file implementation" - -load("//lib:utils.bzl", "is_external_label") - -_WriteSourceFilesInfo = provider( - "Provider to enforce deps are other write_source_files targets", - fields = { - "executable": "Generated update script", - }, -) - -_write_source_files_attrs = { - "in_files": attr.label_list(allow_files = True, allow_empty = True, mandatory = False), - "out_files": attr.label_list(allow_files = True, allow_empty = True, mandatory = False), - "additional_update_targets": attr.label_list(allow_files = False, providers = [_WriteSourceFilesInfo], mandatory = False), - "is_windows": attr.bool(mandatory = True), -} - -def _write_source_files_sh(ctx): - updater = ctx.actions.declare_file( - ctx.label.name + "_update.sh", - ) - - additional_update_scripts = [target[_WriteSourceFilesInfo].executable for target in ctx.attr.additional_update_targets] - - ctx.actions.write( - output = updater, - is_executable = True, - content = """ -#!/usr/bin/env bash -set -o errexit -o nounset -o pipefail -runfiles_dir=$PWD -# BUILD_WORKSPACE_DIRECTORY not set when running as a test, uses the sandbox instead -if [[ ! -z "${BUILD_WORKSPACE_DIRECTORY:-}" ]]; then - cd "$BUILD_WORKSPACE_DIRECTORY" -fi -""" + "\n".join([ - """ -in=$runfiles_dir/{in_file} -out={out_file} - -mkdir -p "$(dirname "$out")" -echo "Copying $in to $out in $PWD" - -if [[ -f "$in" ]]; then - cp -f "$in" "$out" - chmod 664 "$out" -else - mkdir -p "$out" - cp -rf "$in"/* "$out" - chmod 664 "$out"/* -fi -""".format(in_file = ctx.files.in_files[i].short_path, out_file = ctx.files.out_files[i].short_path) - for i in range(len(ctx.attr.in_files)) - ]) + """ -cd "$runfiles_dir" - -# Run the update scripts for all write_source_file deps -""" + "\n".join([""" -{update_script} -""".format(update_script = update_script.short_path) for update_script in additional_update_scripts]), - ) - - return updater - -def _write_source_files_bat(ctx): - updater = ctx.actions.declare_file( - ctx.label.name + "_update.bat", - ) - - additional_update_scripts = [target[_WriteSourceFilesInfo].executable for target in ctx.attr.additional_update_targets] - - content = """ -@rem Generated by write_source_files.bzl, do not edit. -@echo off -set runfiles_dir=%cd% -if defined BUILD_WORKSPACE_DIRECTORY ( - cd %BUILD_WORKSPACE_DIRECTORY% -) -""" + "\n".join([ - """ -set in=%runfiles_dir%\\{in_file} -set out={out_file} - -if not defined BUILD_WORKSPACE_DIRECTORY ( - @rem Because there's no sandboxing in windows, if we copy over the target - @rem file's symlink it will get copied back into the source directory - @rem during tests. Work around this in tests by deleting the target file - @rem symlink before copying over it. - del %out% -) - -echo Copying %in% to %out% in %cd% - -if exist "%in%\\*" ( - mkdir "%out%" >NUL 2>NUL - robocopy "%in%" "%out%" /E >NUL -) else ( - copy %in% %out% >NUL -) -""".format(in_file = ctx.files.in_files[i].short_path.replace("/", "\\"), out_file = ctx.files.out_files[i].short_path.replace("/", "\\")) - for i in range(len(ctx.attr.in_files)) - ]) + """ -cd %runfiles_dir% - -@rem Run the update scripts for all write_source_file deps -""" + "\n".join([""" -call {update_script} -""".format(update_script = update_script.short_path) for update_script in additional_update_scripts]) - - content = content.replace("\n", "\r\n") - - ctx.actions.write( - output = updater, - is_executable = True, - content = content, - ) - return updater - -def _write_source_files_impl(ctx): - if (len(ctx.attr.in_files) != len(ctx.attr.out_files)): - fail("in_files and out_files must be the same length") - - for i in range(len(ctx.attr.in_files)): - out_file_label = ctx.attr.out_files[i].label - if is_external_label(out_file_label): - fail("out file %s must be a source file in the user workspace" % out_file_label) - - if not ctx.files.out_files[i].is_source: - fail("out file %s must be a source file, not a generated file" % out_file_label) - - if out_file_label.package != ctx.label.package: - fail("out file %s (in package '%s') must be a source file within the target's package: '%s'" % (out_file_label, out_file_label.package, ctx.label.package)) - - if ctx.attr.is_windows: - updater = _write_source_files_bat(ctx) - else: - updater = _write_source_files_sh(ctx) - - runfiles = ctx.runfiles(files = ctx.files.in_files) - deps_runfiles = [dep[DefaultInfo].default_runfiles for dep in ctx.attr.additional_update_targets] - if "merge_all" in dir(runfiles): - runfiles = runfiles.merge_all(deps_runfiles) - else: - for dep in deps_runfiles: - runfiles = runfiles.merge(dep) - - return [ - DefaultInfo( - executable = updater, - runfiles = runfiles, - ), - _WriteSourceFilesInfo( - executable = updater, - ), - ] - -write_source_files_lib = struct( - attrs = _write_source_files_attrs, - implementation = _write_source_files_impl, -) diff --git a/lib/tests/default_output_gen.bzl b/lib/tests/default_output_gen.bzl new file mode 100644 index 000000000..2fc9a43e7 --- /dev/null +++ b/lib/tests/default_output_gen.bzl @@ -0,0 +1,30 @@ +"""A simple rule that generates provides a DefaultOutput with some files""" + +def _impl(ctx): + if len(ctx.attr.out_files) != len(ctx.attr.out_contents): + fail("Number of out_files must match number of out_contents") + outputs = [] + for i, file in enumerate(ctx.attr.out_files): + content = ctx.attr.out_contents[i] + out = ctx.actions.declare_file(file) + + # ctx.actions.write creates a FileWriteAction which uses UTF-8 encoding. + ctx.actions.write( + output = out, + content = content, + ) + outputs.append(out) + + return [DefaultInfo( + files = depset(direct = outputs), + runfiles = ctx.runfiles(files = outputs), + )] + +default_output_gen = rule( + implementation = _impl, + provides = [DefaultInfo], + attrs = { + "out_files": attr.string_list(), + "out_contents": attr.string_list(), + }, +) diff --git a/lib/tests/write_source_files/BUILD.bazel b/lib/tests/write_source_files/BUILD.bazel index e4440b021..ec4da553a 100644 --- a/lib/tests/write_source_files/BUILD.bazel +++ b/lib/tests/write_source_files/BUILD.bazel @@ -1,42 +1,84 @@ -load("//lib/tests/write_source_files:write_source_files_test.bzl", "write_source_files_test") +load("//lib/tests/write_source_files:write_source_file_test.bzl", "write_source_file_test") +load("//lib/tests:default_output_gen.bzl", "default_output_gen") load("//lib:write_source_files.bzl", "write_source_files") load("//lib:copy_to_directory.bzl", "copy_to_directory") +load("//lib:directory_path.bzl", "directory_path") +load("//lib:default_info_files.bzl", "default_info_files") genrule( name = "a-desired", outs = ["a-desired.js"], - cmd = "echo 'console.log(\"a*\")' > $@", + cmd = "echo 'console.log(\"a*\");' > $@", ) -genrule( +default_output_gen( + name = "b_c-desired", + out_contents = [ + """console.log(\"b*\"); +""", + "not used!", + ], + out_files = [ + "b-desired.js", + "c-desired.js", + ], +) + +default_info_files( name = "b-desired", - outs = ["b-desired.js"], - cmd = "echo 'console.log(\"b*\")' > $@", + paths = ["%s/b-desired.js" % package_name()], + target = ":b_c-desired", ) genrule( - name = "e", - outs = ["e.js"], - cmd = "echo 'console.log(\"e*\")' > $@", + name = "e-contained", + outs = ["e-contained.js"], + cmd = "echo 'console.log(\"e*\");' > $@", ) copy_to_directory( name = "e_dir-desired", - srcs = [":e"], + srcs = [":e-contained"], ) -write_source_files_test( - name = "write_to_source_files_test", - in_files = [ - ":a-desired", - ":b-desired", - ], - out_files = [ - "a.js", - "b.js", +genrule( + name = "f-contained", + outs = ["f-contained.js"], + cmd = "echo 'console.log(\"f*\");' > $@", +) + +copy_to_directory( + name = "e_f_dir-desired", + srcs = [ + ":e-contained", + ":f-contained", ], ) +directory_path( + name = "f-desired", + directory = ":e_f_dir-desired", + path = "f-contained.js", +) + +write_source_file_test( + name = "write_to_source_files_a_test", + in_file = ":a-desired", + out_file = "a.js", +) + +write_source_file_test( + name = "write_to_source_files_b_test", + in_file = ":b-desired", + out_file = "b.js", +) + +write_source_file_test( + name = "write_to_source_files_f_test", + in_file = ":f-desired", + out_file = "f.js", +) + write_source_files( name = "macro_smoke_test", additional_update_targets = [ @@ -45,6 +87,7 @@ write_source_files( files = { "a2.js": ":a-desired", "b2.js": ":b-desired", - "e_dir": ":e_dir-desired", + "e2_dir": ":e_dir-desired", + "f2.js": ":f-desired", }, ) diff --git a/lib/tests/write_source_files/a2.js b/lib/tests/write_source_files/a2.js index 8c716423d..81c24ad20 100644 --- a/lib/tests/write_source_files/a2.js +++ b/lib/tests/write_source_files/a2.js @@ -1 +1 @@ -console.log("a*") +console.log("a*"); diff --git a/lib/tests/write_source_files/b2.js b/lib/tests/write_source_files/b2.js index 32f7fea05..68266dba5 100644 --- a/lib/tests/write_source_files/b2.js +++ b/lib/tests/write_source_files/b2.js @@ -1 +1 @@ -console.log("b*") +console.log("b*"); diff --git a/lib/tests/write_source_files/e2_dir/e-contained.js b/lib/tests/write_source_files/e2_dir/e-contained.js new file mode 100644 index 000000000..ede7a9241 --- /dev/null +++ b/lib/tests/write_source_files/e2_dir/e-contained.js @@ -0,0 +1 @@ +console.log("e*"); diff --git a/lib/tests/write_source_files/e_dir/e-contained.js b/lib/tests/write_source_files/e_dir/e-contained.js new file mode 100644 index 000000000..2c695c52d --- /dev/null +++ b/lib/tests/write_source_files/e_dir/e-contained.js @@ -0,0 +1 @@ +console.log("e"); diff --git a/lib/tests/write_source_files/e_dir/e.js b/lib/tests/write_source_files/e_dir/e.js deleted file mode 100644 index 955018365..000000000 --- a/lib/tests/write_source_files/e_dir/e.js +++ /dev/null @@ -1 +0,0 @@ -console.log("e*") diff --git a/lib/tests/write_source_files/f.js b/lib/tests/write_source_files/f.js new file mode 100644 index 000000000..c865a5f47 --- /dev/null +++ b/lib/tests/write_source_files/f.js @@ -0,0 +1 @@ +console.log("f"); diff --git a/lib/tests/write_source_files/f2.js b/lib/tests/write_source_files/f2.js new file mode 100644 index 000000000..4338ce4be --- /dev/null +++ b/lib/tests/write_source_files/f2.js @@ -0,0 +1 @@ +console.log("f*"); diff --git a/lib/tests/write_source_files/subdir/BUILD.bazel b/lib/tests/write_source_files/subdir/BUILD.bazel index 1c1be2124..711ecab3b 100644 --- a/lib/tests/write_source_files/subdir/BUILD.bazel +++ b/lib/tests/write_source_files/subdir/BUILD.bazel @@ -8,12 +8,12 @@ genrule( write_source_files( name = "macro_smoke_test", + additional_update_targets = [ + "//lib/tests/write_source_files/subdir/subsubdir:macro_smoke_test", + ], files = { "c.js": ":c-desired", }, suggested_update_target = "//lib/tests/write_source_files:macro_smoke_test", - visibility = ["//visibility:public"], - additional_update_targets = [ - "//lib/tests/write_source_files/subdir/subsubdir:macro_smoke_test", - ], + visibility = ["//lib/tests/write_source_files:__pkg__"], ) diff --git a/lib/tests/write_source_files/subdir/subsubdir/BUILD.bazel b/lib/tests/write_source_files/subdir/subsubdir/BUILD.bazel index 496ade45a..538c8ba6c 100644 --- a/lib/tests/write_source_files/subdir/subsubdir/BUILD.bazel +++ b/lib/tests/write_source_files/subdir/subsubdir/BUILD.bazel @@ -12,5 +12,5 @@ write_source_files( "d.js": ":d-desired", }, suggested_update_target = "//lib/tests/write_source_files:macro_smoke_test", - visibility = ["//visibility:public"], + visibility = ["//lib/tests/write_source_files/subdir:__pkg__"], ) diff --git a/lib/tests/write_source_files/write_source_files_test.bzl b/lib/tests/write_source_files/write_source_file_test.bzl similarity index 57% rename from lib/tests/write_source_files/write_source_files_test.bzl rename to lib/tests/write_source_files/write_source_file_test.bzl index eed77c8ff..e102f2186 100644 --- a/lib/tests/write_source_files/write_source_files_test.bzl +++ b/lib/tests/write_source_files/write_source_file_test.bzl @@ -1,23 +1,23 @@ """Tests for write_source_files""" # Inspired by https://github.com/cgrindel/bazel-starlib/blob/main/updatesrc/private/updatesrc_update_test.bzl -load("//lib/private:write_source_files.bzl", _lib = "write_source_files_lib") +load("//lib/private:write_source_file.bzl", _lib = "write_source_file_lib") +load("//lib/private:directory_path.bzl", "DirectoryPathInfo") -_write_source_files = rule( +_write_source_file = rule( attrs = _lib.attrs, implementation = _lib.implementation, executable = True, ) -def _impl_sh(ctx): +def _impl_sh(ctx, in_file_path, out_file_path): test = ctx.actions.declare_file( ctx.label.name + "_test.sh", ) - ctx.actions.write( - output = test, - is_executable = True, - content = """ + contents = [] + + contents.append(""" #!/usr/bin/env bash set -o errexit -o nounset -o pipefail @@ -32,32 +32,35 @@ assert_same() { local in_file="${1}" local out_file="${2}" diff "${in_file}" "${out_file}" || (echo >&2 "Expected files to be same. in: ${in_file}, out: ${out_file}" && return -1) -} +}""") + contents.append(""" # Check that in and out files are different -""" + "\n".join([ - "assert_different {in_file} {out_file}".format( - in_file = ctx.files.in_files[i].short_path, - out_file = ctx.files.out_files[i].short_path, - ) - for i in range(len(ctx.files.in_files)) - ]) + """ -# Write to the source files +assert_different {in_file} {out_file} +""".format( + in_file = in_file_path, + out_file = out_file_path, + )) + + contents.append("""# Write to the source files {write_source_files} +""".format(write_source_files = ctx.file.write_source_file_target.short_path)) -# Check that in and out files are the same -""".format(write_source_files = ctx.file.write_source_files_target.short_path) + "\n".join([ - "assert_same {in_file} {out_file}".format( - in_file = ctx.files.in_files[i].short_path, - out_file = ctx.files.out_files[i].short_path, - ) - for i in range(len(ctx.files.in_files)) - ]), + contents.append("""# Check that in and out files are the same +assert_same {in_file} {out_file}""".format( + in_file = in_file_path, + out_file = out_file_path, + )) + + ctx.actions.write( + output = test, + is_executable = True, + content = "\n".join(contents), ) return test -def _impl_bat(ctx): +def _impl_bat(ctx, in_file_path, out_file_path): test = ctx.actions.declare_file( ctx.label.name + "_test.bat", ) @@ -66,40 +69,42 @@ def _impl_bat(ctx): # but it is able to execute the actual output script so point to that for now. # # What we would use if this bug didn't exist: - # write_source_files = ctx.executable.write_source_files_target.short_path.replace("/", "\\") + # write_source_files = ctx.executable.write_source_file_target.short_path.replace("/", "\\") # # Instead back out of the runfiles execution directory: # write_to_source_files_test_test.bat.runfiles/aspect_bazel_lib # And point to the output script. - write_source_files = "..\\..\\%s" % ctx.executable.write_source_files_target.basename + write_source_files = "..\\..\\%s" % ctx.executable.write_source_file_target.basename + + contents = [] - content = """ + contents.append(""" @rem Generated by copy_to_directory.bzl, do not edit. @echo off @rem Check that in and out files are different -""" + "\n".join([""" call :assert_different {in_file}, {out_file} if %errorlevel% neq 0 exit /b 1 """.format( - in_file = ctx.files.in_files[i].short_path.replace("/", "\\"), - out_file = ctx.files.out_files[i].short_path.replace("/", "\\"), - ) - for i in range(len(ctx.files.in_files)) - ]) + """ + in_file = in_file_path.replace("/", "\\"), + out_file = out_file_path.replace("/", "\\"), + )) + + contents.append(""" @rem Write to the source files call {write_source_files} if %errorlevel% neq 0 exit /b 1 +""".format(write_source_files = write_source_files)) + contents.append(""" @rem Check that in and out files are the same -""".format(write_source_files = write_source_files) + "\n".join([""" call :assert_same {in_file}, {out_file} if %errorlevel% neq 0 exit /b 1 """.format( - in_file = ctx.files.in_files[i].short_path.replace("/", "\\"), - out_file = ctx.files.out_files[i].short_path.replace("/", "\\"), - ) - for i in range(len(ctx.files.in_files)) - ]) + """ + in_file = in_file_path.replace("/", "\\"), + out_file = out_file_path.replace("/", "\\"), + )) + + contents.append(""" exit /b 0 :assert_different @@ -113,48 +118,54 @@ fc /b %~1 %~2 > nul if %errorlevel% equ 0 exit /b 0 echo Error: %~1 and %~2 are not the same exit /b 1 -""" - content = content.replace("\n", "\r\n") +""") ctx.actions.write( output = test, is_executable = True, - content = content + content = "\n".join(contents).replace("\n", "\r\n"), ) return test def _impl(ctx): + if DirectoryPathInfo in ctx.attr.in_file: + in_file = ctx.attr.in_file[DirectoryPathInfo].directory + in_file_path = "/".join([in_file.short_path, ctx.attr.in_file[DirectoryPathInfo].path]) + else: + if len(ctx.files.in_file) != 1: + fail("in_file must be a single file or a target that provides a DirectoryPathInfo") + in_file = ctx.files.in_file[0] + in_file_path = in_file.short_path + if ctx.attr.is_windows: - test = _impl_bat(ctx) + test = _impl_bat(ctx, in_file_path, ctx.file.out_file.short_path) else: - test = _impl_sh(ctx) + test = _impl_sh(ctx, in_file_path, ctx.file.out_file.short_path) return DefaultInfo( executable = test, runfiles = ctx.runfiles( - files = [ctx.executable.write_source_files_target] + ctx.files.in_files + ctx.files.out_files, + files = [ctx.executable.write_source_file_target, in_file, ctx.file.out_file], ), ) -_write_source_files_test = rule( +_write_source_file_test = rule( implementation = _impl, attrs = { - "write_source_files_target": attr.label( + "write_source_file_target": attr.label( allow_single_file = True, executable = True, # Should be cfg = "exec" but a bazel bug causes a wrong executable symlink on windows cfg = "target", mandatory = True, ), - "out_files": attr.label_list( - allow_files = True, - allow_empty = False, + "out_file": attr.label( + allow_single_file = True, mandatory = True, ), - "in_files": attr.label_list( + "in_file": attr.label( allow_files = True, - allow_empty = False, mandatory = True, ), "is_windows": attr.bool(mandatory = True), @@ -162,13 +173,13 @@ _write_source_files_test = rule( test = True, ) -def write_source_files_test(name, in_files, out_files): +def write_source_file_test(name, in_file, out_file): """Stamp a write_source_files executable and a test to run against it""" - _write_source_files( + _write_source_file( name = name + "_updater", - out_files = out_files, - in_files = in_files, + in_file = in_file, + out_file = out_file, is_windows = select({ "@bazel_tools//src/conditions:host_windows": True, "//conditions:default": False, @@ -177,11 +188,11 @@ def write_source_files_test(name, in_files, out_files): # Note that for testing we update the source files in the sandbox, # not the actual source tree. - _write_source_files_test( + _write_source_file_test( name = name, - write_source_files_target = name + "_updater", - out_files = out_files, - in_files = in_files, + write_source_file_target = name + "_updater", + in_file = in_file, + out_file = out_file, is_windows = select({ "@bazel_tools//src/conditions:host_windows": True, "//conditions:default": False, diff --git a/lib/write_source_files.bzl b/lib/write_source_files.bzl index aafa5650a..029515388 100644 --- a/lib/write_source_files.bzl +++ b/lib/write_source_files.bzl @@ -1,11 +1,14 @@ "Public API for write_source_files" -load("//lib/private:write_source_files.bzl", _lib = "write_source_files_lib") +load( + "//lib/private:write_source_file.bzl", + _lib = "write_source_file_lib", +) load("//lib:utils.bzl", _to_label = "to_label") -load("@bazel_skylib//rules:diff_test.bzl", _diff_test = "diff_test") +load("//lib/private:diff_test.bzl", _diff_test = "diff_test") load("//lib/private:fail_with_message_test.bzl", "fail_with_message_test") -_write_source_files = rule( +_write_source_file = rule( attrs = _lib.attrs, implementation = _lib.implementation, executable = True, @@ -85,41 +88,45 @@ def write_source_files(name, files = {}, additional_update_targets = [], suggest name: Name of the executable target that creates or updates the source file files: A dict where the keys are source files or folders to write to and the values are labels pointing to the desired content. Sources must be within the same bazel package as the target. - additional_update_targets: (Optional) List of other write_source_files targets to update in the same run - suggested_update_target: (Optional) Label of the write_source_files target to suggest running when files are out of date + additional_update_targets: (Optional) List of other write_source_file or other executable updater targets to call in the same run + suggested_update_target: (Optional) Label of the write_source_file target to suggest running when files are out of date **kwargs: Other common named parameters such as `tags` or `visibility` """ - out_files = files.keys() - in_files = [files[f] for f in out_files] - - # Stamp an executable rule that writes to the out file - _write_source_files( - name = name, - in_files = in_files, - out_files = out_files, - additional_update_targets = additional_update_targets, - is_windows = select({ - "@bazel_tools//src/conditions:host_windows": True, - "//conditions:default": False, - }), - visibility = kwargs.get("visibility"), - tags = kwargs.get("tags"), - ) + single_update_target = len(files.keys()) == 1 + update_targets = [] + for i, pair in enumerate(files.items()): + out_file, in_file = pair + + in_file = _to_label(in_file) + out_file = _to_label(out_file) - # Fail if user passes args that would conflict with stamped out targets below - if kwargs.pop("file1", None) != None: - fail("file1 not a valid parameter in write_source_file") - if kwargs.pop("file2", None) != None: - fail("file2 not a valid parameter in write_source_file") - if kwargs.pop("failure_message", None) != None: - fail("failure_message not a valid parameter in write_source_file") + if single_update_target: + update_target_name = name + else: + update_target_name = "%s_%d" % (name, i) + update_targets.append(update_target_name) + + # Runnable target that writes to the out file to the source tree + _write_source_file( + name = update_target_name, + in_file = in_file, + out_file = out_file, + additional_update_targets = additional_update_targets if single_update_target else [], + is_windows = select({ + "@bazel_tools//src/conditions:host_windows": True, + "//conditions:default": False, + }), + visibility = kwargs.get("visibility"), + tags = kwargs.get("tags"), + ) - for i in range(len(out_files)): - out_file = _to_label(out_files[i]) out_file_missing = _is_file_missing(out_file) - name_test = "%s_%d_test" % (name, i) + if single_update_target: + test_target_name = "%s_test" % name + else: + test_target_name = "%s_%d_test" % (name, i) if out_file_missing: if suggested_update_target == None: @@ -147,7 +154,7 @@ To create an update *only* this file, run: # Note that we cannot simply call fail() here since it will fail during the analysis # phase and prevent the user from calling bazel run //update/the:file. fail_with_message_test( - name = name_test, + name = test_target_name, message = message, visibility = kwargs.get("visibility"), tags = kwargs.get("tags"), @@ -176,13 +183,25 @@ To update *only* this file, run: # Stamp out a diff test the check that the source file is up to date _diff_test( - name = name_test, - file1 = in_files[i], + name = test_target_name, + file1 = in_file, file2 = out_file, failure_message = message, **kwargs ) + if not single_update_target: + _write_source_file( + name = name, + additional_update_targets = update_targets + additional_update_targets, + is_windows = select({ + "@bazel_tools//src/conditions:host_windows": True, + "//conditions:default": False, + }), + visibility = kwargs.get("visibility"), + tags = kwargs.get("tags"), + ) + def _is_file_missing(label): """Check if a file is missing by passing its relative path through a glob()