Merge branch 'master' into master
diff --git a/compiler/cli.py b/compiler/cli.py
index 6efde2f..91fd853 100644
--- a/compiler/cli.py
+++ b/compiler/cli.py
@@ -23,6 +23,17 @@
from subpar.compiler import python_archive
+def bool_from_string(raw_value):
+ """Parse a boolean command line argument value"""
+ if raw_value == 'True':
+ return True
+ elif raw_value == 'False':
+ return False
+ else:
+ raise argparse.ArgumentTypeError(
+ 'Value must be True or False, got %r instead.' % raw_value)
+
+
def make_command_line_parser():
"""Return an object that can parse this program's command line"""
parser = argparse.ArgumentParser(
@@ -57,8 +68,8 @@
help='Safe to import modules and access datafiles straight from zip ' +
'archive? If False, all files will be extracted to a temporary ' +
'directory at the start of execution.',
- type=bool,
- default=True)
+ type=bool_from_string,
+ required=True)
return parser
@@ -120,5 +131,6 @@
output_filename=args.outputpar,
manifest_filename=args.manifest_file,
manifest_root=args.manifest_root,
+ zip_safe=args.zip_safe,
)
par.create()
diff --git a/compiler/cli_test.py b/compiler/cli_test.py
index 7db41b4..56a6f18 100644
--- a/compiler/cli_test.py
+++ b/compiler/cli_test.py
@@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+import argparse
import unittest
from subpar.compiler import cli
@@ -21,6 +22,14 @@
class CliTest(unittest.TestCase):
+ def test_bool_from_string(self):
+ self.assertIs(cli.bool_from_string('True'), True)
+ self.assertIs(cli.bool_from_string('False'), False)
+ with self.assertRaises(argparse.ArgumentTypeError):
+ cli.bool_from_string('')
+ with self.assertRaises(argparse.ArgumentTypeError):
+ cli.bool_from_string('Yes')
+
def test_make_command_line_parser(self):
parser = cli.make_command_line_parser()
args = parser.parse_args([
diff --git a/compiler/python_archive.py b/compiler/python_archive.py
index 832cb28..3dfdba0 100755
--- a/compiler/python_archive.py
+++ b/compiler/python_archive.py
@@ -45,7 +45,7 @@
_boilerplate_template = """\
# Boilerplate added by subpar/compiler/python_archive.py
from %(runtime_package)s import support as _
-_.setup(import_roots=%(import_roots)s)
+_.setup(import_roots=%(import_roots)s, zip_safe=%(zip_safe)s)
del _
# End boilerplate
"""
@@ -76,6 +76,7 @@
manifest_filename,
manifest_root,
output_filename,
+ zip_safe,
):
self.main_filename = main_filename
@@ -84,6 +85,7 @@
self.manifest_filename = manifest_filename
self.manifest_root = manifest_root
self.output_filename = output_filename
+ self.zip_safe = zip_safe
self.compression = zipfile.ZIP_DEFLATED
@@ -141,6 +143,7 @@
boilerplate_contents = _boilerplate_template % {
'runtime_package': _runtime_package,
'import_roots': str(import_roots),
+ 'zip_safe': self.zip_safe,
}
return boilerplate_contents.encode('ascii').decode('ascii')
diff --git a/compiler/python_archive_test.py b/compiler/python_archive_test.py
index 67ae497..17d5049 100644
--- a/compiler/python_archive_test.py
+++ b/compiler/python_archive_test.py
@@ -46,6 +46,7 @@
self.output_filename = os.path.join(self.output_dir, 'output.par')
self.interpreter = '/usr/bin/python2'
self.import_roots = []
+ self.zip_safe = True
def _construct(self, manifest_filename=None):
return python_archive.PythonArchive(
@@ -55,6 +56,7 @@
manifest_filename=(manifest_filename or self.manifest_file.name),
manifest_root=os.getcwd(),
output_filename=self.output_filename,
+ zip_safe=self.zip_safe,
)
def test_create_manifest_not_found(self):
diff --git a/runtime/BUILD b/runtime/BUILD
index 5f8edff..7666720 100644
--- a/runtime/BUILD
+++ b/runtime/BUILD
@@ -18,5 +18,6 @@
srcs_version = "PY2AND3",
deps = [
":support",
+ "//compiler:test_utils",
],
)
diff --git a/runtime/support.py b/runtime/support.py
index 8d25dc6..fcfc04e 100644
--- a/runtime/support.py
+++ b/runtime/support.py
@@ -52,10 +52,14 @@
"""
+import atexit
import os
import pkgutil
+import shutil
import sys
+import tempfile
import warnings
+import zipfile
import zipimport
@@ -67,22 +71,53 @@
def _find_archive():
- """Find the path to the currently executing .par file"""
+ """Find the path to the currently executing .par file
+
+ We don't handle the case where prefix is non-empty.
+ """
main = sys.modules.get('__main__')
if not main:
_log('# __main__ module not found')
return None
- main_file = getattr(main, '__file__', None)
- if not main_file:
- _log('# __main__.__file__ not set')
+ main_loader = getattr(main, '__loader__')
+ if not main_loader:
+ _log('# __main__.__loader__ not set')
return None
- archive_path = os.path.dirname(main_file)
- if archive_path == '':
- _log('# unexpected __main__.__file__ is %s' % main_file)
+ prefix = getattr(main_loader, 'prefix')
+ if prefix != '':
+ _log('# unexpected prefix for __main__.__loader__ is %s' %
+ main_loader.prefix)
+ return None
+ archive_path = getattr(main_loader, 'archive')
+ if not archive_path:
+ _log('# missing archive for __main__.__loader__')
return None
return archive_path
+def _extract_files(archive_path):
+ """Extract the contents of this .par file to disk.
+
+ This creates a temporary directory, and registers an atexit
+ handler to clean that directory on program exit. Extraction and
+ cleanup will potentially use significant time and disk space.
+
+ Returns:
+ Directory where contents were extracted to.
+ """
+ extract_dir = tempfile.mkdtemp()
+ def _extract_files_cleanup():
+ shutil.rmtree(extract_dir, ignore_errors=True)
+ atexit.register(_extract_files_cleanup)
+ _log('# extracting %s to %s' % (archive_path, extract_dir))
+
+ zip_file = zipfile.ZipFile(archive_path, mode='r')
+ zip_file.extractall(extract_dir)
+ zip_file.close()
+
+ return extract_dir
+
+
def _version_check_pkg_resources(pkg_resources):
"""Check that pkg_resources supports the APIs we need."""
# Check that pkg_resources is new enough.
@@ -147,8 +182,7 @@
# Convert a virtual filename (full path to file) into a
# zipfile subpath usable with the zipimport directory
# cache for our target archive
- while fspath.endswith(os.sep):
- fspath = fspath[:-1]
+ fspath = fspath.rstrip(os.sep)
if fspath == self.loader.archive:
return ''
if fspath.startswith(self.zip_pre):
@@ -236,22 +270,58 @@
pkg_resources.working_set.add(dist, entry, insert=False,
replace=True)
+def _initialize_import_path(import_roots, import_prefix):
+ """Add extra entries to PYTHONPATH so that modules can be imported."""
+ # We try to match to order of Bazel's stub
+ full_roots = [
+ os.path.join(import_prefix, import_root)
+ for import_root in import_roots]
+ sys.path[1:1] = full_roots
+ _log('# adding %s to sys.path' % full_roots)
-def setup(import_roots=None):
- """Initialize subpar run-time support"""
- # Add third-party library entries to sys.path
+
+def setup(import_roots, zip_safe):
+ """Initialize subpar run-time support
+
+ Args:
+ import_root (list): subdirs inside .par file to add to the
+ module import path at runtime.
+ zip_safe (bool): If False, extract the .par file contents to a
+ temporary directory, and import everything from
+ that directory.
+
+ Returns:
+ True if setup was successful, else False
+ """
archive_path = _find_archive()
if not archive_path:
warnings.warn('Failed to initialize .par file runtime support',
- ImportWarning)
- return
+ UserWarning)
+ return False
+ if sys.path[0] != archive_path:
+ warnings.warn('Failed to initialize .par file runtime support. ' +
+ 'archive_path was %r, sys.path was %r' % (
+ archive_path, sys.path),
+ UserWarning)
+ return False
- # We try to match to order of Bazel's stub
- for import_root in reversed(import_roots or []):
- new_path = os.path.join(archive_path, import_root)
- _log('# adding %s to sys.path' % new_path)
- sys.path.insert(1, new_path)
+ # Extract files to disk if necessary
+ if not zip_safe:
+ extract_dir = _extract_files(archive_path)
+ # sys.path[0] is the name of the executing .par file. Point
+ # it to the extract directory instead, so that Python searches
+ # there for imports.
+ sys.path[0] = extract_dir
+ import_prefix = extract_dir
+ else: # Import directly from .par file
+ extract_dir = None
+ import_prefix = archive_path
+
+ # Initialize import path
+ _initialize_import_path(import_roots, import_prefix)
# Add hook for package metadata
_setup_pkg_resources('pkg_resources')
_setup_pkg_resources('pip._vendor.pkg_resources')
+
+ return True
diff --git a/runtime/support_test.py b/runtime/support_test.py
index de0b9c9..e3a2741 100644
--- a/runtime/support_test.py
+++ b/runtime/support_test.py
@@ -13,13 +13,69 @@
# limitations under the License.
import io
+import os
import sys
+import tempfile
import unittest
+import zipfile
+from subpar.compiler import test_utils
from subpar.runtime import support
+# `import pip` can cause arbitrary sys.path changes,
+# especially if using the Debian `python-pip` package or
+# similar. Do it first to get those changes out of the
+# way.
+old_sys_path = sys.path
+try:
+ mock_sys_path = list(sys.path)
+ sys.path = mock_sys_path
+ import pip # noqa
+except ImportError:
+ pass
+finally:
+ sys.path = old_sys_path
+
+# We assume this test isn't run as a par file.
class SupportTest(unittest.TestCase):
+ @classmethod
+ def setUpClass(cls):
+ # Create zipfile for tests to read
+ tmpdir = test_utils.mkdtemp()
+ zipfile_name = os.path.join(tmpdir, '_support_test_sample.par')
+
+ z = zipfile.ZipFile(zipfile_name, 'w')
+ entry_name = '_support_test_helper_script.py'
+ entry_data = b'print("Hello world")'
+ z.writestr(entry_name, entry_data)
+
+ z.close()
+
+ cls.zipfile_name = zipfile_name
+ cls.entry_name = entry_name
+ cls.entry_data = entry_data
+
+ # Create mock loader object
+ class MockLoader(object): pass
+ mock_loader = MockLoader()
+ mock_loader.archive = zipfile_name
+ mock_loader.prefix = ''
+ main = sys.modules.get('__main__')
+ old_loader = getattr(main, '__loader__', None)
+ main.__loader__ = mock_loader
+
+ cls.mock_loader = mock_loader
+ cls.old_loader = old_loader
+
+ @classmethod
+ def tearDownClass(cls):
+ # Cleanup zipfile
+ os.remove(cls.zipfile_name)
+
+ # Cleanup loader mock
+ main = sys.modules.get('__main__')
+ main.__loader__ = cls.old_loader
def test__log(self):
old_stderr = sys.stderr
@@ -38,8 +94,22 @@
def test__find_archive(self):
# pylint: disable=protected-access
- path = support._find_archive()
- self.assertNotEqual(path, None)
+ archive_path = support._find_archive()
+
+ self.assertNotEqual(archive_path, None)
+ self.assertTrue(zipfile.is_zipfile(archive_path))
+
+ def test__extract_files(self):
+ # Extract zipfile
+ extract_path = support._extract_files(self.zipfile_name)
+
+ # Check results
+ self.assertTrue(os.path.isdir(extract_path))
+ extracted_file = os.path.join(extract_path, self.entry_name)
+ self.assertTrue(os.path.isfile(extracted_file))
+ with open(extracted_file, 'rb') as f:
+ actual_data = f.read()
+ self.assertEqual(actual_data, self.entry_data)
def test__version_check(self):
class MockModule(object): pass
@@ -60,42 +130,48 @@
self.assertTrue(support._version_check_pkg_resources(pkg_resources))
def test_setup(self):
- old_sys_path = sys.path
- mock_sys_path = list(sys.path)
- sys.path = mock_sys_path
- # `import pip` can cause arbitrary sys.path changes,
- # especially if using the Debian `python-pip` package or
- # similar. Do it first to get those changes out of the
- # way.
- try:
- import pip # noqa
- except ImportError:
- pass
- finally:
- sys.path = old_sys_path
-
- # Run setup()
+ # Run setup() without file extraction
old_sys_path = sys.path
try:
mock_sys_path = list(sys.path)
+ mock_sys_path[0] = self.zipfile_name
sys.path = mock_sys_path
- support.setup(import_roots=['some_root', 'another_root'])
+ success = support.setup(import_roots=['some_root', 'another_root'], zip_safe=True)
+ self.assertTrue(success)
finally:
sys.path = old_sys_path
# Check results
- self.assertTrue(mock_sys_path[1].endswith('subpar/runtime/some_root'),
- mock_sys_path)
self.assertTrue(
- mock_sys_path[2].endswith('subpar/runtime/another_root'),
+ mock_sys_path[1].endswith('some_root'),
mock_sys_path)
- self.assertEqual(mock_sys_path[0], sys.path[0])
+ self.assertTrue(
+ mock_sys_path[2].endswith('another_root'),
+ mock_sys_path)
# If we have no pkg_resources, or a really old version of
# pkg_resources, setup skips some things
module = sys.modules.get('pkg_resources', None)
if module and support._version_check_pkg_resources(module):
self.assertEqual(mock_sys_path[3:], sys.path[1:])
+ def test_setup__extract(self):
+ # Run setup() with file extraction
+ old_sys_path = sys.path
+ try:
+ mock_sys_path = list(sys.path)
+ mock_sys_path[0] = self.zipfile_name
+ sys.path = mock_sys_path
+ success = support.setup(import_roots=['some_root'], zip_safe=False)
+ self.assertTrue(success)
+ finally:
+ sys.path = old_sys_path
+
+ # Check results
+ self.assertNotEqual(mock_sys_path[0], self.zipfile_name)
+ self.assertTrue(
+ os.path.isdir(mock_sys_path[0]),
+ mock_sys_path)
+
if __name__ == '__main__':
unittest.main()
diff --git a/subpar.bzl b/subpar.bzl
index 378e328..b433115 100644
--- a/subpar.bzl
+++ b/subpar.bzl
@@ -190,13 +190,14 @@
for arguments and usage.
"""
compiler = kwargs.pop('compiler', DEFAULT_COMPILER)
+ zip_safe = kwargs.pop('zip_safe', True)
native.py_binary(name=name, **kwargs)
+
main = kwargs.get('main', name + '.py')
imports = kwargs.get('imports')
default_python_version = kwargs.get('default_python_version', 'PY2')
visibility = kwargs.get('visibility')
testonly = kwargs.get('testonly', False)
- zip_safe = kwargs.get('zip_safe', True)
parfile(name=name + '.par', src=name, main=main, imports=imports,
default_python_version=default_python_version, visibility=visibility,
compiler=compiler,
@@ -209,13 +210,14 @@
Just like par_binary, but for py_test instead of py_binary. Useful if you
specifically need to test a module's behaviour when used in a .par binary.
"""
+ zip_safe = kwargs.pop('zip_safe', True)
native.py_test(name=name, **kwargs)
+
main = kwargs.get('main', name + '.py')
imports = kwargs.get('imports')
default_python_version = kwargs.get('default_python_version', 'PY2')
visibility = kwargs.get('visibility')
testonly = kwargs.get('testonly', True)
- zip_safe = kwargs.get('zip_safe', True)
parfile_test(
name=name + '.par', src=name, main=main, imports=imports,
default_python_version=default_python_version, visibility=visibility,
diff --git a/tests/BUILD b/tests/BUILD
index 0d215bc..43a602c 100644
--- a/tests/BUILD
+++ b/tests/BUILD
@@ -83,10 +83,22 @@
)
par_binary(
- name = "package_import_roots/import_roots",
+ name = "package_extract/extract",
srcs = [
- "package_import_roots/import_roots.py",
+ "package_extract/extract.py",
+ "package_extract/extract_helper.py",
+ "package_extract/extract_helper_package/__init__.py",
],
+ data = ["package_extract/extract_helper_package/extract_dat.txt"],
+ imports = ["package_extract"],
+ main = "package_extract/extract.py",
+ srcs_version = "PY2AND3",
+ zip_safe = False,
+)
+
+par_binary(
+ name = "package_import_roots/import_roots",
+ srcs = ["package_import_roots/import_roots.py"],
imports = ["package_import_roots"],
main = "package_import_roots/import_roots.py",
srcs_version = "PY2AND3",
@@ -159,6 +171,7 @@
),
("direct_dependency", "//tests/package_b:b", "tests/package_b/b"),
("external_workspace", "//tests:package_e/e", "tests/package_e/e"),
+ ("extract", "//tests:package_extract/extract", "tests/package_extract/extract"),
("import_root", "//tests:package_d/d", "tests/package_d/d"),
(
"import_roots",
diff --git a/tests/package_extract/extract.py b/tests/package_extract/extract.py
new file mode 100644
index 0000000..96c8b71
--- /dev/null
+++ b/tests/package_extract/extract.py
@@ -0,0 +1,43 @@
+# Copyright 2018 Google Inc. 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.
+
+"""Integration test program for Subpar
+
+Tests file extraction functionality (zip_safe=False)
+"""
+
+import os
+import pkgutil
+import sys
+
+
+def main():
+ print('In extract.py main()')
+
+ # Test that imports are from real files on disk. Slightly tricky
+ # to test, since the 'subpar' package is imported before we
+ # extract and setup sys.path, so we can't "import subpar.test.something"
+ import extract_helper
+ assert os.path.isfile(extract_helper.__file__), (
+ extract_helper.__file__, sys.path)
+ import extract_helper_package
+ assert os.path.isfile(extract_helper_package.__file__), (
+ extract_helper_package.__file__, sys.path)
+
+ # Test resource extraction
+ dat = pkgutil.get_data('extract_helper_package', 'extract_dat.txt')
+ assert (dat == b'Dummy data file for extract.py\n'), dat
+
+if __name__ == '__main__':
+ main()
diff --git a/tests/package_extract/extract_PY2_filelist.txt b/tests/package_extract/extract_PY2_filelist.txt
new file mode 100644
index 0000000..bed1830
--- /dev/null
+++ b/tests/package_extract/extract_PY2_filelist.txt
@@ -0,0 +1,11 @@
+__main__.py
+subpar/__init__.py
+subpar/runtime/__init__.py
+subpar/runtime/support.py
+subpar/tests/__init__.py
+subpar/tests/package_extract/__init__.py
+subpar/tests/package_extract/extract
+subpar/tests/package_extract/extract.py
+subpar/tests/package_extract/extract_helper.py
+subpar/tests/package_extract/extract_helper_package/__init__.py
+subpar/tests/package_extract/extract_helper_package/extract_dat.txt
diff --git a/tests/package_extract/extract_PY3_filelist.txt b/tests/package_extract/extract_PY3_filelist.txt
new file mode 100644
index 0000000..bed1830
--- /dev/null
+++ b/tests/package_extract/extract_PY3_filelist.txt
@@ -0,0 +1,11 @@
+__main__.py
+subpar/__init__.py
+subpar/runtime/__init__.py
+subpar/runtime/support.py
+subpar/tests/__init__.py
+subpar/tests/package_extract/__init__.py
+subpar/tests/package_extract/extract
+subpar/tests/package_extract/extract.py
+subpar/tests/package_extract/extract_helper.py
+subpar/tests/package_extract/extract_helper_package/__init__.py
+subpar/tests/package_extract/extract_helper_package/extract_dat.txt
diff --git a/tests/package_extract/extract_helper.py b/tests/package_extract/extract_helper.py
new file mode 100644
index 0000000..2ae2839
--- /dev/null
+++ b/tests/package_extract/extract_helper.py
@@ -0,0 +1 @@
+pass
diff --git a/tests/package_extract/extract_helper_package/__init__.py b/tests/package_extract/extract_helper_package/__init__.py
new file mode 100644
index 0000000..2ae2839
--- /dev/null
+++ b/tests/package_extract/extract_helper_package/__init__.py
@@ -0,0 +1 @@
+pass
diff --git a/tests/package_extract/extract_helper_package/extract_dat.txt b/tests/package_extract/extract_helper_package/extract_dat.txt
new file mode 100644
index 0000000..153a605
--- /dev/null
+++ b/tests/package_extract/extract_helper_package/extract_dat.txt
@@ -0,0 +1 @@
+Dummy data file for extract.py
