From 1c34a08aaed663308277ac645a236465d3ac8e6c Mon Sep 17 00:00:00 2001 From: Andrew Psaltis Date: Thu, 10 Sep 2020 13:59:06 -0400 Subject: [PATCH 1/2] Provide a more concise package "manifest" specification --- In using `pkg_filegroup` in our codebases, we've found that it takes a significant amount of vertical space for complex packages. For simple mappings (e.g. copy a target/file to a destination), we've found that a simple tabular format provides a succinct way to specify contents without sacrificing much in the way of readability. At this time, I'm looking to start a discussion on the idea presented here. The idea itself is essentially agnostic to the implementation of `pkg_filegroup`, but `pkg_filegroup` needs some changes in order to make the usage below more idiomatic: there's currently no means to consolidate multiple differing `pkg_filegroup`-like rules into a single rule. A potential idea for this is discussed in #212. --- This change provides a simple csv-like mechanism for specifying package contents. Instead of specifying multiple `pkg_filegroup`s, you can instead say something like: ```python manifest = [ ("/usr/bin/foo", "copy", "unix=0755", ":foo"), ("/etc/foo.d", "mkdir" "unix=0755", ""), ... ] manifest_rules = pkg_list_manifest( name = "manifest", manifest = manifest, default_attrs = "user=root;group=root", ) pkg_rpm( name = "my-rpm", data = manifest_rules + [ # Complex pkg_filegroup rules here ], ... ) ``` Which, for simple install-and-create operations, is enough and is much easier to read. See the in-file documentation in `pkg/experimental/manifest/manifest.bzl` for more details on how the manifest is intended to be structured. Tests were also provided, runnable as `//experimental/manifest:manifest_test`. They are currently incomplete, but are enough to prove basic correctness so far. The one thing that is not yet clear to us is how we should specify and order the manifests. Some potential examples are provided in `pkg/experimental/manifest/examples`, with further information described in `pkg/experimental/manifest/examples/README.md`. --- pkg/experimental/manifest/BUILD | 25 ++ pkg/experimental/manifest/examples/BUILD | 36 +++ pkg/experimental/manifest/examples/README.md | 65 +++++ .../manifest/examples/action-first-string.bzl | 30 ++ .../manifest/examples/action-first.bzl | 31 ++ .../manifest/examples/dest-first.bzl | 31 ++ pkg/experimental/manifest/manifest.bzl | 266 +++++++++++++++++ pkg/experimental/manifest/manifest_test.py | 270 ++++++++++++++++++ 8 files changed, 754 insertions(+) create mode 100644 pkg/experimental/manifest/BUILD create mode 100644 pkg/experimental/manifest/examples/BUILD create mode 100644 pkg/experimental/manifest/examples/README.md create mode 100644 pkg/experimental/manifest/examples/action-first-string.bzl create mode 100644 pkg/experimental/manifest/examples/action-first.bzl create mode 100644 pkg/experimental/manifest/examples/dest-first.bzl create mode 100644 pkg/experimental/manifest/manifest.bzl create mode 100644 pkg/experimental/manifest/manifest_test.py diff --git a/pkg/experimental/manifest/BUILD b/pkg/experimental/manifest/BUILD new file mode 100644 index 00000000..9636247d --- /dev/null +++ b/pkg/experimental/manifest/BUILD @@ -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. + +load("@rules_python//python:defs.bzl", "py_test") + +exports_files(glob(["*.bzl"])) + +py_test( + name = "manifest_test", + srcs = ["manifest_test.py"], + data = ["manifest.bzl"], + python_version = "PY3", + srcs_version = "PY3", +) diff --git a/pkg/experimental/manifest/examples/BUILD b/pkg/experimental/manifest/examples/BUILD new file mode 100644 index 00000000..c431c0d9 --- /dev/null +++ b/pkg/experimental/manifest/examples/BUILD @@ -0,0 +1,36 @@ +# 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. + +load("//experimental/manifest:manifest.bzl", "pkg_list_manifest") +load("//experimental:rpm.bzl", "pkg_rpm") +load(":dest-first.bzl", "manifest_info") + +# This is an example to demonstrate usage. It currently does not function. All +# rules are tagged as manual to prevent CI from becoming angry. + +manifest = pkg_list_manifest( + name = "manifest", + default_attrs = manifest_info["default_attrs"], + manifest = manifest_info["manifest"], + tags = ["manual"], +) + +pkg_rpm( + name = "nonbuilding-rpm", + data = manifest + [ + # Other pkg_filegroups and friendes here + ] + tags = ["manual"], + # ... +) diff --git a/pkg/experimental/manifest/examples/README.md b/pkg/experimental/manifest/examples/README.md new file mode 100644 index 00000000..34a8f494 --- /dev/null +++ b/pkg/experimental/manifest/examples/README.md @@ -0,0 +1,65 @@ +# Why the examples? + +We are not sure how to consturct the manifests at this time to a few reasons. +What comes to mind is: + +- `buildifier` -- There is currently no way to tell it [to not + reformat](https://github.com/bazelbuild/buildtools/issues/890) some regions of + code, a la `//clang format off/on`. The main result of this is that all + whitespace within each of the manifest rows (tuples) is collapsed when + buildifier is used in any "fixing" mode. + +- Readability: column formatting, file formats? Starlark is a great starting + point here. + +- Maintainability: implementations that don't take direct advantage of + BUILD-loadable Starlark files appear to require much more code than otherwise. + +There may be something we are not aware of, or alternative ideas that we +overlooked. Feedback is greatly appreciated. + +# Options explored + +1. `dest-first.bzl` where the manifest is a list of + `(destination, action, attributes, source)` tuples. This is what is currently implemented. + +2. `action-first.bzl` where the manifest is a list of + `(action, destination, attributes, source)` tuples. + + This is the same as the above, except first two columns are swapped. + +3. `action-first-string.bzl` where the manifest is defined as a space-delimited + string, ordered like in `action-first.bzl`. + +Notes on these: + +- 1) and 2) require no custom parsing other than the `attributes` column, which + is a simple delimited string. + +- 1) and 2) are subject to `buildifier`, 3) is not. + +- 1), however is implemented in the code, because it is highly important that + the destinations are aligned regardless of overall formatting. + +- 3) gives us the most control, but requires writing out a parser. Depending on + the complexity of the file format, this could be impractical to maintain. + +Overall, if we could teach `buildifier` not to reformat a region, our preferred +option is 2), since it has simple code and is easy to read. + +# Options not explored + +- Moving the transformation from manifest to `pkg_filegroup` list to a + repository rule. + + Not explored due to perceived inconvenience and scalability concerns in large + monorepos. + +- Moving the transformation to some other external utility. + + Not explored due to potential implementation costs. Also prevents direct + reuse of the `pkg_filegroup` rule. + + The main concern with this one is that Bazel is not aware of the contents of + the manifest, and will have to be provided additional information that need + not be provided when the manifest is available in Starlark. diff --git a/pkg/experimental/manifest/examples/action-first-string.bzl b/pkg/experimental/manifest/examples/action-first-string.bzl new file mode 100644 index 00000000..571974ee --- /dev/null +++ b/pkg/experimental/manifest/examples/action-first-string.bzl @@ -0,0 +1,30 @@ +_default_attrs = "user=root;group=root" +_manifest = """ +#action dest attributes... source +"copy", "/etc/syslog.conf.d/mycomponent.conf", "unix=0644;section=confignoreplace", "//mycomponent/etc:conf", +"copy", "/opt/mycomponent/lib/", "unix=0755", ":libs_collected", +"copy", "/opt/mycomponent/etc/", "unix=0600;section=confignoreplace", ":secret_properties", +"copy", "/opt/mycomponent/etc/", "unix=0644;section=confignoreplace", "//mycomponent/etc:foo.properties", +"copy", "/opt/mycomponent/etc/", "unix=0644;section=confignoreplace", "//mycomponent/etc:bar.properties", +"copy", "/opt/mycomponent/etc/", "unix=0644;section=confignoreplace", "//mycomponent/etc:foobar.properties", +"copy", "/opt/mycomponent/etc/", "unix=0644;section=confignoreplace", "//mycomponent/etc:idkmybffjill.properties", +"copy", "/opt/mycomponent/etc/", "unix=0644;section=confignoreplace", "//mycomponent/etc/some/subpackage:subpackage.properties" +"copy", "/opt/mycomponent/resources/i18n/en_us", "unix=0644", "@i18n//mycomponent/en_us" +"copy", "/opt/mycomponent/resources/i18n/en_gb", "unix=0644", "@i18n//mycomponent/en_gb" +"copy", "/opt/mycomponent/resources/i18n/es_es", "unix=0644", "@i18n//mycomponent/es_es" +"copy", "/opt/mycomponent/resources/i18n/es_mx", "unix=0644", "@i18n//mycomponent/es_mx" +"copy", "/opt/mycomponent/bin/mycomponent-service.bin", "unix=0755", "//mycomponent/src:service.bin" +"copy", "/opt/mycomponent/bin/mycomponent-runner", "unix=0755", "//mycomponent/src:runner" +"symlink", "/usr/bin/mycomponentd", "", "/opt/mycomponent/bin/mycomponent-service-runner" +"mkdir", "/opt/mycomponent", "unix=0755", "", +"mkdir", "/opt/mycomponent/lib", "unix=0755", "", +"mkdir", "/opt/mycomponent/resources", "unix=0755", "", +"mkdir", "/opt/mycomponent/state", "unix=0755", "", +"mkdir", "/opt/mycomponent/resources/i18n", "unix=0755", "", +""" + + +manifest_info = { + "default_attrs": _default_attrs, + "manifest": _manifest, +} diff --git a/pkg/experimental/manifest/examples/action-first.bzl b/pkg/experimental/manifest/examples/action-first.bzl new file mode 100644 index 00000000..f2c37148 --- /dev/null +++ b/pkg/experimental/manifest/examples/action-first.bzl @@ -0,0 +1,31 @@ +_default_attrs = "user=root;group=root" +_manifest = [ + #action dest attributes... source + ("copy", "/etc/syslog.conf.d/mycomponent.conf", "unix=0644;section=confignoreplace", "//mycomponent/etc:conf"), + ("copy", "/opt/mycomponent/lib/", "unix=0755", ":libs_collected"), + ("copy", "/opt/mycomponent/etc/", "unix=0600;section=confignoreplace", ":secret_properties"), + ("copy", "/opt/mycomponent/etc/", "unix=0644;section=confignoreplace", "//mycomponent/etc:foo.properties"), + ("copy", "/opt/mycomponent/etc/", "unix=0644;section=confignoreplace", "//mycomponent/etc:bar.properties"), + ("copy", "/opt/mycomponent/etc/", "unix=0644;section=confignoreplace", "//mycomponent/etc:foobar.properties"), + ("copy", "/opt/mycomponent/etc/", "unix=0644;section=confignoreplace", "//mycomponent/etc:idkmybffjill.properties"), + ("copy", "/opt/mycomponent/etc/", "unix=0644;section=confignoreplace", "//mycomponent/etc/some/subpackage:subpackage.properties"), + ("copy", "/opt/mycomponent/resources/i18n/en_us", "unix=0644", "@i18n//mycomponent/en_us"), + ("copy", "/opt/mycomponent/resources/i18n/en_gb", "unix=0644", "@i18n//mycomponent/en_gb"), + ("copy", "/opt/mycomponent/resources/i18n/es_es", "unix=0644", "@i18n//mycomponent/es_es"), + ("copy", "/opt/mycomponent/resources/i18n/es_mx", "unix=0644", "@i18n//mycomponent/es_mx"), + ("copy", "/opt/mycomponent/bin/mycomponent-service.bin", "unix=0755", "//mycomponent/src:service.bin"), + ("copy", "/opt/mycomponent/bin/mycomponent-runner", "unix=0755", "//mycomponent/src:runner"), + ("symlink", "/usr/bin/mycomponentd", "", "/opt/mycomponent/bin/mycomponent-service-runner"), + + ("mkdir", "/opt/mycomponent", "unix=0755", ""), + ("mkdir", "/opt/mycomponent/lib", "unix=0755", ""), + ("mkdir", "/opt/mycomponent/resources", "unix=0755", ""), + ("mkdir", "/opt/mycomponent/state", "unix=0755", ""), + ("mkdir", "/opt/mycomponent/resources/i18n", "unix=0755", ""), +] + + +manifest_info = { + "default_attrs": _default_attrs, + "manifest": _manifest, +} diff --git a/pkg/experimental/manifest/examples/dest-first.bzl b/pkg/experimental/manifest/examples/dest-first.bzl new file mode 100644 index 00000000..28cc49ee --- /dev/null +++ b/pkg/experimental/manifest/examples/dest-first.bzl @@ -0,0 +1,31 @@ +_default_attrs = "user=root;group=root" +_manifest = [ + #dest action attributes... source + ("/etc/syslog.conf.d/mycomponent.conf", "copy", "unix=0644;section=config(noreplace)", "//mycomponent/etc:conf"), + ("/opt/mycomponent/lib/", "copy", "unix=0755", ":libs_collected"), + ("/opt/mycomponent/etc/", "copy", "unix=0600;section=config(noreplace)", ":secret_properties"), + ("/opt/mycomponent/etc/", "copy", "unix=0644;section=config(noreplace)", "//mycomponent/etc:foo.properties"), + ("/opt/mycomponent/etc/", "copy", "unix=0644;section=config(noreplace)", "//mycomponent/etc:bar.properties"), + ("/opt/mycomponent/etc/", "copy", "unix=0644;section=config(noreplace)", "//mycomponent/etc:foobar.properties"), + ("/opt/mycomponent/etc/", "copy", "unix=0644;section=config(noreplace)", "//mycomponent/etc:idkmybffjill.properties"), + ("/opt/mycomponent/etc/", "copy", "unix=0644;section=config(noreplace),", "//mycomponent/etc/some/subpackage:subpackage.properties")2, + ("/opt/mycomponent/resources/i18n/en_us", "copy", "unix=0644", "@i18n//mycomponent/en_us"), + ("/opt/mycomponent/resources/i18n/en_gb", "copy", "unix=0644", "@i18n//mycomponent/en_gb"), + ("/opt/mycomponent/resources/i18n/es_es", "copy", "unix=0644", "@i18n//mycomponent/es_es"), + ("/opt/mycomponent/resources/i18n/es_mx", "copy", "unix=0644", "@i18n//mycomponent/es_mx"), + ("/opt/mycomponent/bin/mycomponent-service.bin", "copy", "unix=0755", "//mycomponent/src:service.bin"), + ("/opt/mycomponent/bin/mycomponent-runner", "copy", "unix=0755", "//mycomponent/src:runner"), + ("/usr/bin/mycomponentd", "symlink", "", "/opt/mycomponent/bin/mycomponent-service-runner"), + + ("/opt/mycomponent", "mkdir", "unix=0755", ""), + ("/opt/mycomponent/lib", "mkdir", "unix=0755", ""), + ("/opt/mycomponent/resources", "mkdir", "unix=0755", ""), + ("/opt/mycomponent/state", "mkdir", "unix=0755", ""), + ("/opt/mycomponent/resources/i18n", "mkdir", "unix=0755", ""), + +] + +manifest_info = { + "default_attrs": _default_attrs, + "manifest": _manifest, +} diff --git a/pkg/experimental/manifest/manifest.bzl b/pkg/experimental/manifest/manifest.bzl new file mode 100644 index 00000000..06e9574d --- /dev/null +++ b/pkg/experimental/manifest/manifest.bzl @@ -0,0 +1,266 @@ +# 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. + +"""This module provides an alternate way to specify package contents using a +simple csv-like description specification. + +This allows for a highly succinct descriptions of package contents in many +common cases. + +The following cases are known NOT to be supported by this scheme: + +- Exclusion of any files from a source target with multiple files. +- Renaming of any files from a source target with multiple files. +- Any sort of prefix stripping or path manipulation. + +Normal pkg_filegroup's should be used in the above cases. + +""" + +load("//experimental:pkg_filegroup.bzl", "pkg_filegroup", "pkg_mkdirs", "pkg_mklinks") + +# Example manifest: +# +#[ +# #dest action attributes... source +# ("/some/destination/directory/", "copy", "unix=0755", ":target-or-label"), +# ("/some/destination/binary-name", "copy", "unix=0755", ":target-or-label"), +# ("/dir", "mkdir", "unix=0755", "IGNORED"), +# ("/dir/child", "mkdir", "unix=0755", "IGNORED"), +# ("/dir/child/other-child", "mkdir", "unix=0755", "IGNORED"), +# ("target", "symlink", "unix=0777", "source"), +#] + +_MANIFEST_ROW_SIZE = 4 + +def _manifest_process_copy(name, destination, attrs, source, **kwargs): + allowed_attrs = ["section", "unix", "user", "group"] + + section = None + unix_perms = "-" + user = "-" + group = "-" + for decl in attrs.split(";"): + (attr, _, value) = decl.partition("=") + if attr not in allowed_attrs: + fail("{}: unknown attribute {}".format(name, attr)) + if attr == "section": + section = value + elif attr == "unix": + unix_perms = value + elif attr == "user": + user = value + elif attr == "group": + group = value + + if destination.endswith("/"): + prefix = destination + renames = None + else: + prefix = None + renames = {source: destination} + + pkg_filegroup( + name = name, + srcs = [source], + attrs = {"unix": [unix_perms, user, group]}, + section = section, + renames = renames, + prefix = prefix, + **kwargs + ) + +def _manifest_process_mkdir(name, destination, attrs, source, **kwargs): + allowed_attrs = ["section", "unix", "user", "group"] + + section = None + unix_perms = "-" + user = "-" + group = "-" + for decl in attrs.split(";"): + (attr, _, value) = decl.partition("=") + if attr not in allowed_attrs: + fail("{}: unknown attribute {}".format(name, attr)) + if attr == "section": + section = value + elif attr == "unix": + unix_perms = value + elif attr == "user": + user = value + elif attr == "group": + group = value + + pkg_mkdirs( + name = name, + dirs = [destination], + attrs = {"unix": [unix_perms, user, group]}, + section = section, + **kwargs + ) + +def _manifest_process_symlink(name, destination, attrs, source, **kwargs): + allowed_attrs = ["section", "unix", "user", "group"] + + section = None + unix_perms = "0777" + user = "-" + group = "-" + + if attrs != "-": + for decl in attrs.split(";"): + (attr, _, value) = decl.partition("=") + if attr not in allowed_attrs: + fail("{}: unknown attribute {}".format(name, attr)) + if attr == "section": + section = value + elif attr == "unix": + unix_perms = value + elif attr == "user": + user = value + elif attr == "group": + group = value + + pkg_mklinks( + name = name, + links = {destination: source}, + attrs = {"unix": [unix_perms, user, group]}, + section = section, + **kwargs + ) + +def pkg_list_manifest(name, manifest, default_attrs = "", **kwargs): + """ + Process a "manifest" of package specifications into packaging rules. + + The "manifest" format is a list of tuples that looks like this: + + ``` + (destination, action, attributes, source) + ``` + + Each element is a string. + + Where: + - `destination` refers to the destination path within the package, although + can have special meaning depending on the action + + - `action` is one of: + - "copy", for a simple install-to-destination action, corresponding to `pkg_filegroup` + - "mkdir", for a directory creation action, corresponding to `pkg_mkdirs` + - "symlink", for a symlink creation action, corresponding to `pkg_mklinks` + + - `attributes` refers to various properties and permissions on the + destination targets. They are formatted as a semicolon-separated list of + key=value pairs, e.g. `foo=bar;baz=qux`. + + Common attributes include: + + - "unix": UNIX-style filesystem permissions as four-digit octal (e.g. "0644") + + - "user": Filesystem owning user, as known to your target platform + + - "group: Filesystem owning group, as known to your target platform + + - "section": Package installation property, see each individual action for + details. + + - `source` depends on the action + + For `copy` actions: + + - `destination`: the location within the package where files are installed. + If `destination` ends in "/", the `source` is installed to this path as + the prefix, otherwise, it is renamed to the destination. + + If `destination` refers to a target with more than one output, only the + "/" option is allowed. + + - `attributes`: see "Common attributes", above. "section" corresponds to the + "section" attribute of `pkg_filegroup`. + + - `source`: to a label that specifies the value to be installed. + + For "mkdir" actions: + + - `destination` refers to the path within the package where the directory is created. + + - `attributes: see "Common attributes", above. "section" corresponds to the + "section" attribute of `pkg_mkdirs`. + + - `source` is ignored. + + For "symlink" actions: + + - `destination`: the name of the symbolic link in the target package + + - `attributes: see "Common attributes", above. "section" corresponds to the + "section" attribute of `pkg_mklinks`. + + - `source` refers to the "target" of the symbolic link in question. It may + exist outside of the defined package. + + Args: + name: string value used to influence the output rule names + + manifest: list of tuples, with the format described in the introduction of this rule + + default_attrs: A string representing the default attributes for all + actions in this manifest. Attributes must be specified as though they + were in a manifest. + + **kwargs: Any arguments that should be passed to generated rules. + + Returns: + A list of rules that can be passed to a `pkg_filegroup`-compatible packaging rule. + + The output rules are each named after the rule "name" and the index in + the manifest, which can be useful for finding where precisely errors can + occur. + """ + + rules = [] + + for idx, desc in enumerate(manifest): + if len(desc) != _MANIFEST_ROW_SIZE: + fail("Package description index {} malformed (size {}, must be {})".format( + idx, + len(desc), + _MANIFEST_ROW_SIZE, + )) + + (destination, action, attrs, source) = desc + + if default_attrs != "": + attrs = default_attrs + ";" + attrs + + rule_name = "{}_manifest_elem_{}".format(name, idx) + if action == "copy": + _manifest_process_copy(rule_name, destination, attrs, source, **kwargs) + elif action == "mkdir": + _manifest_process_mkdir(rule_name, destination, attrs, source, **kwargs) + elif action == "symlink": + _manifest_process_symlink(rule_name, destination, attrs, source, **kwargs) + else: + fail("Package description index {} malformed (unknown action {})".format( + idx, + action, + )) + + rules.append(":{}".format(rule_name)) + + # TODO: making this return something like a pkg_filegroup requires some sort + # of "aggregator" rule. The original pkg_filegroup framework was not + # designed this way, and it needs to be rethought to better facilitate this + # purpose. + return rules diff --git a/pkg/experimental/manifest/manifest_test.py b/pkg/experimental/manifest/manifest_test.py new file mode 100644 index 00000000..131c9c49 --- /dev/null +++ b/pkg/experimental/manifest/manifest_test.py @@ -0,0 +1,270 @@ +#!/usr/bin/env python3 + +# 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. + +import itertools +import os +import sys +import unittest + +# Global state used in the below test +files = [] +mkdirs = [] +links = [] + +# Rule stubs +def pkg_filegroup(**kwargs): + global files + files.append(kwargs) + +def pkg_mkdirs(**kwargs): + global mkdirs + mkdirs.append(kwargs) + +def pkg_mklinks(**kwargs): + global links + links.append(kwargs) + +# Failure stub +class StarlarkException(Exception): + pass +def fail(msg, **kwargs): + raise StarlarkException(msg) + +# "load" stub +def load(name, *args): + pass + +# Evaluate the manifest generating code with the above functions known +# +# This tosses a bunch of stuff into the global namespace, which isn't exactly clean. +# +# It also means that this test must be run sequentially. Given how fast this +# will run, I'm not too concerned. +with open("experimental/manifest/manifest.bzl", "r") as fh: + s = fh.read() + exec(s) + + +# Here's an alternate way to do this "modularly". +# +# You'll also need to add all of the pkg_* functions above into the manifest_bzl +# module. To be truly self-contained, this may need to be done inside the test case + +#import importlib +#import builtins +#builtins.load = load +#spec = importlib.util.spec_from_loader("manifest_bzl", +# importlib.machinery.SourceFileLoader("manifest_bzl", "experimental/manifest/manifest.bzl")) +#manifest_bzl = importlib.util.module_from_spec(spec) +#spec.loader.exec_module(manifest_bzl) + +class PkgManifestTest(unittest.TestCase): + def setUp(self): + pass + + def tearDown(self): + global files, mkdirs, links + files.clear() + mkdirs.clear() + links.clear() + + @staticmethod + def _manifest_fmt(name, idx): + return "{}_manifest_elem_{}".format(name, idx) + + def test_copy_action(self): + global files + + manifest = [ + ("/foo/baz/", "copy", "unix=0755;user=user;group=group", "foo:bar"), + ] + + pkg_list_manifest( + name = "manifest_copy", + manifest = manifest, + visibility = ["//visibility:public"] + ) + + # Only one item in the manifest + self.assertEqual(len(files), 1) + + manifest_file = files[0] + + # Named properly + self.assertEqual(manifest_file['name'], self._manifest_fmt("manifest_copy", 0)) + + # Only one source (which is ["foo:bar"]) + self.assertListEqual(manifest_file['srcs'], ["foo:bar"]) + + # Renaming operation -- which does not use a prefix + self.assertEqual(manifest_file['prefix'], '/foo/baz/') + self.assertIsNone(manifest_file['renames']) + + # Assert permissions propagated properly + self.assertDictEqual(manifest_file['attrs'], {"unix": ["0755", "user", "group"]}) + + # Assert kwargs propagated + self.assertListEqual(manifest_file['visibility'], ['//visibility:public']) + + def test_copy_action_rename(self): + global files + + manifest = [ + # Renaming + ("/foo/baz", "copy", "unix=0755;user=user;group=group", "foo:bar"), + ] + + pkg_list_manifest( + name = "manifest_rename", + manifest = manifest, + visibility = ["//visibility:public"] + ) + + # Only one item in the manifest + self.assertEqual(len(files), 1) + + manifest_file = files[0] + + # Named properly + self.assertEqual(manifest_file['name'], self._manifest_fmt("manifest_rename", 0)) + + # Only one source (which is ["foo:bar"]) + self.assertListEqual(manifest_file['srcs'], ["foo:bar"]) + + # Renaming operation -- which does not use a prefix + self.assertIsNone(manifest_file['prefix']) + self.assertDictEqual(manifest_file['renames'], {"foo:bar": "/foo/baz"}) + + # Assert permissions propagated properly + self.assertDictEqual(manifest_file['attrs'], {"unix": ["0755", "user", "group"]}) + + # Assert kwargs propagated + self.assertListEqual(manifest_file['visibility'], ['//visibility:public']) + + def test_mkdir_action(self): + global mkdirs + manifest = [ + ("/foo/bar/qux", "mkdir", "unix=0777;user=user;group=group", "IGNORED"), + ] + + pkg_list_manifest( + name = "manifest_mkdir_single", + manifest = manifest, + visibility = ["//visibility:public"] + ) + + self.assertEqual(len(mkdirs), 1) + + manifest_dir = mkdirs[0] + + self.assertEqual(manifest_dir['name'], self._manifest_fmt("manifest_mkdir_single", 0)) + + self.assertListEqual(manifest_dir['dirs'], ['/foo/bar/qux']) + + # Assert permissions propagated properly + self.assertDictEqual(manifest_dir['attrs'], {"unix": ["0777", "user", "group"]}) + + # Assert kwargs propagated + self.assertListEqual(manifest_dir['visibility'], ['//visibility:public']) + + + # TODO: the below skipped test + def test_symlink_action(self): + # Make a symlink, confirm whether or not its properties propagate properly + self.skipTest("Not implemented") + + def test_multi_manifest(self): + global files, mkdirs, links + + # Make a "full" manifest, see if everything is sufficiently consistent + manifest = [ + ("/foo/baz/", "copy", "unix=0755;user=user;group=group", "foo:bar"), + ("/foo/baz", "copy", "unix=0755;user=user;group=group", "foo:bar"), + ("/foo/bar/qux", "mkdir", "unix=0777;user=user;group=group", "IGNORED"), + ("/symlink-dest", "symlink", "user=user;group=group", "/symlink-src") + ] + + pkg_list_manifest( + name = "manifest_multi", + manifest = manifest, + ) + + self.assertEqual(len(files), 2) + self.assertEqual(len(mkdirs), 1) + self.assertEqual(len(links), 1) + # print(files) + # print(mkdirs) + # print(links) + + for i, f in enumerate(files): + expected_name = self._manifest_fmt("manifest_multi", i) + self.assertEqual(f['name'], expected_name) + # See if this field looks like a pkg_filegroup + self.assertIn('prefix', f, + "copy action '{}' may not be correct (expected 'prefix' attribute)".format(expected_name)) + self.assertIn('renames', f, + "copy action '{}' may not be correct (expected 'renames' attribute)".format(expected_name)) + + for i, d in enumerate(mkdirs, start=2): + expected_name = self._manifest_fmt("manifest_multi", i) + self.assertEqual(d['name'], expected_name) + # See if this field looks like a pkg_mkdirs + self.assertIn('dirs', d, + "directory action '{}' may not be correct (expected 'dirs' attribute)".format(expected_name)) + + for i, l in enumerate(links, start=3): + expected_name = self._manifest_fmt("manifest_multi", i) + self.assertEqual(l['name'], expected_name) + # See if this field looks like a pkg_mkdirs + self.assertIn('links', l, + "symlink action '{}' may not be correct (expected 'links' attribute)".format(expected_name)) + + def test_multi_manifest_with_defaults(self): + # Like the above test, except try layering in some defaults + global files, mkdirs, links + + # Make a "full" manifest, see if everything is sufficiently consistent + manifest = [ + ("/foo/baz/", "copy", "user=user;group=group", "foo:bar"), + ("/foo/baz", "copy", "user=user;group=group", "foo:bar"), + ("/foo/bar/qux", "mkdir", "user=user;group=group", "IGNORED"), + ("/symlink-dest", "symlink", "user=user;group=group", "/symlink-src") + ] + + pkg_list_manifest( + name = "manifest_multi", + manifest = manifest, + default_attrs = "unix=0755;user=root;group=root", + ) + + for idx, entry in enumerate(itertools.chain(files, mkdirs, links)): + # user/group are overridden in each entry, but the permissions + # aren't + self.assertDictEqual(entry['attrs'], {"unix": ["0755", "user", "group"]}, + "Attrs for manifest entry {} are incorrect".format(idx)) + + # TODO: the below skipped tests: + def test_invalid_manifest(self): + # Try passing in a manifest of improper size. See if it's rejected + # Pass in a manifest with an invalid action + self.skipTest("Not implemented") + + def test_invalid_attributes(self): + # Like attribute "fake_attribute" + self.skipTest("Not implemented") + +if __name__ == '__main__': + unittest.main() From 8dd2e54f5edfe310def01edf4831ccd8ab62575f Mon Sep 17 00:00:00 2001 From: Andrew Psaltis Date: Fri, 25 Sep 2020 12:37:04 -0400 Subject: [PATCH 2/2] Actually make sure that `bazel test //...` works My bad. --- pkg/experimental/manifest/examples/BUILD | 4 +++- pkg/experimental/manifest/examples/dest-first.bzl | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/experimental/manifest/examples/BUILD b/pkg/experimental/manifest/examples/BUILD index c431c0d9..f3c0cb10 100644 --- a/pkg/experimental/manifest/examples/BUILD +++ b/pkg/experimental/manifest/examples/BUILD @@ -30,7 +30,9 @@ pkg_rpm( name = "nonbuilding-rpm", data = manifest + [ # Other pkg_filegroups and friendes here - ] + ], tags = ["manual"], + license = "N/A", + summary = "don't build me!", # ... ) diff --git a/pkg/experimental/manifest/examples/dest-first.bzl b/pkg/experimental/manifest/examples/dest-first.bzl index 28cc49ee..03e6f7a6 100644 --- a/pkg/experimental/manifest/examples/dest-first.bzl +++ b/pkg/experimental/manifest/examples/dest-first.bzl @@ -8,14 +8,15 @@ _manifest = [ ("/opt/mycomponent/etc/", "copy", "unix=0644;section=config(noreplace)", "//mycomponent/etc:bar.properties"), ("/opt/mycomponent/etc/", "copy", "unix=0644;section=config(noreplace)", "//mycomponent/etc:foobar.properties"), ("/opt/mycomponent/etc/", "copy", "unix=0644;section=config(noreplace)", "//mycomponent/etc:idkmybffjill.properties"), - ("/opt/mycomponent/etc/", "copy", "unix=0644;section=config(noreplace),", "//mycomponent/etc/some/subpackage:subpackage.properties")2, + ("/opt/mycomponent/etc/", "copy", "unix=0644;section=config(noreplace),", "//mycomponent/etc/some/subpackage:subpackage.properties"), ("/opt/mycomponent/resources/i18n/en_us", "copy", "unix=0644", "@i18n//mycomponent/en_us"), ("/opt/mycomponent/resources/i18n/en_gb", "copy", "unix=0644", "@i18n//mycomponent/en_gb"), ("/opt/mycomponent/resources/i18n/es_es", "copy", "unix=0644", "@i18n//mycomponent/es_es"), ("/opt/mycomponent/resources/i18n/es_mx", "copy", "unix=0644", "@i18n//mycomponent/es_mx"), ("/opt/mycomponent/bin/mycomponent-service.bin", "copy", "unix=0755", "//mycomponent/src:service.bin"), ("/opt/mycomponent/bin/mycomponent-runner", "copy", "unix=0755", "//mycomponent/src:runner"), - ("/usr/bin/mycomponentd", "symlink", "", "/opt/mycomponent/bin/mycomponent-service-runner"), + # FIXME: attributes should be allowed to be empty + ("/usr/bin/mycomponentd", "symlink", "unix=0777", "/opt/mycomponent/bin/mycomponent-service-runner"), ("/opt/mycomponent", "mkdir", "unix=0755", ""), ("/opt/mycomponent/lib", "mkdir", "unix=0755", ""),