Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pkg_zip does not maintain directory structure #434

Closed
hborawski opened this issue Oct 12, 2021 · 10 comments
Closed

pkg_zip does not maintain directory structure #434

hborawski opened this issue Oct 12, 2021 · 10 comments
Labels

Comments

@hborawski
Copy link

pkg_zip does not maintain the directory structure of the files placed inside it, nor does it have an option to.

For the iOS package manager CocoaPods, when archiving code for distribution (rather than relying on git), the directory structure of the decompressed archive must match the globs in the package specification file (.podspec)

if i have a podspec with the following:

package.source_files = ["Sources/**/*.swift"]

and use the same in pkg_zip:

pkg_zip(
    name = "PodName",
    srcs = glob(["Sources/**/*.swift"])
  )

All of the .swift files are unpacked at the root level of the archive. This breaks the CocoaPod import, since the package expects files to be under Sources

While this specific usecase is fixable (with package_dir), CocoaPods supports "subspecs", that allows for optionally importing chunks of code:

package.subspec "Module1" do |sub|
  sub.source_files = ["Sources/package1/**/*.swift"]
end
package.subspec "Module2" do |sub|
  sub.source_files = ["Sources/package2/**/*.swift"]
end

When compressed, all of the code needs to be available in the archive, so adding package_dir as Sources would then leave everything in the Sources directory when decompressed, and the .podspec would not match any files.

In order for these pkg_ rules to work for CocoaPods, there needs to be a way to maintain the directory structure when archiving files.

@aiuto
Copy link
Collaborator

aiuto commented Oct 12, 2021

This should not be done within pkg_zip. You can do it with a glob in a pkg_files.
Have you tried that?
If the docs are not sufficient, maybe the examples here might give an idea.
https://github.com/bazelbuild/rules_pkg/tree/main/examples/rich_structure

@tony-scio
Copy link

I hit the same thing. pkg_files obviously don't produce a zip. So is the recommendation to do a pkg_files then a genrule w/ a manual zip on top of that in order to make a zip that preserves the directory structure?

@nacl
Copy link
Collaborator

nacl commented Nov 9, 2021

The recommendation is to create a one or more pkg_files targets, and then pass them to pkg_zip in its srcs attribute. Note that the path manipulation functions in pkg_files supplant the ones provided by pkg_zip (and also pkg_tar).

For a more concrete example, take a peek at the one @aiuto mentioned in his comment, above.

@aiuto
Copy link
Collaborator

aiuto commented Nov 9, 2021

pkg_files should be the srcs to pkg_zip.

load("//:pkg.bzl", "pkg_zip")
load("//pkg:mappings.bzl", "pkg_files", "strip_prefix")

pkg_files(
    name = "srcs",
    srcs = glob(["Sources/**/*.swift"]),
    strip_prefix = strip_prefix.from_pkg(),
)

pkg_zip(
    name = "pod",
    srcs = ["srcs"],
)
$ bazel build :*
INFO: Analyzed 6 targets (1 packages loaded, 6 targets configured).
...
$ unzip -v ../bazel-bin/i434/pod.zip 
Archive:  ../bazel-bin/i434/pod.zip
 Length   Method    Size  Cmpr    Date    Time   CRC-32   Name
--------  ------  ------- ---- ---------- ----- --------  ----
      20  Defl:N       22 -10% 1980-01-01 00:00 ab34387b  Sources/a/foo.swift
      26  Defl:N       28  -8% 1980-01-01 00:00 0cd071f9  Sources/b/foo.swift
--------          -------  ---                            -------
      46               50  -9%                            2 files

The complaint some people might have that this takes two rules instead of one. That design choice is intentional. pkg_files and related rules like pkg_dirs are a suite of composable rules that allow you do define the structure you want in the final output, and then package that up with any of pkg_{tar, zip, deb, rpm}. This lets us put the mapping logic in one place, rather than reimplementing things like strip_prefix in every rule.

@aiuto aiuto added the wontfix label Nov 9, 2021
@hborawski
Copy link
Author

Sorry to get back to this so late, went down some other related rabbit holes. Using pkg_files works, and I get why, still learning a lot about bazel, so some of the intentional verbosity in the BUILD files isn't what I expect.

Thank you for the help!

@tony-scio
Copy link

Thanks! pkg_files works like a charm. I had incorrectly assumed a filegroup would work in its place.

@aiuto
Copy link
Collaborator

aiuto commented Dec 1, 2021

Just for closure. I do actually agree that pkg_*(srcs=glob(["**/*.switft"])) would be least surprising if it maintained the directory structure. It's just that we have years of people using and expecting the older (and dumb) behavior of flattening the tree. As tempting as it is, I don't want to potentially break a lot of people.

@achew22
Copy link
Member

achew22 commented Dec 2, 2021

@aiuto out of curiosity, WDYT about adding a flatten flag to pkg_* that defaults to false and doing a breaking change? It would let people keep the old (and I agree with you, bad) behavior until they get a chance without increasing the ecosystem wide tech debt over time?

@nacl
Copy link
Collaborator

nacl commented Dec 2, 2021

Perhaps this discussion should be continued in #354, where we are considering this very problem?

I would personally argue that it isn't necessarily bad behavior -- if your default mode of operation has files going to odd locations that have nothing to do with your source paths, you're going to be stripping everything pretty much by default anyway.

Regardless, that behavior may be too surprising to be effective.

@aiuto
Copy link
Collaborator

aiuto commented Dec 2, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants