From 70d718c4f0d4ccd9a2b7f5369b2cc124b042c8e6 Mon Sep 17 00:00:00 2001 From: John Sirois Date: Tue, 22 Jan 2019 09:52:04 -0800 Subject: [PATCH] Make tox -evendor idempotent. This affords adding a CI check to ensure we never commit vendored code with modifications beyond import re-writes. Fixes #649 Fixes #650 --- .travis.yml | 5 +++ pex/environment.py | 4 +-- pex/vendor/__main__.py | 36 +++++++++++++++++--- pex/vendor/_vendored/wheel/wheel/metadata.py | 2 +- tox.ini | 8 +++++ 5 files changed, 47 insertions(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index e8a5490cd..1168db6cb 100644 --- a/.travis.yml +++ b/.travis.yml @@ -99,6 +99,11 @@ matrix: env: *x-py27 script: tox -ve isort-check + - <<: *x-linux-shard + name: TOXENV=vendor-check + env: *x-py27 + script: tox -ve vendor-check + - <<: *x-linux-shard name: TOXENV=py27 env: *x-py27 diff --git a/pex/environment.py b/pex/environment.py index e16125482..bde20e602 100644 --- a/pex/environment.py +++ b/pex/environment.py @@ -32,12 +32,12 @@ def _import_pkg_resources(): try: - import pkg_resources + import pkg_resources # vendor:skip return pkg_resources, False except ImportError: from pex import third_party third_party.install(expose=['setuptools']) - import pkg_resources + import pkg_resources # vendor:skip return pkg_resources, True diff --git a/pex/vendor/__main__.py b/pex/vendor/__main__.py index aff93b50c..d0fdac893 100644 --- a/pex/vendor/__main__.py +++ b/pex/vendor/__main__.py @@ -5,15 +5,17 @@ import os import pkgutil +import shutil import subprocess import sys +import tempfile from collections import OrderedDict from colors import bold, green, yellow -from redbaron import LiteralyEvaluable, NameNode, RedBaron +from redbaron import CommentNode, LiteralyEvaluable, NameNode, RedBaron from pex import third_party -from pex.common import safe_delete, safe_rmtree +from pex.common import safe_delete from pex.vendor import iter_vendor_specs @@ -32,6 +34,14 @@ def _parse(python_file): # losing formatting. See: https://github.com/PyCQA/redbaron return RedBaron(fp.read()) + @staticmethod + def _skip(node): + next_node = node.next_recursive + if isinstance(next_node, CommentNode) and next_node.value.strip() == '# vendor:skip': + print('Skipping {} as directed by {}'.format(node, next_node)) + return True + return False + @staticmethod def _find_literal_node(statement, call_argument): # The list of identifiers is large and they represent disjoint types: @@ -78,6 +88,9 @@ def rewrite(self, python_file): def _modify__import__calls(self, red_baron): # noqa: We want __import__ as part of the name. for call_node in red_baron.find_all('CallNode'): if call_node.previous and call_node.previous.value == '__import__': + if self._skip(call_node): + continue + parent = call_node.parent_find('AtomtrailersNode') original = parent.copy() first_argument = call_node[0] @@ -91,6 +104,9 @@ def _modify__import__calls(self, red_baron): # noqa: We want __import__ as part def _modify_import_statements(self, red_baron): for import_node in red_baron.find_all('ImportNode'): + if self._skip(import_node): + continue + original = import_node.copy() for index, import_module in enumerate(import_node): root_package = import_module[0] @@ -134,6 +150,9 @@ def prefixed_fullname(): def _modify_from_import_statements(self, red_baron): for from_import_node in red_baron.find_all('FromImportNode'): + if self._skip(from_import_node): + continue + if len(from_import_node) == 0: # NB: `from . import ...` has length 0, but we don't care about relative imports which will # point back into vendored code if the origin is within vendored code. @@ -153,15 +172,22 @@ class VendorizeError(Exception): def vendorize(root_dir, vendor_specs, prefix): for vendor_spec in vendor_specs: - cmd = ['pip', 'install', '--upgrade', '--no-compile', '--target', vendor_spec.target_dir, + script_dev_null = tempfile.mkdtemp() + cmd = ['pip', + 'install', + '--upgrade', + '--no-compile', + '--target', vendor_spec.target_dir, + '--install-option', '--install-scripts={}'.format(script_dev_null), vendor_spec.requirement] result = subprocess.call(cmd) + shutil.rmtree(script_dev_null) if result != 0: raise VendorizeError('Failed to vendor {!r}'.format(vendor_spec)) - # We know we can get these as a by-product of a pip install but never need them. - safe_rmtree(os.path.join(vendor_spec.target_dir, 'bin')) + # We know we can get this as a by-product of a pip install but never need it. safe_delete(os.path.join(vendor_spec.target_dir, 'easy_install.py')) + vendor_spec.create_packages() vendored_path = [vendor_spec.target_dir for vendor_spec in vendor_specs] diff --git a/pex/vendor/_vendored/wheel/wheel/metadata.py b/pex/vendor/_vendored/wheel/wheel/metadata.py index 7afdf7876..eca49bc09 100644 --- a/pex/vendor/_vendored/wheel/wheel/metadata.py +++ b/pex/vendor/_vendored/wheel/wheel/metadata.py @@ -13,7 +13,7 @@ # Wheel itself is probably the only program that uses non-extras markers # in METADATA/PKG-INFO. Support its syntax with the extra at the end only. -EXTRA_RE = re.compile(r"^(?P.*?)(;\s*(?P.*?)(extra == '(?P.*?)')?)$") +EXTRA_RE = re.compile("""^(?P.*?)(;\s*(?P.*?)(extra == '(?P.*?)')?)$""") MayRequiresKey = namedtuple('MayRequiresKey', ('condition', 'extra')) diff --git a/tox.ini b/tox.ini index 9d71d7d86..bcc33cc9a 100644 --- a/tox.ini +++ b/tox.ini @@ -25,6 +25,7 @@ deps = whitelist_externals = open bash + git [testenv:integration-tests] deps = @@ -146,6 +147,13 @@ commands = python -m pex.vendor {[testenv:isort-run]commands} +[testenv:vendor-check] +deps = + tox +commands = + tox -e vendor + git diff --quiet + [testenv:docs] changedir = docs deps =