Fix python path discovery if not called python

- Begin a refactor of `delegator.run` invocation to ensure we capture
  and handle failures with our own exception wrappers
- Additoinally capture output and error logging and command information
  when running in verbose mode (should avoid significant repitition in
  the codebase)
- Refactor `which` and `system_which` to fallback to pythonfinder's
implementation
- Abstract `is_python_command` to identify whether we are looking for
  python, this enables us to rely on `pythonfinder.Finder.find_all_python_versions()`
  to ensure we aren't skipping python versions
- Fixes #2783

Signed-off-by: Dan Ryan <[email protected]>

news/2783.bugfix.rst

@@ -0,0 +1,2 @@
+Fixed an issue which caused errors due to reliance on the system utilities ``which`` and ``where`` which may not always exist on some systems.
+- Fixed a bug which caused periodic failures in python discovery when executables named ``python`` were not present on the target ``$PATH``.

pipenv/cmdparse.py

@@ -10,7 +10,7 @@ class ScriptEmptyError(ValueError):
 
 
 def _quote_if_contains(value, pattern):
- if next(re.finditer(pattern, value), None):
+ if next(iter(re.finditer(pattern, value)), None):
  return '"{0}"'.format(re.sub(r'(\\*)"', r'\1\1\\"', value))
  return value
 

pipenv/core.py

@@ -34,7 +34,8 @@
  escape_cmd, escape_grouped_arguments, find_windows_executable,
  get_canonical_names, is_pinned, is_pypi_url, is_required_version, is_star,
  is_valid_url, parse_indexes, pep423_name, prepare_pip_source_args,
- proper_case, python_version, venv_resolve_deps
+ proper_case, python_version, venv_resolve_deps, run_command,
+ is_python_command, find_python
 )
 
 
@@ -86,16 +87,19 @@ def which(command, location=None, allow_global=False):
  location = os.environ.get("VIRTUAL_ENV", None)
  if not (location and os.path.exists(location)) and not allow_global:
  raise RuntimeError("location not created nor specified")
+
+ version_str = "python{0}".format(".".join([str(v) for v in sys.version_info[:2]]))
+ is_python = command in ("python", os.path.basename(sys.executable), version_str)
  if not allow_global:
  if os.name == "nt":
  p = find_windows_executable(os.path.join(location, "Scripts"), command)
  else:
  p = os.path.join(location, "bin", command)
  else:
- if command == "python":
+ if is_python:
  p = sys.executable
  if not os.path.exists(p):
- if command == "python":
+ if is_python:
  p = sys.executable or system_which("python")
  else:
  p = system_which(command)
@@ -323,26 +327,18 @@ def find_a_system_python(line):
  * Search for "python" and "pythonX.Y" executables in PATH to find a match.
  * Nothing fits, return None.
  """
- if not line:
- return None
+
  if os.path.isabs(line):
  return line
  from .vendor.pythonfinder import Finder
-
  finder = Finder(system=False, global_search=True)
+ if not line:
+ return next(iter(finder.find_all_python_versions()), None)
+ # Use the windows finder executable
  if (line.startswith("py ") or line.startswith("py.exe ")) and os.name == "nt":
  line = line.split(" ", 1)[1].lstrip("-")
- elif line.startswith("py"):
- python_entry = finder.which(line)
- if python_entry:
- return python_entry.path.as_posix()
- return None
- python_entry = finder.find_python_version(line)
- if not python_entry:
- python_entry = finder.which("python{0}".format(line))
- if python_entry:
- return python_entry.path.as_posix()
- return None
+ python_entry = find_python(finder, line)
+ return python_entry
 
 
 def ensure_python(three=None, python=None):
@@ -1433,8 +1429,8 @@ def pip_install(
  if "--hash" not in f.read():
  ignore_hashes = True
  else:
- ignore_hashes = True
- install_reqs = requirement.as_line(as_list=True, include_hashes=not ignore_hashes)
+ ignore_hashes = True if not requirement.hashes else False
+ install_reqs = requirement.as_line(as_list=True)
  if not requirement.markers:
  install_reqs = [escape_cmd(r) for r in install_reqs]
  elif len(install_reqs) > 1:
@@ -1507,18 +1503,59 @@ def pip_download(package_name):
  return c
 
 
+def fallback_which(command, location=None, allow_global=False, system=False):
+ """
+ A fallback implementation of the `which` utility command that relies exclusively on
+ searching the path for commands.
+
+ :param str command: The command to search for, optional
+ :param str location: The search location to prioritize (prepend to path), defaults to None
+ :param bool allow_global: Whether to search the global path, defaults to False
+ :param bool system: Whether to use the system python instead of pipenv's python, defaults to False
+ :raises ValueError: Raised if no command is provided
+ :raises TypeError: Raised if the command provided is not a string
+ :return: A path to the discovered command location
+ :rtype: str
+ """
+
+ from .vendor.pythonfinder import Finder
+ if not command:
+ raise ValueError("fallback_which: Must provide a command to search for...")
+ if not isinstance(command, six.string_types):
+ raise TypeError("Provided command must be a string, received {0!r}".format(command))
+ global_search = system or allow_global
+ finder = Finder(system=False, global_search=global_search, path=location)
+ if is_python_command(command):
+ result = find_python(finder, command)
+ if result:
+ return result
+ result = finder.which(command)
+ if result:
+ return result.path.as_posix()
+ return ""
+
+
 def which_pip(allow_global=False):
  """Returns the location of virtualenv-installed pip."""
+
+ location = None
+ if "VIRTUAL_ENV" in os.environ:
+ location = os.environ["VIRTUAL_ENV"]
  if allow_global:
- if "VIRTUAL_ENV" in os.environ:
- return which("pip", location=os.environ["VIRTUAL_ENV"])
+ if location:
+ pip = which("pip", location=location)
+ if pip:
+ return pip
 
  for p in ("pip", "pip3", "pip2"):
  where = system_which(p)
  if where:
  return where
 
- return which("pip")
+ pip = which("pip")
+ if not pip:
+ pip = fallback_which("pip", allow_global=allow_global, location=location)
+ return pip
 
 
 def system_which(command, mult=False):
@@ -1528,6 +1565,7 @@ def system_which(command, mult=False):
  vistir.compat.fs_str(k): vistir.compat.fs_str(val)
  for k, val in os.environ.items()
  }
+ result = None
  try:
  c = delegator.run("{0} {1}".format(_which, command))
  try:
@@ -1542,21 +1580,20 @@ def system_which(command, mult=False):
  )
  assert c.return_code == 0
  except AssertionError:
- return None if not mult else []
+ result = fallback_which(command, allow_global=True)
  except TypeError:
- from .vendor.pythonfinder import Finder
- finder = Finder()
- result = finder.which(command)
- if result:
- return result.path.as_posix()
- return
+ if not result:
+ result = fallback_which(command, allow_global=True)
  else:
- result = c.out.strip() or c.err.strip()
- if mult:
- return result.split("\n")
+ if not result:
+ result = next(iter([c.out, c.err]), "").split("\n")
+ result = next(iter(result)) if not mult else result
+ return result
+ if not result:
+ result = fallback_which(command, allow_global=True)
+ result = [result] if mult else result
+ return result
 
- else:
- return result.split("\n")[0]
 
 
 def format_help(help):
@@ -2173,6 +2210,7 @@ def do_uninstall(
  p for normalized, p in selected_pkg_map.items()
  if normalized in (used_packages - bad_pkgs)
  ]
+ pip_path = None
  for normalized, package_name in selected_pkg_map.items():
  click.echo(
  crayons.white(
@@ -2182,12 +2220,10 @@ def do_uninstall(
  # Uninstall the package.
  if package_name in packages_to_remove:
  with project.environment.activated():
- cmd = "{0} uninstall {1} -y".format(
- escape_grouped_arguments(which_pip(allow_global=system)), package_name,
- )
- if environments.is_verbose():
- click.echo("$ {0}".format(cmd))
- c = delegator.run(cmd)
+ if pip_path is None:
+ pip_path = which_pip(allow_global=system)
+ cmd = [pip_path, "uninstall", package_name, "-y"]
+ c = run_command(cmd)
  click.echo(crayons.blue(c.out))
  if c.return_code != 0:
  failure = True
@@ -2441,6 +2477,7 @@ def do_check(
  args=None,
  pypi_mirror=None,
 ):
+ from pipenv.vendor.vistir.compat import JSONDecodeError
  if not system:
  # Ensure that virtualenv is available.
  ensure_project(
@@ -2471,18 +2508,27 @@ def do_check(
  sys.exit(1)
  else:
  sys.exit(0)
- click.echo(crayons.normal(fix_utf8("Checking PEP 508 requirements…"), bold=True))
- if system:
- python = system_which("python")
- else:
+ click.echo(crayons.normal(decode_for_output("Checking PEP 508 requirements…"), bold=True))
+ pep508checker_path = pep508checker.__file__.rstrip("cdo")
+ safety_path = os.path.join(
+ os.path.dirname(os.path.abspath(__file__)), "patched", "safety.zip"
+ )
+ if not system:
  python = which("python")
+ else:
+ python = system_which("python")
+ _cmd = [python,]
  # Run the PEP 508 checker in the virtualenv.
- c = delegator.run(
- '"{0}" {1}'.format(
- python, escape_grouped_arguments(pep508checker.__file__.rstrip("cdo"))
- )
- )
- results = simplejson.loads(c.out)
+ cmd = _cmd + [pep508checker_path]
+ c = run_command(cmd)
+ try:
+ results = simplejson.loads(c.out.strip())
+ except JSONDecodeError:
+ click.echo("{0}\n{1}".format(
+ crayons.white(decode_for_output("Failed parsing pep508 results: "), bold=True),
+ c.out.strip()
+ ))
+ sys.exit(1)
  # Load the pipfile.
  p = pipfile.Pipfile.load(project.pipfile_location)
  failed = False
@@ -2507,13 +2553,9 @@ def do_check(
  sys.exit(1)
  else:
  click.echo(crayons.green("Passed!"))
- click.echo(crayons.normal(fix_utf8("Checking installed package safety…"), bold=True))
- path = pep508checker.__file__.rstrip("cdo")
- path = os.sep.join(__file__.split(os.sep)[:-1] + ["patched", "safety.zip"])
- if not system:
- python = which("python")
- else:
- python = system_which("python")
+ click.echo(crayons.normal(
+ decode_for_output("Checking installed package safety…"), bold=True)
+ )
  if ignore:
  ignored = "--ignore {0}".format(" --ignore ".join(ignore))
  click.echo(
@@ -2524,17 +2566,13 @@ def do_check(
  )
  else:
  ignored = ""
- c = delegator.run(
- '"{0}" {1} check --json --key={2} {3}'.format(
- python, escape_grouped_arguments(path), PIPENV_PYUP_API_KEY, ignored
- )
- )
+ key = "--key={0}".format(PIPENV_PYUP_API_KEY)
+ cmd = _cmd + [safety_path, "check", "--json", key, ignored]
+ c = run_command(cmd)
  try:
  results = simplejson.loads(c.out)
- except ValueError:
- click.echo("An error occurred:", err=True)
- click.echo(c.err if len(c.err) > 0 else c.out, err=True)
- sys.exit(1)
+ except (ValueError, JSONDecodeError):
+ raise exceptions.JSONParseError(c.out, c.err)
  for (package, resolved, installed, description, vuln) in results:
  click.echo(
  "{0}: {1} {2} resolved ({3} installed)!".format(
@@ -2553,7 +2591,9 @@ def do_check(
 
 
 def do_graph(bare=False, json=False, json_tree=False, reverse=False):
+ from pipenv.vendor.vistir.compat import JSONDecodeError
  import pipdeptree
+ pipdeptree_path = pipdeptree.__file__.rstrip("cdo")
  try:
  python_path = which("python")
  except AttributeError:
@@ -2618,11 +2658,9 @@ def do_graph(bare=False, json=False, json_tree=False, reverse=False):
  err=True,
  )
  sys.exit(1)
- cmd = '"{0}" {1} {2} -l'.format(
- python_path, escape_grouped_arguments(pipdeptree.__file__.rstrip("cdo")), flag
- )
+ cmd_args = [python_path, pipdeptree_path, flag, "-l"]
+ c = run_command(cmd_args)
  # Run dep-tree.
- c = delegator.run(cmd)
  if not bare:
  if json:
  data = []
@@ -2644,9 +2682,14 @@ def traverse(obj):
  obj["dependencies"] = traverse(obj["dependencies"])
  return obj
 
- data = traverse(simplejson.loads(c.out))
- click.echo(simplejson.dumps(data, indent=4))
- sys.exit(0)
+ try:
+ parsed = simplejson.loads(c.out.strip())
+ except JSONDecodeError:
+ raise exceptions.JSONParseError(c.out, c.err)
+ else:
+ data = traverse(parsed)
+ click.echo(simplejson.dumps(data, indent=4))
+ sys.exit(0)
  else:
  for line in c.out.strip().split("\n"):
  # Ignore bad packages as top level.
@@ -2755,8 +2798,8 @@ def do_clean(
  )
  )
  # Uninstall the package.
- cmd_str = Script.parse(cmd + [apparent_bad_package]).cmdify()
- c = delegator.run(cmd_str, block=True)
+ cmd = [which_pip(), "uninstall", apparent_bad_package, "-y"]
+ c = run_command(cmd)
  if c.return_code != 0:
  failure = True
  sys.exit(int(failure))