From 8663e1d2c494a9e136c8c934efe636fc14361ea8 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Fri, 10 Dec 2021 01:13:39 -0500 Subject: [PATCH 1/8] Add .changes file to pkg_deb DefaultInfo - Add tests for that - Improve the example about finding output file paths to show how to get the implicit outputs like the changes file. Fixes #477 --- .bazelignore | 4 ++ examples/where_is_my_output/BUILD | 15 +++++ examples/where_is_my_output/README.md | 37 ++++++++++- ...w_deb_outputs.bzl => show_all_outputs.bzl} | 2 +- pkg/private/deb/deb.bzl | 2 +- tests/deb/BUILD | 10 ++- tests/deb/deb_tests.bzl | 65 ++++++++++++++++++- 7 files changed, 126 insertions(+), 9 deletions(-) rename examples/where_is_my_output/{show_deb_outputs.bzl => show_all_outputs.bzl} (93%) diff --git a/.bazelignore b/.bazelignore index 912ca101..3fb7f4fb 100644 --- a/.bazelignore +++ b/.bazelignore @@ -3,3 +3,7 @@ deb_packages/ examples/ +FUTURE/ +i135/ +i434/ +old_tar/ diff --git a/examples/where_is_my_output/BUILD b/examples/where_is_my_output/BUILD index 848f98f9..2f4605e5 100644 --- a/examples/where_is_my_output/BUILD +++ b/examples/where_is_my_output/BUILD @@ -39,3 +39,18 @@ pkg_deb( package = "mwp", version = "3.14", ) + +# We can also depend just on the .changes file + +filegroup( + name = "the_changes_file", + srcs = [":deb"], + output_group = "changes", +) + +genrule( + name = "use_changes_file", + srcs = [":the_changes_file"], + outs = ["copy_of_changes.txt"], + cmd = "cp $(location :the_changes_file) $@", +) diff --git a/examples/where_is_my_output/README.md b/examples/where_is_my_output/README.md index 7b2f351f..9e3035e5 100644 --- a/examples/where_is_my_output/README.md +++ b/examples/where_is_my_output/README.md @@ -23,7 +23,7 @@ to inspect a target and print exactly what we need. Let's try it: ```shell bazel build :deb -bazel cquery :deb --output=starlark --starlark:file=show_deb_outputs.bzl 2>/dev/null +bazel cquery :deb --output=starlark --starlark:file=show_all_outputs.bzl 2>/dev/null ``` That should produce something like @@ -35,7 +35,7 @@ changes: bazel-out/k8-fastbuild/bin/mwp_3.changes ### How it works -show_deb_outputs.bzl is a Starlark script that must contain a function with the +show_all_outputs.bzl is a Starlark script that must contain a function with the name `format`, that takes a single argument. The argument is typically named target, and is a configured Bazel target, as you might have access to while writing a custom rule. We can inspect its providers and print them in a useful @@ -43,7 +43,7 @@ way. For pkg_deb, there are two files, the .deb file and the .changes, and both are passed along in the rule's OutputGroupInfo provider. This snippet below (from -show_deb_outputs.bzl) prints them. +show_all_outputs.bzl) prints them. ```python def format(target): @@ -64,3 +64,34 @@ def format(target): A full explanation of why this works is beyond the scope of this example. It requires some knowledge of how to write custom Bazel rules. See the Bazel documentation for more information. + +## Using an implicit output as input to another rule. + +Sometimes a rule will create an implicit output that the user does not +explicitly specify as an attribute of the target. The .changes file +from pkg_deb is an example. If we want another rule to depend on an +implicitly created file, we can do that with a filegroup that +specifies the specific output group containing that file. + +In the example below, `:deb` is a rule producing an explicit .deb output +and an implict .changes output. We refer to the .changes file using +the `filegroup` and specifying the desired output group name. Then, +any rule can use this `filegroup` as an input. + +```python + +pkg_deb(name = "deb", ...) + +filegroup( + name = "the_changes_file", + srcs = [":deb"], + output_group = "changes", +) + +genrule( + name = "use_changes_file", + srcs = [":the_changes_file"], + outs = ["copy_of_changes.txt"], + cmd = "cp $(location :the_changes_file) $@", +) +``` diff --git a/examples/where_is_my_output/show_deb_outputs.bzl b/examples/where_is_my_output/show_all_outputs.bzl similarity index 93% rename from examples/where_is_my_output/show_deb_outputs.bzl rename to examples/where_is_my_output/show_all_outputs.bzl index ad78f749..312a4ea4 100644 --- a/examples/where_is_my_output/show_deb_outputs.bzl +++ b/examples/where_is_my_output/show_all_outputs.bzl @@ -16,7 +16,7 @@ # Extract the paths to the various outputs of pkg_deb # # Usage: -# bazel cquery //:debian --output=starlark --starlark:file=show_deb_outputs.bzl +# bazel cquery //:deb --output=starlark --starlark:file=show_all_outputs.bzl # def format(target): diff --git a/pkg/private/deb/deb.bzl b/pkg/private/deb/deb.bzl index fa852239..754f398d 100644 --- a/pkg/private/deb/deb.bzl +++ b/pkg/private/deb/deb.bzl @@ -163,7 +163,7 @@ def _pkg_deb_impl(ctx): return [ OutputGroupInfo(**output_groups), DefaultInfo( - files = depset([output_file]), + files = depset(outputs), runfiles = ctx.runfiles(files = outputs), ), PackageArtifactInfo( diff --git a/tests/deb/BUILD b/tests/deb/BUILD index 488cfc42..0cf5d375 100644 --- a/tests/deb/BUILD +++ b/tests/deb/BUILD @@ -16,7 +16,7 @@ # Tests for pkg_deb specific behavior load("@rules_python//python:defs.bzl", "py_test") -load(":deb_tests.bzl", "package_naming_test") +load(":deb_tests.bzl", "all_files_in_default_info_test", "package_naming_test") load("//pkg:mappings.bzl", "pkg_mklink") load("//pkg:pkg.bzl", "pkg_tar") load("//pkg:deb.bzl", "pkg_deb") @@ -104,8 +104,14 @@ py_test( ], ) +all_files_in_default_info_test( + name = "default_info_test", + expected_basename = "fizzbuzz_test_all", + target_under_test = ":test_deb", +) + package_naming_test( name = "naming_test", - target_under_test = ":test_deb", expected_name = "fizzbuzz_test_all.deb", + target_under_test = ":test_deb", ) diff --git a/tests/deb/deb_tests.bzl b/tests/deb/deb_tests.bzl index 05087049..db66f4f6 100644 --- a/tests/deb/deb_tests.bzl +++ b/tests/deb/deb_tests.bzl @@ -15,6 +15,25 @@ load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") +def assert_contains(env, expected, got, msg = None): + """Asserts that the given `expected` occurs in a set of things. + + Args: + env: The test environment returned by `unittest.begin`. + expected: An expected value. + go: The actual set returned by some computation. + msg: An optional message that will be printed that describes the failure. + If omitted, a default will be used. + """ + if expected in got: + return + expectation_msg = "Expected %s in (%s)" % (expected, got) + if msg: + full_msg = "%s (%s)" % (msg, expectation_msg) + else: + full_msg = expectation_msg + fail(env, full_msg) + def _package_naming_test_impl(ctx): env = analysistest.begin(ctx) target_under_test = analysistest.target_under_test(env) @@ -37,16 +56,58 @@ def _package_naming_test_impl(ctx): if ctx.attr.expected_name: asserts.equals( env, - deb_path.split("/")[-1], # basename(path) ctx.attr.expected_name, + deb_path.split("/")[-1], # basename(path) "Deb package file name is not correct", ) return analysistest.end(env) - package_naming_test = analysistest.make( _package_naming_test_impl, attrs = { "expected_name": attr.string(), }, ) + +def _all_files_in_default_info_impl(ctx): + env = analysistest.begin(ctx) + target_under_test = analysistest.target_under_test(env) + di = target_under_test[DefaultInfo] + files = di.files.to_list() + file_names = ",".join([f.basename for f in files]) + asserts.equals( + env, + 3, + len(files), + file_names, + ) + + expect_output = target_under_test.label.name + ".deb" + assert_contains( + env, + expect_output, + file_names, + ) + + expect_output = ctx.attr.expected_basename + ".deb" + assert_contains( + env, + expect_output, + file_names, + ) + + expect_output = ctx.attr.expected_basename + ".changes" + assert_contains( + env, + expect_output, + file_names, + ) + + return analysistest.end(env) + +all_files_in_default_info_test = analysistest.make( + _all_files_in_default_info_impl, + attrs = { + "expected_basename": attr.string(), + }, +) From e30c2506560d415f1c297b5e8a58b4758797e982 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Fri, 10 Dec 2021 01:31:22 -0500 Subject: [PATCH 2/8] fix-ignore --- .bazelignore | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.bazelignore b/.bazelignore index 3fb7f4fb..912ca101 100644 --- a/.bazelignore +++ b/.bazelignore @@ -3,7 +3,3 @@ deb_packages/ examples/ -FUTURE/ -i135/ -i434/ -old_tar/ From 21298cd436f8e6b3ea6f48d8448e13027e62da15 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Fri, 10 Dec 2021 01:36:14 -0500 Subject: [PATCH 3/8] local change to my .bazelignore --- .bazelignore | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.bazelignore b/.bazelignore index 912ca101..0fc25441 100644 --- a/.bazelignore +++ b/.bazelignore @@ -3,3 +3,8 @@ deb_packages/ examples/ + +FUTURE/ +i135/ +i434/ +old_tar/ From ab76c8dc22df1a78ad54143b6662bec8bea4af86 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Wed, 15 Dec 2021 21:00:22 -0500 Subject: [PATCH 4/8] snorple --- .bazelignore | 5 ----- 1 file changed, 5 deletions(-) diff --git a/.bazelignore b/.bazelignore index 0fc25441..912ca101 100644 --- a/.bazelignore +++ b/.bazelignore @@ -3,8 +3,3 @@ deb_packages/ examples/ - -FUTURE/ -i135/ -i434/ -old_tar/ From 8d69e6caf9a726c06e757c8f4a24df8a64e3155a Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Tue, 21 Dec 2021 23:19:28 -0500 Subject: [PATCH 5/8] lwjrwelkrj --- tests/deb/deb_tests.bzl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/deb/deb_tests.bzl b/tests/deb/deb_tests.bzl index db66f4f6..ccdbd2d9 100644 --- a/tests/deb/deb_tests.bzl +++ b/tests/deb/deb_tests.bzl @@ -16,12 +16,12 @@ load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") def assert_contains(env, expected, got, msg = None): - """Asserts that the given `expected` occurs in a set of things. + """Asserts that `expected` occurs in the iterable (`list`, `dict`) `got`. Args: env: The test environment returned by `unittest.begin`. expected: An expected value. - go: The actual set returned by some computation. + got: The actual set returned by some computation. msg: An optional message that will be printed that describes the failure. If omitted, a default will be used. """ @@ -32,7 +32,7 @@ def assert_contains(env, expected, got, msg = None): full_msg = "%s (%s)" % (msg, expectation_msg) else: full_msg = expectation_msg - fail(env, full_msg) + analysistest.fail(env, full_msg) def _package_naming_test_impl(ctx): env = analysistest.begin(ctx) @@ -74,7 +74,7 @@ def _all_files_in_default_info_impl(ctx): target_under_test = analysistest.target_under_test(env) di = target_under_test[DefaultInfo] files = di.files.to_list() - file_names = ",".join([f.basename for f in files]) + file_names = [f.basename for f in files] asserts.equals( env, 3, From ab62474082d721e3728a63ee3fc065323538d650 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Thu, 13 Jan 2022 17:45:59 -0500 Subject: [PATCH 6/8] revert push to DefaultInfo, leaving just the example --- pkg/private/deb/deb.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/private/deb/deb.bzl b/pkg/private/deb/deb.bzl index 754f398d..fa852239 100644 --- a/pkg/private/deb/deb.bzl +++ b/pkg/private/deb/deb.bzl @@ -163,7 +163,7 @@ def _pkg_deb_impl(ctx): return [ OutputGroupInfo(**output_groups), DefaultInfo( - files = depset(outputs), + files = depset([output_file]), runfiles = ctx.runfiles(files = outputs), ), PackageArtifactInfo( From 852182fadf4a563bba0f4092df95333348f4f96f Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Thu, 13 Jan 2022 18:07:50 -0500 Subject: [PATCH 7/8] revert now unneeded tests --- tests/deb/BUILD | 10 ++----- tests/deb/deb_tests.bzl | 65 ++--------------------------------------- 2 files changed, 4 insertions(+), 71 deletions(-) diff --git a/tests/deb/BUILD b/tests/deb/BUILD index 0cf5d375..488cfc42 100644 --- a/tests/deb/BUILD +++ b/tests/deb/BUILD @@ -16,7 +16,7 @@ # Tests for pkg_deb specific behavior load("@rules_python//python:defs.bzl", "py_test") -load(":deb_tests.bzl", "all_files_in_default_info_test", "package_naming_test") +load(":deb_tests.bzl", "package_naming_test") load("//pkg:mappings.bzl", "pkg_mklink") load("//pkg:pkg.bzl", "pkg_tar") load("//pkg:deb.bzl", "pkg_deb") @@ -104,14 +104,8 @@ py_test( ], ) -all_files_in_default_info_test( - name = "default_info_test", - expected_basename = "fizzbuzz_test_all", - target_under_test = ":test_deb", -) - package_naming_test( name = "naming_test", - expected_name = "fizzbuzz_test_all.deb", target_under_test = ":test_deb", + expected_name = "fizzbuzz_test_all.deb", ) diff --git a/tests/deb/deb_tests.bzl b/tests/deb/deb_tests.bzl index ccdbd2d9..05087049 100644 --- a/tests/deb/deb_tests.bzl +++ b/tests/deb/deb_tests.bzl @@ -15,25 +15,6 @@ load("@bazel_skylib//lib:unittest.bzl", "analysistest", "asserts") -def assert_contains(env, expected, got, msg = None): - """Asserts that `expected` occurs in the iterable (`list`, `dict`) `got`. - - Args: - env: The test environment returned by `unittest.begin`. - expected: An expected value. - got: The actual set returned by some computation. - msg: An optional message that will be printed that describes the failure. - If omitted, a default will be used. - """ - if expected in got: - return - expectation_msg = "Expected %s in (%s)" % (expected, got) - if msg: - full_msg = "%s (%s)" % (msg, expectation_msg) - else: - full_msg = expectation_msg - analysistest.fail(env, full_msg) - def _package_naming_test_impl(ctx): env = analysistest.begin(ctx) target_under_test = analysistest.target_under_test(env) @@ -56,58 +37,16 @@ def _package_naming_test_impl(ctx): if ctx.attr.expected_name: asserts.equals( env, - ctx.attr.expected_name, deb_path.split("/")[-1], # basename(path) + ctx.attr.expected_name, "Deb package file name is not correct", ) return analysistest.end(env) + package_naming_test = analysistest.make( _package_naming_test_impl, attrs = { "expected_name": attr.string(), }, ) - -def _all_files_in_default_info_impl(ctx): - env = analysistest.begin(ctx) - target_under_test = analysistest.target_under_test(env) - di = target_under_test[DefaultInfo] - files = di.files.to_list() - file_names = [f.basename for f in files] - asserts.equals( - env, - 3, - len(files), - file_names, - ) - - expect_output = target_under_test.label.name + ".deb" - assert_contains( - env, - expect_output, - file_names, - ) - - expect_output = ctx.attr.expected_basename + ".deb" - assert_contains( - env, - expect_output, - file_names, - ) - - expect_output = ctx.attr.expected_basename + ".changes" - assert_contains( - env, - expect_output, - file_names, - ) - - return analysistest.end(env) - -all_files_in_default_info_test = analysistest.make( - _all_files_in_default_info_impl, - attrs = { - "expected_basename": attr.string(), - }, -) From 171be5d1c806cb6f44693db4191ef2beeefc54d3 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Fri, 28 Jan 2022 09:15:20 -0500 Subject: [PATCH 8/8] reflow some paragraph text --- examples/where_is_my_output/README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/examples/where_is_my_output/README.md b/examples/where_is_my_output/README.md index 9e3035e5..9b4e16dd 100644 --- a/examples/where_is_my_output/README.md +++ b/examples/where_is_my_output/README.md @@ -68,15 +68,15 @@ documentation for more information. ## Using an implicit output as input to another rule. Sometimes a rule will create an implicit output that the user does not -explicitly specify as an attribute of the target. The .changes file -from pkg_deb is an example. If we want another rule to depend on an -implicitly created file, we can do that with a filegroup that -specifies the specific output group containing that file. +explicitly specify as an attribute of the target. The .changes file from +pkg_deb is an example. If we want another rule to depend on an implicitly +created file, we can do that with a filegroup that specifies the specific +output group containing that file. In the example below, `:deb` is a rule producing an explicit .deb output -and an implict .changes output. We refer to the .changes file using -the `filegroup` and specifying the desired output group name. Then, -any rule can use this `filegroup` as an input. +and an implict .changes output. We refer to the .changes file using the +`filegroup` and specifying the desired output group name. Then, any rule +can use this `filegroup` as an input. ```python