Skip to content

Commit

Permalink
Remove xz compression from Bazel's build in pkg_tar.
Browse files Browse the repository at this point in the history
- clean up Python 2/3 migration hacks
- #11183

RELNOTES:
Dropped fragile xz support from built in pkg_tar. Users requiring xz
compression should switch to bazlebuild/rules_pkg.
PiperOrigin-RevId: 371394411
  • Loading branch information
aiuto authored and copybara-github committed Apr 30, 2021
1 parent 525cefc commit 19a4e6c
Show file tree
Hide file tree
Showing 7 changed files with 21 additions and 62 deletions.
14 changes: 3 additions & 11 deletions tools/build_defs/pkg/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,8 @@ py_test(
],
data = [":archive_testdata"],
python_version = "PY3",
srcs_version = "PY2AND3",
srcs_version = "PY3",
tags = [
# archive.py requires xzcat, which is not available by default on Mac
"noci",
# TODO(laszlocsomor): fix on Windows or describe why it cannot pass.
"no_windows",
],
Expand All @@ -56,7 +54,8 @@ py_test(
name = "path_test",
srcs = ["path_test.py"],
data = ["path.bzl"],
srcs_version = "PY2AND3",
python_version = "PY3",
srcs_version = "PY3",
)

py_binary(
Expand Down Expand Up @@ -103,7 +102,6 @@ genrule(
"",
".gz",
".bz2",
".xz", # This will breaks if xzcat is not installed
]]

[pkg_tar(
Expand All @@ -114,7 +112,6 @@ genrule(
"",
"gz",
"bz2",
"xz",
]]

pkg_tar(
Expand Down Expand Up @@ -207,18 +204,13 @@ sh_test(
":test-tar-inclusion-.tar",
":test-tar-inclusion-bz2.tar",
":test-tar-inclusion-gz.tar",
":test-tar-inclusion-xz.tar",
":test-tar-mtime.tar",
":test-tar-strip_prefix-dot.tar",
":test-tar-strip_prefix-empty.tar",
":test-tar-strip_prefix-etc.tar",
":test-tar-strip_prefix-none.tar",
":test-tar-xz.tar.xz",
":titi_test_all.changes",
],
tags = [
# archive.py requires xzcat, which is not available by default on Mac
"noci",
# TODO(laszlocsomor): fix on Windows or describe why it cannot pass.
"no_windows",
],
Expand Down
50 changes: 9 additions & 41 deletions tools/build_defs/pkg/archive.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import gzip
import io
import os
import subprocess
import tarfile
import six

Expand All @@ -46,7 +45,7 @@ def __init__(self,
Args:
name: the tar file name.
compression: compression type: bzip2, bz2, gz, tgz, xz, lzma.
compression: compression type: bzip2, bz2, gz, tgz.
root_directory: virtual root to prepend to elements in the archive.
default_mtime: default mtime to use for elements in the archive.
May be an integer or the value 'portable' to use the date
Expand All @@ -58,8 +57,6 @@ def __init__(self,
else:
mode = 'w:'
self.gz = compression in ['tgz', 'gz']
# Support xz compression through xz... until we can use Py3
self.xz = compression in ['xz', 'lzma']
self.name = name
self.root_directory = six.ensure_str(root_directory).rstrip('/')

Expand Down Expand Up @@ -283,36 +280,16 @@ def add_tar(self,
compression = 'gz'
elif compression == 'bzip2':
compression = 'bz2'
elif compression == 'lzma':
compression = 'xz'
elif compression not in ['gz', 'bz2', 'xz']:
elif compression not in ['gz', 'bz2']:
compression = ''
if compression == 'xz':
# Python 2 does not support lzma, our py3 support is terrible so let's
# just hack around.
# Note that we buffer the file in memory and it can have an important
# memory footprint but it's probably fine as we don't use them for really
# large files.
# TODO(dmarting): once our py3 support gets better, compile this tools
# with py3 for proper lzma support.
if subprocess.call('which xzcat', shell=True, stdout=subprocess.PIPE):
raise self.Error('Cannot handle .xz and .lzma compression: '
'xzcat not found.')
p = subprocess.Popen('cat %s | xzcat' % tar,
shell=True,
stdout=subprocess.PIPE)
f = io.BytesIO(p.stdout.read())
p.wait()
intar = tarfile.open(fileobj=f, mode='r:')
if compression in ['gz', 'bz2']:
# prevent performance issues due to accidentally-introduced seeks
# during intar traversal by opening in "streaming" mode. gz, bz2
# are supported natively by python 2.7 and 3.x
inmode = 'r|' + six.ensure_str(compression)
else:
if compression in ['gz', 'bz2']:
# prevent performance issues due to accidentally-introduced seeks
# during intar traversal by opening in "streaming" mode. gz, bz2
# are supported natively by python 2.7 and 3.x
inmode = 'r|' + six.ensure_str(compression)
else:
inmode = 'r:' + six.ensure_str(compression)
intar = tarfile.open(name=tar, mode=inmode)
inmode = 'r:' + six.ensure_str(compression)
intar = tarfile.open(name=tar, mode=inmode)
for tarinfo in intar:
if name_filter is None or name_filter(tarinfo.name):
if not self.preserve_mtime:
Expand Down Expand Up @@ -370,12 +347,3 @@ def close(self):
# Close the gzip file object if necessary.
if self.fileobj:
self.fileobj.close()
if self.xz:
# Support xz compression through xz... until we can use Py3
if subprocess.call('which xz', shell=True, stdout=subprocess.PIPE):
raise self.Error('Cannot handle .xz and .lzma compression: '
'xz not found.')
subprocess.call(
'mv {0} {0}.d && xz -z {0}.d && mv {0}.d.xz {0}'.format(self.name),
shell=True,
stdout=subprocess.PIPE)
11 changes: 4 additions & 7 deletions tools/build_defs/pkg/archive_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@
import tarfile
import unittest

# Do not edit this line. Copybara replaces it with PY2 migration helper.
import six

from tools.build_defs.pkg import archive
from tools.build_defs.pkg import testenv

Expand Down Expand Up @@ -83,13 +80,13 @@ def testEmptyTarFile(self):
def assertSimpleFileContent(self, names):
with archive.TarFileWriter(self.tempfile) as f:
for n in names:
f.add_file(n, content=n)
f.add_file(n, content=n.encode("utf-8"))
content = ([{
"name": "."
}] + [{
"name": n,
"size": len(six.ensure_binary(n, "utf-8")),
"data": six.ensure_binary(n, "utf-8")
"size": len(n.encode("utf-8")),
"data": n.encode("utf-8")
} for n in names])
self.assertTarFileContent(self.tempfile, content)

Expand Down Expand Up @@ -170,7 +167,7 @@ def testMergeTar(self):
{"name": "./a", "data": b"a"},
{"name": "./ab", "data": b"ab"},
]
for ext in ["", ".gz", ".bz2", ".xz"]:
for ext in ["", ".gz", ".bz2"]:
with archive.TarFileWriter(self.tempfile) as f:
f.add_tar(os.path.join(testenv.TESTDATA_PATH, "tar_test.tar" + ext),
name_filter=lambda n: n != "./b")
Expand Down
2 changes: 1 addition & 1 deletion tools/build_defs/pkg/build_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ function test_tar() {
./usr/titi
./usr/bin/
./usr/bin/java -> /path/to/bin/java"
for i in "" ".gz" ".bz2" ".xz"; do
for i in "" ".gz" ".bz2"; do
assert_content "test-tar-${i:1}.tar$i"
# Test merging tar files
# We pass a second argument to not test for user and group
Expand Down
4 changes: 3 additions & 1 deletion tools/build_defs/pkg/path_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
import imp
import unittest

pkg_bzl = imp.load_source('pkg_bzl', 'tools/build_defs/pkg/path.bzl')
pkg_bzl = imp.load_source(
'pkg_bzl',
'tools/build_defs/pkg/path.bzl')


class File(object):
Expand Down
2 changes: 1 addition & 1 deletion tools/build_defs/pkg/pkg.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ load(":path.bzl", "compute_data_path", "dest_path")
load("//tools/config:common_settings.bzl", "BuildSettingInfo")

# Filetype to restrict inputs
tar_filetype = [".tar", ".tar.gz", ".tgz", ".tar.xz", ".tar.bz2"]
tar_filetype = [".tar", ".tar.gz", ".tgz", ".tar.bz2"]

def _remap(remap_paths, path):
"""If path starts with a key in remap_paths, rewrite it."""
Expand Down
Binary file removed tools/build_defs/pkg/testdata/tar_test.tar.xz
Binary file not shown.

0 comments on commit 19a4e6c

Please sign in to comment.