From bda5c632beeec53b58e9b8d23ac166e9091d1e93 Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Tue, 15 Mar 2022 17:36:22 -0700 Subject: [PATCH] feat: replace default_info_files with output_files which adds output_group attribute (#50) --- docs/BUILD.bazel | 4 +- docs/default_info_files.md | 49 ------------- docs/directory_path.md | 12 ++-- docs/output_files.md | 51 ++++++++++++++ lib/BUILD.bazel | 6 +- lib/default_info_files.bzl | 11 --- lib/output_files.bzl | 11 +++ lib/private/BUILD.bazel | 8 +-- lib/private/default_info_files.bzl | 70 ------------------- lib/private/directory_path.bzl | 14 ++-- lib/private/output_files.bzl | 87 ++++++++++++++++++++++++ lib/tests/default_output_gen.bzl | 30 -------- lib/tests/generate_outputs.bzl | 34 +++++++++ lib/tests/write_source_files/BUILD.bazel | 40 +++++++++-- lib/tests/write_source_files/g.js | 1 + lib/tests/write_source_files/g2.js | 1 + 16 files changed, 244 insertions(+), 185 deletions(-) delete mode 100644 docs/default_info_files.md create mode 100644 docs/output_files.md delete mode 100644 lib/default_info_files.bzl create mode 100644 lib/output_files.bzl delete mode 100644 lib/private/default_info_files.bzl create mode 100644 lib/private/output_files.bzl delete mode 100644 lib/tests/default_output_gen.bzl create mode 100644 lib/tests/generate_outputs.bzl create mode 100644 lib/tests/write_source_files/g.js create mode 100644 lib/tests/write_source_files/g2.js diff --git a/docs/BUILD.bazel b/docs/BUILD.bazel index dca1bd950..7ef3c1895 100644 --- a/docs/BUILD.bazel +++ b/docs/BUILD.bazel @@ -49,8 +49,8 @@ stardoc_with_diff_test( ) stardoc_with_diff_test( - name = "default_info_files", - bzl_library_target = "//lib:default_info_files", + name = "output_files", + bzl_library_target = "//lib:output_files", ) update_docs() diff --git a/docs/default_info_files.md b/docs/default_info_files.md deleted file mode 100644 index 20d30814a..000000000 --- a/docs/default_info_files.md +++ /dev/null @@ -1,49 +0,0 @@ - - -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 6e6b484a2..e2c1356a9 100644 --- a/docs/directory_path.md +++ b/docs/directory_path.md @@ -50,7 +50,7 @@ Joins a label pointing to a TreeArtifact with a path nested within that director ## make_directory_path
-make_directory_path(name, directory, path)
+make_directory_path(name, directory, path, kwargs)
 
Helper function to generate a directory_path target and return its label. @@ -60,9 +60,10 @@ Helper function to generate a directory_path target and return its label. | Name | Description | Default Value | | :------------- | :------------- | :------------- | -| name | Unique name for the generated directory_path target. | none | -| directory | a TreeArtifact (ctx.actions.declare_directory) | none | -| path | path relative to the directory | none | +| name | unique name for the generated directory_path target | none | +| directory | directory attribute passed to generated directory_path target | none | +| path | path attribute passed to generated directory_path target | none | +| kwargs | parameters to pass to generated output_files target | none | **RETURNS** @@ -74,7 +75,7 @@ The label `name` ## make_directory_paths
-make_directory_paths(name, dict)
+make_directory_paths(name, dict, kwargs)
 
Helper function to convert a dict of directory to path mappings to directory_path targets and labels. @@ -128,6 +129,7 @@ and the list of targets is returned, | :------------- | :------------- | :------------- | | name | The target name to use for the generated targets & labels.

The names are generated as zero-indexed name + "_" + i | none | | dict | The dictionary of directory keys to path or path list values. | none | +| kwargs | additional parameters to pass to each generated target | none | **RETURNS** diff --git a/docs/output_files.md b/docs/output_files.md new file mode 100644 index 000000000..6790dbc44 --- /dev/null +++ b/docs/output_files.md @@ -0,0 +1,51 @@ + + +A rule that provides file(s) specific via DefaultInfo from a given target's DefaultInfo or OutputGroupInfo + + + + +## output_files + +
+output_files(name, output_group, paths, target)
+
+ +A rule that provides file(s) specific via DefaultInfo from a given target's DefaultInfo or OutputGroupInfo + +**ATTRIBUTES** + + +| Name | Description | Type | Mandatory | Default | +| :------------- | :------------- | :------------- | :------------- | :------------- | +| name | A unique name for this target. | Name | required | | +| output_group | if set, we look in the specified output group for paths instead of DefaultInfo | String | optional | "" | +| paths | the paths of the file(s), relative to their roots, to provide via DefaultInfo from the given target's DefaultInfo or OutputGroupInfo | List of strings | required | | +| target | the target to look in for requested paths in its' DefaultInfo or OutputGroupInfo | Label | required | | + + + + +## make_output_files + +
+make_output_files(name, target, paths, kwargs)
+
+ +Helper function to generate a output_files target and return its label. + +**PARAMETERS** + + +| Name | Description | Default Value | +| :------------- | :------------- | :------------- | +| name | unique name for the generated output_files target | none | +| target | target attribute passed to generated output_files target | none | +| paths | paths attribute passed to generated output_files target | none | +| kwargs | parameters to pass to generated output_files target | none | + +**RETURNS** + +The label `name` + + diff --git a/lib/BUILD.bazel b/lib/BUILD.bazel index 150115dbf..c2db6cd1a 100644 --- a/lib/BUILD.bazel +++ b/lib/BUILD.bazel @@ -64,10 +64,10 @@ bzl_library( ) bzl_library( - name = "default_info_files", - srcs = ["default_info_files.bzl"], + name = "output_files", + srcs = ["output_files.bzl"], visibility = ["//visibility:public"], - deps = ["//lib/private:default_info_files"], + deps = ["//lib/private:output_files"], ) bzl_library( diff --git a/lib/default_info_files.bzl b/lib/default_info_files.bzl deleted file mode 100644 index 5eb9a1ad8..000000000 --- a/lib/default_info_files.bzl +++ /dev/null @@ -1,11 +0,0 @@ -"""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/output_files.bzl b/lib/output_files.bzl new file mode 100644 index 000000000..a32915557 --- /dev/null +++ b/lib/output_files.bzl @@ -0,0 +1,11 @@ +"""A rule that provides file(s) specific via DefaultInfo from a given target's DefaultInfo or OutputGroupInfo +""" + +load( + "//lib/private:output_files.bzl", + _make_output_files = "make_output_files", + _output_files = "output_files", +) + +output_files = _output_files +make_output_files = _make_output_files diff --git a/lib/private/BUILD.bazel b/lib/private/BUILD.bazel index 66436f097..88cbb7bd6 100644 --- a/lib/private/BUILD.bazel +++ b/lib/private/BUILD.bazel @@ -10,8 +10,8 @@ bzl_library( srcs = ["copy_to_directory.bzl"], visibility = ["//lib:__subpackages__"], deps = [ - ":default_info_files", ":directory_path", + ":output_files", ":paths", "@bazel_skylib//lib:paths", ], @@ -66,8 +66,8 @@ bzl_library( srcs = ["write_source_file.bzl"], visibility = ["//lib:__subpackages__"], deps = [ - ":default_info_files", ":directory_path", + ":output_files", "//lib:utils", ], ) @@ -86,8 +86,8 @@ bzl_library( ) bzl_library( - name = "default_info_files", - srcs = ["default_info_files.bzl"], + name = "output_files", + srcs = ["output_files.bzl"], visibility = ["//lib:__subpackages__"], deps = ["//lib:utils"], ) diff --git a/lib/private/default_info_files.bzl b/lib/private/default_info_files.bzl deleted file mode 100644 index 849804d27..000000000 --- a/lib/private/default_info_files.bzl +++ /dev/null @@ -1,70 +0,0 @@ -"""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/directory_path.bzl b/lib/private/directory_path.bzl index 6ba7c9b8d..1e7c650d6 100644 --- a/lib/private/directory_path.bzl +++ b/lib/private/directory_path.bzl @@ -36,13 +36,14 @@ Otherwise there is no way to give a Bazel label for it.""", provides = [DirectoryPathInfo], ) -def make_directory_path(name, directory, path): +def make_directory_path(name, directory, path, **kwargs): """Helper function to generate a directory_path target and return its label. Args: - name: Unique name for the generated `directory_path` target. - directory: a TreeArtifact (ctx.actions.declare_directory) - path: path relative to the directory + name: unique name for the generated `directory_path` target + directory: `directory` attribute passed to generated `directory_path` target + path: `path` attribute passed to generated `directory_path` target + **kwargs: parameters to pass to generated `output_files` target Returns: The label `name` @@ -51,10 +52,11 @@ def make_directory_path(name, directory, path): name = name, directory = directory, path = path, + **kwargs ) return _to_label(name) -def make_directory_paths(name, dict): +def make_directory_paths(name, dict, **kwargs): """Helper function to convert a dict of directory to path mappings to directory_path targets and labels. For example, @@ -104,6 +106,7 @@ def make_directory_paths(name, dict): The names are generated as zero-indexed `name + "_" + i` dict: The dictionary of directory keys to path or path list values. + **kwargs: additional parameters to pass to each generated target Returns: The label of the generated `directory_path` targets named `name + "_" + i` @@ -124,5 +127,6 @@ def make_directory_paths(name, dict): "%s_%d" % (name, i), directory, path, + **kwargs )) return labels diff --git a/lib/private/output_files.bzl b/lib/private/output_files.bzl new file mode 100644 index 000000000..4706ff257 --- /dev/null +++ b/lib/private/output_files.bzl @@ -0,0 +1,87 @@ +"""output_files implementation +""" + +load("//lib:utils.bzl", _to_label = "to_label") + +def _output_files(ctx): + files = [] + files_depset = depset() + if ctx.attr.output_group: + if OutputGroupInfo not in ctx.attr.target: + msg = "%s output_group is specified but %s does not provide an OutputGroupInfo" % (ctx.attr.output_group, ctx.attr.target) + fail(msg) + if ctx.attr.output_group not in ctx.attr.target[OutputGroupInfo]: + msg = "%s output_group is specified but %s does not provide this output group" % (ctx.attr.output_group, ctx.attr.target) + fail(msg) + files_depset = ctx.attr.target[OutputGroupInfo][ctx.attr.output_group] + else: + files_depset = ctx.attr.target[DefaultInfo].files + for path in ctx.attr.paths: + file = _find_short_path_in_files_depset(files_depset, path) + if not file: + if ctx.attr.output_group: + msg = "%s file not found within the %s output group of %s" % (path, ctx.attr.output_group, ctx.attr.target) + else: + msg = "%s file not found within the DefaultInfo of %s" % (path, ctx.attr.target) + fail(msg) + files.append(file) + return [DefaultInfo( + files = depset(direct = files), + runfiles = ctx.runfiles(files = files), + )] + +output_files = rule( + doc = "A rule that provides file(s) specific via DefaultInfo from a given target's DefaultInfo or OutputGroupInfo", + implementation = _output_files, + attrs = { + "target": attr.label( + doc = "the target to look in for requested paths in its' DefaultInfo or OutputGroupInfo", + mandatory = True, + ), + "paths": attr.string_list( + doc = "the paths of the file(s), relative to their roots, to provide via DefaultInfo from the given target's DefaultInfo or OutputGroupInfo", + mandatory = True, + allow_empty = False, + ), + "output_group": attr.string( + doc = "if set, we look in the specified output group for paths instead of DefaultInfo", + ), + }, + provides = [DefaultInfo], +) + +def make_output_files(name, target, paths, **kwargs): + """Helper function to generate a output_files target and return its label. + + Args: + name: unique name for the generated `output_files` target + target: `target` attribute passed to generated `output_files` target + paths: `paths` attribute passed to generated `output_files` target + **kwargs: parameters to pass to generated `output_files` target + + Returns: + The label `name` + """ + output_files( + name = name, + target = target, + paths = paths, + **kwargs + ) + return _to_label(name) + +def _find_short_path_in_files_depset(files_depset, short_path): + """Helper function find a file in a DefaultInfo by short path + + Args: + files_depset: a depset + short_path: the short path (path relative to root) to search for + + Returns: + The File if found else None + """ + if files_depset: + for file in files_depset.to_list(): + if file.short_path == short_path: + return file + return None diff --git a/lib/tests/default_output_gen.bzl b/lib/tests/default_output_gen.bzl deleted file mode 100644 index 2fc9a43e7..000000000 --- a/lib/tests/default_output_gen.bzl +++ /dev/null @@ -1,30 +0,0 @@ -"""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/generate_outputs.bzl b/lib/tests/generate_outputs.bzl new file mode 100644 index 000000000..bba5a0430 --- /dev/null +++ b/lib/tests/generate_outputs.bzl @@ -0,0 +1,34 @@ +"""A simple rule that generates provides a DefaultOutput with some files""" + +def _impl(ctx): + if len(ctx.attr.output_files) != len(ctx.attr.output_contents): + fail("Number of output_files must match number of output_contents") + outputs = [] + for i, file in enumerate(ctx.attr.output_files): + content = ctx.attr.output_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) + + provide = [] + if ctx.attr.output_group: + kwargs = {ctx.attr.output_group: depset(outputs)} + provide.append(OutputGroupInfo(**kwargs)) + else: + provide.append(DefaultInfo(files = depset(outputs))) + return provide + +generate_outputs = rule( + implementation = _impl, + provides = [DefaultInfo], + attrs = { + "output_files": attr.string_list(), + "output_contents": attr.string_list(), + "output_group": attr.string(), + }, +) diff --git a/lib/tests/write_source_files/BUILD.bazel b/lib/tests/write_source_files/BUILD.bazel index ec4da553a..4586eefaf 100644 --- a/lib/tests/write_source_files/BUILD.bazel +++ b/lib/tests/write_source_files/BUILD.bazel @@ -1,9 +1,9 @@ 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/tests:generate_outputs.bzl", "generate_outputs") 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") +load("//lib:output_files.bzl", "output_files") genrule( name = "a-desired", @@ -11,20 +11,20 @@ genrule( cmd = "echo 'console.log(\"a*\");' > $@", ) -default_output_gen( +generate_outputs( name = "b_c-desired", - out_contents = [ + output_contents = [ """console.log(\"b*\"); """, "not used!", ], - out_files = [ + output_files = [ "b-desired.js", "c-desired.js", ], ) -default_info_files( +output_files( name = "b-desired", paths = ["%s/b-desired.js" % package_name()], target = ":b_c-desired", @@ -61,6 +61,27 @@ directory_path( path = "f-contained.js", ) +generate_outputs( + name = "g_h-desired", + output_contents = [ + """console.log(\"g*\"); +""", + "not used!", + ], + output_files = [ + "g-desired.js", + "h-desired.js", + ], + output_group = "gh_output_group", +) + +output_files( + name = "g-desired", + output_group = "gh_output_group", + paths = ["%s/g-desired.js" % package_name()], + target = ":g_h-desired", +) + write_source_file_test( name = "write_to_source_files_a_test", in_file = ":a-desired", @@ -79,6 +100,12 @@ write_source_file_test( out_file = "f.js", ) +write_source_file_test( + name = "write_to_source_files_g_test", + in_file = ":g-desired", + out_file = "g.js", +) + write_source_files( name = "macro_smoke_test", additional_update_targets = [ @@ -89,5 +116,6 @@ write_source_files( "b2.js": ":b-desired", "e2_dir": ":e_dir-desired", "f2.js": ":f-desired", + "g2.js": ":g-desired", }, ) diff --git a/lib/tests/write_source_files/g.js b/lib/tests/write_source_files/g.js new file mode 100644 index 000000000..0e1da4703 --- /dev/null +++ b/lib/tests/write_source_files/g.js @@ -0,0 +1 @@ +console.log("g"); diff --git a/lib/tests/write_source_files/g2.js b/lib/tests/write_source_files/g2.js new file mode 100644 index 000000000..f6c481c3b --- /dev/null +++ b/lib/tests/write_source_files/g2.js @@ -0,0 +1 @@ +console.log("g*");