From 829e41976f6397eb0985bf11bd3e2edae75b1bb9 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Thu, 11 Jul 2019 01:37:36 -0400 Subject: [PATCH 1/4] Ensure we always respect --site-packages - Sometimes we fail to respect `--site-packages` when it is passed with `--install` - This resolves that and ensures we always pass it to `ensure_project` - Fixes #3718 Signed-off-by: Dan Ryan --- news/3718.bugfix.rst | 1 + pipenv/cli/command.py | 16 ++++--- pipenv/core.py | 2 + .../requirementslib/models/setup_info.py | 48 +++++++++++++++++-- 4 files changed, 57 insertions(+), 10 deletions(-) create mode 100644 news/3718.bugfix.rst diff --git a/news/3718.bugfix.rst b/news/3718.bugfix.rst new file mode 100644 index 00000000..7a90ea50 --- /dev/null +++ b/news/3718.bugfix.rst @@ -0,0 +1 @@ +Fixed a bug which sometimes caused pipenv to fail to respect the ``--site-packages`` flag when passed with ``pipenv install``. diff --git a/pipenv/cli/command.py b/pipenv/cli/command.py index d1bcaa93..6d89f15a 100644 --- a/pipenv/cli/command.py +++ b/pipenv/cli/command.py @@ -18,7 +18,7 @@ from .options import ( general_options, install_options, lock_options, pass_state, pypi_mirror_option, python_option, requirementstxt_option, skip_lock_option, sync_options, system_option, three_option, - uninstall_options, verbose_option + uninstall_options, verbose_option, site_packages_option ) @@ -217,6 +217,7 @@ def cli( @system_option @code_option @deploy_option +@site_packages_option @skip_lock_option @install_options @pass_state @@ -249,6 +250,7 @@ def install( extra_index_url=state.extra_index_urls, packages=state.installstate.packages, editable_packages=state.installstate.editables, + site_packages=state.site_packages ) if retcode: ctx.abort() @@ -310,11 +312,10 @@ def lock( ): """Generates Pipfile.lock.""" from ..core import ensure_project, do_init, do_lock - # Ensure that virtualenv is available. ensure_project( three=state.three, python=state.python, pypi_mirror=state.pypi_mirror, - warn=(not state.quiet) + warn=(not state.quiet), site_packages=state.site_packages, clear=state.clear ) if state.installstate.requirementstxt: do_init( @@ -475,8 +476,10 @@ def update( do_sync, project, ) - - ensure_project(three=state.three, python=state.python, warn=True, pypi_mirror=state.pypi_mirror) + ensure_project( + three=state.three, python=state.python, pypi_mirror=state.pypi_mirror, + warn=(not state.quiet), site_packages=state.site_packages, clear=state.clear + ) if not outdated: outdated = bool(dry_run) if outdated: @@ -505,12 +508,13 @@ def update( err=True, ) ctx.abort() - do_lock( + ctx=ctx, clear=state.clear, pre=state.installstate.pre, keep_outdated=state.installstate.keep_outdated, pypi_mirror=state.pypi_mirror, + write=not state.quiet, ) do_sync( ctx=ctx, diff --git a/pipenv/core.py b/pipenv/core.py index cf427728..266a83c2 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -1861,6 +1861,7 @@ def do_install( deploy=False, keep_outdated=False, selective_upgrade=False, + site_packages=False, ): from .environments import PIPENV_VIRTUALENV, PIPENV_USE_SYSTEM from .vendor.pip_shims.shims import PipError @@ -1888,6 +1889,7 @@ def do_install( deploy=deploy, skip_requirements=skip_requirements, pypi_mirror=pypi_mirror, + site_packages=site_packages, ) # Don't attempt to install develop and default packages if Pipfile is missing if not project.pipfile_exists and not (package_args or dev) and not code: diff --git a/pipenv/vendor/requirementslib/models/setup_info.py b/pipenv/vendor/requirementslib/models/setup_info.py index 454cbfcc..551f3e87 100644 --- a/pipenv/vendor/requirementslib/models/setup_info.py +++ b/pipenv/vendor/requirementslib/models/setup_info.py @@ -195,7 +195,7 @@ def make_base_requirements(reqs): requirements.add(req) elif isinstance(req, pkg_resources_requirements.Requirement): requirements.add(BaseRequirement.from_req(req)) - elif req and not req.startswith("#"): + elif req and isinstance(req, six.string_types) and not req.startswith("#"): requirements.add(BaseRequirement.from_string(req)) return requirements @@ -240,10 +240,15 @@ def get_package_dir_from_setupcfg(parser, base_dir=None): if "package_dir" in setup_py: package_lookup = setup_py["package_dir"] if not isinstance(package_lookup, Mapping): - return package_lookup - return package_lookup.get( + package_dir = package_lookup + package_dir = package_lookup.get( next(iter(list(package_lookup.keys()))), package_dir ) + if not os.path.isabs(package_dir): + if not base_dir: + package_dir = os.path.join(os.path.getcwd(), package_dir) + else: + package_dir = os.path.join(base_dir, package_dir) return package_dir @@ -296,7 +301,8 @@ def parse_setup_cfg(setup_cfg_path): parser = configparser.ConfigParser(default_opts) parser.read(setup_cfg_path) results = {} - package_dir = get_package_dir_from_setupcfg(parser, base_dir=os.getcwd()) + base_dir = os.path.dirname(os.path.abspath(setup_cfg_path)) + package_dir = get_package_dir_from_setupcfg(parser, base_dir=base_dir) name, version = get_name_and_version_from_setupcfg(parser, package_dir) results["name"] = name results["version"] = version @@ -638,6 +644,8 @@ class Analyzer(ast.NodeVisitor): self.functions = [] self.strings = [] self.assignments = {} + self.binOps = [] + self.binOps_map = {} super(Analyzer, self).__init__() def generic_visit(self, node): @@ -652,6 +660,17 @@ class Analyzer(ast.NodeVisitor): self.assignments.update(ast_unparse(node, initial_mapping=True)) super(Analyzer, self).generic_visit(node) + def visit_BinOp(self, node): + left = ast_unparse(node.left, initial_mapping=True) + right = ast_unparse(node.right, initial_mapping=True) + node.left = left + node.right = right + self.binOps.append(node) + + def unmap_binops(self): + for binop in self.binOps: + self.binOps_map[binop] = ast_unparse(binop, analyzer=self) + def match_assignment_str(self, match): return next( iter(k for k in self.assignments if getattr(k, "id", "") == match), None @@ -678,6 +697,26 @@ def ast_unparse(item, initial_mapping=False, analyzer=None, recurse=True): # no unparsed = item.s elif isinstance(item, ast.Subscript): unparsed = unparse(item.value) + elif isinstance(item, ast.BinOp): + if analyzer and item in analyzer.binOps_map: + unparsed = analyzer.binOps_map[item] + elif isinstance(item.op, ast.Add): + if not initial_mapping: + right_item = unparse(item.right) + left_item = unparse(item.left) + if not all( + isinstance(side, (six.string_types, int, float, list, tuple)) + for side in (left_item, right_item) + ): + item.left = left_item + item.right = right_item + unparsed = item + else: + unparsed = right_item + left_item + else: + unparsed = item + elif isinstance(item.op, ast.Sub): + unparsed = unparse(item.left) - unparse(item.right) elif isinstance(item, ast.Name): if not initial_mapping: unparsed = item.id @@ -787,6 +826,7 @@ def ast_parse_setup_py(path): # type: (S) -> Dict[Any, Any] ast_analyzer = ast_parse_file(path) setup = {} # type: Dict[Any, Any] + ast_analyzer.unmap_binops() for k, v in ast_analyzer.function_map.items(): fn_name = "" if isinstance(k, ast.Name): From 4f33dc8928afb32aaf534a02515c8e90485d1177 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Sat, 13 Jul 2019 13:31:55 -0400 Subject: [PATCH 2/4] Fix modifications to site packages argument Signed-off-by: Dan Ryan --- pipenv/cli/command.py | 2 +- pipenv/cli/options.py | 8 ++++---- pipenv/core.py | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pipenv/cli/command.py b/pipenv/cli/command.py index 6d89f15a..9bc6fe34 100644 --- a/pipenv/cli/command.py +++ b/pipenv/cli/command.py @@ -70,7 +70,7 @@ def cli( man=False, support=None, help=False, - site_packages=False, + site_packages=None, **kwargs ): # Handle this ASAP to make shell startup fast. diff --git a/pipenv/cli/options.py b/pipenv/cli/options.py index 56b0827e..b2183fec 100644 --- a/pipenv/cli/options.py +++ b/pipenv/cli/options.py @@ -61,7 +61,7 @@ class State(object): self.python = None self.two = None self.three = None - self.site_packages = False + self.site_packages = None self.clear = False self.system = False self.installstate = InstallState() @@ -263,9 +263,9 @@ def site_packages_option(f): state = ctx.ensure_object(State) state.site_packages = value return value - return option("--site-packages", is_flag=True, default=False, type=click.types.BOOL, - help="Enable site-packages for the virtualenv.", callback=callback, - expose_value=False, show_envvar=True)(f) + return option("--site-packages/--no-site-packages", is_flag=True, default=None, + type=click.types.BOOL, help="Enable site-packages for the virtualenv.", + callback=callback, expose_value=False, show_envvar=True)(f) def clear_option(f): diff --git a/pipenv/core.py b/pipenv/core.py index 266a83c2..b7845892 100644 --- a/pipenv/core.py +++ b/pipenv/core.py @@ -466,7 +466,7 @@ def ensure_python(three=None, python=None): return path_to_python -def ensure_virtualenv(three=None, python=None, site_packages=False, pypi_mirror=None): +def ensure_virtualenv(three=None, python=None, site_packages=None, pypi_mirror=None): """Creates a virtualenv, if one doesn't exist.""" from .environments import PIPENV_USE_SYSTEM @@ -500,7 +500,7 @@ def ensure_virtualenv(three=None, python=None, site_packages=False, pypi_mirror= cleanup_virtualenv(bare=False) sys.exit(1) # If --three, --two, or --python were passed… - elif (python) or (three is not None) or (site_packages is not False): + elif (python) or (three is not None) or (site_packages is not None): USING_DEFAULT_PYTHON = False # Ensure python is installed before deleting existing virtual env python = ensure_python(three=three, python=python) @@ -536,7 +536,7 @@ def ensure_project( validate=True, system=False, warn=True, - site_packages=False, + site_packages=None, deploy=False, skip_requirements=False, pypi_mirror=None, @@ -891,7 +891,7 @@ def convert_three_to_python(three, python): return python -def do_create_virtualenv(python=None, site_packages=False, pypi_mirror=None): +def do_create_virtualenv(python=None, site_packages=None, pypi_mirror=None): """Creates a virtualenv.""" click.echo( @@ -1861,7 +1861,7 @@ def do_install( deploy=False, keep_outdated=False, selective_upgrade=False, - site_packages=False, + site_packages=None, ): from .environments import PIPENV_VIRTUALENV, PIPENV_USE_SYSTEM from .vendor.pip_shims.shims import PipError From 9d913476d9364665ff9b3fe62da9ad21a43e1bf1 Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Sat, 13 Jul 2019 13:40:55 -0400 Subject: [PATCH 3/4] Remove click type and add validation for site packages Signed-off-by: Dan Ryan --- pipenv/cli/options.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/pipenv/cli/options.py b/pipenv/cli/options.py index b2183fec..fc45256f 100644 --- a/pipenv/cli/options.py +++ b/pipenv/cli/options.py @@ -261,11 +261,12 @@ def quiet_option(f): def site_packages_option(f): def callback(ctx, param, value): state = ctx.ensure_object(State) + validate_bool_or_none(ctx, param, value) state.site_packages = value return value return option("--site-packages/--no-site-packages", is_flag=True, default=None, - type=click.types.BOOL, help="Enable site-packages for the virtualenv.", - callback=callback, expose_value=False, show_envvar=True)(f) + help="Enable site-packages for the virtualenv.", callback=callback, + expose_value=False, show_envvar=True)(f) def clear_option(f): @@ -356,6 +357,12 @@ def validate_python_path(ctx, param, value): return value +def validate_bool_or_none(ctx, param, value): + if value is not None: + return click.types.BOOL(value) + return False + + def validate_pypi_mirror(ctx, param, value): if value and not is_valid_url(value): raise BadParameter("Invalid PyPI mirror URL: %s" % value) From 5ffa946160fdf2545a77fe137d5222629dd907ab Mon Sep 17 00:00:00 2001 From: Dan Ryan Date: Sun, 14 Jul 2019 01:28:49 -0400 Subject: [PATCH 4/4] Don't pass clear along to ensure_project during lock Signed-off-by: Dan Ryan --- pipenv/cli/command.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pipenv/cli/command.py b/pipenv/cli/command.py index 9bc6fe34..7d1d8ee7 100644 --- a/pipenv/cli/command.py +++ b/pipenv/cli/command.py @@ -313,9 +313,11 @@ def lock( """Generates Pipfile.lock.""" from ..core import ensure_project, do_init, do_lock # Ensure that virtualenv is available. + # Note that we don't pass clear on to ensure_project as it is also + # handled in do_lock ensure_project( three=state.three, python=state.python, pypi_mirror=state.pypi_mirror, - warn=(not state.quiet), site_packages=state.site_packages, clear=state.clear + warn=(not state.quiet), site_packages=state.site_packages, ) if state.installstate.requirementstxt: do_init(