From a0b9b27c7ac3147a4ef3e614ceb5fdad44b7da19 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Sat, 27 Jun 2020 11:56:35 -0400 Subject: [PATCH 01/13] PackageNamingInfo PoC: start --- pkg/pkg.bzl | 41 +++++++++++++++++++++++----- pkg/providers.bzl | 31 ++++++++++++++++++++++ pkg/tests/BUILD | 50 ++++++++++++++++++++++------------- pkg/tests/build_test_tar.sh | 4 +++ pkg/tests/my_package_name.bzl | 25 ++++++++++++++++++ 5 files changed, 127 insertions(+), 24 deletions(-) create mode 100644 pkg/providers.bzl create mode 100644 pkg/tests/my_package_name.bzl diff --git a/pkg/pkg.bzl b/pkg/pkg.bzl index 679f3d83..191e6439 100644 --- a/pkg/pkg.bzl +++ b/pkg/pkg.bzl @@ -14,6 +14,7 @@ """Rules for manipulation of various packaging.""" load(":path.bzl", "compute_data_path", "dest_path") +load(":providers.bzl", "PackageArtifactInfo", "PackageNamingInfo") # Filetype to restrict inputs tar_filetype = [".tar", ".tar.gz", ".tgz", ".tar.xz", ".tar.bz2"] @@ -37,8 +38,20 @@ def _pkg_tar_impl(ctx): # Files needed by rule implementation at runtime files = [] + output_name = ctx.label.name + "." + ctx.attr.extension + if ctx.attr.package_file_name: + naming_info = ctx.attr.naming_info[PackageNamingInfo] + if naming_info: + # This is obviously wrong. We need a templater that takes a dict. + output_name = ctx.attr.package_file_name.replace( + "${cpu}", naming_info.values['cpu']).replace( + "${opt}", naming_info.values['opt']) + print("======= Writing: %s", output_name) + output_file = ctx.actions.declare_file(output_name) + # print("======= Writing: %s", output_file.path) + # Compute the relative path - data_path = compute_data_path(ctx.outputs.out, ctx.attr.strip_prefix) + data_path = compute_data_path(output_file, ctx.attr.strip_prefix) # Find a list of path remappings to apply. remap_paths = ctx.attr.remap_paths @@ -54,7 +67,7 @@ def _pkg_tar_impl(ctx): # Start building the arguments. args = [ - "--output=" + ctx.outputs.out.path, + "--output=" + output_file.path, package_dir_arg, "--mode=" + ctx.attr.mode, "--owner=" + ctx.attr.owner, @@ -131,7 +144,7 @@ def _pkg_tar_impl(ctx): inputs = file_inputs + ctx.files.deps + files, executable = ctx.executable.build_tar, arguments = ["@" + arg_file.path], - outputs = [ctx.outputs.out], + outputs = [output_file], env = { "LANG": "en_US.UTF-8", "LC_CTYPE": "UTF-8", @@ -140,7 +153,19 @@ def _pkg_tar_impl(ctx): }, use_default_shell_env = True, ) - return OutputGroupInfo(out = [ctx.outputs.out]) + return [ + # WHY a seperate OutputGroup? + # OutputGroupInfo(out = [output_file]), + DefaultInfo( + files = depset([output_file]), + # This is sort of ugly that the object itself must be runfiles. + runfiles = ctx.runfiles(files = [output_file]), + ), + PackageArtifactInfo( + label = ctx.label.name, + file_name = output_name, + ) + ] def _pkg_deb_impl(ctx): """The implementation for the pkg_deb rule.""" @@ -285,9 +310,13 @@ pkg_tar_impl = rule( "include_runfiles": attr.bool(), "empty_dirs": attr.string_list(), "remap_paths": attr.string_dict(), + "package_file_name": attr.string(), + "naming_info": attr.label( + providers = [PackageNamingInfo], + ), # Outputs - "out": attr.output(), + "out": attr.output(mandatory=False), # Implicit dependencies. "build_tar": attr.label( @@ -314,7 +343,7 @@ def pkg_tar(**kwargs): kwargs["srcs"] = kwargs.pop("files") extension = kwargs.get("extension") or "tar" pkg_tar_impl( - out = kwargs["name"] + "." + extension, + # out = kwargs["name"] + "." + extension, **kwargs ) diff --git a/pkg/providers.bzl b/pkg/providers.bzl new file mode 100644 index 00000000..50b9b637 --- /dev/null +++ b/pkg/providers.bzl @@ -0,0 +1,31 @@ +# Copyright 2020 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. + +"""Packaging related providers.""" + + +PackageArtifactInfo = provider( + doc = """Metadata about a package artifact.""", + fields = { + "label": "Label which produced it", + "file_name": "The file name of the artifact.", + }, +) + +PackageNamingInfo = provider( + doc = """Metadata about a package artifact.""", + fields = { + "values": "Dict of name/value pairs", + }, +) diff --git a/pkg/tests/BUILD b/pkg/tests/BUILD index 30c85d00..db622299 100644 --- a/pkg/tests/BUILD +++ b/pkg/tests/BUILD @@ -5,6 +5,10 @@ load("//:pkg.bzl", "pkg_deb", "pkg_tar", "pkg_zip") load("//:rpm.bzl", "pkg_rpm") load("@rules_python//python:defs.bzl", "py_test") +load(":my_package_name.bzl", "my_package_naming") + +my_package_naming(name = "naming_info") + genrule( name = "generate_files", outs = [ @@ -14,6 +18,15 @@ genrule( cmd = "for i in $(OUTS); do echo 1 >$$i; done", ) +pkg_tar( + name = "test_naming", + srcs = [ + ":BUILD", + ], + package_file_name = "test_naming-${cpu}-${opt}.tar", + naming_info = ":naming_info", +) + pkg_tar( name = "test_tar_package_dir", srcs = [ @@ -182,7 +195,7 @@ pkg_deb( "/etc/other", ], config = ":testdata/config", - data = ":test-tar-gz.tar.gz", + data = ":test-tar-gz", depends = [ "dep1", "dep2", @@ -217,7 +230,7 @@ pkg_deb( "/etc/other", ], config = ":testdata/config", - data = ":test-tar-gz.tar.gz", + data = ":test-tar-gz", depends = [ "dep1", "dep2", @@ -242,6 +255,7 @@ pkg_deb( ], build_tar = "//:build_tar", extension = "tar%s" % ext, + # extension = ext[1:], mode = "0644", modes = {"usr/titi": "0755"}, owner = "42.24", @@ -366,22 +380,22 @@ sh_test( data = [ "testenv.sh", ":archive_testdata", - ":test-tar-.tar", - ":test-tar-bz2.tar.bz2", - ":test-tar-empty_dirs.tar", - ":test-tar-empty_files.tar", - ":test-tar-files_dict.tar", - ":test-tar-gz.tar.gz", - ":test-tar-inclusion-.tar", - ":test-tar-inclusion-bz2.tar", - ":test-tar-inclusion-gz.tar", - ":test-tar-inclusion-xz.tar", - ":test-tar-mtime.tar", - ":test-tar-strip_prefix-dot.tar", - ":test-tar-strip_prefix-empty.tar", - ":test-tar-strip_prefix-etc.tar", - ":test-tar-strip_prefix-none.tar", - ":test-tar-xz.tar.xz", + ":test-tar-", + ":test-tar-bz2", + ":test-tar-empty_dirs", + ":test-tar-empty_files", + ":test-tar-files_dict", + ":test-tar-gz", + ":test-tar-inclusion-", + ":test-tar-inclusion-bz2", + ":test-tar-inclusion-gz", + ":test-tar-inclusion-xz", + ":test-tar-mtime", + ":test-tar-strip_prefix-dot", + ":test-tar-strip_prefix-empty", + ":test-tar-strip_prefix-etc", + ":test-tar-strip_prefix-none", + ":test-tar-xz", ":titi_test_all.changes", ], tags = [ diff --git a/pkg/tests/build_test_tar.sh b/pkg/tests/build_test_tar.sh index da4e3b0f..72c12e36 100755 --- a/pkg/tests/build_test_tar.sh +++ b/pkg/tests/build_test_tar.sh @@ -102,6 +102,10 @@ function test_tar() { ./usr/bin/ ./usr/bin/java -> /path/to/bin/java" for i in "" ".gz" ".bz2" ".xz"; do + # !!!!! + # !!!!! TODO: This needs work. Unless you have the PackageNamingInfo, + # !!!!! you would not know this true filename. + # !!!!! assert_content "test-tar-${i:1}.tar$i" # Test merging tar files # We pass a second argument to not test for user and group diff --git a/pkg/tests/my_package_name.bzl b/pkg/tests/my_package_name.bzl new file mode 100644 index 00000000..91f4d19d --- /dev/null +++ b/pkg/tests/my_package_name.bzl @@ -0,0 +1,25 @@ +# Copyright 2020 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. + +"""Sample rule to show package naming.""" + +load("//:providers.bzl", "PackageNamingInfo") + +def _my_package_naming_impl(ctx): + return PackageNamingInfo(values = {'cpu': 'arm48', 'opt': 'debug'}) + +my_package_naming = rule( + implementation = _my_package_naming_impl, +) + From 0f89ade43603fa9ef4b7952ad936f951bf99b8a5 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Sat, 27 Jun 2020 23:39:41 -0400 Subject: [PATCH 02/13] WTF --- pkg/tests/BUILD | 7 +++++-- pkg/tests/build_test_tar.sh | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/tests/BUILD b/pkg/tests/BUILD index db622299..18bc1219 100644 --- a/pkg/tests/BUILD +++ b/pkg/tests/BUILD @@ -19,7 +19,7 @@ genrule( ) pkg_tar( - name = "test_naming", + name = "test_tar_naming", srcs = [ ":BUILD", ], @@ -275,7 +275,10 @@ pkg_deb( [pkg_tar( name = "test-tar-inclusion-%s" % ext, build_tar = "//:build_tar", - deps = [":test-tar-%s" % ext], + deps = [ + ":test-tar-%s" % ext, + # ":test_tar_naming", + ], ) for ext in [ "", "gz", diff --git a/pkg/tests/build_test_tar.sh b/pkg/tests/build_test_tar.sh index 72c12e36..dfa9444c 100755 --- a/pkg/tests/build_test_tar.sh +++ b/pkg/tests/build_test_tar.sh @@ -96,7 +96,7 @@ function assert_content() { function test_tar() { local listing="./ ./etc/ -./etc/nsswitch.conf +XXXXXXXXXXXXX ./etc/nsswitch.conf ./usr/ ./usr/titi ./usr/bin/ From 15fb7a0f803c616668daa8fd48dcbe8e1df225d4 Mon Sep 17 00:00:00 2001 From: David Schneider Date: Wed, 26 Aug 2020 20:18:53 -0700 Subject: [PATCH 03/13] Add attribute strip_prefix to pkg_zip (#221) * Add attribute strip_prefix to pkg_zip * Add tests for pkg_zip strip_prefix behavior --- pkg/pkg.bzl | 14 ++++++------ pkg/tests/BUILD | 52 +++++++++++++++++++++++++++++++++++++++++++ pkg/tests/zip_test.py | 34 +++++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 8 deletions(-) diff --git a/pkg/pkg.bzl b/pkg/pkg.bzl index 2a4bc9d1..24b511f9 100644 --- a/pkg/pkg.bzl +++ b/pkg/pkg.bzl @@ -390,9 +390,6 @@ def pkg_deb(name, package, **kwargs): **kwargs ) -def _format_zip_file_arg(f): - return "%s=%s" % (_quote(f.path), dest_path(f, strip_prefix = None)) - def _pkg_zip_impl(ctx): args = ctx.actions.args() @@ -401,10 +398,12 @@ def _pkg_zip_impl(ctx): args.add("-t", ctx.attr.timestamp) args.add("-m", ctx.attr.mode) - args.add_all( - ctx.files.srcs, - map_each = _format_zip_file_arg, - ) + for f in ctx.files.srcs: + arg = "%s=%s" % ( + _quote(f.path), + dest_path(f, compute_data_path(ctx.outputs.out, ctx.attr.strip_prefix)) + ) + args.add(arg) args.set_param_file_format("multiline") args.use_param_file("@%s") @@ -434,6 +433,7 @@ pkg_zip_impl = rule( "timestamp": attr.int(default = 315532800), "mode": attr.string(default = "0555"), "out": attr.output(), + "strip_prefix": attr.string(), # Implicit dependencies. "build_zip": attr.label( default = Label("//:build_zip"), diff --git a/pkg/tests/BUILD b/pkg/tests/BUILD index c7803a58..ab9af684 100644 --- a/pkg/tests/BUILD +++ b/pkg/tests/BUILD @@ -5,6 +5,7 @@ licenses(["notice"]) load("//:pkg.bzl", "pkg_deb", "pkg_tar", "pkg_zip") load("//:rpm.bzl", "pkg_rpm") load("@rules_python//python:defs.bzl", "py_test") +load("@bazel_skylib//rules:copy_file.bzl", "copy_file") genrule( name = "generate_files", @@ -15,6 +16,12 @@ genrule( cmd = "for i in $(OUTS); do echo 1 >$$i; done", ) +copy_file( + name = "zipcontent_loremipsum", + src = "testdata/loremipsum.txt", + out = "zipcontent/loremipsum.txt", +) + pkg_tar( name = "test_tar_package_dir", srcs = [ @@ -79,6 +86,47 @@ pkg_zip( timestamp = 0, ) +pkg_zip( + name = "test-zip-strip_prefix-empty", + srcs = [ + "zipcontent_loremipsum", + ], + strip_prefix = "", +) + +pkg_zip( + name = "test-zip-strip_prefix-none", + srcs = [ + "zipcontent_loremipsum", + ], +) + +pkg_zip( + name = "test-zip-strip_prefix-zipcontent", + srcs = [ + "zipcontent_loremipsum", + ], + strip_prefix = "zipcontent", +) + +pkg_zip( + name = "test-zip-strip_prefix-dot", + srcs = [ + "zipcontent_loremipsum", + ], + strip_prefix = ".", +) + +filegroup( + name = "test_zip_strip_prefix", + srcs = [ + "test-zip-strip_prefix-empty", + "test-zip-strip_prefix-none", + "test-zip-strip_prefix-zipcontent", + "test-zip-strip_prefix-dot", + ], +) + # All of these should produce the same zip file. [pkg_zip( name = "test_zip_package_dir" + str(idx), @@ -104,6 +152,10 @@ py_test( "test_zip_package_dir0.zip", "test_zip_timestamp.zip", "test_zip_permissions.zip", + "test-zip-strip_prefix-empty.zip", + "test-zip-strip_prefix-none.zip", + "test-zip-strip_prefix-zipcontent.zip", + "test-zip-strip_prefix-dot.zip", # these could be replaced with diff_test() rules (from skylib) "test_zip_basic_timestamp_before_epoch.zip", diff --git a/pkg/tests/zip_test.py b/pkg/tests/zip_test.py index d73c0433..be67510e 100644 --- a/pkg/tests/zip_test.py +++ b/pkg/tests/zip_test.py @@ -82,7 +82,7 @@ def testPermissions(self): self.assertZipFileContent( "test_zip_permissions.zip", [ - { + { "filename": "executable.sh", "crc": EXECUTABLE_CRC, "timestamp": 1234567890, @@ -100,6 +100,38 @@ def testPackageDir(self): ], ) + def testZipStripPrefixEmpty(self): + self.assertZipFileContent( + "test-zip-strip_prefix-empty.zip", + [ + {"filename": "loremipsum.txt", "crc": LOREM_CRC}, + ], + ) + + def testZipStripPrefixNone(self): + self.assertZipFileContent( + "test-zip-strip_prefix-none.zip", + [ + {"filename": "loremipsum.txt", "crc": LOREM_CRC}, + ], + ) + + def testZipStripPrefixZipcontent(self): + self.assertZipFileContent( + "test-zip-strip_prefix-zipcontent.zip", + [ + {"filename": "loremipsum.txt", "crc": LOREM_CRC}, + ], + ) + + def testZipStripPrefixDot(self): + self.assertZipFileContent( + "test-zip-strip_prefix-dot.zip", + [ + {"filename": "zipcontent/loremipsum.txt", "crc": LOREM_CRC}, + ], + ) + class ZipEquivalency(ZipTest): """Check that some generated zip files are equivalent to each-other.""" From d01c5cc52d86dc904e9020c1deb3acd93bd2ff35 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Thu, 24 Sep 2020 11:31:13 -0400 Subject: [PATCH 04/13] always write output --- pkg/BUILD | 8 ++ pkg/pkg.bzl | 30 ++++--- pkg/tests/BUILD | 2 +- pkg/tests/build_test_tar.sh | 154 ---------------------------------- pkg/tests/my_package_name.bzl | 6 +- 5 files changed, 31 insertions(+), 169 deletions(-) delete mode 100755 pkg/tests/build_test_tar.sh diff --git a/pkg/BUILD b/pkg/BUILD index 5ccc99fe..17cc13d0 100644 --- a/pkg/BUILD +++ b/pkg/BUILD @@ -26,6 +26,14 @@ exports_files( visibility = ["//distro:__pkg__"], ) + +# Exposes the value of the --stamp blaze flag so that codegen modules +# which can use volatile build info know whether to depend on it. +config_setting( + name = "is_stamped_build", + values = {"stamp": "1"}, +) + py_library( name = "archive", srcs = [ diff --git a/pkg/pkg.bzl b/pkg/pkg.bzl index 1d1e48c8..540f2f5f 100644 --- a/pkg/pkg.bzl +++ b/pkg/pkg.bzl @@ -21,6 +21,7 @@ tar_filetype = [".tar", ".tar.gz", ".tgz", ".tar.xz", ".tar.bz2"] deb_filetype = [".deb", ".udeb"] _DEFAULT_MTIME = -1 + def _remap(remap_paths, path): """If path starts with a key in remap_paths, rewrite it.""" for prefix, replacement in remap_paths.items(): @@ -32,6 +33,7 @@ def _quote(filename, protect = "="): """Quote the filename, by escaping = by \\= and \\ by \\\\""" return filename.replace("\\", "\\\\").replace(protect, "\\" + protect) + def _pkg_tar_impl(ctx): """Implementation of the pkg_tar rule.""" @@ -41,18 +43,21 @@ def _pkg_tar_impl(ctx): output_name = ctx.label.name + "." + ctx.attr.extension if ctx.attr.package_file_name: naming_info = ctx.attr.naming_info[PackageNamingInfo] - if naming_info: - # This is obviously wrong. We need a templater that takes a dict. - output_name = ctx.attr.package_file_name.replace( - "${cpu}", naming_info.values['cpu']).replace( - "${opt}", naming_info.values['opt']) - print("======= Writing: %s", output_name) - output_file = ctx.actions.declare_file(output_name) - # print("======= Writing: %s", output_file.path) + if not naming_info: + fail('attribute package_file_name requires naming_info') + output_name = ctx.attr.package_file_name.format(**naming_info.values) + output_file = ctx.actions.declare_file(output_name) + ctx.actions.symlink( + output = ctx.outputs.out, + target_file = output_file + ) + else: + output_file = ctx.outputs.out + print("======= Writing: %s", output_file.path) # Compute the relative path - data_path = compute_data_path(ctx.outputs.out, ctx.attr.strip_prefix) - data_path_without_prefix = compute_data_path(ctx.outputs.out, '.') + data_path = compute_data_path(output_file, ctx.attr.strip_prefix) + data_path_without_prefix = compute_data_path(output_file, '.') # Find a list of path remappings to apply. remap_paths = ctx.attr.remap_paths @@ -345,11 +350,10 @@ def pkg_tar(name, **kwargs): kwargs["srcs"] = kwargs.pop("files") archive_name = kwargs.get("archive_name") or name extension = kwargs.get("extension") or "tar" - pkg_tar_impl( name = name, - # out = archive_name + "." + extension, - **kwargs + out = kwargs.get("out") or (archive_name + "." + extension), + **kwargs, ) # A rule for creating a deb file, see README.md diff --git a/pkg/tests/BUILD b/pkg/tests/BUILD index 97859ce7..34e0723a 100644 --- a/pkg/tests/BUILD +++ b/pkg/tests/BUILD @@ -31,7 +31,7 @@ pkg_tar( srcs = [ ":BUILD", ], - package_file_name = "test_naming-${cpu}-${opt}.tar", + package_file_name = "test_naming-{cpu}-{opt}-{BUILD_USER}.tar", naming_info = ":naming_info", ) diff --git a/pkg/tests/build_test_tar.sh b/pkg/tests/build_test_tar.sh deleted file mode 100755 index dfa9444c..00000000 --- a/pkg/tests/build_test_tar.sh +++ /dev/null @@ -1,154 +0,0 @@ -#!/bin/bash -# -*- coding: utf-8 -*- - -# Copyright 2015 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. - -# Unit tests for pkg_tar - -DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) -source ${DIR}/testenv.sh || { echo "testenv.sh not found!" >&2; exit 1; } - -readonly OS=$(uname) - -# Linux and macos report 1999-12-31 19:00 in different ways -readonly TAR_OUTPUT_FIXERS=$(mktemp) -trap '/bin/rm $TAR_OUTPUT_FIXERS' 0 1 2 3 15 -( - echo 's# Dec 31 1999 # 1999-12-31 19:00 #' - echo 's# Jan 1 2000 # 1999-12-31 19:00 #' - echo 's# *0 tata titi # tata/titi #' - echo 's# *0 titi tata # titi/tata #' - echo 's# *0 0 0 # 0/0 #' - echo 's# *0 24 42 # 24/42 #' - echo 's# *0 42 24 # 42/24 #' -) >"$TAR_OUTPUT_FIXERS" - -function get_tar_listing() { - local input=$1 - local test_data="${TEST_DATA_DIR}/${input}" - # We strip unused prefixes rather than dropping "v" flag for tar, because we - # want to preserve symlink information. - tar tvf "${test_data}" | sed -f "$TAR_OUTPUT_FIXERS" | sed -e 's/^.*:00 //' -} - -function get_tar_verbose_listing() { - local input=$1 - local test_data="${TEST_DATA_DIR}/${input}" - TZ="UTC" tar tvf "${test_data}" | sed -f "$TAR_OUTPUT_FIXERS" -} - -function get_tar_owner() { - local input=$1 - local file=$2 - local test_data="${TEST_DATA_DIR}/${input}" - tar tvf "${test_data}" | sed -f "$TAR_OUTPUT_FIXERS" | grep "00 $file\$" | cut -d " " -f 2 -} - -function get_numeric_tar_owner() { - local input=$1 - local file=$2 - local test_data="${TEST_DATA_DIR}/${input}" - tar --numeric-owner -tvf "${test_data}" | sed -f "$TAR_OUTPUT_FIXERS" | grep "00 $file\$" | cut -d " " -f 2 -} - -function get_tar_permission() { - local input=$1 - local file=$2 - local test_data="${TEST_DATA_DIR}/${input}" - tar tvf "${test_data}" | sed -f "$TAR_OUTPUT_FIXERS" | fgrep "00 $file" | cut -d " " -f 1 -} - -function assert_content() { - local listing="./ -./etc/ -./etc/nsswitch.conf -./usr/ -./usr/titi -./usr/bin/ -./usr/bin/java -> /path/to/bin/java" - check_eq "$listing" "$(get_tar_listing $1)" - check_eq "-rwxr-xr-x" "$(get_tar_permission $1 ./usr/titi)" - check_eq "-rw-r--r--" "$(get_tar_permission $1 ./etc/nsswitch.conf)" - check_eq "24/42" "$(get_numeric_tar_owner $1 ./etc/)" - check_eq "24/42" "$(get_numeric_tar_owner $1 ./etc/nsswitch.conf)" - check_eq "42/24" "$(get_numeric_tar_owner $1 ./usr/)" - check_eq "42/24" "$(get_numeric_tar_owner $1 ./usr/titi)" - if [ -z "${2-}" ]; then - check_eq "tata/titi" "$(get_tar_owner $1 ./etc/)" - check_eq "tata/titi" "$(get_tar_owner $1 ./etc/nsswitch.conf)" - check_eq "titi/tata" "$(get_tar_owner $1 ./usr/)" - check_eq "titi/tata" "$(get_tar_owner $1 ./usr/titi)" - fi -} - -function test_tar() { - local listing="./ -./etc/ -XXXXXXXXXXXXX ./etc/nsswitch.conf -./usr/ -./usr/titi -./usr/bin/ -./usr/bin/java -> /path/to/bin/java" - for i in "" ".gz" ".bz2" ".xz"; do - # !!!!! - # !!!!! TODO: This needs work. Unless you have the PackageNamingInfo, - # !!!!! you would not know this true filename. - # !!!!! - assert_content "test-tar-${i:1}.tar$i" - # Test merging tar files - # We pass a second argument to not test for user and group - # names because tar merging ask for numeric owners. - assert_content "test-tar-inclusion-${i:1}.tar" "true" - done; - - check_eq "./ -./nsswitch.conf" "$(get_tar_listing test-tar-strip_prefix-empty.tar)" - check_eq "./ -./nsswitch.conf" "$(get_tar_listing test-tar-strip_prefix-none.tar)" - check_eq "./ -./nsswitch.conf" "$(get_tar_listing test-tar-strip_prefix-etc.tar)" - check_eq "./ -./etc/ -./etc/nsswitch.conf -./external/ -./external/bazel_tools/ -./external/bazel_tools/tools/ -./external/bazel_tools/tools/python/ -./external/bazel_tools/tools/python/runfiles/ -./external/bazel_tools/tools/python/runfiles/runfiles.py" "$(get_tar_listing test-tar-strip_prefix-dot.tar)" - check_eq "./ -./not-etc/ -./not-etc/mapped-filename.conf" "$(get_tar_listing test-tar-files_dict.tar)" - -# TODO(aiuto): Make these tests less brittle w.r.t. tar variations, so that -# we can run them on something besides Linux. -if [[ "$OS" == 'Linux' ]] ; then - check_eq "drwxr-xr-x 0/0 0 2000-01-01 00:00 ./ --rwxrwxrwx 0/0 0 2000-01-01 00:00 ./a --rwxrwxrwx 0/0 0 2000-01-01 00:00 ./b" \ - "$(get_tar_verbose_listing test-tar-empty_files.tar)" - check_eq "drwxr-xr-x 0/0 0 2000-01-01 00:00 ./ -drwxrwxrwx 0/0 0 2000-01-01 00:00 ./tmp/ -drwxrwxrwx 0/0 0 2000-01-01 00:00 ./pmt/" \ - "$(get_tar_verbose_listing test-tar-empty_dirs.tar)" - check_eq \ - "drwxr-xr-x 0/0 0 1999-12-31 23:59 ./ --r-xr-xr-x 0/0 2 1999-12-31 23:59 ./nsswitch.conf" \ - "$(get_tar_verbose_listing test-tar-mtime.tar)" -fi -} - - -run_suite "build_test" diff --git a/pkg/tests/my_package_name.bzl b/pkg/tests/my_package_name.bzl index 91f4d19d..e678ebde 100644 --- a/pkg/tests/my_package_name.bzl +++ b/pkg/tests/my_package_name.bzl @@ -15,9 +15,13 @@ """Sample rule to show package naming.""" load("//:providers.bzl", "PackageNamingInfo") +load("//:package_naming.bzl", "default_package_naming") def _my_package_naming_impl(ctx): - return PackageNamingInfo(values = {'cpu': 'arm48', 'opt': 'debug'}) + values = default_package_naming(ctx) + values['cpu'] = 'arm48' + values['opt'] = 'debug' + return PackageNamingInfo(values = values) my_package_naming = rule( implementation = _my_package_naming_impl, From 861c72fce88f5517fa0f6bc33f6b992aa1643a3a Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Thu, 24 Sep 2020 15:16:21 -0400 Subject: [PATCH 05/13] checkpoing --- pkg/package_naming.bzl | 14 ++++++++++++++ pkg/tests/BUILD | 2 +- pkg/tests/my_package_name.bzl | 8 ++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 pkg/package_naming.bzl diff --git a/pkg/package_naming.bzl b/pkg/package_naming.bzl new file mode 100644 index 00000000..412d93fa --- /dev/null +++ b/pkg/package_naming.bzl @@ -0,0 +1,14 @@ + + +def default_package_naming(ctx): + if in ctx.attr["stamp"]: + stamp = str(ctx.attrs.stamp) + else: + stamp = "NO_STAMP" + values = {} + values['stamp'] = stamp + values['BUILD_HOST'] = 'host' + values['BUILD_USER'] = 'user' + # Jan 1, 1980 + values['BUILD_TIMESTAMP'] = 315532800 + return values diff --git a/pkg/tests/BUILD b/pkg/tests/BUILD index 34e0723a..64c3e9b6 100644 --- a/pkg/tests/BUILD +++ b/pkg/tests/BUILD @@ -31,7 +31,7 @@ pkg_tar( srcs = [ ":BUILD", ], - package_file_name = "test_naming-{cpu}-{opt}-{BUILD_USER}.tar", + package_file_name = "test_naming-{cpu}-{opt}-{BUILD_USER}-{stamp}.tar", naming_info = ":naming_info", ) diff --git a/pkg/tests/my_package_name.bzl b/pkg/tests/my_package_name.bzl index e678ebde..ddf7436d 100644 --- a/pkg/tests/my_package_name.bzl +++ b/pkg/tests/my_package_name.bzl @@ -25,5 +25,13 @@ def _my_package_naming_impl(ctx): my_package_naming = rule( implementation = _my_package_naming_impl, + attrs = { + "stamp": attr.int( + values = [-1, 0, 1], + default = -1, + # TODO(aiuto): Improve doc. Matches cc rules, but that it should be a convention. + doc = "-1: use --stamp, 0: do not stamp, 1: stamp", + ), + } ) From 4d1a22aa036998d4f10cc745304a8c86d668a1f9 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Thu, 24 Sep 2020 18:35:06 -0400 Subject: [PATCH 06/13] buff for review --- pkg/BUILD | 8 -------- pkg/package_naming.bzl | 14 -------------- pkg/package_variables.bzl | 22 ++++++++++++++++++++++ pkg/pkg.bzl | 16 ++++++++-------- pkg/providers.bzl | 4 ++-- pkg/tests/BUILD | 19 ++++++++++++++++--- pkg/tests/my_package_name.bzl | 29 +++++++++++++++++------------ 7 files changed, 65 insertions(+), 47 deletions(-) delete mode 100644 pkg/package_naming.bzl create mode 100644 pkg/package_variables.bzl diff --git a/pkg/BUILD b/pkg/BUILD index 0ed3b4a3..9f753594 100644 --- a/pkg/BUILD +++ b/pkg/BUILD @@ -27,14 +27,6 @@ exports_files( visibility = ["//distro:__pkg__"], ) - -# Exposes the value of the --stamp blaze flag so that codegen modules -# which can use volatile build info know whether to depend on it. -config_setting( - name = "is_stamped_build", - values = {"stamp": "1"}, -) - py_library( name = "archive", srcs = [ diff --git a/pkg/package_naming.bzl b/pkg/package_naming.bzl deleted file mode 100644 index 412d93fa..00000000 --- a/pkg/package_naming.bzl +++ /dev/null @@ -1,14 +0,0 @@ - - -def default_package_naming(ctx): - if in ctx.attr["stamp"]: - stamp = str(ctx.attrs.stamp) - else: - stamp = "NO_STAMP" - values = {} - values['stamp'] = stamp - values['BUILD_HOST'] = 'host' - values['BUILD_USER'] = 'user' - # Jan 1, 1980 - values['BUILD_TIMESTAMP'] = 315532800 - return values diff --git a/pkg/package_variables.bzl b/pkg/package_variables.bzl new file mode 100644 index 00000000..e07649d2 --- /dev/null +++ b/pkg/package_variables.bzl @@ -0,0 +1,22 @@ +# Copyright 2020 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. + +"""Utility methods to populate PackageVariablesInfo instances.""" + +load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") + +def add_cpp_variables(ctx, values): + cc_toolchain = find_cpp_toolchain(ctx) + values['cpu'] = cc_toolchain.cpu + return values diff --git a/pkg/pkg.bzl b/pkg/pkg.bzl index 540f2f5f..07946fc6 100644 --- a/pkg/pkg.bzl +++ b/pkg/pkg.bzl @@ -14,7 +14,7 @@ """Rules for manipulation of various packaging.""" load(":path.bzl", "compute_data_path", "dest_path") -load(":providers.bzl", "PackageArtifactInfo", "PackageNamingInfo") +load(":providers.bzl", "PackageArtifactInfo", "PackageVariablesInfo") # Filetype to restrict inputs tar_filetype = [".tar", ".tar.gz", ".tgz", ".tar.xz", ".tar.bz2"] @@ -42,10 +42,10 @@ def _pkg_tar_impl(ctx): output_name = ctx.label.name + "." + ctx.attr.extension if ctx.attr.package_file_name: - naming_info = ctx.attr.naming_info[PackageNamingInfo] - if not naming_info: - fail('attribute package_file_name requires naming_info') - output_name = ctx.attr.package_file_name.format(**naming_info.values) + package_variables = ctx.attr.package_variables[PackageVariablesInfo] + if not package_variables: + fail('attribute package_file_name requires package_variables') + output_name = ctx.attr.package_file_name.format(**package_variables.values) output_file = ctx.actions.declare_file(output_name) ctx.actions.symlink( output = ctx.outputs.out, @@ -53,7 +53,6 @@ def _pkg_tar_impl(ctx): ) else: output_file = ctx.outputs.out - print("======= Writing: %s", output_file.path) # Compute the relative path data_path = compute_data_path(output_file, ctx.attr.strip_prefix) @@ -148,6 +147,7 @@ def _pkg_tar_impl(ctx): ctx.actions.run( mnemonic = "PackageTar", + progress_message = "Writing: %s" % output_file.path, inputs = file_inputs + ctx.files.deps + files, executable = ctx.executable.build_tar, arguments = ["@" + arg_file.path], @@ -318,8 +318,8 @@ pkg_tar_impl = rule( "empty_dirs": attr.string_list(), "remap_paths": attr.string_dict(), "package_file_name": attr.string(), - "naming_info": attr.label( - providers = [PackageNamingInfo], + "package_variables": attr.label( + providers = [PackageVariablesInfo], ), # Outputs diff --git a/pkg/providers.bzl b/pkg/providers.bzl index 50b9b637..d17aabd3 100644 --- a/pkg/providers.bzl +++ b/pkg/providers.bzl @@ -23,8 +23,8 @@ PackageArtifactInfo = provider( }, ) -PackageNamingInfo = provider( - doc = """Metadata about a package artifact.""", +PackageVariablesInfo = provider( + doc = """Variables which may be substituted into package names and content.""", fields = { "values": "Dict of name/value pairs", }, diff --git a/pkg/tests/BUILD b/pkg/tests/BUILD index 64c3e9b6..79dd838d 100644 --- a/pkg/tests/BUILD +++ b/pkg/tests/BUILD @@ -9,7 +9,20 @@ load("@bazel_skylib//rules:copy_file.bzl", "copy_file") load(":my_package_name.bzl", "my_package_naming") -my_package_naming(name = "naming_info") +# Exposes the value of the -c mode to the package naming. +config_setting( + name = "debug_build", + values = {"compilation_mode": "dbg"}, +) + +my_package_naming( + name = "naming_info", + label = "-alpha", + compilation_mode = select({ + ":debug_build": "-debug", + "//conditions:default": "", + }), +) genrule( name = "generate_files", @@ -31,8 +44,8 @@ pkg_tar( srcs = [ ":BUILD", ], - package_file_name = "test_naming-{cpu}-{opt}-{BUILD_USER}-{stamp}.tar", - naming_info = ":naming_info", + package_file_name = "test_naming-{cpu}{compilation_mode}{label}.tar", + package_variables = ":naming_info", ) pkg_tar( diff --git a/pkg/tests/my_package_name.bzl b/pkg/tests/my_package_name.bzl index ddf7436d..111c187b 100644 --- a/pkg/tests/my_package_name.bzl +++ b/pkg/tests/my_package_name.bzl @@ -14,24 +14,29 @@ """Sample rule to show package naming.""" -load("//:providers.bzl", "PackageNamingInfo") -load("//:package_naming.bzl", "default_package_naming") +load("//:providers.bzl", "PackageVariablesInfo") +load("//:package_variables.bzl", "add_cpp_variables") def _my_package_naming_impl(ctx): - values = default_package_naming(ctx) - values['cpu'] = 'arm48' - values['opt'] = 'debug' - return PackageNamingInfo(values = values) + values = {} + # get things from the platform and toolchain + add_cpp_variables(ctx, values) + # then add in my own custom values + values['label'] = ctx.attr.label + values['compilation_mode'] = ctx.attr.compilation_mode + return PackageVariablesInfo(values = values) my_package_naming = rule( implementation = _my_package_naming_impl, + # must ask for cpp fragement to use add_cpp_variables(). + fragments = ["cpp"], attrs = { - "stamp": attr.int( - values = [-1, 0, 1], - default = -1, - # TODO(aiuto): Improve doc. Matches cc rules, but that it should be a convention. - doc = "-1: use --stamp, 0: do not stamp, 1: stamp", + "label": attr.string(doc = "A label that matters to me."), + "compilation_mode": attr.string( + doc = "Another label for the sake of the sample." + ), + "_cc_toolchain": attr.label( + default = Label("@bazel_tools//tools/cpp:current_cc_toolchain") ), } ) - From 3e48005e9774c320e179d912fee3ee9cc033d387 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Thu, 24 Sep 2020 18:50:43 -0400 Subject: [PATCH 07/13] docs --- pkg/distro/BUILD | 2 ++ pkg/package_variables.bzl | 3 +++ pkg/tests/BUILD | 6 +++--- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/pkg/distro/BUILD b/pkg/distro/BUILD index c8310db5..b2bbfa0d 100644 --- a/pkg/distro/BUILD +++ b/pkg/distro/BUILD @@ -75,8 +75,10 @@ genrule( bzl_library( name = "rules_pkg_lib", srcs = [ + "//:package_variables.bzl", "//:path.bzl", "//:pkg.bzl", + "//:providers.bzl", "//:rpm.bzl", "//:version.bzl", ], diff --git a/pkg/package_variables.bzl b/pkg/package_variables.bzl index e07649d2..90b4f7b8 100644 --- a/pkg/package_variables.bzl +++ b/pkg/package_variables.bzl @@ -18,5 +18,8 @@ load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") def add_cpp_variables(ctx, values): cc_toolchain = find_cpp_toolchain(ctx) + # TODO(aiuto): Expand this to include target OS. Maybe also compilation + # mode, ABI and libc version, since they are sometimes used in package file + # names. values['cpu'] = cc_toolchain.cpu return values diff --git a/pkg/tests/BUILD b/pkg/tests/BUILD index 79dd838d..de9352c0 100644 --- a/pkg/tests/BUILD +++ b/pkg/tests/BUILD @@ -9,14 +9,14 @@ load("@bazel_skylib//rules:copy_file.bzl", "copy_file") load(":my_package_name.bzl", "my_package_naming") -# Exposes the value of the -c mode to the package naming. +# Exposes the value of the compilation mode to the package naming. config_setting( name = "debug_build", values = {"compilation_mode": "dbg"}, ) my_package_naming( - name = "naming_info", + name = "my_package_variables", label = "-alpha", compilation_mode = select({ ":debug_build": "-debug", @@ -45,7 +45,7 @@ pkg_tar( ":BUILD", ], package_file_name = "test_naming-{cpu}{compilation_mode}{label}.tar", - package_variables = ":naming_info", + package_variables = ":my_package_variables", ) pkg_tar( From eb0d591325bc5ec33c1dae606670db590e1ed5bf Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Thu, 24 Sep 2020 18:58:21 -0400 Subject: [PATCH 08/13] only require package_variables if there is a substitution --- pkg/pkg.bzl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/pkg.bzl b/pkg/pkg.bzl index 07946fc6..d509bb7b 100644 --- a/pkg/pkg.bzl +++ b/pkg/pkg.bzl @@ -43,9 +43,10 @@ def _pkg_tar_impl(ctx): output_name = ctx.label.name + "." + ctx.attr.extension if ctx.attr.package_file_name: package_variables = ctx.attr.package_variables[PackageVariablesInfo] - if not package_variables: + if package_variables: + output_name = ctx.attr.package_file_name.format(**package_variables.values) + elif ctx.attr.package_file_name.find('{') >= 0: fail('attribute package_file_name requires package_variables') - output_name = ctx.attr.package_file_name.format(**package_variables.values) output_file = ctx.actions.declare_file(output_name) ctx.actions.symlink( output = ctx.outputs.out, From d0376c2388f9dd02085d452f062ec0e97c1fa20d Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Thu, 24 Sep 2020 21:24:02 -0400 Subject: [PATCH 09/13] put the renamed pkg_tar file in the tar test --- pkg/tests/BUILD | 2 +- pkg/tests/pkg_tar_test.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/pkg/tests/BUILD b/pkg/tests/BUILD index de9352c0..7129ed91 100644 --- a/pkg/tests/BUILD +++ b/pkg/tests/BUILD @@ -320,7 +320,7 @@ pkg_rpm( build_tar = "//:build_tar", deps = [ ":test-tar-basic-%s" % ext, - # ":test_tar_naming", + ":test_tar_naming", ], ) for ext in [ "", diff --git a/pkg/tests/pkg_tar_test.py b/pkg/tests/pkg_tar_test.py index 88ce7dce..9015a657 100644 --- a/pkg/tests/pkg_tar_test.py +++ b/pkg/tests/pkg_tar_test.py @@ -175,6 +175,7 @@ def test_file_inclusion(self): {'name': './usr/titi', 'mode': 0o755, 'uid': 42, 'gid': 24}, {'name': './usr/bin'}, {'name': './usr/bin/java', 'linkname': '/path/to/bin/java'}, + {'name': './BUILD'}, ] for ext in ('', '.gz', '.bz2', '.xz'): with self.subTest(ext=ext): From 5394c05d0bbc5e8092e638b64a4728036a78ca97 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Thu, 24 Sep 2020 21:45:33 -0400 Subject: [PATCH 10/13] remove pointless comments --- pkg/pkg.bzl | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/pkg.bzl b/pkg/pkg.bzl index d509bb7b..f6208bc4 100644 --- a/pkg/pkg.bzl +++ b/pkg/pkg.bzl @@ -162,11 +162,8 @@ def _pkg_tar_impl(ctx): use_default_shell_env = True, ) return [ - # WHY a seperate OutputGroup? - # OutputGroupInfo(out = [output_file]), DefaultInfo( files = depset([output_file]), - # This is sort of ugly that the object itself must be runfiles. runfiles = ctx.runfiles(files = [output_file]), ), PackageArtifactInfo( @@ -324,7 +321,7 @@ pkg_tar_impl = rule( ), # Outputs - "out": attr.output(mandatory=False), + "out": attr.output(), # Implicit dependencies. "build_tar": attr.label( From 0712ea9d0d62283f34894d68f66b6cfd9c45b00c Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Thu, 24 Sep 2020 22:02:42 -0400 Subject: [PATCH 11/13] fix path naming in ArtifactInfo --- pkg/pkg.bzl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/pkg.bzl b/pkg/pkg.bzl index f6208bc4..a5a21240 100644 --- a/pkg/pkg.bzl +++ b/pkg/pkg.bzl @@ -40,11 +40,11 @@ def _pkg_tar_impl(ctx): # Files needed by rule implementation at runtime files = [] - output_name = ctx.label.name + "." + ctx.attr.extension if ctx.attr.package_file_name: + output_name = ctx.attr.package_file_name package_variables = ctx.attr.package_variables[PackageVariablesInfo] if package_variables: - output_name = ctx.attr.package_file_name.format(**package_variables.values) + output_name = output_name.format(**package_variables.values) elif ctx.attr.package_file_name.find('{') >= 0: fail('attribute package_file_name requires package_variables') output_file = ctx.actions.declare_file(output_name) @@ -54,6 +54,7 @@ def _pkg_tar_impl(ctx): ) else: output_file = ctx.outputs.out + output_name = ctx.outputs.out.basename # Compute the relative path data_path = compute_data_path(output_file, ctx.attr.strip_prefix) From 4d9efcbe476d00b42405a75cd0e78ba1e7df337e Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Mon, 28 Sep 2020 12:39:46 -0400 Subject: [PATCH 12/13] review fixes --- pkg/package_variables.bzl | 12 ++++-------- pkg/pkg.bzl | 6 +++--- pkg/tests/BUILD | 15 ++++++++++----- pkg/tests/my_package_name.bzl | 15 +++++---------- 4 files changed, 22 insertions(+), 26 deletions(-) diff --git a/pkg/package_variables.bzl b/pkg/package_variables.bzl index 90b4f7b8..865fefdd 100644 --- a/pkg/package_variables.bzl +++ b/pkg/package_variables.bzl @@ -14,12 +14,8 @@ """Utility methods to populate PackageVariablesInfo instances.""" -load("@bazel_tools//tools/cpp:toolchain_utils.bzl", "find_cpp_toolchain") - -def add_cpp_variables(ctx, values): - cc_toolchain = find_cpp_toolchain(ctx) - # TODO(aiuto): Expand this to include target OS. Maybe also compilation - # mode, ABI and libc version, since they are sometimes used in package file - # names. - values['cpu'] = cc_toolchain.cpu +def add_ctx_variables(ctx, values): + """Add selected variables from ctx.""" + values['target_cpu'] = ctx.var.get("TARGET_CPU") + values['compilation_mode'] = ctx.var.get("COMPILATION_MODE") return values diff --git a/pkg/pkg.bzl b/pkg/pkg.bzl index a5a21240..680b386d 100644 --- a/pkg/pkg.bzl +++ b/pkg/pkg.bzl @@ -42,11 +42,11 @@ def _pkg_tar_impl(ctx): if ctx.attr.package_file_name: output_name = ctx.attr.package_file_name - package_variables = ctx.attr.package_variables[PackageVariablesInfo] - if package_variables: + if ctx.attr.package_variables: + package_variables = ctx.attr.package_variables[PackageVariablesInfo] output_name = output_name.format(**package_variables.values) elif ctx.attr.package_file_name.find('{') >= 0: - fail('attribute package_file_name requires package_variables') + fail("package_variables is required when using '{' in package_file_name") output_file = ctx.actions.declare_file(output_name) ctx.actions.symlink( output = ctx.outputs.out, diff --git a/pkg/tests/BUILD b/pkg/tests/BUILD index 7129ed91..aaa3ce4b 100644 --- a/pkg/tests/BUILD +++ b/pkg/tests/BUILD @@ -11,15 +11,18 @@ load(":my_package_name.bzl", "my_package_naming") # Exposes the value of the compilation mode to the package naming. config_setting( - name = "debug_build", - values = {"compilation_mode": "dbg"}, + name = "special_build", + values = {"define": "SPECIAL=1"}, ) my_package_naming( name = "my_package_variables", label = "-alpha", - compilation_mode = select({ - ":debug_build": "-debug", + # There is no important reason to use a config_setting and a select. This + # is really just to show a technique. + # TBD: Move this into a cookbook of examples. + special_build = select({ + ":special_build": "-IsSpecial", "//conditions:default": "", }), ) @@ -39,12 +42,14 @@ copy_file( out = "zipcontent/loremipsum.txt", ) +# Try building with various flag combinations to show the abilities +# bazel build :test_tar_naming -c dbg --define=SPECIAL=1 pkg_tar( name = "test_tar_naming", srcs = [ ":BUILD", ], - package_file_name = "test_naming-{cpu}{compilation_mode}{label}.tar", + package_file_name = "test_naming-{target_cpu}-{compilation_mode}{label}{special_build}.tar", package_variables = ":my_package_variables", ) diff --git a/pkg/tests/my_package_name.bzl b/pkg/tests/my_package_name.bzl index 111c187b..c0fc5f4c 100644 --- a/pkg/tests/my_package_name.bzl +++ b/pkg/tests/my_package_name.bzl @@ -15,28 +15,23 @@ """Sample rule to show package naming.""" load("//:providers.bzl", "PackageVariablesInfo") -load("//:package_variables.bzl", "add_cpp_variables") +load("//:package_variables.bzl", "add_ctx_variables") def _my_package_naming_impl(ctx): values = {} - # get things from the platform and toolchain - add_cpp_variables(ctx, values) + # Add variables which are always present + add_ctx_variables(ctx, values) # then add in my own custom values values['label'] = ctx.attr.label - values['compilation_mode'] = ctx.attr.compilation_mode + values['special_build'] = ctx.attr.special_build return PackageVariablesInfo(values = values) my_package_naming = rule( implementation = _my_package_naming_impl, - # must ask for cpp fragement to use add_cpp_variables(). - fragments = ["cpp"], attrs = { "label": attr.string(doc = "A label that matters to me."), - "compilation_mode": attr.string( + "special_build": attr.string( doc = "Another label for the sake of the sample." ), - "_cc_toolchain": attr.label( - default = Label("@bazel_tools//tools/cpp:current_cc_toolchain") - ), } ) From ac2e47cff7ca1b20ae7f77be8307ae549b0c2213 Mon Sep 17 00:00:00 2001 From: Tony Aiuto Date: Mon, 28 Sep 2020 12:42:46 -0400 Subject: [PATCH 13/13] review fixes --- pkg/pkg.bzl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/pkg.bzl b/pkg/pkg.bzl index 680b386d..2ee3e67a 100644 --- a/pkg/pkg.bzl +++ b/pkg/pkg.bzl @@ -40,6 +40,7 @@ def _pkg_tar_impl(ctx): # Files needed by rule implementation at runtime files = [] + outputs = [ctx.outputs.out] if ctx.attr.package_file_name: output_name = ctx.attr.package_file_name if ctx.attr.package_variables: @@ -48,6 +49,7 @@ def _pkg_tar_impl(ctx): elif ctx.attr.package_file_name.find('{') >= 0: fail("package_variables is required when using '{' in package_file_name") output_file = ctx.actions.declare_file(output_name) + outputs.append(output_file) ctx.actions.symlink( output = ctx.outputs.out, target_file = output_file @@ -164,7 +166,7 @@ def _pkg_tar_impl(ctx): ) return [ DefaultInfo( - files = depset([output_file]), + files = depset(outputs), runfiles = ctx.runfiles(files = [output_file]), ), PackageArtifactInfo(