From 47ff6643696f4ac9265c2c1b8c5f89dd3c1ce5a5 Mon Sep 17 00:00:00 2001 From: Fahrzin Hemmati Date: Fri, 23 Mar 2018 13:43:24 -0700 Subject: [PATCH 01/19] Handle extra arguments to pass to the compiler (#74) --- subpar.bzl | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/subpar.bzl b/subpar.bzl index f12230d..075c5a2 100644 --- a/subpar.bzl +++ b/subpar.bzl @@ -74,7 +74,7 @@ def _parfile_impl(ctx): zip_safe = ctx.attr.zip_safe # Assemble command line for .par compiler - args = [ + args = ctx.attr.compiler_args + [ '--manifest_file', sources_file.path, '--outputpar', ctx.outputs.executable.path, '--stub_file', stub_file, @@ -123,6 +123,7 @@ parfile_attrs = { executable = True, cfg = "host", ), + "compiler_args": attr.string_list(default = []), "zip_safe": attr.bool(default=True), } @@ -190,6 +191,7 @@ def par_binary(name, **kwargs): for arguments and usage. """ compiler = kwargs.pop('compiler', None) + compiler_args = kwargs.pop('compiler_args', []) zip_safe = kwargs.pop('zip_safe', True) native.py_binary(name=name, **kwargs) @@ -200,6 +202,7 @@ def par_binary(name, **kwargs): testonly = kwargs.get('testonly', False) parfile( compiler=compiler, + compiler_args=compiler_args, default_python_version=default_python_version, imports=imports, main=main, From 1c91ab83c784f098718024229256110aa5595b57 Mon Sep 17 00:00:00 2001 From: Ed Bardsley Date: Tue, 10 Apr 2018 15:13:46 -0700 Subject: [PATCH 02/19] Fix tests on MacOS. (#77) Two issues: - BSD head/tail don't support the +/- specifications. Use "-q -q" to unzip instead to remove headers and footers. - "python2" isn't guaranteed to exist. Use the current interpreter. --- compiler/python_archive_test.py | 3 ++- tests/test_harness.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/python_archive_test.py b/compiler/python_archive_test.py index 3fd5ec2..b918399 100644 --- a/compiler/python_archive_test.py +++ b/compiler/python_archive_test.py @@ -14,6 +14,7 @@ import os import subprocess +import sys import time import unittest import zipfile @@ -45,7 +46,7 @@ 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 diff --git a/tests/test_harness.sh b/tests/test_harness.sh index b3a14d0..59ef338 100755 --- a/tests/test_harness.sh +++ b/tests/test_harness.sh @@ -41,7 +41,7 @@ TMP_EXECUTABLE="$TEST_TMPDIR"/$(basename "$EXECUTABLE") # Compare list of files in zipfile with expected list if [ "$PAR" -eq 1 ]; then diff \ - <(unzip -l "$EXECUTABLE" | awk '{print $NF}' | head -n -2 | tail -n +4) \ + <(unzip -l -q -q "$EXECUTABLE" | awk '{print $NF}') \ "$FILELIST" \ || die 'FATAL: zipfile contents do not match expected' fi From 6c7e64ada41cb0440bcd8a1796f3fba212539882 Mon Sep 17 00:00:00 2001 From: Ed Bardsley Date: Tue, 10 Apr 2018 15:20:04 -0700 Subject: [PATCH 03/19] Pass use_default_shell_env=True to ctx.action. (#76) This is necessary under --python_path=python3 and other relative paths, which is in turn necessary to support builds across multiple environments with python3 installed in different locations. --- subpar.bzl | 1 + 1 file changed, 1 insertion(+) diff --git a/subpar.bzl b/subpar.bzl index 075c5a2..e3d1e25 100644 --- a/subpar.bzl +++ b/subpar.bzl @@ -88,6 +88,7 @@ def _parfile_impl(ctx): executable=ctx.executable.compiler, arguments=args, mnemonic='PythonCompile', + use_default_shell_env=True, ) # .par file itself has no runfiles and no providers From 751ca6725d3888cb6922906ffd47627804eaeb83 Mon Sep 17 00:00:00 2001 From: Douglas Greiman Date: Tue, 17 Apr 2018 13:30:37 -0700 Subject: [PATCH 04/19] Minor fixes and cleanups to run_tests.sh script (#79) Fixed usage of `expr` that is not supported on Mac OS. Fixed case when $(which python3) returned false. --- run_tests.sh | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/run_tests.sh b/run_tests.sh index 1dc7403..25ffac1 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -16,8 +16,13 @@ set -euo pipefail +function die { + echo "$*" 1>&2 + exit 1 +} + # Find various tools in environment -PYTHON2=$(which python) +PYTHON2=$(which python||true) if [ -z "${PYTHON2}" ]; then PYTHON2=$(which python2) fi @@ -29,22 +34,21 @@ fi # https://www.python.org/dev/peps/pep-0394/ if [ -n "${PYTHON2}" ]; then VERSION=$("${PYTHON2}" -V 2>&1) - if ! expr match "${VERSION}" "Python 2." >/dev/null ; then + if ! expr "${VERSION}" : "Python 2." >/dev/null ; then PYTHON2= fi fi -PYTHON3=$(which python3) +PYTHON3=$(which python3||true) -VIRTUALENV=$(which virtualenv) +VIRTUALENV=$(which virtualenv) || die "virtualenv not installed" VIRTUALENVDIR=$(dirname $0)/.env # Virtualenv `activate` needs $PS1 set PS1='$ ' # Must have at least one Python interpreter to test if [ -z "${PYTHON2}" -a -z "${PYTHON3}" ]; then - echo "ERROR: Could not find Python 2 or 3 interpreter on $PATH" 1>&2 - exit 1 + die "Could not find Python 2 or 3 interpreter on $PATH" fi # Run test matrix From 07ff5feb7c7b113eea593eb6ec50b51099cf0261 Mon Sep 17 00:00:00 2001 From: Ali Date: Thu, 26 Apr 2018 11:10:40 -0700 Subject: [PATCH 05/19] make python_archive compatible with python 2.6 (#80) --- compiler/python_archive.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/python_archive.py b/compiler/python_archive.py index 8a119fd..fc810c6 100755 --- a/compiler/python_archive.py +++ b/compiler/python_archive.py @@ -29,6 +29,7 @@ """ from datetime import datetime +import contextlib import io import logging import os @@ -274,7 +275,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 From b57a337e3eca2342ed5f2fa614c62f53d6565ce6 Mon Sep 17 00:00:00 2001 From: Alex Thompson Date: Mon, 19 Nov 2018 09:37:19 -0500 Subject: [PATCH 06/19] Python interpreter can now be overridden --- compiler/cli.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/compiler/cli.py b/compiler/cli.py index 083f931..7a34b4e 100644 --- a/compiler/cli.py +++ b/compiler/cli.py @@ -60,6 +60,9 @@ def make_command_line_parser(): '--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. # @@ -138,6 +141,9 @@ def main(argv): # Parse information from stub file that's too hard to compute in Skylark import_roots, 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, From fe1e114d107b3bf46a5bf035d1e1c3cdc47202e2 Mon Sep 17 00:00:00 2001 From: Laurent Le Brun Date: Tue, 27 Nov 2018 20:55:02 +0100 Subject: [PATCH 07/19] Reformat subpar.bzl with buildifier --- subpar.bzl | 146 ++++++++++++++++++++++++++++------------------------- 1 file changed, 78 insertions(+), 68 deletions(-) diff --git a/subpar.bzl b/subpar.bzl index e3d1e25..3c630bf 100644 --- a/subpar.bzl +++ b/subpar.bzl @@ -16,19 +16,20 @@ load("//:debug.bzl", "dump") -DEFAULT_COMPILER = '//compiler:compiler.par' +DEFAULT_COMPILER = "//compiler:compiler.par" def _parfile_impl(ctx): """Implementation of parfile() rule""" + # Find the main entry point py_files = ctx.files.main if len(py_files) == 0: - fail('Expected exactly one .py file, found none', 'main') + fail("Expected exactly one .py file, found none", "main") elif len(py_files) > 1: - fail('Expected exactly one .py file, found these: [%s]' % py_files, 'main') + fail("Expected exactly one .py file, found these: [%s]" % py_files, "main") main_py_file = py_files[0] if main_py_file not in ctx.attr.src.data_runfiles.files: - fail('Main entry point [%s] not listed in srcs' % main_py_file, 'main') + fail("Main entry point [%s] not listed in srcs" % main_py_file, "main") # Find the list of things that must be built before this thing is built # TODO: also handle ctx.attr.src.data_runfiles.symlinks @@ -37,28 +38,32 @@ def _parfile_impl(ctx): # Make a manifest of files to store in the .par file. The # runfiles manifest is not quite right, so we make our own. sources_map = {} + # First, add the zero-length __init__.py files for empty in ctx.attr.src.default_runfiles.empty_filenames: stored_path = _prepend_workspace(empty, ctx) - local_path = '' + local_path = "" sources_map[stored_path] = local_path + # Now add the regular (source and generated) files for input_file in inputs: stored_path = _prepend_workspace(input_file.short_path, ctx) local_path = input_file.path sources_map[stored_path] = local_path + # Now make a nice sorted list sources_lines = [] - for k,v in sorted(sources_map.items()): - sources_lines.append('%s %s' % (k, v)) - sources_content = '\n'.join(sources_lines) + '\n' + for k, v in sorted(sources_map.items()): + sources_lines.append("%s %s" % (k, v)) + sources_content = "\n".join(sources_lines) + "\n" # Write the list to the manifest file - sources_file = ctx.new_file(ctx.label.name + '_SOURCES') + sources_file = ctx.new_file(ctx.label.name + "_SOURCES") ctx.file_action( - output=sources_file, - content=sources_content, - executable=False) + output = sources_file, + content = sources_content, + executable = False, + ) # Find the list of directories to add to sys.path # TODO(b/29227737): Use 'imports' provider from Bazel @@ -69,26 +74,30 @@ def _parfile_impl(ctx): sources_file, ctx.attr.src.files_to_run.executable, ctx.attr.src.files_to_run.runfiles_manifest, - ] + ] zip_safe = ctx.attr.zip_safe # Assemble command line for .par compiler args = ctx.attr.compiler_args + [ - '--manifest_file', sources_file.path, - '--outputpar', ctx.outputs.executable.path, - '--stub_file', stub_file, - '--zip_safe', str(zip_safe), + "--manifest_file", + sources_file.path, + "--outputpar", + ctx.outputs.executable.path, + "--stub_file", + stub_file, + "--zip_safe", + str(zip_safe), main_py_file.path, ] ctx.action( - inputs=inputs + extra_inputs, - outputs=[ctx.outputs.executable], - progress_message='Building par file %s' % ctx.label, - executable=ctx.executable.compiler, - arguments=args, - mnemonic='PythonCompile', - use_default_shell_env=True, + inputs = inputs + extra_inputs, + outputs = [ctx.outputs.executable], + progress_message = "Building par file %s" % ctx.label, + executable = ctx.executable.compiler, + arguments = args, + mnemonic = "PythonCompile", + use_default_shell_env = True, ) # .par file itself has no runfiles and no providers @@ -96,18 +105,19 @@ def _parfile_impl(ctx): def _prepend_workspace(path, ctx): """Given a path, prepend the workspace name as the parent directory""" + # It feels like there should be an easier, less fragile way. - if path.startswith('../'): + if path.startswith("../"): # External workspace, for example # '../protobuf/python/google/protobuf/any_pb2.py' - stored_path = path[len('../'):] - elif path.startswith('external/'): + stored_path = path[len("../"):] + elif path.startswith("external/"): # External workspace, for example # 'external/protobuf/python/__init__.py' - stored_path = path[len('external/'):] + stored_path = path[len("external/"):] else: # Main workspace, for example 'mypackage/main.py' - stored_path = ctx.workspace_name + '/' + path + stored_path = ctx.workspace_name + "/" + path return stored_path parfile_attrs = { @@ -125,7 +135,7 @@ parfile_attrs = { cfg = "host", ), "compiler_args": attr.string_list(default = []), - "zip_safe": attr.bool(default=True), + "zip_safe": attr.bool(default = True), } # Rule to create a parfile given a py_binary() as input @@ -191,27 +201,27 @@ def par_binary(name, **kwargs): See [py_binary](http://www.bazel.io/docs/be/python.html#py_binary) for arguments and usage. """ - compiler = kwargs.pop('compiler', None) - compiler_args = kwargs.pop('compiler_args', []) - 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) + compiler = kwargs.pop("compiler", None) + compiler_args = kwargs.pop("compiler_args", []) + 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) parfile( - compiler=compiler, - compiler_args=compiler_args, - default_python_version=default_python_version, - imports=imports, - main=main, - name=name + '.par', - src=name, - testonly=testonly, - visibility=visibility, - zip_safe=zip_safe, + compiler = compiler, + compiler_args = compiler_args, + default_python_version = default_python_version, + imports = imports, + main = main, + name = name + ".par", + src = name, + testonly = testonly, + visibility = visibility, + zip_safe = zip_safe, ) def par_test(name, **kwargs): @@ -220,23 +230,23 @@ def par_test(name, **kwargs): 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. """ - compiler = kwargs.pop('compiler', None) - 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) + compiler = kwargs.pop("compiler", None) + 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) parfile_test( - compiler=compiler, - default_python_version=default_python_version, - imports=imports, - main=main, - name=name + '.par', - src=name, - testonly=testonly, - visibility=visibility, - zip_safe=zip_safe, + compiler = compiler, + default_python_version = default_python_version, + imports = imports, + main = main, + name = name + ".par", + src = name, + testonly = testonly, + visibility = visibility, + zip_safe = zip_safe, ) From 95d029a0b2c7622117f5c2aac83c2d54b9e69112 Mon Sep 17 00:00:00 2001 From: Laurent Le Brun Date: Tue, 27 Nov 2018 20:55:29 +0100 Subject: [PATCH 08/19] Apply buildifier --lint=fix --- subpar.bzl | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/subpar.bzl b/subpar.bzl index 3c630bf..5e3809a 100644 --- a/subpar.bzl +++ b/subpar.bzl @@ -14,8 +14,6 @@ """Build self-contained python executables.""" -load("//:debug.bzl", "dump") - DEFAULT_COMPILER = "//compiler:compiler.par" def _parfile_impl(ctx): @@ -58,11 +56,11 @@ def _parfile_impl(ctx): sources_content = "\n".join(sources_lines) + "\n" # Write the list to the manifest file - sources_file = ctx.new_file(ctx.label.name + "_SOURCES") - ctx.file_action( + sources_file = ctx.actions.declare_file(ctx.label.name + "_SOURCES") + ctx.actions.write( output = sources_file, content = sources_content, - executable = False, + is_executable = False, ) # Find the list of directories to add to sys.path @@ -90,7 +88,7 @@ def _parfile_impl(ctx): str(zip_safe), main_py_file.path, ] - ctx.action( + ctx.actions.run( inputs = inputs + extra_inputs, outputs = [ctx.outputs.executable], progress_message = "Building par file %s" % ctx.label, @@ -124,8 +122,7 @@ parfile_attrs = { "src": attr.label(mandatory = True), "main": attr.label( mandatory = True, - allow_files = True, - single_file = True, + allow_single_file = True, ), "imports": attr.string_list(default = []), "default_python_version": attr.string(mandatory = True), From 9662e6b5fafb273bcd37b55ea70c0a539e009e6a Mon Sep 17 00:00:00 2001 From: Laurent Le Brun Date: Tue, 27 Nov 2018 20:58:00 +0100 Subject: [PATCH 09/19] Fix other Bazel incompatible issues - depset is not iterable - tools should be separate from other inputs --- subpar.bzl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/subpar.bzl b/subpar.bzl index 5e3809a..3a3354b 100644 --- a/subpar.bzl +++ b/subpar.bzl @@ -26,19 +26,19 @@ def _parfile_impl(ctx): elif len(py_files) > 1: fail("Expected exactly one .py file, found these: [%s]" % py_files, "main") main_py_file = py_files[0] - if main_py_file not in ctx.attr.src.data_runfiles.files: + if main_py_file not in ctx.attr.src.data_runfiles.files.to_list(): fail("Main entry point [%s] not listed in srcs" % main_py_file, "main") # Find the list of things that must be built before this thing is built # TODO: also handle ctx.attr.src.data_runfiles.symlinks - inputs = list(ctx.attr.src.default_runfiles.files) + inputs = ctx.attr.src.default_runfiles.files.to_list() # Make a manifest of files to store in the .par file. The # runfiles manifest is not quite right, so we make our own. sources_map = {} # First, add the zero-length __init__.py files - for empty in ctx.attr.src.default_runfiles.empty_filenames: + for empty in ctx.attr.src.default_runfiles.empty_filenames.to_list(): stored_path = _prepend_workspace(empty, ctx) local_path = "" sources_map[stored_path] = local_path @@ -70,7 +70,6 @@ def _parfile_impl(ctx): # Inputs to the action, but don't actually get stored in the .par file extra_inputs = [ sources_file, - ctx.attr.src.files_to_run.executable, ctx.attr.src.files_to_run.runfiles_manifest, ] @@ -90,6 +89,7 @@ def _parfile_impl(ctx): ] ctx.actions.run( inputs = inputs + extra_inputs, + tools = [ctx.attr.src.files_to_run.executable], outputs = [ctx.outputs.executable], progress_message = "Building par file %s" % ctx.label, executable = ctx.executable.compiler, From 6550931223702123193aa21f0c41d0da698f74f3 Mon Sep 17 00:00:00 2001 From: Alex Thompson Date: Wed, 19 Dec 2018 11:59:15 -0500 Subject: [PATCH 10/19] Added test case for compiler interpreter argument --- compiler/cli_test.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/compiler/cli_test.py b/compiler/cli_test.py index 56a6f18..677b092 100644 --- a/compiler/cli_test.py +++ b/compiler/cli_test.py @@ -42,6 +42,19 @@ def test_make_command_line_parser(self): ]) self.assertEqual(args.manifest_file, 'bar') + 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', + '--outputpar=baz', + '--stub_file=quux', + '--zip_safe=False', + '--interpreter=foobar', + 'foo', + ]) + self.assertEqual(args.interpreter, 'foobar') + def test_stub(self): valid_cases = [ # Empty list From 1f34493d833f20fbf713f18aa4574c8c8dee38df Mon Sep 17 00:00:00 2001 From: Alex Thompson Date: Wed, 19 Dec 2018 12:02:42 -0500 Subject: [PATCH 11/19] Fixed copy/paste error --- compiler/cli_test.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/compiler/cli_test.py b/compiler/cli_test.py index 677b092..a30c067 100644 --- a/compiler/cli_test.py +++ b/compiler/cli_test.py @@ -42,18 +42,18 @@ def test_make_command_line_parser(self): ]) self.assertEqual(args.manifest_file, 'bar') - 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', - '--outputpar=baz', - '--stub_file=quux', - '--zip_safe=False', - '--interpreter=foobar', - 'foo', - ]) - self.assertEqual(args.interpreter, 'foobar') + 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', + '--outputpar=baz', + '--stub_file=quux', + '--zip_safe=False', + '--interpreter=foobar', + 'foo', + ]) + self.assertEqual(args.interpreter, 'foobar') def test_stub(self): valid_cases = [ From 76137f15167a5460853fe861732dff588292876d Mon Sep 17 00:00:00 2001 From: Jon Brandvein Date: Thu, 7 Mar 2019 17:07:50 -0500 Subject: [PATCH 12/19] Update documentation generation dependencies and regenerate (#95) This updates to a recent version of Skydoc so that doc generation works on modern versions of Bazel. --- docs/BUILD | 11 ++++++----- docs/WORKSPACE | 38 +++++++++++++++++++++++++------------- docs/index.html | 4 ++-- docs/index.md | 4 ++-- docs/main.css | 4 +--- docs/subpar.html | 36 ++++++++++++++++++++++++++++++++---- docs/subpar.md | 36 ++++++++++++++++++++++++++++++++---- 7 files changed, 100 insertions(+), 33 deletions(-) diff --git a/docs/BUILD b/docs/BUILD index f72a6ea..555d460 100644 --- a/docs/BUILD +++ b/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,7 +17,7 @@ skylark_doc( format = "markdown", overview = True, site_root = ".", - deps = ["subpar-rules"], + srcs = [":subpar-rules"], ) skylark_doc( @@ -24,5 +25,5 @@ skylark_doc( format = "html", overview = True, site_root = ".", - deps = ["subpar-rules"], + srcs = [":subpar-rules"], ) diff --git a/docs/WORKSPACE b/docs/WORKSPACE index 77a5e97..23bddbd 100644 --- a/docs/WORKSPACE +++ b/docs/WORKSPACE @@ -1,28 +1,40 @@ workspace(name = "subpar_docs") +load("@bazel_tools//tools/build_defs/repo:git.bzl", "git_repository") + local_repository( name = "subpar", path = "..", ) -# Dependency of skydoc -git_repository( - name = "io_bazel_rules_sass", - remote = "https://github.com/bazelbuild/rules_sass.git", - tag = "0.0.3", -) - -load("@io_bazel_rules_sass//sass:sass.bzl", "sass_repositories") +# Skydoc / stardoc and dependencies. -sass_repositories() - -# Used to create documentation git_repository( name = "io_bazel_skydoc", remote = "https://github.com/bazelbuild/skydoc.git", - commit = "b36d22cc4436a7a7933e36a111bfc00fd494b9fb", + commit = "13063139c7c2bca7c725f382fa1e78bdfe93c887", # 2019-02-27 ) -load("@io_bazel_skydoc//skylark:skylark.bzl", "skydoc_repositories") +# This initialization comes from Stardoc's WORKSPACE file as of commit +# 13063139c7c2bca7c725f382fa1e78bdfe93c887. +load("@io_bazel_skydoc//:setup.bzl", "skydoc_repositories") skydoc_repositories() + +load("@io_bazel_rules_sass//:package.bzl", "rules_sass_dependencies") +rules_sass_dependencies() + +load("@build_bazel_rules_nodejs//:defs.bzl", "node_repositories") +node_repositories() + +load("@io_bazel_rules_sass//:defs.bzl", "sass_repositories") +sass_repositories() + +# This definition is also needed for (legacy) Skydoc. +# TODO(#93): Migrate to Stardoc. + +git_repository( + name = "com_google_protobuf", + remote = "https://github.com/protocolbuffers/protobuf.git", + commit = "7b28271a61a3da0a37f6fda399b0c4c86464e5b3", # 2018-11-16 +) diff --git a/docs/index.html b/docs/index.html index f46d709..d685f9e 100644 --- a/docs/index.html +++ b/docs/index.html @@ -70,7 +70,7 @@

Rule sets

-

debug Rules

+

debug Rules

Macros

@@ -92,7 +92,7 @@

Macros

-

Build self-contained python executables.

+

Build self-contained python executables.

Rules

diff --git a/docs/index.md b/docs/index.md index a62b873..f4f0c2b 100644 --- a/docs/index.md +++ b/docs/index.md @@ -10,7 +10,7 @@ -

debug Rules

+

debug Rules

Macros

@@ -32,7 +32,7 @@
-

Build self-contained python executables.

+

Build self-contained python executables.

Rules

diff --git a/docs/main.css b/docs/main.css index f506e12..89f66fe 100755 --- a/docs/main.css +++ b/docs/main.css @@ -1,3 +1 @@ -body{background-color:#fafafa}pre,code{font-family:'Liberation Mono', Consolas, Monaco, 'Andale Mono', monospace}pre{background-color:#eee;padding:20px;overflow-x:auto;word-wrap:normal}pre code{overflow-wrap:normal;white-space:pre}code{display:inline-block;font-size:90%;white-space:pre-wrap}.mdl-layout__drawer{background-color:#fff}.mdl-layout__drawer .mdl-layout-title{border-bottom:1px solid #e0e0e0;padding-left:24px}.drawer-nav ul{list-style:none;padding-left:0}.drawer-nav ul li{display:block;padding:0}.drawer-nav ul li ul li a{padding-left:44px;font-weight:400}.drawer-nav ul li a{display:block;flex-shrink:0;padding:15px 0 15px 22px;margin:0;font-weight:600;color:#757575;line-height:1em;text-decoration:none;cursor:pointer}.drawer-nav ul li a:active,.drawer-nav ul li a:hover{background-color:#f0f0f0}.drawer-nav ul li.active a{color:#4caf50;font-weight:500}h1.page-title{font-size:34px;font-weight:400;line-height:40px;margin-bottom:30px;color:#4caf50}p.lead{font-size:20px;line-height:32px}table{border-collapse:collapse;border-spacing:0;background-color:#fff;table-layout:auto}table thead th{background-color:#fafafa;border:1px solid #eee;color:#757575;padding:12px 12px 12px 24px;vertical-align:top}table tbody td{border:1px solid #eee;padding:12px 12px 12px 24px;vertical-align:top}table.params-table{width:100%}table.params-table col.col-param{width:25%}table.params-table col.col-description{width:75%}table.overview-table{width:100%}table.overview-table col.col-name{width:25%}table.overview-table col.col-description{width:75%}table.overview-table td p{margin:0}hr{margin-top:80px;margin-bottom:80px}nav.toc{border-left:5px solid #4caf50;padding-left:20px;margin-bottom:48px}nav.toc h1,nav.toc h2{font-size:15px;line-height:16px;padding-bottom:12px;margin-bottom:0;font-weight:400;color:#757575}nav.toc ul{list-style:none;margin-top:0;padding-left:0}nav.toc ul li{font-size:20px;line-height:40px}nav.toc ul li a{color:#4caf50}.page-content{margin-left:auto;margin-right:auto;padding-top:60px;padding-bottom:60px;width:760px}.page-content a{text-decoration:none}.page-content h1{font-size:34px;font-weight:400;line-height:40px;margin-bottom:30px;color:#4caf50}.page-content h2{font-size:24px;font-weight:400;line-height:32px;margin-bottom:30px;color:#4caf50}.page-content h3{font-size:20px;font-weight:400;line-height:32px;margin-bottom:30px;color:#4caf50}@media (max-width: 768px){.page-content{width:360px}}@media (min-width: 768px){.page-content{width:760px}}@media (min-width: 1476px){.page-content{width:1160px}}.mdl-mini-footer{padding-left:40px} - -/*# sourceMappingURL=main.css.map */ \ No newline at end of file +body{background-color:#fafafa}pre,code{font-family:"Liberation Mono",Consolas,Monaco,"Andale Mono",monospace}pre{background-color:#eee;padding:20px;overflow-x:auto;word-wrap:normal}pre code{overflow-wrap:normal;white-space:pre}code{display:inline-block;font-size:90%;white-space:pre-wrap}.mdl-layout__drawer{background-color:#fff}.mdl-layout__drawer .mdl-layout-title{border-bottom:1px solid #e0e0e0;padding-left:24px}.drawer-nav ul{list-style:none;padding-left:0}.drawer-nav ul li{display:block;padding:0}.drawer-nav ul li ul li a{padding-left:44px;font-weight:400}.drawer-nav ul li a{display:block;flex-shrink:0;padding:15px 0 15px 22px;margin:0;font-weight:600;color:#757575;line-height:1em;text-decoration:none;cursor:pointer}.drawer-nav ul li a:active,.drawer-nav ul li a:hover{background-color:#f0f0f0}.drawer-nav ul li.active a{color:#4caf50;font-weight:500}h1.page-title{font-size:34px;font-weight:400;line-height:40px;margin-bottom:30px;color:#4caf50}p.lead{font-size:20px;line-height:32px}table{border-collapse:collapse;border-spacing:0;background-color:#fff;table-layout:auto}table thead th{background-color:#fafafa;border:1px solid #eee;color:#757575;padding:12px 12px 12px 24px;vertical-align:top}table tbody td{border:1px solid #eee;padding:12px 12px 12px 24px;vertical-align:top}table.params-table{width:100%}table.params-table col.col-param{width:25%}table.params-table col.col-description{width:75%}table.overview-table{width:100%}table.overview-table col.col-name{width:25%}table.overview-table col.col-description{width:75%}table.overview-table td p{margin:0}hr{margin-top:40px;margin-bottom:40px}nav.toc{border-left:5px solid #4caf50;padding-left:20px;margin-bottom:48px}nav.toc h1,nav.toc h2{font-size:15px;line-height:16px;padding-bottom:12px;margin-bottom:0;font-weight:400;color:#757575}nav.toc ul{list-style:none;margin-top:0;padding-left:0}nav.toc ul li{font-size:20px;line-height:40px}nav.toc ul li a{color:#4caf50}.page-content{margin-left:auto;margin-right:auto;padding-top:60px;padding-bottom:60px;width:760px}.page-content a{text-decoration:none}.page-content h1{font-size:34px;font-weight:400;line-height:40px;margin-bottom:30px;color:#4caf50}.page-content h2{font-size:24px;font-weight:400;line-height:32px;margin-bottom:30px;color:#4caf50}.page-content h3{font-size:20px;font-weight:400;line-height:32px;margin-bottom:30px;color:#4caf50}@media(max-width: 768px){.page-content{width:360px}}@media(min-width: 768px){.page-content{width:760px}}@media(min-width: 1476px){.page-content{width:1160px}}.mdl-mini-footer{padding-left:40px}/*# sourceMappingURL=main.css.map */ diff --git a/docs/subpar.html b/docs/subpar.html index 1009446..d1fc5d1 100644 --- a/docs/subpar.html +++ b/docs/subpar.html @@ -77,7 +77,7 @@

Macros

par_binary

- 
par_binary(name)
+ 
par_binary(name, **kwargs)

An executable Python program.

par_binary() is a drop-in replacement for py_binary() that also @@ -106,13 +106,20 @@

Attributes

A unique name for this rule.

+ + + +
**kwargs +

Unknown; Optional

+ +

par_test

- 
par_test(name)
+ 
par_test(name, **kwargs)

An executable Python test.

Just like par_binary, but for py_test instead of py_binary. Useful if you @@ -134,13 +141,20 @@

Attributes

A unique name for this rule.

+ + **kwargs + +

Unknown; Optional

+ + +

parfile

- 
parfile(name, src, compiler, default_python_version, imports, main, zip_safe)
+ 
parfile(name, src, compiler, compiler_args, default_python_version, imports, main, zip_safe)

A self-contained, single-file Python program, with a .par file extension.

You probably want to use par_binary() instead of this.

@@ -177,6 +191,13 @@

Attributes

Internal use only.

+ + compiler_args + +

List of strings; Optional; Default is []

+ + + default_python_version @@ -218,7 +239,7 @@

Attributes

parfile_test

- 
parfile_test(name, src, compiler, default_python_version, imports, main, zip_safe)
+ 
parfile_test(name, src, compiler, compiler_args, default_python_version, imports, main, zip_safe)

Identical to par_binary, but the rule is marked as being a test.

You probably want to use par_test() instead of this.

@@ -253,6 +274,13 @@

Attributes

Internal use only.

+ + compiler_args + +

List of strings; Optional; Default is []

+ + + default_python_version diff --git a/docs/subpar.md b/docs/subpar.md index 1d71527..aaf19ab 100644 --- a/docs/subpar.md +++ b/docs/subpar.md @@ -21,7 +21,7 @@ Documentation generated by Skydoc ## par_binary 
-par_binary(name)
+par_binary(name, **kwargs)
An executable Python program. @@ -56,13 +56,20 @@ for arguments and usage.

A unique name for this rule.

+ + **kwargs + +

Unknown; Optional

+ + + ## par_test 
-par_test(name)
+par_test(name, **kwargs)
An executable Python test. @@ -88,13 +95,20 @@ specifically need to test a module's behaviour when used in a .par binary.

A unique name for this rule.

+ + **kwargs + +

Unknown; Optional

+ + + ## parfile 
-parfile(name, src, compiler, default_python_version, imports, main, zip_safe)
+parfile(name, src, compiler, compiler_args, default_python_version, imports, main, zip_safe)
A self-contained, single-file Python program, with a .par file extension. @@ -136,6 +150,13 @@ is a bug, don't use or depend on it.

Internal use only.

+ + compiler_args + +

List of strings; Optional; Default is []

+ + + default_python_version @@ -177,7 +198,7 @@ par file executes.

## parfile_test 
-parfile_test(name, src, compiler, default_python_version, imports, main, zip_safe)
+parfile_test(name, src, compiler, compiler_args, default_python_version, imports, main, zip_safe)
Identical to par_binary, but the rule is marked as being a test. @@ -216,6 +237,13 @@ You probably want to use par_test() instead of this.

Internal use only.

+ + compiler_args + +

List of strings; Optional; Default is []

+ + + default_python_version From 0356bef3fbbabec5f0e196ecfacdeb6db62d48c0 Mon Sep 17 00:00:00 2001 From: Jon Brandvein Date: Thu, 7 Mar 2019 17:26:02 -0500 Subject: [PATCH 13/19] Fixes for --all_incompatible_changes (#96) Two tiny fixes to rename `default_python_version` for a `py_binary` target and to change the syntax of a return. This does not rename the `default_python_version` attribute on the `parfile` rule. That attribute is currently marked mandatory, though it doesn't appear to be used. If we want to keep it, and if we want a gentle migration period, we'd need to introduce a new `python_version` attribute for `parfile`, make them both non-mandatory, and validate them at anaylsis time, similarly to how Bazel handled the rename of this attribute for `py_binary`/`py_test`. --- compiler/BUILD | 2 +- subpar.bzl | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/BUILD b/compiler/BUILD index 321b692..e69b9f3 100644 --- a/compiler/BUILD +++ b/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"], diff --git a/subpar.bzl b/subpar.bzl index 3a3354b..1b70b5d 100644 --- a/subpar.bzl +++ b/subpar.bzl @@ -99,7 +99,7 @@ def _parfile_impl(ctx): ) # .par file itself has no runfiles and no providers - return struct() + return [] def _prepend_workspace(path, ctx): """Given a path, prepend the workspace name as the parent directory""" From a25a2f2f9a0a491346df78e933e777d2af76ac27 Mon Sep 17 00:00:00 2001 From: Adam Liddell Date: Thu, 9 May 2019 18:00:45 +0000 Subject: [PATCH 14/19] Fix incompatibility with Python Toolchains (#99) * Make remove_if_present atomic * Fix typo * Pass import roots directly from Bazel to compiler via args * Replace default python toolchain wrappers with /usr/bin/env python[23] * Replace Skylark with Starlark * Update tests for CLI * Update CLI args for naming consistency * Fail when python interpreter is a relative path and improve comments --- compiler/cli.py | 61 ++++++++++++++++++++++++-------------- compiler/cli_test.py | 52 +++++++++++++++----------------- compiler/python_archive.py | 10 +++++-- subpar.bzl | 13 ++++---- 4 files changed, 79 insertions(+), 57 deletions(-) diff --git a/compiler/cli.py b/compiler/cli.py index 7a34b4e..d1fcb59 100644 --- a/compiler/cli.py +++ b/compiler/cli.py @@ -53,7 +53,7 @@ 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( @@ -69,7 +69,7 @@ def make_command_line_parser(): # "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', @@ -87,50 +87,67 @@ 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') 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): @@ -138,17 +155,17 @@ def main(argv): 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, diff --git a/compiler/cli_test.py b/compiler/cli_test.py index a30c067..4bdaf34 100644 --- a/compiler/cli_test.py +++ b/compiler/cli_test.py @@ -35,19 +35,27 @@ 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', + '--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', - '--outputpar=baz', + '--output_par=baz', '--stub_file=quux', '--zip_safe=False', '--interpreter=foobar', @@ -57,36 +65,26 @@ def test_make_command_line_parser_for_interprerter(self): 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: @@ -94,15 +92,13 @@ def test_stub(self): 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' """, ] diff --git a/compiler/python_archive.py b/compiler/python_archive.py index fc810c6..2c69c58 100755 --- a/compiler/python_archive.py +++ b/compiler/python_archive.py @@ -30,6 +30,7 @@ from datetime import datetime import contextlib +import errno import io import logging import os @@ -290,9 +291,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): diff --git a/subpar.bzl b/subpar.bzl index 1b70b5d..ae685bf 100644 --- a/subpar.bzl +++ b/subpar.bzl @@ -64,8 +64,7 @@ def _parfile_impl(ctx): ) # Find the list of directories to add to sys.path - # TODO(b/29227737): Use 'imports' provider from Bazel - stub_file = ctx.attr.src.files_to_run.executable.path + import_roots = ctx.attr.src[PyInfo].imports.to_list() # Inputs to the action, but don't actually get stored in the .par file extra_inputs = [ @@ -79,14 +78,18 @@ def _parfile_impl(ctx): args = ctx.attr.compiler_args + [ "--manifest_file", sources_file.path, - "--outputpar", + "--output_par", ctx.outputs.executable.path, "--stub_file", - stub_file, + ctx.attr.src.files_to_run.executable.path, "--zip_safe", str(zip_safe), - main_py_file.path, ] + for import_root in import_roots: + args.extend(['--import_root', import_root]) + args.append(main_py_file.path) + + # Run compiler ctx.actions.run( inputs = inputs + extra_inputs, tools = [ctx.attr.src.files_to_run.executable], From 9c7b3e73b2c1f2befb3a789194bb46982ea336c8 Mon Sep 17 00:00:00 2001 From: Jon Brandvein Date: Mon, 13 May 2019 10:48:53 -0400 Subject: [PATCH 15/19] Fix tests for --incompatible_use_python_toolchains (#104) This adds a hook file for run_tests.sh to write toolchain info to before each bazel invocation. This replaces the legacy way of passing an interpreter in via --python_path. It also replaces a config_setting that was used to control whether PY2 or PY3 was used, with a simple constant symbol consumed at loading time. This is needed in order to make the target aware at analysis time of which version it is building for. Fixes #98, fixes #102. --- run_tests.sh | 105 ++++++++++++++++++++++++++++++++++++--- tests/BUILD | 38 ++++++-------- tests/package_f/f_PY3.py | 2 +- toolchain_test_hook.bzl | 4 ++ 4 files changed, 117 insertions(+), 32 deletions(-) create mode 100644 toolchain_test_hook.bzl diff --git a/run_tests.sh b/run_tests.sh index 25ffac1..21a259f 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -21,6 +21,78 @@ function die { exit 1 } +# This sets up the toolchain hook to run tests for the given version of Python +# whose interpreter is located at the given absolute path. +# +# $1 may be either "PY2" or "PY3". If it is PY2, then the Python 2 runtime is +# set to the path in $2, and the Python 3 runtime is set to the default value +# given by $PYTHON3. If $1 is PY3, then $2 is the Python 3 runtime and the +# Python 2 runtime is given by $PYTHON2. +# +# The PYVER constant is also set to $1. It is consumed at loading time by +# //tests:BUILD. +# +# Note that even though tests are only run for one version of Python at a time, +# we still need to provide both runtimes in the toolchain for the sake of +# tools. In particular, the par compiler itself requires PY2, even if the par +# that we are compiling uses PY3. +function set_toolchain_hook { + pyver=$1 + if [ $pyver == "PY3"]; then + py2_path="$PYTHON2" + py3_path="$2" + else + py2_path="$2" + py3_path="$PYTHON3" + fi + + cat > toolchain_test_hook.bzl << EOF +load("@bazel_tools//tools/python:toolchain.bzl", "py_runtime_pair") + +PYVER = "$pyver" + +def define_toolchain_for_testing(): + native.py_runtime( + name = "py2_runtime", + interpreter_path = "$py2_path", + python_version = "PY2", + ) + + native.py_runtime( + name = "py3_runtime", + interpreter_path = "$py3_path", + python_version = "PY3", + ) + + py_runtime_pair( + name = "runtime_pair_for_testing", + py2_runtime = ":py2_runtime", + py3_runtime = ":py3_runtime", + visibility = ["//visibility:public"], + ) + + native.toolchain( + name = "toolchain_for_testing", + toolchain = ":runtime_pair_for_testing", + toolchain_type = "@bazel_tools//tools/python:toolchain_type", + visibility = ["//visibility:public"], + ) +EOF +} + +# Clear the toolchain hook back to its original no-op contents. +# +# If the test exits abnormally and this function isn't run, we may be left with +# a modified version of this file in our source tree. +function clear_toolchain_hook { + cat > toolchain_test_hook.bzl << EOF +PYVER = "PY3" + +def define_toolchain_for_testing(): + pass +EOF +} + # Find various tools in environment PYTHON2=$(which python||true) if [ -z "${PYTHON2}" ]; then @@ -46,9 +118,18 @@ VIRTUALENVDIR=$(dirname $0)/.env # Virtualenv `activate` needs $PS1 set PS1='$ ' -# Must have at least one Python interpreter to test -if [ -z "${PYTHON2}" -a -z "${PYTHON3}" ]; then - die "Could not find Python 2 or 3 interpreter on $PATH" +# Must have both Python interpreters to test. +if [ -z "${PYTHON2}" ]; then + die "Could not find Python 2 on $PATH" +fi +if [ -z "${PYTHON3}" ]; then + die "Could not find Python 3 on $PATH" +fi + +# Must be able to locate toolchain_test_hook.bzl. This will fail if cwd is not +# the root of the subpar workspace. +if [ ! -f "toolchain_test_hook.bzl" ]; then + die "Could not locate toolchain_test_hook.bzl (are we in the workspace root?)" fi # Run test matrix @@ -57,15 +138,19 @@ for PYTHON_INTERPRETER in "${PYTHON2}" "${PYTHON3}"; do continue; fi + BAZEL_TEST="bazel test --test_output=errors \ +--incompatible_use_python_toolchains \ +--extra_toolchains=//tests:toolchain_for_testing" if [ "${PYTHON_INTERPRETER}" = "${PYTHON3}" ]; then - BAZEL_TEST="bazel test --define subpar_test_python_version=3" + PYVER="PY3" else - BAZEL_TEST="bazel test" + PYVER="PY2" fi echo "Testing ${PYTHON_INTERPRETER}" bazel clean - ${BAZEL_TEST} --python_path="${PYTHON_INTERPRETER}" --test_output=errors //... + set_toolchain_hook "$PYVER" "$PYTHON_INTERPRETER" + ${BAZEL_TEST} //... if [ -n "${VIRTUALENV}" ]; then echo "Testing bare virtualenv" @@ -76,7 +161,8 @@ for PYTHON_INTERPRETER in "${PYTHON2}" "${PYTHON3}"; do "${VIRTUALENVDIR}" source "${VIRTUALENVDIR}"/bin/activate bazel clean - ${BAZEL_TEST} --python_path=$(which python) --test_output=errors //... + set_toolchain_hook $PYVER $(which python) + ${BAZEL_TEST} //... deactivate for REQUIREMENTS in tests/requirements-test-*.txt; do @@ -88,8 +174,11 @@ for PYTHON_INTERPRETER in "${PYTHON2}" "${PYTHON3}"; do source "${VIRTUALENVDIR}"/bin/activate pip install -r "${REQUIREMENTS}" bazel clean - ${BAZEL_TEST} --python_path=$(which python) --test_output=errors //... + set_toolchain_hook $PYVER $(which python) + ${BAZEL_TEST} //... deactivate done fi done + +clear_toolchain_hook diff --git a/tests/BUILD b/tests/BUILD index 0e72f9d..f38083b 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -11,14 +11,13 @@ exports_files([ ]) load("//:subpar.bzl", "par_binary") +load("//:toolchain_test_hook.bzl", "PYVER", "define_toolchain_for_testing") -# Configurations -config_setting( - name = "subpar_test_python_version_3", - define_values = { - "subpar_test_python_version": "3", - }, -) +# Creates the target //tests:toolchain_for_testing. +# +# This hook is used by run_tests.sh to inject a Python toolchain into the build +# during tests. When not running tests this hook is a no-op. +define_toolchain_for_testing() # Utility targets sh_library( @@ -65,14 +64,11 @@ par_binary( par_binary( name = "package_f/f", - srcs = select({ - "subpar_test_python_version_3": ["package_f/f_PY3.py"], - "//conditions:default": ["package_f/f_PY2.py"], - }), - main = select({ - "subpar_test_python_version_3": "package_f/f_PY3.py", - "//conditions:default": "package_f/f_PY2.py", - }), + # We need to use the PYVER constant defined by the test hook, rather than + # select(), because python_version is not a configurable attribute. + srcs = ["package_f/f_PY3.py"] if PYVER == "PY3" else ["package_f/f_PY2.py"], + main = "package_f/f_PY3.py" if PYVER == "PY3" else "package_f/f_PY2.py", + python_version = "PY3" if PYVER == "PY3" else "PY2" ) par_binary( @@ -151,16 +147,12 @@ par_binary( args = [ "--par", "%s.par" % path, - ] + select({ - "subpar_test_python_version_3": ["%s_PY3_filelist.txt" % path], - "//conditions:default": ["%s_PY2_filelist.txt" % path], - }), + ("%s_PY3_filelist.txt" % path) if PYVER == "PY3" else ("%s_PY2_filelist.txt" % path) + ], data = [ "%s.par" % label, - ] + select({ - "subpar_test_python_version_3": ["%s_PY3_filelist.txt" % label], - "//conditions:default": ["%s_PY2_filelist.txt" % label], - }), + ("%s_PY3_filelist.txt" % label) if PYVER == "PY3" else ("%s_PY2_filelist.txt" % label) + ], ), ) for name, label, path in [ ("basic", "//tests/package_a:a", "tests/package_a/a"), diff --git a/tests/package_f/f_PY3.py b/tests/package_f/f_PY3.py index c5ac97d..5b42e49 100644 --- a/tests/package_f/f_PY3.py +++ b/tests/package_f/f_PY3.py @@ -22,7 +22,7 @@ def main(): assert sys.version_info.major == 3, sys.version - print('In f_PY2.py main()') + print('In f_PY3.py main()') if __name__ == '__main__': diff --git a/toolchain_test_hook.bzl b/toolchain_test_hook.bzl new file mode 100644 index 0000000..c2e3d7d --- /dev/null +++ b/toolchain_test_hook.bzl @@ -0,0 +1,4 @@ +PYVER = "PY3" + +def define_toolchain_for_testing(): + pass From 5b0f501356b86f16da5aac25bc2069e25e74b67d Mon Sep 17 00:00:00 2001 From: Jon Brandvein Date: Mon, 13 May 2019 17:43:00 -0400 Subject: [PATCH 16/19] Fix test failures when running directly instead of via script (#106) Fixes #105 by 1) enabling toolchains in the workspace .bazelrc, and 2) making the test harness filter out runfiles added by the autodetecting toolchain. Previously, the tests worked when run from run_tests.sh but failed in `bazel test //...`. This caused travis to pass but buildkite to fail. Also fixed a typo that caused run_test.sh to fail on some versions of bash. --- .bazelrc | 3 +++ run_tests.sh | 2 +- tests/test_harness.sh | 6 +++++- 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 .bazelrc diff --git a/.bazelrc b/.bazelrc new file mode 100644 index 0000000..04615fa --- /dev/null +++ b/.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 diff --git a/run_tests.sh b/run_tests.sh index 21a259f..acc5410 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -38,7 +38,7 @@ function die { # that we are compiling uses PY3. function set_toolchain_hook { pyver=$1 - if [ $pyver == "PY3"]; then + if [ $pyver == "PY3" ]; then py2_path="$PYTHON2" py3_path="$2" else diff --git a/tests/test_harness.sh b/tests/test_harness.sh index 59ef338..7c4b380 100755 --- a/tests/test_harness.sh +++ b/tests/test_harness.sh @@ -40,8 +40,12 @@ TMP_EXECUTABLE="$TEST_TMPDIR"/$(basename "$EXECUTABLE") # Compare list of files in zipfile with expected list if [ "$PAR" -eq 1 ]; then + # Exclude runfiles of the autodetecting toolchain. The test is still brittle + # with respect to runfiles introduced by any other toolchain. When tests are + # invoked by run_tests.sh, a custom toolchain with no runfiles is used. diff \ - <(unzip -l -q -q "$EXECUTABLE" | awk '{print $NF}') \ + <(unzip -l -q -q "$EXECUTABLE" | awk '{print $NF}' \ + | grep -v 'bazel_tools/tools/python/py.wrapper\.sh') \ "$FILELIST" \ || die 'FATAL: zipfile contents do not match expected' fi From 35bb9f0092f71ea56b742a520602da9b3638a24f Mon Sep 17 00:00:00 2001 From: Jon Brandvein Date: Tue, 14 May 2019 15:59:21 -0400 Subject: [PATCH 17/19] Add note on bazel version compatibility to WORKSPACE (#107) This will be useful to keep up-to-date for documentation purposes, and so we know when a release cut breaks compatibility with Bazel and therefore needs to be a major release. --- WORKSPACE | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/WORKSPACE b/WORKSPACE index 09df09f..0ddafb5 100644 --- a/WORKSPACE +++ b/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", From a994ae8ba29943743a7eec470047bdf5d390768a Mon Sep 17 00:00:00 2001 From: Serge Bazanski Date: Fri, 5 Oct 2018 11:32:51 -0700 Subject: [PATCH 18/19] implement no_remove flag --- compiler/cli.py | 9 ++++++++- compiler/cli_test.py | 2 ++ compiler/python_archive.py | 6 +++++- compiler/python_archive_test.py | 2 ++ runtime/support.py | 13 +++++++------ runtime/support_test.py | 7 ++++--- subpar.bzl | 11 +++++++++++ tests/BUILD | 1 + 8 files changed, 40 insertions(+), 11 deletions(-) diff --git a/compiler/cli.py b/compiler/cli.py index d1fcb59..a721c13 100644 --- a/compiler/cli.py +++ b/compiler/cli.py @@ -92,7 +92,13 @@ def make_command_line_parser(): help='Path to add to sys.path, may be repeated to provide multiple roots.', action='append', default=[], - dest='import_roots') + 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 @@ -170,5 +176,6 @@ def main(argv): manifest_root=args.manifest_root, timestamp=args.timestamp, zip_safe=args.zip_safe, + no_remove=args.no_remove, ) par.create() diff --git a/compiler/cli_test.py b/compiler/cli_test.py index 4bdaf34..bd375e6 100644 --- a/compiler/cli_test.py +++ b/compiler/cli_test.py @@ -38,6 +38,7 @@ def test_make_command_line_parser(self): '--output_par=baz', '--stub_file=quux', '--zip_safe=False', + '--no_remove=False', '--import_root=root1', '--import_root=root2', 'foo', @@ -58,6 +59,7 @@ def test_make_command_line_parser_for_interprerter(self): '--output_par=baz', '--stub_file=quux', '--zip_safe=False', + '--no_remove=False', '--interpreter=foobar', 'foo', ]) diff --git a/compiler/python_archive.py b/compiler/python_archive.py index 2c69c58..a4a9093 100755 --- a/compiler/python_archive.py +++ b/compiler/python_archive.py @@ -48,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 """ @@ -102,6 +103,7 @@ def __init__(self, output_filename, timestamp, zip_safe, + no_remove, ): self.main_filename = main_filename @@ -114,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 @@ -172,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') diff --git a/compiler/python_archive_test.py b/compiler/python_archive_test.py index b918399..05400d7 100644 --- a/compiler/python_archive_test.py +++ b/compiler/python_archive_test.py @@ -51,6 +51,7 @@ def setUp(self): 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( @@ -62,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): diff --git a/runtime/support.py b/runtime/support.py index e8ec253..b5bcb84 100644 --- a/runtime/support.py +++ b/runtime/support.py @@ -95,7 +95,7 @@ def _find_archive(): return archive_path -def _extract_files(archive_path): +def _extract_files(archive_path, no_remove): """Extract the contents of this .par file to disk. This creates a temporary directory, and registers an atexit @@ -107,9 +107,10 @@ def _extract_files(archive_path): """ extract_dir = tempfile.mkdtemp() - def _extract_files_cleanup(): - shutil.rmtree(extract_dir, ignore_errors=True) - atexit.register(_extract_files_cleanup) + if not no_remove: + 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') @@ -282,7 +283,7 @@ def _initialize_import_path(import_roots, import_prefix): _log('# adding %s to sys.path' % full_roots) -def setup(import_roots, zip_safe): +def setup(import_roots, zip_safe, no_remove): """Initialize subpar run-time support Args: @@ -309,7 +310,7 @@ def setup(import_roots, zip_safe): # Extract files to disk if necessary if not zip_safe: - extract_dir = _extract_files(archive_path) + extract_dir = _extract_files(archive_path, no_remove) # 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. diff --git a/runtime/support_test.py b/runtime/support_test.py index 565adb1..8dda700 100644 --- a/runtime/support_test.py +++ b/runtime/support_test.py @@ -101,7 +101,7 @@ def test__find_archive(self): def test__extract_files(self): # Extract zipfile - extract_path = support._extract_files(self.zipfile_name) + extract_path = support._extract_files(self.zipfile_name, False) # Check results self.assertTrue(os.path.isdir(extract_path)) @@ -140,7 +140,7 @@ def test_setup(self): mock_sys_path[0] = self.zipfile_name sys.path = mock_sys_path success = support.setup(import_roots=['some_root', 'another_root'], - zip_safe=True) + zip_safe=True, no_remove=False) self.assertTrue(success) finally: sys.path = old_sys_path @@ -165,7 +165,8 @@ def test_setup__extract(self): 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) + success = support.setup(import_roots=['some_root'], zip_safe=False, + no_remove=False) self.assertTrue(success) finally: sys.path = old_sys_path diff --git a/subpar.bzl b/subpar.bzl index ae685bf..11e3b18 100644 --- a/subpar.bzl +++ b/subpar.bzl @@ -73,6 +73,7 @@ def _parfile_impl(ctx): ] zip_safe = ctx.attr.zip_safe + no_remove = ctx.attr.no_remove # Assemble command line for .par compiler args = ctx.attr.compiler_args + [ @@ -84,6 +85,8 @@ def _parfile_impl(ctx): ctx.attr.src.files_to_run.executable.path, "--zip_safe", str(zip_safe), + '--no_remove', + str(no_remove), ] for import_root in import_roots: args.extend(['--import_root', import_root]) @@ -136,6 +139,7 @@ parfile_attrs = { ), "compiler_args": attr.string_list(default = []), "zip_safe": attr.bool(default = True), + "no_remove": attr.bool(default = False), } # Rule to create a parfile given a py_binary() as input @@ -171,6 +175,9 @@ Args: extracted to a temporary directory on disk each time the par file executes. + no_remove: Whether to keep the extracted temporary directory after the + program finishes, if zip_safe is enabled. + TODO(b/27502830): A directory foo.par.runfiles is also created. This is a bug, don't use or depend on it. """ @@ -204,6 +211,7 @@ def par_binary(name, **kwargs): compiler = kwargs.pop("compiler", None) compiler_args = kwargs.pop("compiler_args", []) zip_safe = kwargs.pop("zip_safe", True) + no_remove = kwargs.pop('no_remove', False) native.py_binary(name = name, **kwargs) main = kwargs.get("main", name + ".py") @@ -222,6 +230,7 @@ def par_binary(name, **kwargs): testonly = testonly, visibility = visibility, zip_safe = zip_safe, + no_remove=no_remove, ) def par_test(name, **kwargs): @@ -232,6 +241,7 @@ def par_test(name, **kwargs): """ compiler = kwargs.pop("compiler", None) zip_safe = kwargs.pop("zip_safe", True) + no_remove = kwargs.pop('no_remove', False) native.py_test(name = name, **kwargs) main = kwargs.get("main", name + ".py") @@ -249,4 +259,5 @@ def par_test(name, **kwargs): testonly = testonly, visibility = visibility, zip_safe = zip_safe, + no_remove=no_remove, ) diff --git a/tests/BUILD b/tests/BUILD index f38083b..00030fd 100644 --- a/tests/BUILD +++ b/tests/BUILD @@ -90,6 +90,7 @@ par_binary( main = "package_extract/extract.py", srcs_version = "PY2AND3", zip_safe = False, + no_remove = False, ) par_binary( From 5dd9fb4586616c69df9b3f5aba12f08f85d708d1 Mon Sep 17 00:00:00 2001 From: Sergiusz Bazanski Date: Tue, 16 Jul 2019 13:45:05 +0200 Subject: [PATCH 19/19] docs: bump --- docs/subpar.html | 20 ++++++++++++++++++-- docs/subpar.md | 20 ++++++++++++++++++-- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/docs/subpar.html b/docs/subpar.html index d1fc5d1..d510172 100644 --- a/docs/subpar.html +++ b/docs/subpar.html @@ -154,7 +154,7 @@

Attributes

parfile

- 
parfile(name, src, compiler, compiler_args, default_python_version, imports, main, zip_safe)
+ 
parfile(name, src, compiler, compiler_args, default_python_version, imports, main, no_remove, zip_safe)

A self-contained, single-file Python program, with a .par file extension.

You probably want to use par_binary() instead of this.

@@ -223,6 +223,14 @@

Attributes

See py_binary.main

+ + no_remove + +

Boolean; Optional; Default is False

+

Whether to keep the extracted temporary directory after the +program finishes, if zip_safe is enabled.

+ + zip_safe @@ -239,7 +247,7 @@

Attributes

parfile_test

- 
parfile_test(name, src, compiler, compiler_args, default_python_version, imports, main, zip_safe)
+ 
parfile_test(name, src, compiler, compiler_args, default_python_version, imports, main, no_remove, zip_safe)

Identical to par_binary, but the rule is marked as being a test.

You probably want to use par_test() instead of this.

@@ -306,6 +314,14 @@

Attributes

See py_binary.main

+ + no_remove + +

Boolean; Optional; Default is False

+

Whether to keep the extracted temporary directory after the +program finishes, if zip_safe is enabled.

+ + zip_safe diff --git a/docs/subpar.md b/docs/subpar.md index aaf19ab..ddde9df 100644 --- a/docs/subpar.md +++ b/docs/subpar.md @@ -108,7 +108,7 @@ specifically need to test a module's behaviour when used in a .par binary. ## parfile 
-parfile(name, src, compiler, compiler_args, default_python_version, imports, main, zip_safe)
+parfile(name, src, compiler, compiler_args, default_python_version, imports, main, no_remove, zip_safe)
A self-contained, single-file Python program, with a .par file extension. @@ -182,6 +182,14 @@ the application.

See py_binary.main

+ + no_remove + +

Boolean; Optional; Default is False

+

Whether to keep the extracted temporary directory after the +program finishes, if zip_safe is enabled.

+ + zip_safe @@ -198,7 +206,7 @@ par file executes.

## parfile_test 
-parfile_test(name, src, compiler, compiler_args, default_python_version, imports, main, zip_safe)
+parfile_test(name, src, compiler, compiler_args, default_python_version, imports, main, no_remove, zip_safe)
Identical to par_binary, but the rule is marked as being a test. @@ -269,6 +277,14 @@ the application.

See py_binary.main

+ + no_remove + +

Boolean; Optional; Default is False

+

Whether to keep the extracted temporary directory after the +program finishes, if zip_safe is enabled.

+ + zip_safe