Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed the default paths for conda on windows where the python.exe found was not the correct one #310

Merged
merged 9 commits into from
Jun 21, 2020
16 changes: 9 additions & 7 deletions nox/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

import os
import sys
from typing import Any, Iterable, Optional, Sequence, Union
from typing import Any, Iterable, Optional, Sequence, Union, List

import py
from nox.logger import logger
Expand All @@ -29,12 +29,12 @@ def __init__(self, reason: str = None) -> None:
self.reason = reason


def which(program: str, path: Optional[str]) -> str:
def which(program: str, paths: Optional[List[str]]) -> str:
"""Finds the full path to an executable."""
full_path = None

if path:
full_path = py.path.local.sysfind(program, paths=[path])
if paths:
full_path = py.path.local.sysfind(program, paths=paths)

if full_path:
return full_path.strpath
Expand Down Expand Up @@ -66,7 +66,7 @@ def run(
*,
env: Optional[dict] = None,
silent: bool = False,
path: Optional[str] = None,
paths: Optional[List[str]] = None,
success_codes: Optional[Iterable[int]] = None,
log: bool = True,
external: bool = False,
Expand All @@ -80,12 +80,14 @@ def run(
cmd, args = args[0], args[1:]
full_cmd = "{} {}".format(cmd, " ".join(args))

cmd_path = which(cmd, path)
cmd_path = which(cmd, paths)

if log:
logger.info(full_cmd)

is_external_tool = path is not None and not cmd_path.startswith(path)
is_external_tool = paths is not None and not any(
cmd_path.startswith(path) for path in paths
)
if is_external_tool:
if external == "error":
logger.error(
Expand Down
12 changes: 9 additions & 3 deletions nox/sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,16 @@ def python(self) -> Optional[Union[str, Sequence[str], bool]]:
"""The python version passed into ``@nox.session``."""
return self._runner.func.python

@property
def bin_paths(self) -> Optional[List[str]]:
smarie marked this conversation as resolved.
Show resolved Hide resolved
"""The bin directories for the virtualenv."""
return self.virtualenv.bin_paths

@property
def bin(self) -> Optional[str]:
"""The bin directory for the virtualenv."""
return self.virtualenv.bin
"""The first bin directory for the virtualenv."""
paths = self.bin_paths
return paths[0] if paths is not None else None

def create_tmp(self) -> str:
"""Create, and return, a temporary directory."""
Expand Down Expand Up @@ -279,7 +285,7 @@ def _run(self, *args: str, env: Mapping[str, str] = None, **kwargs: Any) -> Any:
kwargs["external"] = True

# Run a shell command.
return nox.command.run(args, env=env, path=self.bin, **kwargs)
return nox.command.run(args, env=env, paths=self.bin_paths, **kwargs)

def conda_install(self, *args: str, **kwargs: Any) -> None:
"""Install invokes `conda install`_ to install packages inside of the
Expand Down
33 changes: 21 additions & 12 deletions nox/virtualenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import re
import shutil
import sys
from typing import Any, Mapping, Optional, Tuple, Union
from typing import Any, Mapping, Optional, Tuple, Union, List

import nox.command
import py
Expand Down Expand Up @@ -48,8 +48,8 @@ class ProcessEnv:
# Special programs that aren't included in the environment.
allowed_globals = () # type: _typing.ClassVar[Tuple[Any, ...]]

def __init__(self, bin: None = None, env: Mapping[str, str] = None) -> None:
self._bin = bin
def __init__(self, bin_paths: None = None, env: Mapping[str, str] = None) -> None:
self._bin_paths = bin_paths
self.env = os.environ.copy()

if env is not None:
Expand All @@ -58,12 +58,20 @@ def __init__(self, bin: None = None, env: Mapping[str, str] = None) -> None:
for key in _BLACKLISTED_ENV_VARS:
self.env.pop(key, None)

if self.bin:
self.env["PATH"] = os.pathsep.join([self.bin, self.env.get("PATH", "")])
if self.bin_paths:
self.env["PATH"] = os.pathsep.join(
self.bin_paths + [self.env.get("PATH", "")]
)

@property
def bin_paths(self) -> Optional[List[str]]:
return self._bin_paths

@property
def bin(self) -> Optional[str]:
return self._bin
"""The first bin directory for the virtualenv."""
paths = self.bin_paths
return paths[0] if paths is not None else None

def create(self) -> bool:
raise NotImplementedError("ProcessEnv.create should be overwritten in subclass")
Expand Down Expand Up @@ -191,12 +199,13 @@ def __init__(
_clean_location = _clean_location

@property
def bin(self) -> str:
def bin_paths(self) -> List[str]:
"""Returns the location of the conda env's bin folder."""
# see https://docs.anaconda.com/anaconda/user-guide/tasks/integration/python-path/#examples
if _SYSTEM == "Windows":
return os.path.join(self.location, "Scripts")
return [self.location, os.path.join(self.location, "Scripts")]
else:
return os.path.join(self.location, "bin")
return [os.path.join(self.location, "bin")]

def create(self) -> bool:
"""Create the conda env."""
Expand Down Expand Up @@ -339,12 +348,12 @@ def _resolved_interpreter(self) -> str:
raise self._resolved

@property
def bin(self) -> str:
def bin_paths(self) -> List[str]:
"""Returns the location of the virtualenv's bin folder."""
if _SYSTEM == "Windows":
return os.path.join(self.location, "Scripts")
return [os.path.join(self.location, "Scripts")]
else:
return os.path.join(self.location, "bin")
return [os.path.join(self.location, "bin")]

def create(self) -> bool:
"""Create the virtualenv or venv."""
Expand Down
10 changes: 5 additions & 5 deletions tests/test_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def test_run_path_nonexistent():
result = nox.command.run(
[PYTHON, "-c", "import sys; print(sys.executable)"],
silent=True,
path="/non/existent",
paths=["/non/existent"],
)

assert "/non/existent" not in result
Expand All @@ -135,14 +135,14 @@ def test_run_path_existent(tmpdir, monkeypatch):

with mock.patch("nox.command.popen") as mock_command:
mock_command.return_value = (0, "")
nox.command.run(["testexc"], silent=True, path=tmpdir.strpath)
nox.command.run(["testexc"], silent=True, paths=[tmpdir.strpath])
mock_command.assert_called_with([executable.strpath], env=None, silent=True)


def test_run_external_warns(tmpdir, caplog):
caplog.set_level(logging.WARNING)

nox.command.run([PYTHON, "--version"], silent=True, path=tmpdir.strpath)
nox.command.run([PYTHON, "--version"], silent=True, paths=[tmpdir.strpath])

assert "external=True" in caplog.text

Expand All @@ -151,7 +151,7 @@ def test_run_external_silences(tmpdir, caplog):
caplog.set_level(logging.WARNING)

nox.command.run(
[PYTHON, "--version"], silent=True, path=tmpdir.strpath, external=True
[PYTHON, "--version"], silent=True, paths=[tmpdir.strpath], external=True
)

assert "external=True" not in caplog.text
Expand All @@ -162,7 +162,7 @@ def test_run_external_raises(tmpdir, caplog):

with pytest.raises(nox.command.CommandFailed):
nox.command.run(
[PYTHON, "--version"], silent=True, path=tmpdir.strpath, external="error"
[PYTHON, "--version"], silent=True, paths=[tmpdir.strpath], external="error"
)

assert "external=True" in caplog.text
Expand Down
25 changes: 18 additions & 7 deletions tests/test_sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ def make_session_and_runner(self):
)
runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv)
runner.venv.env = {}
runner.venv.bin = "/no/bin/for/you"
runner.venv.bin_paths = ["/no/bin/for/you"]
return nox.sessions.Session(runner=runner), runner

def test_create_tmp(self):
Expand All @@ -100,9 +100,17 @@ def test_properties(self):
assert session.env is runner.venv.env
assert session.posargs is runner.global_config.posargs
assert session.virtualenv is runner.venv
assert session.bin is runner.venv.bin
assert session.bin_paths is runner.venv.bin_paths
assert session.bin is runner.venv.bin_paths[0]
assert session.python is runner.func.python

def test_no_bin_paths(self):
session, runner = self.make_session_and_runner()

runner.venv.bin_paths = None
assert session.bin is None
assert session.bin_paths is None

def test_virtualenv_as_none(self):
session, runner = self.make_session_and_runner()

Expand Down Expand Up @@ -187,7 +195,7 @@ def test_run_install_only_should_install(self):
("pip", "install", "spam"),
env=mock.ANY,
external=mock.ANY,
path=mock.ANY,
paths=mock.ANY,
silent=mock.ANY,
)

Expand Down Expand Up @@ -225,7 +233,7 @@ def test_run_external_not_a_virtualenv(self):
session.run(sys.executable, "--version")

run.assert_called_once_with(
(sys.executable, "--version"), external=True, env=mock.ANY, path=None
(sys.executable, "--version"), external=True, env=mock.ANY, paths=None
)

def test_run_external_condaenv(self):
Expand All @@ -234,14 +242,17 @@ def test_run_external_condaenv(self):
runner.venv = mock.create_autospec(nox.virtualenv.CondaEnv)
runner.venv.allowed_globals = ("conda",)
runner.venv.env = {}
runner.venv.bin = "/path/to/env/bin"
runner.venv.bin_paths = ["/path/to/env/bin"]
runner.venv.create.return_value = True

with mock.patch("nox.command.run", autospec=True) as run:
session.run("conda", "--version")

run.assert_called_once_with(
("conda", "--version"), external=True, env=mock.ANY, path="/path/to/env/bin"
("conda", "--version"),
external=True,
env=mock.ANY,
paths=["/path/to/env/bin"],
)

def test_run_external_with_error_on_external_run(self):
Expand All @@ -256,7 +267,7 @@ def test_run_external_with_error_on_external_run_condaenv(self):
session, runner = self.make_session_and_runner()
runner.venv = mock.create_autospec(nox.virtualenv.CondaEnv)
runner.venv.env = {}
runner.venv.bin = "/path/to/env/bin"
runner.venv.bin_paths = ["/path/to/env/bin"]

runner.global_config.error_on_external_run = True

Expand Down
18 changes: 13 additions & 5 deletions tests/test_virtualenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def mock_sysfind(arg):

def test_process_env_constructor():
penv = nox.virtualenv.ProcessEnv()
assert not penv.bin
assert not penv.bin_paths

penv = nox.virtualenv.ProcessEnv(env={"SIGIL": "123"})
assert penv.env["SIGIL"] == "123"
Expand Down Expand Up @@ -186,7 +186,7 @@ def test_condaenv_create_interpreter(make_conda):
@mock.patch("nox.virtualenv._SYSTEM", new="Windows")
def test_condaenv_bin_windows(make_conda):
venv, dir_ = make_conda()
assert dir_.join("Scripts").strpath == venv.bin
assert [dir_.strpath, dir_.join("Scripts").strpath] == venv.bin_paths


def test_constructor_defaults(make_one):
Expand All @@ -209,13 +209,16 @@ def test_env(monkeypatch, make_one):
monkeypatch.setenv("SIGIL", "123")
venv, _ = make_one()
assert venv.env["SIGIL"] == "123"
assert venv.bin in venv.env["PATH"]
assert venv.bin not in os.environ["PATH"]
assert len(venv.bin_paths) == 1
assert venv.bin_paths[0] in venv.env["PATH"]
assert venv.bin_paths[0] not in os.environ["PATH"]


def test_blacklisted_env(monkeypatch, make_one):
monkeypatch.setenv("__PYVENV_LAUNCHER__", "meep")
venv, _ = make_one()
assert len(venv.bin_paths) == 1
assert venv.bin_paths[0] == venv.bin
assert "__PYVENV_LAUNCHER__" not in venv.bin


Expand Down Expand Up @@ -249,9 +252,12 @@ def test__clean_location(monkeypatch, make_one):
assert venv._clean_location()


def test_bin(make_one):
def test_bin_paths(make_one):
venv, dir_ = make_one()

assert len(venv.bin_paths) == 1
assert venv.bin_paths[0] == venv.bin

if IS_WINDOWS:
assert dir_.join("Scripts").strpath == venv.bin
else:
Expand All @@ -261,6 +267,8 @@ def test_bin(make_one):
@mock.patch("nox.virtualenv._SYSTEM", new="Windows")
def test_bin_windows(make_one):
venv, dir_ = make_one()
assert len(venv.bin_paths) == 1
assert venv.bin_paths[0] == venv.bin
assert dir_.join("Scripts").strpath == venv.bin


Expand Down