From 5b3b7d7794327db560183e6f552a9fc3ac12320a Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Wed, 10 Jan 2024 15:07:03 -0800 Subject: [PATCH] feat: allow write_source_file(s) to write source files to bazel packages outside of the target's package (#717) --- docs/write_source_files.md | 10 ++++--- lib/private/write_source_file.bzl | 30 +++++++++++++------ lib/tests/BUILD.bazel | 2 ++ lib/tests/a.js | 1 + lib/tests/write_source_files/BUILD.bazel | 16 ++++++++++ .../write_source_files/subpkg/BUILD.bazel | 1 + lib/tests/write_source_files/subpkg/a.js | 1 + .../write_source_file_test.bzl | 3 +- lib/write_source_files.bzl | 11 +++++-- 9 files changed, 59 insertions(+), 16 deletions(-) create mode 100644 lib/tests/a.js create mode 100644 lib/tests/write_source_files/subpkg/BUILD.bazel create mode 100644 lib/tests/write_source_files/subpkg/a.js diff --git a/docs/write_source_files.md b/docs/write_source_files.md index 1a253bfd2..04b10c3a8 100644 --- a/docs/write_source_files.md +++ b/docs/write_source_files.md @@ -8,7 +8,7 @@ Public API for write_source_files
 write_source_file(name, in_file, out_file, executable, additional_update_targets,
-                  suggested_update_target, diff_test, kwargs)
+                  suggested_update_target, diff_test, check_that_out_file_exists, kwargs)
 
Write a file or directory to the source tree. @@ -25,11 +25,12 @@ To disable the exists check and up-to-date test set `diff_test` to `False`. | :------------- | :------------- | :------------- | | name | Name of the runnable target that creates or updates the source tree file or directory. | none | | 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. | None | -| out_file | The file or directory to write to in the source tree. Must be within the same containing Bazel package as this target. | None | +| out_file | The file or directory to write to in the source tree.

The output file or directory must be within the same containing Bazel package as this target if check_that_out_file_exists is True. See check_that_out_file_exists docstring for more info. | None | | executable | Whether source tree file or files within the source tree directory written should be made executable. | False | | additional_update_targets | List of other write_source_files or write_source_file 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. | None | | diff_test | Test that the source tree file or directory exist and is up to date. | True | +| check_that_out_file_exists | Test that the output file exists and print a helpful error message if it doesn't.

If True, the output file or directory must be in the same containing Bazel package as the target since the underlying mechanism for this check is limited to files in the same Bazel package. | True | | kwargs | Other common named parameters such as tags or visibility | none | **RETURNS** @@ -43,7 +44,7 @@ Name of the generated test target if requested, otherwise None.
 write_source_files(name, files, executable, additional_update_targets, suggested_update_target,
-                   diff_test, kwargs)
+                   diff_test, check_that_out_file_exists, kwargs)
 
Write one or more files and/or directories to the source tree. @@ -136,11 +137,12 @@ NOTE: If you run formatters or linters on your codebase, it is advised that you | Name | Description | Default Value | | :------------- | :------------- | :------------- | | 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.

Destination files and directories must be within the same containing Bazel package as this target. | {} | +| 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.

Destination files and directories must be within the same containing Bazel package as this target if check_that_out_file_exists is True. See check_that_out_file_exists docstring for more info. | {} | | 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 write_source_file 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. | None | | diff_test | Test that the source tree files and/or directories exist and are up to date. | True | +| check_that_out_file_exists | Test that each output file exists and print a helpful error message if it doesn't.

If True, destination files and directories must be in the same containing Bazel package as the target since the underlying mechanism for this check is limited to files in the same Bazel package. | True | | kwargs | Other common named parameters such as tags or visibility | none | diff --git a/lib/private/write_source_file.bzl b/lib/private/write_source_file.bzl index f177118e8..8b72681f2 100644 --- a/lib/private/write_source_file.bzl +++ b/lib/private/write_source_file.bzl @@ -20,6 +20,7 @@ def write_source_file( additional_update_targets = [], suggested_update_target = None, diff_test = True, + check_that_out_file_exists = True, **kwargs): """Write a file or directory to the source tree. @@ -34,7 +35,10 @@ def write_source_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 containing Bazel package as this target. + out_file: The file or directory to write to in the source tree. + + The output file or directory must be within the same containing Bazel package as this target if `check_that_out_file_exists` is `True`. + See `check_that_out_file_exists` docstring for more info. executable: Whether source tree file or files within the source tree directory written should be made executable. @@ -44,6 +48,11 @@ def write_source_file( diff_test: Test that the source tree file or directory exist and is up to date. + check_that_out_file_exists: Test that the output file exists and print a helpful error message if it doesn't. + + If `True`, the output file or directory must be in the same containing Bazel package as the target since the underlying mechanism + for this check is limited to files in the same Bazel package. + **kwargs: Other common named parameters such as `tags` or `visibility` Returns: @@ -63,14 +72,15 @@ def write_source_file( if utils.is_external_label(out_file): msg = "out file {} must be in the user workspace".format(out_file) fail(msg) - if out_file.package != native.package_name(): - msg = "out file {} (in package '{}') must be a source file within the target's package: '{}'".format(out_file, out_file.package, native.package_name()) + + if check_that_out_file_exists and out_file.package != native.package_name(): + msg = "out file {} (in package '{}') must be a source file within the target's package: '{}'; set check_that_out_file_exists to False to work-around this requirement".format(out_file, out_file.package, native.package_name()) fail(msg) _write_source_file( name = name, in_file = in_file, - out_file = out_file.name if out_file else None, + out_file = str(out_file) if out_file else None, executable = executable, additional_update_targets = additional_update_targets, **kwargs @@ -79,7 +89,7 @@ def write_source_file( if not in_file or not out_file or not diff_test: return None - out_file_missing = _is_file_missing(out_file) + out_file_missing = check_that_out_file_exists and _is_file_missing(out_file) test_target_name = "%s_test" % name if out_file_missing: @@ -304,15 +314,17 @@ if exist "%in%\\*" ( def _write_source_file_impl(ctx): is_windows = ctx.target_platform_has_constraint(ctx.attr._windows_constraint[platform_common.ConstraintValueInfo]) - if ctx.attr.out_file and not ctx.attr.in_file: + out_file = Label(ctx.attr.out_file) if ctx.attr.out_file else None + + if out_file and not ctx.attr.in_file: fail("in_file must be specified if out_file is set") - if ctx.attr.in_file and not ctx.attr.out_file: + if ctx.attr.in_file and not out_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 ctx.attr.in_file and out_file: if DirectoryPathInfo in ctx.attr.in_file: in_path = "/".join([ ctx.attr.in_file[DirectoryPathInfo].directory.short_path, @@ -328,7 +340,7 @@ def _write_source_file_impl(ctx): msg = "in file {} must be a single file or a target that provides a DirectoryPathInfo".format(ctx.attr.in_file.label) fail(msg) - out_path = "/".join([ctx.label.package, ctx.attr.out_file]) if ctx.label.package else ctx.attr.out_file + out_path = "/".join([out_file.package, out_file.name]) if out_file.package else out_file.name paths.append((in_path, out_path)) if is_windows: diff --git a/lib/tests/BUILD.bazel b/lib/tests/BUILD.bazel index deb004e30..5e98a2a8f 100644 --- a/lib/tests/BUILD.bazel +++ b/lib/tests/BUILD.bazel @@ -13,6 +13,8 @@ load(":paths_test.bzl", "paths_test_suite") load(":strings_tests.bzl", "strings_test_suite") load(":utils_test.bzl", "utils_test_suite") +exports_files(["a.js"]) + config_setting( name = "allow_unresolved_symlinks", values = {bazel_features.flags.allow_unresolved_symlinks: "true"}, diff --git a/lib/tests/a.js b/lib/tests/a.js new file mode 100644 index 000000000..7b2a34601 --- /dev/null +++ b/lib/tests/a.js @@ -0,0 +1 @@ +console.log("a"); diff --git a/lib/tests/write_source_files/BUILD.bazel b/lib/tests/write_source_files/BUILD.bazel index 3ceabd77d..fca6e1f6b 100644 --- a/lib/tests/write_source_files/BUILD.bazel +++ b/lib/tests/write_source_files/BUILD.bazel @@ -204,3 +204,19 @@ write_source_files( "skylib_LICENSE": "@bazel_skylib//:LICENSE", }, ) + +# Test that we can write to a sub package +write_source_file_test( + name = "a_subpkg_test", + check_that_out_file_exists = False, + in_file = ":a-desired", + out_file = "//lib/tests/write_source_files/subpkg:a.js", +) + +# Test that we can write to a parent package +write_source_file_test( + name = "a_parentpkg_test", + check_that_out_file_exists = False, + in_file = ":a-desired", + out_file = "//lib/tests:a.js", +) diff --git a/lib/tests/write_source_files/subpkg/BUILD.bazel b/lib/tests/write_source_files/subpkg/BUILD.bazel new file mode 100644 index 000000000..ce07f4f05 --- /dev/null +++ b/lib/tests/write_source_files/subpkg/BUILD.bazel @@ -0,0 +1 @@ +exports_files(["a.js"]) diff --git a/lib/tests/write_source_files/subpkg/a.js b/lib/tests/write_source_files/subpkg/a.js new file mode 100644 index 000000000..7b2a34601 --- /dev/null +++ b/lib/tests/write_source_files/subpkg/a.js @@ -0,0 +1 @@ +console.log("a"); diff --git a/lib/tests/write_source_files/write_source_file_test.bzl b/lib/tests/write_source_files/write_source_file_test.bzl index b96252cea..0bebe6c13 100644 --- a/lib/tests/write_source_files/write_source_file_test.bzl +++ b/lib/tests/write_source_files/write_source_file_test.bzl @@ -172,7 +172,7 @@ _write_source_file_test = rule( test = True, ) -def write_source_file_test(name, in_file, out_file): +def write_source_file_test(name, in_file, out_file, check_that_out_file_exists = True): """Stamp a write_source_files executable and a test to run against it""" _write_source_file( @@ -180,6 +180,7 @@ def write_source_file_test(name, in_file, out_file): in_file = in_file, out_file = out_file, diff_test = False, + check_that_out_file_exists = check_that_out_file_exists, ) # Note that for testing we update the source files in the sandbox, diff --git a/lib/write_source_files.bzl b/lib/write_source_files.bzl index f93a09d03..bd2d6e04f 100644 --- a/lib/write_source_files.bzl +++ b/lib/write_source_files.bzl @@ -12,6 +12,7 @@ def write_source_files( additional_update_targets = [], suggested_update_target = None, diff_test = True, + check_that_out_file_exists = True, **kwargs): """Write one or more files and/or directories to the source tree. @@ -102,7 +103,8 @@ def write_source_files( 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. - Destination files and directories must be within the same containing Bazel package as this target. + Destination files and directories must be within the same containing Bazel package as this target if + `check_that_out_file_exists` is True. See `check_that_out_file_exists` docstring for more info. executable: Whether source tree files written should be made executable. @@ -116,6 +118,11 @@ def write_source_files( diff_test: Test that the source tree files and/or directories exist and are up to date. + check_that_out_file_exists: Test that each output file exists and print a helpful error message if it doesn't. + + If `True`, destination files and directories must be in the same containing Bazel package as the target since the underlying + mechanism for this check is limited to files in the same Bazel package. + **kwargs: Other common named parameters such as `tags` or `visibility` """ @@ -143,6 +150,7 @@ def write_source_files( additional_update_targets = additional_update_targets if single_update_target else [], suggested_update_target = this_suggested_update_target, diff_test = diff_test, + check_that_out_file_exists = check_that_out_file_exists, **kwargs ) @@ -162,7 +170,6 @@ def write_source_files( name = name, additional_update_targets = update_targets + additional_update_targets, suggested_update_target = suggested_update_target, - diff_test = False, **kwargs )