From 9059f8fa5ee1dc3a1c95102ff2a5a95fbe7fb8e6 Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Fri, 2 Dec 2022 23:23:57 -0800 Subject: [PATCH] feat: add executable attribute to write_source_files --- CONTRIBUTING.md | 2 +- docs/base64.md | 0 docs/copy_to_bin.md | 0 docs/glob_match.md | 0 docs/host_repo.md | 0 docs/platform_utils.md | 0 docs/repo_utils.md | 0 docs/stamping.md | 0 docs/testing.md | 0 docs/write_source_files.md | 39 +++++++---- docs/yq.md | 0 e2e/copy_to_directory/BUILD.bazel | 21 +++--- e2e/run_jq_symlinked_bin.sh | 5 +- e2e/run_yq_symlinked_bin.sh | 5 +- e2e/write_source_files.sh | 31 +++++++++ e2e/write_source_files_gitignored.sh | 6 -- e2e/write_source_files_root.sh | 22 ++++-- e2e/write_source_files_subdir.sh | 33 +++++++++ ...write_source_files_subdir_multiple_runs.sh | 9 --- e2e/write_source_files_symlinks.sh | 49 ++++++++++---- lib/diff_test.bzl | 3 + lib/private/diff_test.bzl | 3 + lib/private/diff_test_tmpl.sh | 2 +- lib/private/write_source_file.bzl | 62 ++++++++++++++--- lib/tests/BUILD.bazel | 4 +- lib/tests/generate_outputs.bzl | 2 +- lib/tests/write_source_files/.gitignore | 2 + lib/tests/write_source_files/BUILD.bazel | 67 ++++++++++++++++--- .../write_source_files/e2_dir/e-contained.js | 0 .../write_source_files/e_dir/e-contained.js | 2 +- .../es2_dir/subdir/e-contained.js | 1 + .../es_dir/subdir/e-contained.js | 1 + lib/write_source_files.bzl | 56 ++++++++++++---- 33 files changed, 328 insertions(+), 99 deletions(-) mode change 100755 => 100644 docs/base64.md mode change 100755 => 100644 docs/copy_to_bin.md mode change 100755 => 100644 docs/glob_match.md mode change 100755 => 100644 docs/host_repo.md mode change 100755 => 100644 docs/platform_utils.md mode change 100755 => 100644 docs/repo_utils.md mode change 100755 => 100644 docs/stamping.md mode change 100755 => 100644 docs/testing.md mode change 100755 => 100644 docs/yq.md create mode 100755 e2e/write_source_files.sh delete mode 100755 e2e/write_source_files_gitignored.sh create mode 100755 e2e/write_source_files_subdir.sh delete mode 100755 e2e/write_source_files_subdir_multiple_runs.sh mode change 100644 => 100755 lib/tests/write_source_files/e2_dir/e-contained.js create mode 100755 lib/tests/write_source_files/es2_dir/subdir/e-contained.js create mode 100755 lib/tests/write_source_files/es_dir/subdir/e-contained.js diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3907641..d25c986 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -17,7 +17,7 @@ Otherwise later tooling on CI may yell at you about formatting/linting violation Some targets are generated from sources. Currently this is just the `bzl_library` targets. -Run `bazel run //:gazelle` to keep them up-to-date. +Run `bazel run //:gazelle` to keep them up to date. ## Using this as a development dependency of other rules diff --git a/docs/base64.md b/docs/base64.md old mode 100755 new mode 100644 diff --git a/docs/copy_to_bin.md b/docs/copy_to_bin.md old mode 100755 new mode 100644 diff --git a/docs/glob_match.md b/docs/glob_match.md old mode 100755 new mode 100644 diff --git a/docs/host_repo.md b/docs/host_repo.md old mode 100755 new mode 100644 diff --git a/docs/platform_utils.md b/docs/platform_utils.md old mode 100755 new mode 100644 diff --git a/docs/repo_utils.md b/docs/repo_utils.md old mode 100755 new mode 100644 diff --git a/docs/stamping.md b/docs/stamping.md old mode 100755 new mode 100644 diff --git a/docs/testing.md b/docs/testing.md old mode 100755 new mode 100644 diff --git a/docs/write_source_files.md b/docs/write_source_files.md index 5bd921e..892ede4 100644 --- a/docs/write_source_files.md +++ b/docs/write_source_files.md @@ -7,11 +7,15 @@ Public API for write_source_files ## write_source_files 
-write_source_files(name, files, additional_update_targets, suggested_update_target, diff_test,
-                   kwargs)
+write_source_files(name, files, executable, additional_update_targets, suggested_update_target,
+                   diff_test, kwargs)
-Write to one or more files or folders in the source tree. Stamp out tests that ensure the sources exist and are up to date. +Write one or more files and/or directories to the source tree. + +By default, `diff_test` targets are generated that ensure the source tree files and/or directories to be written to +are up to date and the rule also checks that all source tree files and/or directories to be written to exist. +To disable the exists check and up-to-date tests set `diff_test` to `False`. Usage: @@ -27,11 +31,16 @@ write_source_files( ``` To update the source file, run: + ```bash bazel run //:write_foobar ``` -A test will fail if the source file doesn't exist or if it's out of date with instructions on how to create/update it. +The generated `diff_test` will fail if the file is out of date and print out instructions on +how to update it. + +If the file does not exist, Bazel will fail at analysis time and print out instructions on +how to create it. You can declare a tree of generated source file targets: @@ -54,7 +63,8 @@ And update them with a single run: bazel run //:write_all ``` -When a file is out of date, you can leave a suggestion to run a target further up in the tree by specifying `suggested_update_target`. E.g., +When a file is out of date, you can leave a suggestion to run a target further up in the tree by specifying `suggested_update_target`. +For example, ```starlark write_source_files( @@ -66,7 +76,7 @@ write_source_files( ) ``` -A test failure from foo.json being out of date will yield the following message: +A test failure from `foo.json` being out of date will yield the following message: ``` //a/b:c:foo.json is out of date. To update this and other generated files, run: @@ -78,9 +88,11 @@ To update *only* this file, run: bazel run //a/b/c:write_foo ``` -If you have many sources that you want to update as a group, we recommend wrapping write_source_files in a macro that defaults `suggested_update_target` to the umbrella update target. +If you have many `write_source_files` targets that you want to update as a group, we recommend wrapping +`write_source_files` in a macro that defaults `suggested_update_target` to the umbrella update target. -NOTE: If you run formatters or linters on your codebase, it is advised that you exclude/ignore the outputs of this rule from those formatters/linters so as to avoid causing collisions and failing tests. +NOTE: If you run formatters or linters on your codebase, it is advised that you exclude/ignore the outputs of this + rule from those formatters/linters so as to avoid causing collisions and failing tests. **PARAMETERS** @@ -88,11 +100,12 @@ NOTE: If you run formatters or linters on your codebase, it is advised that you | Name | Description | Default Value | | :------------- | :------------- | :------------- | -| 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_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 | -| diff_test | (Optional) Generate a test target to check that the source file(s) exist and are up to date with the generated files(s). | True | +| name | Name of the runnable target that creates or updates the source tree files and/or directories. | none | +| files | A dict where the keys are files or directories in the source tree to write to and the values are labels pointing to the desired content, typically file or directory outputs of other targets.

Source tree files and directories must be within the same bazel package as the target. | {} | +| executable | Whether source tree files written should be made executable.

This applies to all source tree files written by this target. This attribute is not propagated to additional_update_targets.

To set different executable permissions on different source tree files use multiple write_source_files targets. | False | +| additional_update_targets | List of other write_source_files or other executable updater targets to call in the same run. | [] | +| suggested_update_target | Label of the write_source_files target to suggest running when files are out of date. | None | +| diff_test | Test that the source tree files and/or directories exist and are up to date. | True | | kwargs | Other common named parameters such as tags or visibility | none | diff --git a/docs/yq.md b/docs/yq.md old mode 100755 new mode 100644 diff --git a/e2e/copy_to_directory/BUILD.bazel b/e2e/copy_to_directory/BUILD.bazel index 629f949..7657522 100644 --- a/e2e/copy_to_directory/BUILD.bazel +++ b/e2e/copy_to_directory/BUILD.bazel @@ -1,9 +1,6 @@ load("@aspect_bazel_lib//lib:copy_to_directory.bzl", "copy_to_directory") load("@aspect_bazel_lib//lib:diff_test.bzl", "diff_test") -# TODO: figure out ERROR: cannot compare a directory against a file -_TODO = ["no-remote-exec"] - # Test special cases of using "." and "./**" patterns in the root package copy_to_directory( @@ -30,14 +27,16 @@ diff_test( name = "test1", file1 = ":dir1", file2 = ":expected1", - tags = _TODO, + # Source directories are not support on remote execution. + tags = ["no-remote-exec"], ) diff_test( name = "test1b", file1 = ":dir1b", file2 = ":expected1", - tags = _TODO, + # Source directories are not support on remote execution. + tags = ["no-remote-exec"], ) copy_to_directory( @@ -66,14 +65,16 @@ diff_test( name = "test2", file1 = ":dir2", file2 = ":expected2", - tags = _TODO, + # Source directories are not support on remote execution. + tags = ["no-remote-exec"], ) diff_test( name = "test2b", file1 = ":dir2b", file2 = ":expected2", - tags = _TODO, + # Source directories are not support on remote execution. + tags = ["no-remote-exec"], ) copy_to_directory( @@ -102,12 +103,14 @@ diff_test( name = "test3", file1 = ":dir3", file2 = ":expected3", - tags = _TODO, + # Source directories are not support on remote execution. + tags = ["no-remote-exec"], ) diff_test( name = "test3b", file1 = ":dir3b", file2 = ":expected3", - tags = _TODO, + # Source directories are not support on remote execution. + tags = ["no-remote-exec"], ) diff --git a/e2e/run_jq_symlinked_bin.sh b/e2e/run_jq_symlinked_bin.sh index bac6253..ed748b3 100755 --- a/e2e/run_jq_symlinked_bin.sh +++ b/e2e/run_jq_symlinked_bin.sh @@ -1,6 +1,5 @@ -#!/bin/bash - -set -e +#!/usr/bin/env bash +set -o errexit -o nounset -o pipefail case "$(uname -s)" in CYGWIN*|MINGW32*|MSYS*|MINGW*) diff --git a/e2e/run_yq_symlinked_bin.sh b/e2e/run_yq_symlinked_bin.sh index 9b32c60..cc3a0df 100755 --- a/e2e/run_yq_symlinked_bin.sh +++ b/e2e/run_yq_symlinked_bin.sh @@ -1,6 +1,5 @@ -#!/bin/bash - -set -e +#!/usr/bin/env bash +set -o errexit -o nounset -o pipefail case "$(uname -s)" in CYGWIN*|MINGW32*|MSYS*|MINGW*) diff --git a/e2e/write_source_files.sh b/e2e/write_source_files.sh new file mode 100755 index 0000000..ed34be3 --- /dev/null +++ b/e2e/write_source_files.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +set -o errexit -o nounset -o pipefail + +function run_test { + bazel run //lib/tests/write_source_files:write_dist + local expected_out="lib/tests/write_source_files/dist.js" + if [ ! -e "$expected_out" ]; then + echo "ERROR: expected $expected_out to exist" + exit 1 + fi + if [ -x "$expected_out" ]; then + echo "ERROR: expected $expected_out to not be executable" + exit 1 + fi + + bazel run //lib/tests/write_source_files:write_dist_executable + local expected_out="lib/tests/write_source_files/dist_executable.js" + if [ ! -e "$expected_out" ]; then + echo "ERROR: expected $expected_out to exist" + exit 1 + fi + if [ ! -x "$expected_out" ]; then + echo "ERROR: expected $expected_out to be executable" + exit 1 + fi +} + +# Run twice to make sure we can have permission to overwrite the outputs of a previous run +run_test +run_test +echo "All tests passed" diff --git a/e2e/write_source_files_gitignored.sh b/e2e/write_source_files_gitignored.sh deleted file mode 100755 index 4ca907d..0000000 --- a/e2e/write_source_files_gitignored.sh +++ /dev/null @@ -1,6 +0,0 @@ -#!/bin/bash - -set -e - -bazel run //lib/tests/write_source_files:write_dist -[ -e lib/tests/write_source_files/dist.js ] diff --git a/e2e/write_source_files_root.sh b/e2e/write_source_files_root.sh index 9d23e62..1df2823 100755 --- a/e2e/write_source_files_root.sh +++ b/e2e/write_source_files_root.sh @@ -1,6 +1,20 @@ -#!/bin/bash +#!/usr/bin/env bash +set -o errexit -o nounset -o pipefail -set -e +function run_test { + bazel run //:write_source_file_root-test + local expected_out="test-out/dist/write_source_file_root-test/test.txt" + if [ ! -e "$expected_out" ]; then + echo "ERROR: expected $expected_out to exist" + exit 1 + fi + if [ -x "$expected_out" ]; then + echo "ERROR: expected $expected_out to not be executable" + exit 1 + fi +} -bazel run //:write_source_file_root-test -[ -e test-out/dist/write_source_file_root-test/test.txt ] +# Run twice to make sure we can have permission to overwrite the outputs of a previous run +run_test +run_test +echo "All tests passed" diff --git a/e2e/write_source_files_subdir.sh b/e2e/write_source_files_subdir.sh new file mode 100755 index 0000000..d5f591f --- /dev/null +++ b/e2e/write_source_files_subdir.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +set -o errexit -o nounset -o pipefail + +function run_test { + bazel run //lib/tests/write_source_files:write_subdir + local expected_out="lib/tests/write_source_files/subdir_test/a/b/c/test.txt" + if [ ! -e "$expected_out" ]; then + echo "ERROR: expected $expected_out to exist" + exit 1 + fi + if [ -x "$expected_out" ]; then + echo "ERROR: expected $expected_out to not be executable" + exit 1 + fi + + bazel run //lib/tests/write_source_files:write_subdir_executable + local expected_out="lib/tests/write_source_files/subdir_executable_test/a/b/c/test.txt" + if [ ! -e "$expected_out" ]; then + echo "ERROR: expected $expected_out to exist" + exit 1 + fi + if [ ! -x "$expected_out" ]; then + echo "ERROR: expected $expected_out to be executable" + exit 1 + fi +} + +# Run twice to make sure we can have permission to overwrite the outputs of a previous run +rm -rf lib/tests/write_source_files/subdir_test +rm -rf lib/tests/write_source_files/subdir_executable_test +run_test +run_test +echo "All tests passed" diff --git a/e2e/write_source_files_subdir_multiple_runs.sh b/e2e/write_source_files_subdir_multiple_runs.sh deleted file mode 100755 index 5ec7048..0000000 --- a/e2e/write_source_files_subdir_multiple_runs.sh +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/bash - -set -e - -bazel run //lib/tests/write_source_files:write_subdir -[ -e lib/tests/write_source_files/subdir_test/a/b/c/test.txt ] - -bazel run //lib/tests/write_source_files:write_subdir -[ -e lib/tests/write_source_files/subdir_test/a/b/c/test.txt ] diff --git a/e2e/write_source_files_symlinks.sh b/e2e/write_source_files_symlinks.sh index 7b4dde7..7438da5 100755 --- a/e2e/write_source_files_symlinks.sh +++ b/e2e/write_source_files_symlinks.sh @@ -1,18 +1,39 @@ -#!/bin/bash +#!/usr/bin/env bash +set -o errexit -o nounset -o pipefail -set -e +function run_test { + bazel run //lib/tests/write_source_files:write_symlinks -bazel run //lib/tests/write_source_files:write_symlinks + local expected_out="lib/tests/write_source_files/symlink_test/a/test.txt" + if [ ! -e "$expected_out" ]; then + echo "ERROR: expected $expected_out to exist" + exit 1 + fi + if [ -x "$expected_out" ]; then + echo "ERROR: expected $expected_out to not be executable" + exit 1 + fi + if [ -L "$expected_out" ]; then + echo "ERROR: expected $expected_out to not be a symlink" + exit 1 + fi -# Ensure exists -[ -e lib/tests/write_source_files/symlink_test/a/test.txt ] -[ -e lib/tests/write_source_files/symlink_test/b/test.txt ] + local expected_out="lib/tests/write_source_files/symlink_test/b/test.txt" + if [ ! -e "$expected_out" ]; then + echo "ERROR: expected $expected_out to exist" + exit 1 + fi + if [ -x "$expected_out" ]; then + echo "ERROR: expected $expected_out to not be executable" + exit 1 + fi + if [ -L "$expected_out" ]; then + echo "ERROR: expected $expected_out to not be a symlink" + exit 1 + fi +} -# Exit if any symlinks -if [ -L lib/tests/write_source_files/symlink_test/a/test.txt ]; then - exit 1 -fi - -if [ -L lib/tests/write_source_files/symlink_test/b/test.txt ]; then - exit 1 -fi +# Run twice to make sure we can have permission to overwrite the outputs of a previous run +run_test +run_test +echo "All tests passed" diff --git a/lib/diff_test.bzl b/lib/diff_test.bzl index 5fa8783..55fcc31 100644 --- a/lib/diff_test.bzl +++ b/lib/diff_test.bzl @@ -14,6 +14,9 @@ """A test rule that compares two binary files or two directories. +Similar to `bazel-skylib`'s [`diff_test`](https://github.com/bazelbuild/bazel-skylib/blob/main/rules/diff_test.bzl) +but also supports comparing directories. + 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). """ diff --git a/lib/private/diff_test.bzl b/lib/private/diff_test.bzl index 3908dd7..6e10e44 100644 --- a/lib/private/diff_test.bzl +++ b/lib/private/diff_test.bzl @@ -14,6 +14,9 @@ """A test rule that compares two binary files or two directories. +Similar to `bazel-skylib`'s [`diff_test`](https://github.com/bazelbuild/bazel-skylib/blob/main/rules/diff_test.bzl) +but also supports comparing directories. + 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). """ diff --git a/lib/private/diff_test_tmpl.sh b/lib/private/diff_test_tmpl.sh index 4051dc5..c32f50f 100644 --- a/lib/private/diff_test_tmpl.sh +++ b/lib/private/diff_test_tmpl.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -set -euo pipefail +set -o errexit -o nounset -o pipefail escape() { echo "$1" \ | sed 's/&/\&/g; s//\>/g; s/"/\"/g; s/'"'"'/\'/g' \ diff --git a/lib/private/write_source_file.bzl b/lib/private/write_source_file.bzl index 9eda020..b0baa35 100644 --- a/lib/private/write_source_file.bzl +++ b/lib/private/write_source_file.bzl @@ -16,19 +16,34 @@ def write_source_file( name, in_file = None, out_file = None, + executable = False, additional_update_targets = [], suggested_update_target = None, diff_test = True, **kwargs): - """Write a file or folder to the output tree. Stamp out tests that ensure the sources exist and are up to date. + """Write a file or directory to the source tree. + + By default, `diff_test` targets are generated that ensure the source tree file or directory to be written to + is up to date and the rule also checks that the source tree file or directory to be written to exists. + To disable the exists check and up-to-date test set `diff_test` to `False`. Args: - name: Name of the executable target that creates or updates the source file - in_file: File to use as the desired content to write to out_file. If in_file is a TreeArtifact then entire directory contents are copied. - out_file: The file to write to in the source tree. Must be within the same bazel package as the target. - additional_update_targets: List of other write_source_file or other executable updater targets to call in the same run - suggested_update_target: Label of the write_source_file target to suggest running when files are out of date - diff_test: Generate a test target to check that the source file(s) exist and are up to date with the generated files(s). + name: Name of the runnable target that creates or updates the source tree file or directory. + + in_file: File or directory to use as the desired content to write to `out_file`. + + This is typically a file or directory output of another target. If `in_file` is a directory then entire directory contents are copied. + + out_file: The file or directory to write to in the source tree. Must be within the same bazel package as the target. + + executable: Whether source tree file or files within the source tree directory written should be made executable. + + additional_update_targets: List of other `write_source_files`, write_source_file` or other executable updater targets to call in the same run. + + suggested_update_target: Label of the `write_source_files` or `write_source_file` target to suggest running when files are out of date. + + diff_test: Test that the source tree file or directory exist and is up to date. + **kwargs: Other common named parameters such as `tags` or `visibility` Returns: @@ -57,6 +72,7 @@ def write_source_file( name = name, in_file = in_file, out_file = out_file.name if out_file else None, + executable = executable, additional_update_targets = additional_update_targets, **kwargs ) @@ -140,12 +156,16 @@ _write_source_file_attrs = { # and it goes into an infinite update, notify loop when running this target. # See https://github.com/aspect-build/bazel-lib/pull/52 for more context. "out_file": attr.string(mandatory = False), + "executable": attr.bool(), # buildifier: disable=attr-cfg "additional_update_targets": attr.label_list(cfg = "host", mandatory = False, providers = [WriteSourceFileInfo]), "_windows_constraint": attr.label(default = "@platforms//os:windows"), + "_macos_constraint": attr.label(default = "@platforms//os:macos"), } def _write_source_file_sh(ctx, paths): + is_macos = ctx.target_platform_has_constraint(ctx.attr._macos_constraint[platform_common.ConstraintValueInfo]) + updater = ctx.actions.declare_file( ctx.label.name + "_update.sh", ) @@ -162,24 +182,46 @@ if [[ ! -z "${BUILD_WORKSPACE_DIRECTORY:-}" ]]; then cd "$BUILD_WORKSPACE_DIRECTORY" fi"""] + if ctx.attr.executable: + executable_file = "chmod +x \"$out\"" + executable_dir = "chmod -R +x \"$out\"/*" + else: + executable_file = "chmod -x \"$out\"" + if is_macos: + # -x+X doesn't work on macos so we have to find files and remove the execute bits only from those + executable_dir = "find \"$out\" -type file | xargs chmod -x" + else: + # Remove execute/search bit recursively from files bit not directories: https://superuser.com/a/434418 + executable_dir = "chmod -R -x+X \"$out\"/*" + 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 + echo "Copying file $in to $out in $PWD" + rm -Rf "$out" cp -f "$in" "$out" + # cp should make the file writable but call chmod anyway as a defense in depth chmod ug+w "$out" + # cp should make the file not-executable but set the desired execute bit in both cases as a defense in depth + {executable_file} else + echo "Copying directory $in to $out in $PWD" rm -Rf "$out"/* mkdir -p "$out" cp -fRL "$in"/* "$out" chmod -R ug+w "$out"/* + {executable_dir} fi -""".format(in_path = in_path, out_path = out_path)) +""".format( + in_path = in_path, + out_path = out_path, + executable_file = executable_file, + executable_dir = executable_dir, + )) contents.extend([ "cd \"$runfiles_dir\"", diff --git a/lib/tests/BUILD.bazel b/lib/tests/BUILD.bazel index 2caf15a..d55bd96 100644 --- a/lib/tests/BUILD.bazel +++ b/lib/tests/BUILD.bazel @@ -24,8 +24,8 @@ write_file( name = "gen_template", out = "template.txt", content = [ - "#!/bin/bash", - "set -o errexit", + "#!/usr/bin/env bash", + "set -o errexit -o nounset -o pipefail", """[ "{thing}" == "stuff" ]""", """[ "%path%" == "{BINDIR}/lib/tests/template.txt" ]""", ], diff --git a/lib/tests/generate_outputs.bzl b/lib/tests/generate_outputs.bzl index 269f649..9704f90 100644 --- a/lib/tests/generate_outputs.bzl +++ b/lib/tests/generate_outputs.bzl @@ -1,4 +1,4 @@ -"""A simple rule that generates provides a DefaultOutput with some files""" +"""A simple rule that generates and provides a DefaultOutput with some files""" def _generate_outputs_impl(ctx): if len(ctx.attr.output_files) != len(ctx.attr.output_contents): diff --git a/lib/tests/write_source_files/.gitignore b/lib/tests/write_source_files/.gitignore index 38bb718..2b56454 100644 --- a/lib/tests/write_source_files/.gitignore +++ b/lib/tests/write_source_files/.gitignore @@ -1,3 +1,5 @@ dist.js +dist_executable.js subdir_test +subdir_executable_test symlink_test \ No newline at end of file diff --git a/lib/tests/write_source_files/BUILD.bazel b/lib/tests/write_source_files/BUILD.bazel index ea54786..c6285f9 100644 --- a/lib/tests/write_source_files/BUILD.bazel +++ b/lib/tests/write_source_files/BUILD.bazel @@ -1,6 +1,7 @@ load("//lib/tests/write_source_files:write_source_file_test.bzl", "write_source_file_test") load("//lib/tests:generate_outputs.bzl", "generate_outputs") load("//lib:write_source_files.bzl", "write_source_files") +load("//lib/private:write_source_file.bzl", "write_source_file") load("//lib:copy_to_directory.bzl", "copy_to_directory") load("//lib:directory_path.bzl", "directory_path") load("//lib:output_files.bzl", "output_files") @@ -51,6 +52,14 @@ copy_to_directory( srcs = [":e-contained"], ) +copy_to_directory( + name = "es_dir-desired", + srcs = [":e-contained"], + replace_prefixes = { + "e-contained.js": "subdir/e-contained.js", + }, +) + genrule( name = "f-contained", outs = ["f-contained.js"], @@ -93,25 +102,47 @@ output_files( ) write_source_file_test( - name = "write_to_source_files_a_test", + name = "a_test", in_file = ":a-desired", out_file = "a.js", ) write_source_file_test( - name = "write_to_source_files_b_test", + name = "b_test", in_file = ":b-desired", out_file = "b.js", ) +write_source_file( + name = "e_dir_test", + in_file = ":e_dir-desired", + out_file = "e_dir", + # The diff_test that is contained within this macro takes `out_file` which is a source directory + # as an input but source directories are not supported on remote execution so we must tag that + # rule with "no-remote-exec". + # ERROR: cannot compare a directory "lib/tests/write_source_files/e_dir-desired" against a file "lib/tests/write_source_files/e_dir" + tags = ["no-remote-exec"], +) + +write_source_file( + name = "es_dir_test", + in_file = ":es_dir-desired", + out_file = "es_dir", + # The diff_test that is contained within this macro takes `out_file` which is a source directory + # as an input but source directories are not supported on remote execution so we must tag that + # rule with "no-remote-exec". + # ERROR: cannot compare a directory "lib/tests/write_source_files/es_dir-desired" against a file "lib/tests/write_source_files/es_dir" + tags = ["no-remote-exec"], +) + write_source_file_test( - name = "write_to_source_files_f_test", + name = "f_test", in_file = ":f-desired", out_file = "f.js", ) write_source_file_test( - name = "write_to_source_files_g_test", + name = "g_test", in_file = ":g-desired", out_file = "g.js", ) @@ -125,10 +156,13 @@ write_source_files( "a2.js": ":a-desired", "b2.js": ":b-desired", "e2_dir": ":e_dir-desired", + "es2_dir": ":es_dir-desired", "f2.js": ":f-desired", "g2.js": ":g-desired", }, - # TODO: figure out ERROR: cannot compare a directory against a file + # The diff_test that is contained within this macro takes output directories `es_dir` and + # `es2dir` which are source directories as inputs but source directories are not supported on + # remote execution so we must tag that rule with "no-remote-exec". tags = ["no-remote-exec"], ) @@ -138,22 +172,28 @@ genrule( cmd = "echo 'dist' > $@", ) -# ibazel run //lib/tests/write_source_files:write_dist -# See e2e/write_source_files_gitignored +# See e2e/write_source_files write_source_files( name = "write_dist", diff_test = False, files = {"dist.js": ":dist"}, ) -# Generate a readonly file in nested readonly directories +# See e2e/write_source_files +write_source_files( + name = "write_dist_executable", + diff_test = False, + executable = True, + files = {"dist_executable.js": ":dist"}, +) + +# Generate a file in nested directories genrule( name = "subdir", outs = ["subdir_test"], cmd = ";".join([ "mkdir -p $@/a/b/c", "echo 'test' > $@/a/b/c/test.txt", - "chmod -R -w $@/*", ]), ) @@ -165,6 +205,15 @@ write_source_files( files = {"subdir_test": ":subdir"}, ) +# Write nested subdirectories to source +# See e2e/write_source_files_subdir_multiple_runs +write_source_files( + name = "write_subdir_executable", + diff_test = False, + executable = True, + files = {"subdir_executable_test": ":subdir"}, +) + # Generate some directories including symlinks genrule( name = "symlinks", diff --git a/lib/tests/write_source_files/e2_dir/e-contained.js b/lib/tests/write_source_files/e2_dir/e-contained.js old mode 100644 new mode 100755 diff --git a/lib/tests/write_source_files/e_dir/e-contained.js b/lib/tests/write_source_files/e_dir/e-contained.js index 2c695c5..ede7a92 100644 --- a/lib/tests/write_source_files/e_dir/e-contained.js +++ b/lib/tests/write_source_files/e_dir/e-contained.js @@ -1 +1 @@ -console.log("e"); +console.log("e*"); diff --git a/lib/tests/write_source_files/es2_dir/subdir/e-contained.js b/lib/tests/write_source_files/es2_dir/subdir/e-contained.js new file mode 100755 index 0000000..ede7a92 --- /dev/null +++ b/lib/tests/write_source_files/es2_dir/subdir/e-contained.js @@ -0,0 +1 @@ +console.log("e*"); diff --git a/lib/tests/write_source_files/es_dir/subdir/e-contained.js b/lib/tests/write_source_files/es_dir/subdir/e-contained.js new file mode 100755 index 0000000..ede7a92 --- /dev/null +++ b/lib/tests/write_source_files/es_dir/subdir/e-contained.js @@ -0,0 +1 @@ +console.log("e*"); diff --git a/lib/write_source_files.bzl b/lib/write_source_files.bzl index 3330e4b..8e20dc9 100644 --- a/lib/write_source_files.bzl +++ b/lib/write_source_files.bzl @@ -5,14 +5,22 @@ load( _write_source_file = "write_source_file", ) +# TODO: Make write_source_file part of the public API +# write_source_file = _write_source_file + def write_source_files( name, files = {}, + executable = False, additional_update_targets = [], suggested_update_target = None, diff_test = True, **kwargs): - """Write to one or more files or folders in the source tree. Stamp out tests that ensure the sources exist and are up to date. + """Write one or more files and/or directories to the source tree. + + By default, `diff_test` targets are generated that ensure the source tree files and/or directories to be written to + are up to date and the rule also checks that all source tree files and/or directories to be written to exist. + To disable the exists check and up-to-date tests set `diff_test` to `False`. Usage: @@ -28,11 +36,16 @@ def write_source_files( ``` To update the source file, run: + ```bash bazel run //:write_foobar ``` - A test will fail if the source file doesn't exist or if it's out of date with instructions on how to create/update it. + The generated `diff_test` will fail if the file is out of date and print out instructions on + how to update it. + + If the file does not exist, Bazel will fail at analysis time and print out instructions on + how to create it. You can declare a tree of generated source file targets: @@ -55,7 +68,8 @@ def write_source_files( bazel run //:write_all ``` - When a file is out of date, you can leave a suggestion to run a target further up in the tree by specifying `suggested_update_target`. E.g., + When a file is out of date, you can leave a suggestion to run a target further up in the tree by specifying `suggested_update_target`. + For example, ```starlark write_source_files( @@ -67,7 +81,7 @@ def write_source_files( ) ``` - A test failure from foo.json being out of date will yield the following message: + A test failure from `foo.json` being out of date will yield the following message: ``` //a/b:c:foo.json is out of date. To update this and other generated files, run: @@ -79,17 +93,32 @@ def write_source_files( bazel run //a/b/c:write_foo ``` - If you have many sources that you want to update as a group, we recommend wrapping write_source_files in a macro that defaults `suggested_update_target` to the umbrella update target. - - NOTE: If you run formatters or linters on your codebase, it is advised that you exclude/ignore the outputs of this rule from those formatters/linters so as to avoid causing collisions and failing tests. + If you have many `write_source_files` targets that you want to update as a group, we recommend wrapping + `write_source_files` in a macro that defaults `suggested_update_target` to the umbrella update target. + + NOTE: If you run formatters or linters on your codebase, it is advised that you exclude/ignore the outputs of this + rule from those formatters/linters so as to avoid causing collisions and failing tests. Args: - 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_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 - diff_test: (Optional) Generate a test target to check that the source file(s) exist and are up to date with the generated files(s). + name: Name of the runnable target that creates or updates the source tree files and/or directories. + + files: A dict where the keys are files or directories in the source tree to write to and the values are labels + pointing to the desired content, typically file or directory outputs of other targets. + + Source tree files and directories must be within the same bazel package as the target. + + executable: Whether source tree files written should be made executable. + + This applies to all source tree files written by this target. This attribute is not propagated to `additional_update_targets`. + + To set different executable permissions on different source tree files use multiple `write_source_files` targets. + + additional_update_targets: List of other `write_source_files` or other executable updater targets to call in the same run. + + suggested_update_target: Label of the `write_source_files` target to suggest running when files are out of date. + + diff_test: Test that the source tree files and/or directories exist and are up to date. + **kwargs: Other common named parameters such as `tags` or `visibility` """ @@ -113,6 +142,7 @@ def write_source_files( name = update_target_name, in_file = in_file, out_file = out_file, + executable = executable, additional_update_targets = additional_update_targets if single_update_target else [], suggested_update_target = this_suggested_update_target, diff_test = diff_test,