Fix bug in merged tars with big file names on Python 3.8

[andrewalker]

When adding tarballs as input for other tarballs (i.e. pkg_tar that depends on another pkg_tar), every file with a name that exceeds 98 characters will not land in the new root directory (package_dir attribute), but rather on the actual root.

Consider the following example:

pkg_tar(
    name = "foo",
    package_dir = "/foo",
    strip_prefix = ".",
    srcs = [ ... ],
    deps = [":big-names"],
)

pkg_tar(
    name = "big-names",
    strip_prefix = ".",
    srcs = [
        "ninety-eight-characters-seems-to-be-the-maximum-limit-for-merged-tarballs-in-the-rules_pkg-for-bzl",
        "ninety-eight-characters-seems-to-be-the-maximum-limit-for-merged-tarballs-in-the-rules_pkg-for-bzl-",
    ],
)

The foo tarball will have the following structure:

.
├── foo
│   └── ninety-eight-characters-seems-to-be-the-maximum-limit-for-merged-tarballs-in-the-rules_pkg-for-bzl
└── ninety-eight-characters-seems-to-be-the-maximum-limit-for-merged-tarballs-in-the-rules_pkg-for-bzl-

Filenames with 98 characters or less get included in the right directory, but the larger ones go to the root. A deep directory structure behaves the same way:

.
├── foo
│   └── ninety/eight/characters/seems/to/be/the/maximum/limit/for/merged/tarballs/in/the/rules_pkg/for/bzl
└── ninety/eight/characters/seems/to/be/the/maximum/limit/for/merged/tarballs/in/the/rules_pkg/for/bzl-

And the size of the package_dir has no influence: "/f", "/foo" or "/foo/bar/baz/quux" behaves exactly the same way.

This only happens on Python 3.8. The reason is the default format changed from tarfile.GNU_FORMAT to tarfile.PAX_FORMAT. It's also possible to reproduce this by changing the format in archive.py to PAX_FORMAT explicitly.

To fix it, I'm explicitly requesting GNU_FORMAT even on 3.8, so that it continues to work as before.

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[andrewalker]

Huh, pipeline passed. I get a broken test on ArchLinux, bazel 3.3.0- (@non-git).

[andrewalker]

It's also worth mentioning that if I change the format to GNU_FORMAT, I don't get the error:

diff --git a/pkg/archive.py b/pkg/archive.py
index f15b425..a2de674 100644
--- a/pkg/archive.py
+++ b/pkg/archive.py
@@ -146,7 +146,7 @@ class TarFileWriter(object):
       # Instead, we manually re-implement gzopen from tarfile.py and set mtime.
       self.fileobj = gzip.GzipFile(
           filename=name, mode='w', compresslevel=9, mtime=self.default_mtime)
-    self.tar = tarfile.open(name=name, mode=mode, fileobj=self.fileobj)
+    self.tar = tarfile.open(name=name, mode=mode, fileobj=self.fileobj, format=tarfile.GNU_FORMAT)
     self.members = set([])
     self.directories = set([])

[andrewalker]

OK, found the error. I'm running Python 3.8, and according to the docs: "Changed in version 3.8: The default format for new archives was changed to PAX_FORMAT from GNU_FORMAT". If I force format=tarfile.PAX_FORMAT on archive.py, it fails consistently everywhere. On my machine that's the default, which is why I can fix it by using GNU_FORMAT.

[aiuto]

See #203

[DhashS]

@andrewalker I think you need to sign the CLA to get this merged. We're looking forward to it, as this oneliner makes the behaviour of rules_pkg not break horribly depending on your python version

[aiuto]

Sorry I took so long. I wanted to fix the testing of debian packages first. One #211 goes in, I will look at this and #201 and see which to apply.

[andrewalker]

Hey, sorry I haven't signed the CLA yet. I was going to sign it as part of my company (so the corporate version), but that got a bit delayed in internal bureaucracy. I see #217 was merged, is there still interest in merging the test?

[aiuto]

I would like to have some tests to prove behavior. I am behind on this myself. I want to understand what appears contradictory

• python 3.8 switched a tarfile default to PAX on the grounds that it provides better UTF-8 and long file support
• our experience with .deb files seems to disagree.

So, what I need is more tests beyond what you have in this CL. And, I don't have python3.8 in our test machines yet, so anything I do is a stab in the dark. Nor do I have a Windows machine in my home office, so some interoperbility there is hard to test. If you have no objections, I would take your test case and use it in a larger set of CLs.