bazelbuild / rules_pkg

Bazel rules for creating packages of many types (zip, tar, deb, rpm, ...)
Apache License 2.0
226 stars 175 forks source link

Make the default behavior of pkg_files#strip_prefix more intuitive; rename it to srcs_strip_prefix #656

Open nacl opened 1 year ago

nacl commented 1 year ago

pkg_files#strip_prefix provides the capability to remove path segments from files prior to them entering the output package. The default for this attribute is to flatten the outputs (e.g. remove all directory structure), which is potentially confusing to users and can cause odd problems. What is generally more expected (and error-prone) is to preserve the package-relative path, as specified when you pass strip_prefix.from_pkg() to pkg_files,

This change implements this update to the default, and implements the following additional changes:

Given that this is a breaking change, we have provided a migration script that users can run to implement this change in their trees. I have partially tested this in an internal tree, and the outputs are identical. This script uses libcst to process the BUILD files and implement transformations. Updates to .bzl files were not attempted due to complexity requirements; users will have to apply them manually.

All BUILD changes were implemented by applying the script like so:

git ls-files '**/BUILD' | xargs bazel run //migration:strip_prefix --

bazel run the above target for more details. It can be run as @rules_pkg//migration:strip_prefix if the appropriate WORKSPACE stuffs are added.

Fixes #354. Supplants #492.

TODO: provide instructions for migrating without downloading rules_pkg -- copying and tweaking some of the stuff added to the WORKSPACE file should be enough.

aiuto commented 1 year ago

This seems to be different from what I thought we agreed on, which was that strip prefix would strip from root or package relative, and flatten was a separate attribute to flatten paths after the stripping.

Also, quick side question. does strip_prefix='.' still do the right thing, which is to strip the package and not flatten the rest.

nacl commented 1 year ago

This seems to be different from what I thought we agreed on, which was that strip prefix would strip from root or package relative, and flatten was a separate attribute to flatten paths after the stripping.

I think we were thinking different things when we came to an agreement on this in December. Could you provide an example of what you intend the behavior to be with the separate flatten attribute ? I tried reasoning about it a bit, and there doesn't seem to be much that couldn't be otherwise be accomplished by either creating a new BUILD file in a subdirectory, or by specifying prefix.

Also, quick side question. does strip_prefix='.' still do the right thing, which is to strip the package and not flatten the rest.

I believe so; that code hasn't changed.