From 6f25b273102b87882d77f862bb83f48cbb6a58fe Mon Sep 17 00:00:00 2001 From: Andrew Psaltis Date: Thu, 5 Jan 2023 17:34:08 -0500 Subject: [PATCH 1/7] Implement the changes - `strip_prefix#files_only()` is now `strip_prefix#flatten()` - `pkg_files#strip_prefix` is now `pkg_files#srcs_strip_prefix` - The default for (now) `pkg_files#srcs_strip_prefix` is now to strip prefix from the package root instead of flattening. --- pkg/mappings.bzl | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/pkg/mappings.bzl b/pkg/mappings.bzl index 6b3ab6ad..2366e706 100644 --- a/pkg/mappings.bzl +++ b/pkg/mappings.bzl @@ -36,7 +36,7 @@ _PKGFILEGROUP_STRIP_ALL = "." REMOVE_BASE_DIRECTORY = "\0" -def _sp_files_only(): +def _sp_flatten(): return _PKGFILEGROUP_STRIP_ALL def _sp_from_pkg(path = ""): @@ -52,11 +52,11 @@ def _sp_from_root(path = ""): return "/" + path strip_prefix = struct( - _doc = """pkg_files `strip_prefix` helper. Instructs `pkg_files` what to do with directory prefixes of files. + _doc = """pkg_files `srcs_strip_prefix` helper. Instructs `pkg_files` what to do with directory prefixes of files. Each member is a function that equates to: - - `files_only()`: strip all directory components from all paths + - `flatten()`: strip all directory components from all paths - `from_pkg(path)`: strip all directory components up to the current package, plus what's in `path`, if provided. @@ -67,7 +67,7 @@ strip_prefix = struct( Prefix stripping is applied to each `src` in a `pkg_files` rule independently. """, - files_only = _sp_files_only, + flatten = _sp_flatten, from_pkg = _sp_from_pkg, from_root = _sp_from_root, ) @@ -198,15 +198,15 @@ def _pkg_files_impl(ctx): # Exclude excludes srcs = [f for f in ctx.files.srcs if f not in ctx.files.excludes] - if ctx.attr.strip_prefix == _PKGFILEGROUP_STRIP_ALL: + if ctx.attr.srcs_strip_prefix == _PKGFILEGROUP_STRIP_ALL: src_dest_paths_map = {src: paths.join(ctx.attr.prefix, src.basename) for src in srcs} - elif ctx.attr.strip_prefix.startswith("/"): + elif ctx.attr.srcs_strip_prefix.startswith("/"): # Relative to workspace/repository root src_dest_paths_map = {src: paths.join( ctx.attr.prefix, _do_strip_prefix( _path_relative_to_repo_root(src), - ctx.attr.strip_prefix[1:], + ctx.attr.srcs_strip_prefix[1:], src, ), ) for src in srcs} @@ -216,7 +216,7 @@ def _pkg_files_impl(ctx): ctx.attr.prefix, _do_strip_prefix( _path_relative_to_package(src), - ctx.attr.strip_prefix, + ctx.attr.srcs_strip_prefix, src, ), ) for src in srcs} @@ -337,19 +337,18 @@ pkg_files = rule( """, default = "", ), - "strip_prefix": attr.string( - doc = """What prefix of a file's path to discard prior to installation. + "srcs_strip_prefix": attr.string( + doc = """What prefix of a file's source path to discard prior to installation. This specifies what prefix of an incoming file's path should not be included in the output package at after being appended to the install prefix (the `prefix` attribute). Note that this is only - applied to full directory names, see `strip_prefix` for more - details. + applied to full directory names, see the `strip_prefix` struct + for more details. - Use the `strip_prefix` struct to define this attribute. If this - attribute is not specified, all directories will be stripped from - all files prior to being included in packages - (`strip_prefix.files_only()`). + Use the `strip_prefix` psuedo-structure to define this attribute. + If this attribute is not specified, paths will be preserved relative + to their current package (equivalent to `strip_prefix.from_pkg()`). If prefix stripping fails on any file provided in `srcs`, the build will fail. @@ -359,7 +358,7 @@ pkg_files = rule( TreeArtifacts (directory outputs), or the directories themselves. See also #269. """, - default = strip_prefix.files_only(), + default = strip_prefix.from_pkg(), ), "excludes": attr.label_list( doc = """List of files or labels to exclude from the inputs to this rule. @@ -377,13 +376,13 @@ pkg_files = rule( `pkg_file`s relative to the `prefix` attribute. Keys to the dict are source files/labels, values are destinations relative to the `prefix`, ignoring whatever value was provided for - `strip_prefix`. + `srcs_strip_prefix`. If the key refers to a TreeArtifact (directory output), you may specify the constant `REMOVE_BASE_DIRECTORY` as the value, which will result in all containing files and directories being installed relative to the otherwise specified install prefix (via the `prefix` - and `strip_prefix` attributes), not the directory name. + and `srcs_strip_prefix` attributes), not the directory name. The following keys are rejected: From a0e439ff1d3dbf0200976630e593315a088096dc Mon Sep 17 00:00:00 2001 From: Andrew Psaltis Date: Thu, 5 Jan 2023 17:34:55 -0500 Subject: [PATCH 2/7] Implement the migration script This script migrates BUILD files to the new behavior. It is written using libCST, which provides concrete syntax tree functionality for python -- in particular, it knows how to preserve whitespace. --- WORKSPACE | 12 ++ migration/BUILD | 30 ++++ migration/migrate_strip_prefix.py | 257 ++++++++++++++++++++++++++++++ migration/migration_deps.txt | 1 + migration/migration_deps_lock.txt | 88 ++++++++++ pkg/deps.bzl | 4 +- 6 files changed, 390 insertions(+), 2 deletions(-) create mode 100644 migration/BUILD create mode 100644 migration/migrate_strip_prefix.py create mode 100644 migration/migration_deps.txt create mode 100644 migration/migration_deps_lock.txt diff --git a/WORKSPACE b/WORKSPACE index 58ab9170..e0d036f2 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -71,3 +71,15 @@ http_archive( load("@bazel_stardoc//:setup.bzl", "stardoc_repositories") stardoc_repositories() + +load("@rules_python//python:pip.bzl", "pip_parse") + +pip_parse( + name = "rules_pkg_migration_deps", + requirements_lock = "//migration:migration_deps_lock.txt", +) + +load("@rules_pkg_migration_deps//:requirements.bzl", migration_install_deps = "install_deps") + +migration_install_deps() + diff --git a/migration/BUILD b/migration/BUILD new file mode 100644 index 00000000..b3ef73ca --- /dev/null +++ b/migration/BUILD @@ -0,0 +1,30 @@ +# Copyright 2023 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_binary") +load("@rules_python//python:pip.bzl", "compile_pip_requirements") +load("@rules_pkg_migration_deps//:requirements.bzl", "requirement") + +py_binary( + name = "strip_prefix", + srcs = ["migrate_strip_prefix.py"], + main = "migrate_strip_prefix.py", + deps = [requirement("libcst")], +) + +compile_pip_requirements( + name = "requirements", + requirements_in = "migration_deps.txt", + requirements_txt = "migration_deps_lock.txt", +) diff --git a/migration/migrate_strip_prefix.py b/migration/migrate_strip_prefix.py new file mode 100644 index 00000000..92f6a74b --- /dev/null +++ b/migration/migrate_strip_prefix.py @@ -0,0 +1,257 @@ +#!/usr/bin/env python3 + +# Copyright 2023 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 argparse +import pathlib +import os +import sys + +import libcst as cst +import libcst.codemod as codemod +import libcst.matchers as cstm + +class PkgFilesStripPrefixTransformer(codemod.ContextAwareTransformer): + def __init__(self, context): + super().__init__(context) + self.load_node = None + self.may_need_to_amend_load = None + + @cstm.call_if_inside(cstm.Call(func = cstm.Name("pkg_files"))) + @cstm.leave(cstm.Arg(keyword = cstm.Name("strip_prefix"))) + def rename_strip_prefix_to_srcs_strip_prefix(self, + original_node: cst.Arg, + updated_node: cst.Arg) -> cst.Arg: + return updated_node.with_changes(keyword = cst.Name("srcs_strip_prefix")) + + + @cstm.call_if_inside(cstm.Call(func = cstm.Attribute(value = cstm.Name("strip_prefix")))) + @cstm.leave(cstm.Attribute(value = cstm.Name("strip_prefix"))) + def rename_strip_prefix_files_only_to_flatten(self, + original_node: cst.Attribute, + updated_node: cst.Attribute) -> cst.Attribute: + return updated_node.with_changes( + attr = cst.Name('flatten') if updated_node.attr.value == 'files_only' else updated_node.attr + ) + + @cstm.call_if_inside(cstm.Call( + func = cstm.Name("pkg_files"), + args = [cstm.ZeroOrMore(~cstm.OneOf( + cstm.Arg(keyword=cstm.Name('strip_prefix')), + # Be idempotent + cstm.Arg(keyword=cstm.Name('srcs_strip_prefix')) + ))], + )) + @cstm.leave(cstm.Arg(keyword = cstm.Name("srcs"))) + def strip_prefix_account_for_new_default(self, + original_node: cst.Arg, + updated_node: cst.Arg) -> cst.Arg: + new_node = updated_node.with_changes( + keyword = cst.Name("srcs_strip_prefix"), + value = cst.Call( + func = cst.Attribute( + value = cst.Name("strip_prefix"), + attr = cst.Name("flatten"), + ) + ) + ) + may_need_to_amend_load = True + return cst.FlattenSentinel([ + updated_node, + new_node, + ]) + + # FIXME: need to incorporate a hack to allow the decorator's contents to be + # evaluated at runtime so we can name the rules_pkg repository as a CLI + # argument. Perhaps one like https://stackoverflow.com/q/11731136, which is + # a bit complex. + @cstm.leave( + cstm.Call( + func=cstm.Name("load"), + args=[ + cstm.OneOf( + cstm.Arg(value=cstm.SimpleString('"//:mappings.bzl"')), + cstm.Arg(value=cstm.SimpleString('"//pkg:mappings.bzl"')), + cstm.Arg(value=cstm.SimpleString('"@rules_pkg//:mappings.bzl"')), + cstm.Arg(value=cstm.SimpleString('"@rules_pkg//pkg:mappings.bzl"')), + ), + cstm.ZeroOrMore(~cstm.Arg(value=cstm.SimpleString('"strip_prefix"'))) + ], + ) + ) + def find_eligible_rules_pkg_node(self, + original_node: cst.Call, + updated_node: cst.Call) -> cst.Call: + self.load_node = updated_node + return updated_node + + def leave_Module(self, + original_node: cst.Module, + updated_node: cst.Module) -> cst.Module: + if self.load_node is None or self.may_need_to_amend_load == False: + # Nothing to do + return updated_node + + # This is somewhat convoluted, but it works. I guess. + + # Get the args in the original call. Needs to be made a list because + # we'll be changing it later. + args = list(self.load_node.args) + + ## Figure out where "strip_prefix" will appear in the arg list. + + # If the args list is already sensibly sorted, keep it that way. + # Else, we will be applying the entry to the end. + + # libcst nodes are not comparable. Instead derive our own metric to + # preserve the sort order. + def arg_value(a): + if isinstance(a.value, cst.SimpleString): + # It's a string. Get the value without the quotes. + return a.value.value[1:-1] + else: + # It's a `name = "string"` arrangement + return a.value + + # Do that sorting thing. The label argument (if properly formatted) + # should come first regardless. + args_sorted = sorted(args, key=arg_value) + + # Also of note is that libcst objects are only immediately compared by + # identity. + is_sorted = args_sorted == args + + if is_sorted: + new_args = sorted(args_sorted + [ + cst.Arg( + value=cst.SimpleString(value='"strip_prefix"') + ) + ], key = arg_value) + else: + # Just stick `strip_prefix` on the back. They can sort the list + # again later if they want + new_args = args + [ + cst.Arg( + value=cst.SimpleString(value='"strip_prefix"') + )] + + ## Given the existing arg list as a template, create the new one. + + # This is largely to keep track of whitespace. In libcst arg lists, + # it's associated with the comma at the end of each entry. + + # Set aside the first one and the last one. + starting_arg = args[0] + final_arg = args[-1] + + # Do the overlaying. Everything but the last arg is formatted the same + # way. + formatted_new_args = [new_args[0]] + for a in new_args[1:-1]: + formatted_new_args.append( + starting_arg.with_changes( + value = a.value, + keyword = a.keyword, + ) + ) + formatted_new_args.append( + final_arg.with_changes( + value = new_args[-1].value, + keyword = new_args[-1].keyword, + ) + ) + + new_load_node = self.load_node.with_changes( + args = formatted_new_args + ) + + return updated_node.deep_replace( + old_node = self.load_node, + new_node = new_load_node + ) + +def main(argv): + if os.name == 'nt': + exit('This script does not support windows. If this support is desired, please file an issue against rules_pkg.') + + parser = argparse.ArgumentParser() + + parser.add_argument('-d', '--diff', action='store_true', default=False) + parser.add_argument('-q', '--quiet', action='store_true', default=False) + parser.add_argument("paths", nargs='+', type=pathlib.Path, action='store') + + # TODO(nacl): this doesn't quite work with the decorators (since they're evaluated + # at compile-time), but we can work around it if that ends up being an issue. + #parser.add_argument('-n', '--rules_pkg_repo_name', action='store', default='rules_pkg') + + # TODO(nacl): See comment around `jobs` in + #`codemod.parallel_exec_transform_with_prettyprint()` + #parser.add_argument('-j', '--jobs', type=int, action='store', default=1) + + args = parser.parse_args(argv) + + cwd = pathlib.Path(os.environ["BUILD_WORKING_DIRECTORY"]) + + def real_filepath(p): + real_p = p + if not real_p.is_absolute(): + real_p = cwd / real_p + if real_p.is_dir(): + if (real_p / "BUILD").exists(): + return real_p / "BUILD" + elif (real_p / "BUILD.bazel").exists(): + return real_p / "BUILD.bazel" + else: + exit("Directory {} does not contain any valid BUILD files".format(real_p)) + else: + return real_p + + paths = [str(real_filepath(p)) for p in args.paths] + + # Calculate something close to the repo root to placate libcst (which was + # designed for python) + if len(paths) == 1: + repo_root = os.path.dirname(paths[0]) + else: + repo_root = os.path.commonprefix(paths) + + codemod_ctx = codemod.CodemodContext() + transformer = PkgFilesStripPrefixTransformer(codemod_ctx) + + # NOTE: this can't be interrupted, because the underlying transform + # execution catches KeyboardInterrupt and + # parallel_exec_transform_with_prettyprint thinks of it as a "skip". + print("") + print("!!! Interrupt with C-\\ !!!") + print("") + + # NOTE: doesn't work on Windows + res = codemod.parallel_exec_transform_with_prettyprint( + transformer, + paths, + # TODO(nacl): We cannot parallelize this due to issues pickling some objects + # in libcst. File a bug for this. + jobs = 1, # defaults to None, which means "use all cores" + unified_diff = args.diff, + # Otherwise libcst can't calculate module paths and emits (for us) + # spurious warnings. + repo_root = repo_root, + hide_progress = args.quiet, + ) + + print(res) + +if __name__ == '__main__': + exit(main(sys.argv[1:])) diff --git a/migration/migration_deps.txt b/migration/migration_deps.txt new file mode 100644 index 00000000..f4debef7 --- /dev/null +++ b/migration/migration_deps.txt @@ -0,0 +1 @@ +libcst == 0.4.9 diff --git a/migration/migration_deps_lock.txt b/migration/migration_deps_lock.txt new file mode 100644 index 00000000..e7774b80 --- /dev/null +++ b/migration/migration_deps_lock.txt @@ -0,0 +1,88 @@ +# +# This file is autogenerated by pip-compile +# To update, run: +# +# bazel run //migration:requirements.update +# +libcst==0.4.9 \ + --hash=sha256:01786c403348f76f274dbaf3888ae237ffb73e6ed6973e65eba5c1fc389861dd \ + --hash=sha256:045b3b0b06413cdae6e9751b5f417f789ffa410f2cb2815e3e0e0ea6bef10ec0 \ + --hash=sha256:10479371d04ee8dc978c889c1774bbf6a83df88fa055fcb0159a606f6679c565 \ + --hash=sha256:1266530bf840cc40633a04feb578bb4cac1aa3aea058cc3729e24eab09a8e996 \ + --hash=sha256:132bec627b064bd567e7e4cd6c89524d02842151eb0d8f5f3f7ffd2579ec1b09 \ + --hash=sha256:1350d375d3fb9b20a6cf10c09b2964baca9be753a033dde7c1aced49d8e58387 \ + --hash=sha256:15ded11ff7f4572f91635e02b519ae959f782689fdb4445bbebb7a3cc5c71d75 \ + --hash=sha256:183636141b839aa35b639e100883813744523bc7c12528906621121731b28443 \ + --hash=sha256:27be8db54c0e5fe440021a771a38b81a7dbc23cd630eb8b0e9828b7717f9b702 \ + --hash=sha256:3822056dc13326082362db35b3f649e0f4a97e36ddb4e487441da8e0fb9db7b3 \ + --hash=sha256:3cf48d7aec6dc54b02aec0b1bb413c5bb3b02d852fd6facf1f05c7213e61a176 \ + --hash=sha256:400166fc4efb9aa06ce44498d443aa78519082695b1894202dd73cd507d2d712 \ + --hash=sha256:46123863fba35cc84f7b54dd68826419cabfd9504d8a101c7fe3313ea03776f9 \ + --hash=sha256:4f9e42085c403e22201e5c41e707ef73e4ea910ad9fc67983ceee2368097f54e \ + --hash=sha256:596860090aeed3ee6ad1e59c35c6c4110a57e4e896abf51b91cae003ec720a11 \ + --hash=sha256:5b266867b712a120fad93983de432ddb2ccb062eb5fd2bea748c9a94cb200c36 \ + --hash=sha256:7415569ab998a85b0fc9af3a204611ea7fadb2d719a12532c448f8fc98f5aca4 \ + --hash=sha256:76491f67431318c3145442e97dddcead7075b074c59eac51be7cc9e3fffec6ee \ + --hash=sha256:786e562b54bbcd17a060d1244deeef466b7ee07fe544074c252c4a169e38f1ee \ + --hash=sha256:794250d2359edd518fb698e5d21c38a5bdfc5e4a75d0407b4c19818271ce6742 \ + --hash=sha256:7a98286cbbfa90a42d376900c875161ad02a5a2a6b7c94c0f7afd9075e329ce4 \ + --hash=sha256:7e33b66762efaa014c38819efae5d8f726dd823e32d5d691035484411d2a2a69 \ + --hash=sha256:9b3348c6b7711a5235b133bd8e11d22e903c388db42485b8ceb5f2aa0fae9b9f \ + --hash=sha256:aa53993e9a2853efb3ed3605da39f2e7125df6430f613eb67ef886c1ce4f94b5 \ + --hash=sha256:d67bc87e0d8db9434f2ea063734938a320f541f4c6da1074001e372f840f385d \ + --hash=sha256:e316da5a126f2a9e1d7680f95f907b575f082a35e2f8bd5620c59b2aaaebfe0a \ + --hash=sha256:e799add8fba4976628b9c1a6768d73178bf898f0ed1bd1322930c2d3db9063ba \ + --hash=sha256:f4487608258109f774300466d4ca97353df29ae6ac23d1502e13e5509423c9d5 \ + --hash=sha256:f6ce794483d4c605ef0f5b199a49fb6996f9586ca938b7bfef213bd13858d7ab \ + --hash=sha256:f9679177391ccb9b0cdde3185c22bf366cb672457c4b7f4031fcb3b5e739fbd6 + # via -r migration/migration_deps.txt +mypy-extensions==0.4.3 \ + --hash=sha256:090fedd75945a69ae91ce1303b5824f428daf5a028d2f6ab8a299250a846f15d \ + --hash=sha256:2d82818f5bb3e369420cb3c4060a7970edba416647068eb4c5343488a6c604a8 + # via typing-inspect +pyyaml==6.0 \ + --hash=sha256:0283c35a6a9fbf047493e3a0ce8d79ef5030852c51e9d911a27badfde0605293 \ + --hash=sha256:055d937d65826939cb044fc8c9b08889e8c743fdc6a32b33e2390f66013e449b \ + --hash=sha256:07751360502caac1c067a8132d150cf3d61339af5691fe9e87803040dbc5db57 \ + --hash=sha256:0b4624f379dab24d3725ffde76559cff63d9ec94e1736b556dacdfebe5ab6d4b \ + --hash=sha256:0ce82d761c532fe4ec3f87fc45688bdd3a4c1dc5e0b4a19814b9009a29baefd4 \ + --hash=sha256:1e4747bc279b4f613a09eb64bba2ba602d8a6664c6ce6396a4d0cd413a50ce07 \ + --hash=sha256:213c60cd50106436cc818accf5baa1aba61c0189ff610f64f4a3e8c6726218ba \ + --hash=sha256:231710d57adfd809ef5d34183b8ed1eeae3f76459c18fb4a0b373ad56bedcdd9 \ + --hash=sha256:277a0ef2981ca40581a47093e9e2d13b3f1fbbeffae064c1d21bfceba2030287 \ + --hash=sha256:2cd5df3de48857ed0544b34e2d40e9fac445930039f3cfe4bcc592a1f836d513 \ + --hash=sha256:40527857252b61eacd1d9af500c3337ba8deb8fc298940291486c465c8b46ec0 \ + --hash=sha256:473f9edb243cb1935ab5a084eb238d842fb8f404ed2193a915d1784b5a6b5fc0 \ + --hash=sha256:48c346915c114f5fdb3ead70312bd042a953a8ce5c7106d5bfb1a5254e47da92 \ + --hash=sha256:50602afada6d6cbfad699b0c7bb50d5ccffa7e46a3d738092afddc1f9758427f \ + --hash=sha256:68fb519c14306fec9720a2a5b45bc9f0c8d1b9c72adf45c37baedfcd949c35a2 \ + --hash=sha256:77f396e6ef4c73fdc33a9157446466f1cff553d979bd00ecb64385760c6babdc \ + --hash=sha256:819b3830a1543db06c4d4b865e70ded25be52a2e0631ccd2f6a47a2822f2fd7c \ + --hash=sha256:897b80890765f037df3403d22bab41627ca8811ae55e9a722fd0392850ec4d86 \ + --hash=sha256:98c4d36e99714e55cfbaaee6dd5badbc9a1ec339ebfc3b1f52e293aee6bb71a4 \ + --hash=sha256:9df7ed3b3d2e0ecfe09e14741b857df43adb5a3ddadc919a2d94fbdf78fea53c \ + --hash=sha256:9fa600030013c4de8165339db93d182b9431076eb98eb40ee068700c9c813e34 \ + --hash=sha256:a80a78046a72361de73f8f395f1f1e49f956c6be882eed58505a15f3e430962b \ + --hash=sha256:b3d267842bf12586ba6c734f89d1f5b871df0273157918b0ccefa29deb05c21c \ + --hash=sha256:b5b9eccad747aabaaffbc6064800670f0c297e52c12754eb1d976c57e4f74dcb \ + --hash=sha256:c5687b8d43cf58545ade1fe3e055f70eac7a5a1a0bf42824308d868289a95737 \ + --hash=sha256:cba8c411ef271aa037d7357a2bc8f9ee8b58b9965831d9e51baf703280dc73d3 \ + --hash=sha256:d15a181d1ecd0d4270dc32edb46f7cb7733c7c508857278d3d378d14d606db2d \ + --hash=sha256:d4db7c7aef085872ef65a8fd7d6d09a14ae91f691dec3e87ee5ee0539d516f53 \ + --hash=sha256:d4eccecf9adf6fbcc6861a38015c2a64f38b9d94838ac1810a9023a0609e1b78 \ + --hash=sha256:d67d839ede4ed1b28a4e8909735fc992a923cdb84e618544973d7dfc71540803 \ + --hash=sha256:daf496c58a8c52083df09b80c860005194014c3698698d1a57cbcfa182142a3a \ + --hash=sha256:e61ceaab6f49fb8bdfaa0f92c4b57bcfbea54c09277b1b4f7ac376bfb7a7c174 \ + --hash=sha256:f84fbc98b019fef2ee9a1cb3ce93e3187a6df0b2538a651bfb890254ba9f90b5 + # via libcst +typing-extensions==4.0.1 \ + --hash=sha256:4ca091dea149f945ec56afb48dae714f21e8692ef22a395223bcd328961b6a0e \ + --hash=sha256:7f001e5ac290a0c0401508864c7ec868be4e701886d5b573a9528ed3973d9d3b + # via + # libcst + # typing-inspect +typing-inspect==0.7.1 \ + --hash=sha256:047d4097d9b17f46531bf6f014356111a1b6fb821a24fe7ac909853ca2a782aa \ + --hash=sha256:3cd7d4563e997719a710a3bfe7ffb544c6b72069b6812a02e9b414a8fa3aaa6b \ + --hash=sha256:b1f56c0783ef0f25fb064a01be6e5407e54cf4a4bf4f3ba3fe51e0bd6dcea9e5 + # via libcst diff --git a/pkg/deps.bzl b/pkg/deps.bzl index 3f9d342e..9f5cb067 100644 --- a/pkg/deps.bzl +++ b/pkg/deps.bzl @@ -33,8 +33,8 @@ def rules_pkg_dependencies(): maybe( http_archive, name = "rules_python", - url = "https://github.com/bazelbuild/rules_python/releases/download/0.1.0/rules_python-0.1.0.tar.gz", - sha256 = "b6d46438523a3ec0f3cead544190ee13223a52f6a6765a29eae7b7cc24cc83a0", + url = "https://github.com/bazelbuild/rules_python/releases/download/0.5.0/rules_python-0.5.0.tar.gz", + sha256 = "cd6730ed53a002c56ce4e2f396ba3b3be262fd7cb68339f0377a45e8227fe332", ) maybe( http_archive, From f23749f044aff6c08c83cc03ebc2c7d384802571 Mon Sep 17 00:00:00 2001 From: Andrew Psaltis Date: Thu, 5 Jan 2023 17:34:55 -0500 Subject: [PATCH 3/7] Implement manual updates The migration script, due to its relative simplicity, only knows how to operate on BUILD files. This change provides analogous changes within our macros, which are mostly used in our unit tests. --- tests/mappings/external_repo/pkg/test.bzl | 2 +- .../mappings/mappings_external_repo_test.bzl | 8 ++--- tests/mappings/mappings_test.bzl | 35 ++++++++++--------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/tests/mappings/external_repo/pkg/test.bzl b/tests/mappings/external_repo/pkg/test.bzl index 4951cd71..b9599f64 100644 --- a/tests/mappings/external_repo/pkg/test.bzl +++ b/tests/mappings/external_repo/pkg/test.bzl @@ -57,7 +57,7 @@ def test_referencing_remote_file(name): prefix = "usr/share", srcs = ["@//tests:loremipsum_txt"], # The prefix in rules_pkg. Why yes, this is knotty - strip_prefix = strip_prefix.from_root("tests"), + srcs_strip_prefix = strip_prefix.from_root("tests"), tags = ["manual"], ) diff --git a/tests/mappings/mappings_external_repo_test.bzl b/tests/mappings/mappings_external_repo_test.bzl index aeb32235..bd7a8a68 100644 --- a/tests/mappings/mappings_external_repo_test.bzl +++ b/tests/mappings/mappings_external_repo_test.bzl @@ -43,7 +43,7 @@ def _test_pkg_files_extrepo(): pkg_files( name = "pf_extrepo_strip_from_pkg_g", srcs = ["@mappings_test_external_repo//pkg:dir/script"], - strip_prefix = strip_prefix.from_pkg("dir"), + srcs_strip_prefix = strip_prefix.from_pkg("dir"), tags = ["manual"], ) pkg_files_contents_test( @@ -59,7 +59,7 @@ def _test_pkg_files_extrepo(): pkg_files( name = "pf_extrepo_strip_from_root_g", srcs = ["@mappings_test_external_repo//pkg:dir/script"], - strip_prefix = strip_prefix.from_root("pkg"), + srcs_strip_prefix = strip_prefix.from_root("pkg"), tags = ["manual"], ) pkg_files_contents_test( @@ -80,7 +80,7 @@ def _test_pkg_files_extrepo(): srcs = [":extrepo_test_fg"], # Files within filegroups should be considered relative to their # destination paths. - strip_prefix = strip_prefix.from_pkg(""), + srcs_strip_prefix = strip_prefix.from_pkg(""), ) pkg_files_contents_test( name = "pf_extrepo_filegroup_strip_from_pkg", @@ -94,7 +94,7 @@ def _test_pkg_files_extrepo(): srcs = [":extrepo_test_fg"], # Files within filegroups should be considered relative to their # destination paths. - strip_prefix = strip_prefix.from_root("pkg"), + srcs_strip_prefix = strip_prefix.from_root("pkg"), ) pkg_files_contents_test( name = "pf_extrepo_filegroup_strip_from_root", diff --git a/tests/mappings/mappings_test.bzl b/tests/mappings/mappings_test.bzl index fcb46393..202517a3 100644 --- a/tests/mappings/mappings_test.bzl +++ b/tests/mappings/mappings_test.bzl @@ -94,7 +94,7 @@ pkg_files_contents_test = analysistest.make( ) def _test_pkg_files_contents(): - # Test stripping when no arguments are provided (same as strip_prefix.files_only()) + # Test stripping when no arguments are provided (same as strip_prefix.flatten()) pkg_files( name = "pf_no_strip_prefix_g", srcs = ["testdata/hello.txt"], @@ -120,17 +120,17 @@ def _test_pkg_files_contents(): ), ) - # And now, files_only = True + # And now, flatten = True pkg_files( - name = "pf_files_only_g", + name = "pf_flatten_g", srcs = ["testdata/hello.txt"], - strip_prefix = strip_prefix.files_only(), + srcs_strip_prefix = strip_prefix.flatten(), tags = ["manual"], ) pkg_files_contents_test( - name = "pf_files_only", - target_under_test = ":pf_files_only_g", + name = "pf_flatten", + target_under_test = ":pf_flatten_g", expected_dests = ["hello.txt"], ) @@ -148,7 +148,7 @@ def _test_pkg_files_contents(): "testdata/hello.txt", ":testdata/test_script", ], - strip_prefix = strip_prefix.from_pkg("testdata/"), + srcs_strip_prefix = strip_prefix.from_pkg("testdata/"), tags = ["manual"], ) @@ -174,7 +174,7 @@ def _test_pkg_files_contents(): pkg_files( name = "pf_from_root_g", srcs = [":testdata/test_script"], - strip_prefix = strip_prefix.from_root("tests/mappings"), + srcs_strip_prefix = strip_prefix.from_root("tests/mappings"), tags = ["manual"], ) @@ -194,7 +194,7 @@ def _test_pkg_files_contents(): pkg_files( name = "pf_attributes_mode_overlay_if_not_provided_g", srcs = ["foo"], - strip_prefix = strip_prefix.from_pkg(), + srcs_strip_prefix = strip_prefix.from_pkg(), attributes = pkg_attributes( user = "foo", group = "bar", @@ -219,6 +219,7 @@ def _test_pkg_files_contents(): pkg_files( name = "pf_destination_collision_invalid_g", srcs = ["foo", "bar/foo"], + srcs_strip_prefix = strip_prefix.flatten(), tags = ["manual"], ) generic_negative_test( @@ -230,7 +231,7 @@ def _test_pkg_files_contents(): pkg_files( name = "pf_strip_prefix_from_package_invalid_g", srcs = ["foo/foo", "bar/foo"], - strip_prefix = strip_prefix.from_pkg("bar"), + srcs_strip_prefix = strip_prefix.from_pkg("bar"), tags = ["manual"], ) generic_negative_test( @@ -242,7 +243,7 @@ def _test_pkg_files_contents(): pkg_files( name = "pf_strip_prefix_from_root_invalid_g", srcs = ["foo", "bar"], - strip_prefix = strip_prefix.from_root("not/the/root"), + srcs_strip_prefix = strip_prefix.from_root("not/the/root"), tags = ["manual"], ) generic_negative_test( @@ -293,7 +294,7 @@ def _test_pkg_files_exclusions(): name = "pf_exclude_by_label_strip_from_pkg_g", srcs = [":test_base_fg"], excludes = ["//tests/mappings:testdata/config"], - strip_prefix = strip_prefix.from_pkg("testdata"), + srcs_strip_prefix = strip_prefix.from_pkg("testdata"), tags = ["manual"], ) pkg_files_contents_test( @@ -306,7 +307,7 @@ def _test_pkg_files_exclusions(): name = "pf_exclude_by_filename_strip_from_pkg_g", srcs = [":test_base_fg"], excludes = ["testdata/config"], - strip_prefix = strip_prefix.from_pkg("testdata"), + srcs_strip_prefix = strip_prefix.from_pkg("testdata"), tags = ["manual"], ) pkg_files_contents_test( @@ -320,7 +321,7 @@ def _test_pkg_files_exclusions(): name = "pf_exclude_by_label_strip_from_root_g", srcs = [":test_base_fg"], excludes = ["//tests/mappings:testdata/config"], - strip_prefix = strip_prefix.from_root("tests/mappings"), + srcs_strip_prefix = strip_prefix.from_root("tests/mappings"), tags = ["manual"], ) pkg_files_contents_test( @@ -333,7 +334,7 @@ def _test_pkg_files_exclusions(): name = "pf_exclude_by_filename_strip_from_root_g", srcs = [":test_base_fg"], excludes = ["testdata/config"], - strip_prefix = strip_prefix.from_root("tests/mappings"), + srcs_strip_prefix = strip_prefix.from_root("tests/mappings"), tags = ["manual"], ) pkg_files_contents_test( @@ -874,7 +875,7 @@ def _test_pkg_filegroup(name): def _strip_prefix_test_impl(ctx): env = unittest.begin(ctx) - asserts.equals(env, ".", strip_prefix.files_only()) + asserts.equals(env, ".", strip_prefix.flatten()) asserts.equals(env, "path", strip_prefix.from_pkg("path")) asserts.equals(env, "path", strip_prefix.from_pkg("/path")) asserts.equals(env, "/path", strip_prefix.from_root("path")) @@ -903,7 +904,7 @@ def mappings_analysis_tests(): # buildifier: don't sort # Simple tests ":pf_no_strip_prefix", - ":pf_files_only", + ":pf_flatten", ":pf_strip_testdata_from_pkg", ":pf_strip_prefix_from_root", ":pf_attributes_mode_overlay_if_not_provided", From c2850e7bd4829b9d0492fb4b088708d7b50360b8 Mon Sep 17 00:00:00 2001 From: Andrew Psaltis Date: Thu, 5 Jan 2023 17:34:55 -0500 Subject: [PATCH 4/7] Apply the migration script This is the most substantial part of the change -- we automatically update all of rules_pkg to account for this new behavior. --- examples/rich_structure/BUILD | 2 +- examples/rich_structure/resources/l10n/BUILD | 6 +++--- examples/rich_structure/src/client/BUILD | 6 +++++- examples/rich_structure/src/server/BUILD | 4 +++- tests/BUILD | 2 +- tests/deb/BUILD | 2 +- tests/install/BUILD | 4 +++- tests/mappings/BUILD | 4 +++- tests/mappings/external_repo/pkg/BUILD | 2 +- tests/mappings/filter_directory/BUILD | 2 +- tests/rpm/BUILD | 4 ++++ tests/rpm/source_date_epoch/BUILD | 3 ++- tests/rpm/tree_artifacts/BUILD | 4 +++- tests/tar/BUILD | 2 +- tests/zip/BUILD | 2 +- 15 files changed, 33 insertions(+), 16 deletions(-) diff --git a/examples/rich_structure/BUILD b/examples/rich_structure/BUILD index 6bff0f64..34521b21 100644 --- a/examples/rich_structure/BUILD +++ b/examples/rich_structure/BUILD @@ -32,7 +32,7 @@ pkg_files( # Where it should be in the final package prefix = "usr/share/doc/foo", # Required, but why?: see #354 - strip_prefix = strip_prefix.from_pkg(), + srcs_strip_prefix = strip_prefix.from_pkg(), ) pkg_filegroup( diff --git a/examples/rich_structure/resources/l10n/BUILD b/examples/rich_structure/resources/l10n/BUILD index 823cab85..25739c86 100644 --- a/examples/rich_structure/resources/l10n/BUILD +++ b/examples/rich_structure/resources/l10n/BUILD @@ -28,7 +28,7 @@ pkg_files( # We know they should be read only. We don't know who should own them. mode = "0444", ), - strip_prefix = strip_prefix.from_pkg(), + srcs_strip_prefix = strip_prefix.from_pkg(), ) # Use the English catalog again under a different name. @@ -44,7 +44,7 @@ pkg_files( mode = "0444", ), prefix = "foo/en_GB", - strip_prefix = strip_prefix.from_pkg(), + srcs_strip_prefix = strip_prefix.from_pkg(), ) # This gathers together all our message catalogs, and adds a prefix based @@ -65,7 +65,7 @@ pkg_files( attributes = pkg_attributes( mode = "0444", ), - strip_prefix = strip_prefix.from_root("resources"), + srcs_strip_prefix = strip_prefix.from_root("resources"), ) pkg_tar( diff --git a/examples/rich_structure/src/client/BUILD b/examples/rich_structure/src/client/BUILD index 28a84d53..c99d0a5e 100644 --- a/examples/rich_structure/src/client/BUILD +++ b/examples/rich_structure/src/client/BUILD @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("@rules_pkg//pkg:mappings.bzl", "pkg_attributes", "pkg_filegroup", "pkg_files", "pkg_mklink") +load("@rules_pkg//pkg:mappings.bzl", "pkg_attributes", "pkg_filegroup", "pkg_files", "pkg_mklink", "strip_prefix") load("@rules_pkg//pkg:tar.bzl", "pkg_tar") load("//:foo_defs.bzl", "shared_object_path_selector") @@ -56,6 +56,8 @@ pkg_files( ":fooctl", ], # Where it should be in the final package + srcs_strip_prefix = strip_prefix.flatten(), + # Where it should be in the final package prefix = select(shared_object_path_selector) + "/bin", # Should this work? # runfiles_prefix = select(shared_object_path_selector) + "/bin/runfiles", @@ -66,6 +68,7 @@ pkg_files( srcs = [ ":foolib", ], + srcs_strip_prefix = strip_prefix.flatten(), prefix = "usr/lib/foo", ) @@ -83,6 +86,7 @@ pkg_files( srcs = [ ":man", ], + srcs_strip_prefix = strip_prefix.flatten(), attributes = pkg_attributes( group = "man", mode = "0444", diff --git a/examples/rich_structure/src/server/BUILD b/examples/rich_structure/src/server/BUILD index aaf93837..779249d9 100644 --- a/examples/rich_structure/src/server/BUILD +++ b/examples/rich_structure/src/server/BUILD @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -load("@rules_pkg//pkg:mappings.bzl", "pkg_attributes", "pkg_filegroup", "pkg_files", "pkg_mkdirs") +load("@rules_pkg//pkg:mappings.bzl", "pkg_attributes", "pkg_filegroup", "pkg_files", "pkg_mkdirs", "strip_prefix") load("@rules_pkg//pkg:tar.bzl", "pkg_tar") load("//:foo_defs.bzl", "shared_object_path_selector") @@ -41,6 +41,7 @@ pkg_files( srcs = [ ":food", ], + srcs_strip_prefix = strip_prefix.flatten(), attributes = pkg_attributes( group = "root", mode = "0551", @@ -63,6 +64,7 @@ pkg_files( srcs = [ ":mansrc", ], + srcs_strip_prefix = strip_prefix.flatten(), attributes = pkg_attributes( group = "man", mode = "0444", diff --git a/tests/BUILD b/tests/BUILD index 7f4c9894..9242b4cb 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -18,7 +18,7 @@ load("@bazel_skylib//rules:copy_file.bzl", "copy_file") load(":my_package_name.bzl", "my_package_naming") load(":path_test.bzl", "path_tests") load("//pkg:deb.bzl", "pkg_deb") -load("//pkg:mappings.bzl", "pkg_attributes", "pkg_mkdirs") +load("//pkg:mappings.bzl", "pkg_attributes", "pkg_mkdirs", "strip_prefix") load("//pkg:tar.bzl", "pkg_tar") load("//pkg:zip.bzl", "pkg_zip") diff --git a/tests/deb/BUILD b/tests/deb/BUILD index d81a98f4..2063209d 100644 --- a/tests/deb/BUILD +++ b/tests/deb/BUILD @@ -17,7 +17,7 @@ load("@rules_python//python:defs.bzl", "py_test") load(":deb_tests.bzl", "package_naming_test") -load("//pkg:mappings.bzl", "pkg_mklink") +load("//pkg:mappings.bzl", "pkg_mklink", "strip_prefix") load("//pkg:deb.bzl", "pkg_deb") load("//pkg:tar.bzl", "pkg_tar") load("//tests:my_package_name.bzl", "my_package_naming") diff --git a/tests/install/BUILD b/tests/install/BUILD index f083b3f7..49458fc6 100644 --- a/tests/install/BUILD +++ b/tests/install/BUILD @@ -14,7 +14,7 @@ load("@rules_python//python:defs.bzl", "py_test") load("//pkg:install.bzl", "pkg_install") -load("//pkg:mappings.bzl", "pkg_attributes", "pkg_files", "pkg_mkdirs") +load("//pkg:mappings.bzl", "pkg_attributes", "pkg_files", "pkg_mkdirs", "strip_prefix") load("//tests/util:defs.bzl", "fake_artifact") package(default_applicable_licenses = ["//:license"]) @@ -56,6 +56,7 @@ fake_artifact( pkg_files( name = "artifact-in-owned-dir", srcs = ["artifact"], + srcs_strip_prefix = strip_prefix.flatten(), attributes = pkg_attributes(mode = "0700"), prefix = "owned-dir", ) @@ -63,6 +64,7 @@ pkg_files( pkg_files( name = "artifact-in-unowned-dir", srcs = ["artifact"], + srcs_strip_prefix = strip_prefix.flatten(), attributes = pkg_attributes(mode = "0755"), prefix = "unowned-dir", ) diff --git a/tests/mappings/BUILD b/tests/mappings/BUILD index b9f55aa3..06607bd7 100644 --- a/tests/mappings/BUILD +++ b/tests/mappings/BUILD @@ -62,6 +62,7 @@ directory( pkg_files( name = "directory-with-contents", srcs = ["treeartifact"], + srcs_strip_prefix = strip_prefix.flatten(), attributes = pkg_attributes(mode = "0644"), ) @@ -82,6 +83,7 @@ pkg_files( srcs = [ "mappings_test.bzl", ], +srcs_strip_prefix = strip_prefix.flatten(), ) # TODO: this seems to write the JSON in a particular order; regardless of input @@ -110,7 +112,7 @@ pkg_files( srcs = [ "//tests/testdata/utf8:files", ], - strip_prefix = strip_prefix.from_pkg(), + srcs_strip_prefix = strip_prefix.from_pkg(), ) write_content_manifest( diff --git a/tests/mappings/external_repo/pkg/BUILD b/tests/mappings/external_repo/pkg/BUILD index 8800ce3f..4c098b39 100644 --- a/tests/mappings/external_repo/pkg/BUILD +++ b/tests/mappings/external_repo/pkg/BUILD @@ -32,7 +32,7 @@ pkg_files( name = "extproj_script_pf", srcs = ["dir/extproj.sh"], prefix = "usr/bin", - strip_prefix = strip_prefix.from_pkg(), + srcs_strip_prefix = strip_prefix.from_pkg(), tags = ["manual"], ) diff --git a/tests/mappings/filter_directory/BUILD b/tests/mappings/filter_directory/BUILD index 4b0399b0..4ca3da4c 100644 --- a/tests/mappings/filter_directory/BUILD +++ b/tests/mappings/filter_directory/BUILD @@ -14,7 +14,7 @@ load("@rules_python//python:defs.bzl", "py_test") load(":defs.bzl", "inspect_directory_test") -load("//pkg:mappings.bzl", "filter_directory") +load("//pkg:mappings.bzl", "filter_directory", "strip_prefix") load("//tests/util:defs.bzl", "directory") # TODO: the below tests only check for rule success and confirmation that diff --git a/tests/rpm/BUILD b/tests/rpm/BUILD index 5f91da8d..cd212e21 100644 --- a/tests/rpm/BUILD +++ b/tests/rpm/BUILD @@ -23,6 +23,7 @@ load( "pkg_files", "pkg_mkdirs", "pkg_mklink", + "strip_prefix", ) load("//pkg:rpm.bzl", "pkg_rpm") load("analysis_tests.bzl", "analysis_tests") @@ -65,6 +66,7 @@ pkg_files( srcs = [ ":ars", ], + srcs_strip_prefix = strip_prefix.flatten(), attributes = pkg_attributes( group = "root", mode = "0755", @@ -82,6 +84,7 @@ genrule( pkg_files( name = "config_file", srcs = [":config_empty"], + srcs_strip_prefix = strip_prefix.flatten(), attributes = pkg_attributes( group = "root", mode = "0644", @@ -329,6 +332,7 @@ pkg_files( srcs = [ ":config_empty", ], + srcs_strip_prefix = strip_prefix.flatten(), attributes = pkg_attributes(mode = "0644"), prefix = "dir", ) diff --git a/tests/rpm/source_date_epoch/BUILD b/tests/rpm/source_date_epoch/BUILD index c6bacede..2f4fa2bc 100644 --- a/tests/rpm/source_date_epoch/BUILD +++ b/tests/rpm/source_date_epoch/BUILD @@ -13,7 +13,7 @@ # limitations under the License. load("@rules_python//python:defs.bzl", "py_test") -load("//pkg:mappings.bzl", "pkg_filegroup", "pkg_files") +load("//pkg:mappings.bzl", "pkg_filegroup", "pkg_files", "strip_prefix") load("//pkg:rpm.bzl", "pkg_rpm") ############################################################################ @@ -72,6 +72,7 @@ pkg_filegroup( pkg_files( name = "pf", srcs = [":files"], + srcs_strip_prefix = strip_prefix.flatten(), prefix = "test_dir", ) diff --git a/tests/rpm/tree_artifacts/BUILD b/tests/rpm/tree_artifacts/BUILD index 04d7d8eb..db5cf0ad 100644 --- a/tests/rpm/tree_artifacts/BUILD +++ b/tests/rpm/tree_artifacts/BUILD @@ -107,7 +107,7 @@ pkg_files( mode = "0644", user = "root", ), - strip_prefix = strip_prefix.from_pkg(""), + srcs_strip_prefix = strip_prefix.from_pkg(""), ) directory( @@ -125,6 +125,7 @@ directory( pkg_files( name = "test_files_from_rules", srcs = [":my_files"], + srcs_strip_prefix = strip_prefix.flatten(), attributes = pkg_attributes( group = "root", mode = "0644", @@ -246,6 +247,7 @@ pkg_files( srcs = [ ":test_dir1", ], + srcs_strip_prefix = strip_prefix.flatten(), attributes = pkg_attributes( group = "root", mode = "0644", diff --git a/tests/tar/BUILD b/tests/tar/BUILD index 19504e1c..7a7bfe89 100644 --- a/tests/tar/BUILD +++ b/tests/tar/BUILD @@ -279,7 +279,7 @@ pkg_files( srcs = [ ":generate_tree", ], - strip_prefix = "generate_tree", + srcs_strip_prefix = "generate_tree", ) pkg_tar( diff --git a/tests/zip/BUILD b/tests/zip/BUILD index 56c739b4..9c9c18e8 100644 --- a/tests/zip/BUILD +++ b/tests/zip/BUILD @@ -13,7 +13,7 @@ # limitations under the License. # -*- coding: utf-8 -*- -load("//pkg:mappings.bzl", "pkg_attributes", "pkg_mkdirs", "pkg_mklink") +load("//pkg:mappings.bzl", "pkg_attributes", "pkg_mkdirs", "pkg_mklink", "strip_prefix") load("//pkg:zip.bzl", "pkg_zip") load("//tests:my_package_name.bzl", "my_package_naming") load("//tests/util:defs.bzl", "directory", "fake_artifact") From 8ea822b624788a3978784c909eebe29e0c754dae Mon Sep 17 00:00:00 2001 From: Andrew Psaltis Date: Thu, 5 Jan 2023 17:43:15 -0500 Subject: [PATCH 5/7] Update examples to account for new behavior Examples should reflect how you should do things, and specifying `srcs_strip_prefix` everywhere in the intuitive case is no longer necessary. --- examples/rich_structure/BUILD | 2 -- examples/rich_structure/resources/l10n/BUILD | 2 -- 2 files changed, 4 deletions(-) diff --git a/examples/rich_structure/BUILD b/examples/rich_structure/BUILD index 34521b21..4599d433 100644 --- a/examples/rich_structure/BUILD +++ b/examples/rich_structure/BUILD @@ -31,8 +31,6 @@ pkg_files( ], # Where it should be in the final package prefix = "usr/share/doc/foo", - # Required, but why?: see #354 - srcs_strip_prefix = strip_prefix.from_pkg(), ) pkg_filegroup( diff --git a/examples/rich_structure/resources/l10n/BUILD b/examples/rich_structure/resources/l10n/BUILD index 25739c86..e5297012 100644 --- a/examples/rich_structure/resources/l10n/BUILD +++ b/examples/rich_structure/resources/l10n/BUILD @@ -28,7 +28,6 @@ pkg_files( # We know they should be read only. We don't know who should own them. mode = "0444", ), - srcs_strip_prefix = strip_prefix.from_pkg(), ) # Use the English catalog again under a different name. @@ -44,7 +43,6 @@ pkg_files( mode = "0444", ), prefix = "foo/en_GB", - srcs_strip_prefix = strip_prefix.from_pkg(), ) # This gathers together all our message catalogs, and adds a prefix based From e72567908b4aee70ca918b63acfca225801631c9 Mon Sep 17 00:00:00 2001 From: Andrew Psaltis Date: Thu, 5 Jan 2023 18:04:44 -0500 Subject: [PATCH 6/7] Include in distro tarball --- distro/BUILD | 1 + migration/BUILD | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/distro/BUILD b/distro/BUILD index 024faa24..2dea25da 100644 --- a/distro/BUILD +++ b/distro/BUILD @@ -36,6 +36,7 @@ pkg_tar( srcs = [ ":small_workspace", "//:standard_package", + "//migration:standard_package", "//pkg:standard_package", "//pkg/private:standard_package", "//pkg/private/deb:standard_package", diff --git a/migration/BUILD b/migration/BUILD index b3ef73ca..c44ac516 100644 --- a/migration/BUILD +++ b/migration/BUILD @@ -16,6 +16,18 @@ load("@rules_python//python:defs.bzl", "py_binary") load("@rules_python//python:pip.bzl", "compile_pip_requirements") load("@rules_pkg_migration_deps//:requirements.bzl", "requirement") +filegroup( + name = "standard_package", + srcs = glob([ + "*.py" + ] + [ + "BUILD", + "migration_deps.txt", + "migration_deps_lock.txt", + ]), + visibility = ["//distro:__pkg__"], +) + py_binary( name = "strip_prefix", srcs = ["migrate_strip_prefix.py"], From 3b1c7848026a9840c45ce8490f357ad8c66350fd Mon Sep 17 00:00:00 2001 From: Andrew Psaltis Date: Tue, 10 Jan 2023 13:29:52 -0500 Subject: [PATCH 7/7] Subclass from a different codemod transformer; fill in bugs; fix indentation --- migration/migrate_strip_prefix.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/migration/migrate_strip_prefix.py b/migration/migrate_strip_prefix.py index 92f6a74b..41c9b413 100644 --- a/migration/migrate_strip_prefix.py +++ b/migration/migrate_strip_prefix.py @@ -23,7 +23,7 @@ import libcst.codemod as codemod import libcst.matchers as cstm -class PkgFilesStripPrefixTransformer(codemod.ContextAwareTransformer): +class PkgFilesStripPrefixTransformer(codemod.VisitorBasedCodemodCommand): def __init__(self, context): super().__init__(context) self.load_node = None @@ -32,8 +32,8 @@ def __init__(self, context): @cstm.call_if_inside(cstm.Call(func = cstm.Name("pkg_files"))) @cstm.leave(cstm.Arg(keyword = cstm.Name("strip_prefix"))) def rename_strip_prefix_to_srcs_strip_prefix(self, - original_node: cst.Arg, - updated_node: cst.Arg) -> cst.Arg: + original_node: cst.Arg, + updated_node: cst.Arg) -> cst.Arg: return updated_node.with_changes(keyword = cst.Name("srcs_strip_prefix")) @@ -67,7 +67,7 @@ def strip_prefix_account_for_new_default(self, ) ) ) - may_need_to_amend_load = True + self.may_need_to_amend_load = True return cst.FlattenSentinel([ updated_node, new_node, @@ -233,6 +233,8 @@ def real_filepath(p): # NOTE: this can't be interrupted, because the underlying transform # execution catches KeyboardInterrupt and # parallel_exec_transform_with_prettyprint thinks of it as a "skip". + # + # See also https://github.com/Instagram/LibCST/issues/849 print("") print("!!! Interrupt with C-\\ !!!") print("") @@ -241,8 +243,9 @@ def real_filepath(p): res = codemod.parallel_exec_transform_with_prettyprint( transformer, paths, - # TODO(nacl): We cannot parallelize this due to issues pickling some objects - # in libcst. File a bug for this. + # FIXME: We cannot parallelize this due to issues with some of the + # matchers in libcst. See also + # https://github.com/Instagram/LibCST/issues/848. jobs = 1, # defaults to None, which means "use all cores" unified_diff = args.diff, # Otherwise libcst can't calculate module paths and emits (for us)