Add no_remove flag

.bazelrc

@@ -0,0 +1,3 @@
+# //tests:version_test asserts on the Python interpreter version, so enable
+# toolchains to ensure we get the right one (avoid bazelbuild/bazel#4815).
+build --incompatible_use_python_toolchains

WORKSPACE

@@ -1,5 +1,13 @@
 workspace(name = "subpar")
 
+# These version vars aren't consumed by anything but are good to know for
+# documentation / compatibility purposes.
+#
+# Because of use of `PyInfo` provider.
+MIN_BAZEL_VERSION = "0.23.0"
+# Because tests require --incompatible_use_python_toolchains.
+MIN_BAZEL_VERSION_FOR_TESTS = "0.25.0"
+
 # Used by integration tests
 local_repository(
     name = "test_workspace",

compiler/BUILD

@@ -30,7 +30,7 @@ py_library(
 py_binary(
     name = "compiler",
     srcs = ["compiler.py"],
-    default_python_version = "PY2",
+    python_version = "PY2",
     main = "compiler.py",
     srcs_version = "PY2AND3",
     deps = [":compiler_lib"],

compiler/cli.py

@@ -53,20 +53,23 @@ def make_command_line_parser():
         help='Root directory of all relative paths in manifest file.',
         default=os.getcwd())
     parser.add_argument(
-        '--outputpar',
+        '--output_par',
         help='Filename of generated par file.',
         required=True)
     parser.add_argument(
         '--stub_file',
         help='Read imports and interpreter path from the specified stub file',
         required=True)
+    parser.add_argument(
+        '--interpreter',
+        help='Interpreter to use instead of determining it from the stub file')
     # The default timestamp is "Jan 1 1980 00:00:00 utc", which is the
     # earliest time that can be stored in a zip file.
     #
     # "Seconds since Unix epoch" was chosen to be compatible with
     # the SOURCE_DATE_EPOCH standard
     #
-    # Numeric calue is from running this:
+    # Numeric value is from running this:
     #   "date --date='Jan 1 1980 00:00:00 utc' --utc +%s"
     parser.add_argument(
         '--timestamp',
@@ -84,68 +87,95 @@ def make_command_line_parser():
         'directory at the start of execution.',
         type=bool_from_string,
         required=True)
+    parser.add_argument(
+        '--import_root',
+        help='Path to add to sys.path, may be repeated to provide multiple roots.',
+        action='append',
+        default=[],
+        dest='import_roots'),
+    parser.add_argument(
+        '--no_remove',
+        help='Keep extracted files after program finishes if --zip_safe is ' +
+        'enabled.',
+        type=bool_from_string,
+        required=True)
     return parser
 
 
 def parse_stub(stub_filename):
-    """Parse the imports and interpreter path from a py_binary() stub.
+    """Parse interpreter path from a py_binary() stub.
 
     We assume the stub is utf-8 encoded.
 
-    TODO(b/29227737): Remove this once we can access imports from skylark.
+    TODO(bazelbuild/bazel#7805): Remove this once we can access the py_runtime from Starlark.
 
-    Returns (list of relative paths, path to Python interpreter)
+    Returns path to Python interpreter
     """
 
-    # Find the list of import roots
-    imports_regex = re.compile(r'''^  python_imports = '([^']*)'$''')
+    # Find the interpreter
     interpreter_regex = re.compile(r'''^PYTHON_BINARY = '([^']*)'$''')
-    import_roots = None
     interpreter = None
     with io.open(stub_filename, 'rt', encoding='utf8') as stub_file:
         for line in stub_file:
-            importers_match = imports_regex.match(line)
-            if importers_match:
-                import_roots = importers_match.group(1).split(':')
-                # Filter out empty paths
-                import_roots = [x for x in import_roots if x]
             interpreter_match = interpreter_regex.match(line)
             if interpreter_match:
                 interpreter = interpreter_match.group(1)
-    if import_roots is None or not interpreter:
+    if not interpreter:
         raise error.Error('Failed to parse stub file [%s]' % stub_filename)
 
-    # Find the Python interpreter, matching the search logic in
-    # stub_template.txt
+    # Determine the Python interpreter, checking for default toolchain.
+    #
+    # This somewhat mirrors the logic in python_stub_template.txt, but we don't support
+    # relative paths (i.e., in-workspace interpreters). This is because the interpreter
+    # will be used in the .par file's shebang, and putting a relative path in a shebang
+    # is extremely brittle and non-relocatable. (The reason the standard py_binary rule
+    # can use an in-workspace interpreter is that its stub script runs in a separate
+    # process and has a shebang referencing the system interpreter). As a special case,
+    # if the Python target is using the autodetecting Python toolchain, which is
+    # technically an in-workspace runtime, we rewrite it to "/usr/bin/env python[2|3]"
+    # rather than fail.
     if interpreter.startswith('//'):
         raise error.Error('Python interpreter must not be a label [%s]' %
                           stub_filename)
     elif interpreter.startswith('/'):
         pass
+    elif interpreter == 'bazel_tools/tools/python/py3wrapper.sh': # Default toolchain
+        # Replace default toolchain python3 wrapper with default python3 on path
+        interpreter = '/usr/bin/env python3'
+    elif interpreter == 'bazel_tools/tools/python/py2wrapper.sh': # Default toolchain
+        # Replace default toolchain python2 wrapper with default python2 on path
+        interpreter = '/usr/bin/env python2'
     elif '/' in interpreter:
-        pass
+        raise error.Error(
+            'par files require a Python runtime that is ' +
+            'installed on the system, not defined inside the workspace. Use ' +
+            'a `py_runtime` with an absolute path, not a label.')
     else:
         interpreter = '/usr/bin/env %s' % interpreter
 
-    return (import_roots, interpreter)
+    return interpreter
 
 
 def main(argv):
     """Command line interface to Subpar"""
     parser = make_command_line_parser()
     args = parser.parse_args(argv[1:])
 
-    # Parse information from stub file that's too hard to compute in Skylark
-    import_roots, interpreter = parse_stub(args.stub_file)
+    # Parse interpreter from stub file that's not available in Starlark
+    interpreter = parse_stub(args.stub_file)
+
+    if args.interpreter:
+        interpreter = args.interpreter
 
     par = python_archive.PythonArchive(
         main_filename=args.main_filename,
-        import_roots=import_roots,
+        import_roots=args.import_roots,
         interpreter=interpreter,
-        output_filename=args.outputpar,
+        output_filename=args.output_par,
         manifest_filename=args.manifest_file,
         manifest_root=args.manifest_root,
         timestamp=args.timestamp,
         zip_safe=args.zip_safe,
+        no_remove=args.no_remove,
     )
     par.create()

compiler/cli_test.py

@@ -35,61 +35,72 @@ def test_make_command_line_parser(self):
         args = parser.parse_args([
             '--manifest_file=bar',
             '--manifest_root=bazz',
-            '--outputpar=baz',
+            '--output_par=baz',
             '--stub_file=quux',
             '--zip_safe=False',
+            '--no_remove=False',
+            '--import_root=root1',
+            '--import_root=root2',
             'foo',
         ])
         self.assertEqual(args.manifest_file, 'bar')
+        self.assertEqual(args.manifest_root, 'bazz')
+        self.assertEqual(args.output_par, 'baz')
+        self.assertEqual(args.stub_file, 'quux')
+        self.assertEqual(args.zip_safe, False)
+        self.assertEqual(args.import_roots, ['root1', 'root2'])
+        self.assertEqual(args.main_filename, 'foo')
+
+    def test_make_command_line_parser_for_interprerter(self):
+        parser = cli.make_command_line_parser()
+        args = parser.parse_args([
+            '--manifest_file=bar',
+            '--manifest_root=bazz',
+            '--output_par=baz',
+            '--stub_file=quux',
+            '--zip_safe=False',
+            '--no_remove=False',
+            '--interpreter=foobar',
+            'foo',
+        ])
+        self.assertEqual(args.interpreter, 'foobar')
 
     def test_stub(self):
         valid_cases = [
-            # Empty list
+            # Absolute path to interpreter
             [b"""
-  python_imports = ''
 PYTHON_BINARY = '/usr/bin/python'
 """,
-             ([], '/usr/bin/python')],
-            # Single import
-            [b"""
-  python_imports = 'myworkspace/spam/eggs'
-PYTHON_BINARY = '/usr/bin/python'
-""",
-             (['myworkspace/spam/eggs'], '/usr/bin/python')],
-            # Multiple imports
+             '/usr/bin/python'],
+            # Search for interpreter on $PATH
             [b"""
-  python_imports = 'myworkspace/spam/eggs:otherworkspace'
-PYTHON_BINARY = '/usr/bin/python'
+PYTHON_BINARY = 'python'
 """,
-             (['myworkspace/spam/eggs', 'otherworkspace'], '/usr/bin/python')],
-            # Relative path to interpreter
+             '/usr/bin/env python'],
+            # Default toolchain wrapped python3 interpreter
             [b"""
-  python_imports = ''
-PYTHON_BINARY = 'mydir/python'
+PYTHON_BINARY = 'bazel_tools/tools/python/py3wrapper.sh'
 """,
-             ([], 'mydir/python')],
-            # Search for interpreter on $PATH
+             '/usr/bin/env python3'],
+            # Default toolchain wrapped python2 interpreter
             [b"""
-  python_imports = ''
-PYTHON_BINARY = 'python'
+PYTHON_BINARY = 'bazel_tools/tools/python/py2wrapper.sh'
 """,
-             ([], '/usr/bin/env python')],
+             '/usr/bin/env python2'],
         ]
         for content, expected in valid_cases:
             with test_utils.temp_file(content) as stub_file:
                 actual = cli.parse_stub(stub_file.name)
                 self.assertEqual(actual, expected)
 
         invalid_cases = [
+            # No interpreter
             b'',
             b'\n\n',
-            # No interpreter
-            b"  python_imports = 'myworkspace/spam/eggs'",
-            # No imports
-            b"PYTHON_BINARY = 'python'\n",
+            # Relative interpreter path
+            b"PYTHON_BINARY = 'mydir/python'",
             # Interpreter is label
             b"""
-  python_imports = ''
 PYTHON_BINARY = '//mypackage:python'
 """,
             ]

compiler/python_archive.py

@@ -29,6 +29,8 @@
 """
 
 from datetime import datetime
+import contextlib
+import errno
 import io
 import logging
 import os
@@ -46,7 +48,8 @@
 _boilerplate_template = """\
 # Boilerplate added by subpar/compiler/python_archive.py
 from %(runtime_package)s import support as _
-_.setup(import_roots=%(import_roots)s, zip_safe=%(zip_safe)s)
+_.setup(import_roots=%(import_roots)s, zip_safe=%(zip_safe)s,
+    no_remove=%(no_remove)s)
 del _
 # End boilerplate
 """
@@ -100,6 +103,7 @@ def __init__(self,
                  output_filename,
                  timestamp,
                  zip_safe,
+                 no_remove,
                  ):
         self.main_filename = main_filename
 
@@ -112,6 +116,7 @@ def __init__(self,
         t = datetime.utcfromtimestamp(timestamp)
         self.timestamp_tuple = t.timetuple()[0:6]
         self.zip_safe = zip_safe
+        self.no_remove = no_remove
 
         self.compression = zipfile.ZIP_DEFLATED
 
@@ -170,6 +175,7 @@ def generate_boilerplate(self, import_roots):
             'runtime_package': _runtime_package,
             'import_roots': str(import_roots),
             'zip_safe': self.zip_safe,
+            'no_remove': self.no_remove,
         }
         return boilerplate_contents.encode('ascii').decode('ascii')
 
@@ -274,7 +280,7 @@ def write_zip_data(self, temp_parfile, stored_resources):
         """
 
         logging.debug('Storing Files...')
-        with zipfile.ZipFile(temp_parfile, 'w', self.compression) as z:
+        with contextlib.closing(zipfile.ZipFile(temp_parfile, 'w', self.compression)) as z:
             items = sorted(stored_resources.items())
             for relative_path, resource in items:
                 assert resource.zipinfo.filename == relative_path
@@ -289,9 +295,14 @@ def create_final_from_temp(self, temp_parfile_name):
 
 
 def remove_if_present(filename):
-    """Delete any existing file"""
-    if os.path.exists(filename):
+    """Delete a file if it exists"""
+    try:
+        # Remove atomically
         os.remove(filename)
+    except OSError as exc:
+        # Ignore failure if file does not exist
+        if exc.errno != errno.ENOENT:
+            raise
 
 
 def fetch_support_file(name, timestamp_tuple):

compiler/python_archive_test.py

@@ -14,6 +14,7 @@
 
 import os
 import subprocess
+import sys
 import time
 import unittest
 import zipfile
@@ -45,11 +46,12 @@ def setUp(self):
         if not os.path.exists(self.output_dir):
             os.makedirs(self.output_dir)
         self.output_filename = os.path.join(self.output_dir, 'output.par')
-        self.interpreter = '/usr/bin/python2'
+        self.interpreter = sys.executable
         self.import_roots = []
         self.date_time_tuple = (1980, 1, 1, 0, 0, 0)
         self.timestamp = 315532800
         self.zip_safe = True
+        self.no_remove = False
 
     def _construct(self, manifest_filename=None):
         return python_archive.PythonArchive(
@@ -61,6 +63,7 @@ def _construct(self, manifest_filename=None):
             output_filename=self.output_filename,
             timestamp=self.timestamp,
             zip_safe=self.zip_safe,
+            no_remove=self.no_remove,
         )
 
     def test_create_manifest_not_found(self):

docs/BUILD

@@ -1,9 +1,10 @@
 # To regenerate html docs, run:
-#   ./update_docs.sh
+#   ../update_docs.sh
 
-load("@io_bazel_skydoc//skylark:skylark.bzl", "skylark_doc", "skylark_library")
+load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
+load("@io_bazel_skydoc//skylark:skylark.bzl", "skylark_doc")
 
-skylark_library(
+bzl_library(
     name = "subpar-rules",
     srcs = [
         "@subpar//:debug.bzl",
@@ -16,13 +17,13 @@ skylark_doc(
     format = "markdown",
     overview = True,
     site_root = ".",
-    deps = ["subpar-rules"],
+    srcs = [":subpar-rules"],
 )
 
 skylark_doc(
     name = "docs-html",
     format = "html",
     overview = True,
     site_root = ".",
-    deps = ["subpar-rules"],
+    srcs = [":subpar-rules"],
 )