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

Warn when programs not in the virtualenv are used, allow erroring and silencing the warning. #147

Merged
merged 1 commit into from
Oct 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ The following options can be specified in the Noxfile:
* ``nox.options.reuse_existing_virtualenvs`` is equivalent to specifying :ref:`--reuse-existing-virtualenvs <opt-reuse-existing-virtualenvs>`. You can force this off by specifying ``--no-reuse-existing-virtualenvs`` during invocation.
* ``nox.options.stop_on_first_error`` is equivalent to specifying :ref:`--stop-on-first-error <opt-stop-on-first-error>`. You can force this off by specifying ``--no-stop-on-first-error`` during invocation.
* ``nox.options.error_on_missing_interpreters`` is equivalent to specifying :ref:`--error-on-missing-interpreters <opt-error-on-missing-interpreters>`. You can force this off by specifying ``--no-error-on-missing-interpreters`` during invocation.
* ``nox.options.error_on_external_run`` is equivalent to specifying :ref:`--error-on-external-run <opt-error-on-external-run>`. You can force this off by specifying ``--no-error-on-external-run`` during invocation.
* ``nox.options.report`` is equivalent to specifying :ref:`--report <opt-report>`.


Expand Down
11 changes: 11 additions & 0 deletions docs/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,17 @@ By default, Nox will skip sessions where the Python interpreter can't be found.

If the Noxfile sets ``nox.options.error_on_missing_interpreters``, you can override the Noxfile setting from the command line by using ``--no-error-on-missing-interpreters``.

.. _opt-error-on-external-run:

Disallowing external programs
-----------------------------

By default Nox will warn but ultimately allow you to run programs not installed in the session's virtualenv. You can use ``--error-on-external-run`` to make Nox fail the session if it uses any external program without explicitly passing ``external=True`` into :func:`session.run <nox.session.Session.run>`::

nox --error-on-external-run

If the Noxfile sets ``nox.options.error_on_external_run``, you can override the Noxfile setting from the command line by using ``--no-error-on-external-run``.

Specifying a different configuration file
-----------------------------------------

Expand Down
18 changes: 18 additions & 0 deletions nox/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ def __init__(self, args):
self.no_stop_on_first_error = args.no_stop_on_first_error
self.error_on_missing_interpreters = args.error_on_missing_interpreters
self.no_error_on_missing_interpreters = args.no_error_on_missing_interpreters
self.error_on_external_run = args.error_on_external_run
self.no_error_on_external_run = args.no_error_on_external_run
self.posargs = args.posargs
self.report = args.report

Expand Down Expand Up @@ -89,6 +91,11 @@ def merge_from_options(self, options):
options.error_on_missing_interpreters,
self.no_error_on_missing_interpreters,
)
self.error_on_external_run = _default_with_off_flag(
self.error_on_external_run,
options.error_on_external_run,
self.no_error_on_external_run,
)
self.report = self.report or options.report


Expand Down Expand Up @@ -187,6 +194,17 @@ def main():
help="Disables --error-on-missing-interpreters if it is enabled in the Noxfile.",
)

secondary.add_argument(
"--error-on-external-run",
action="store_true",
help="Error if run() is used to execute a program that isn't installed in a session's virtualenv.",
)
secondary.add_argument(
"--no-error-on-external-run",
action="store_true",
help="Disables --error-on-external-run if it is enabled in the Noxfile.",
)

secondary.add_argument(
"--report", help="Output a report of all sessions to the given filename."
)
Expand Down
1 change: 1 addition & 0 deletions nox/_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,5 @@ class options:
reuse_existing_virtualenvs = False
stop_on_first_error = False
error_on_missing_interpreters = False
error_on_external_run = False
report = None
29 changes: 28 additions & 1 deletion nox/command.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,16 @@ def _clean_env(env):
return clean_env


def run(args, *, env=None, silent=False, path=None, success_codes=None, log=True):
def run(
args,
*,
env=None,
silent=False,
path=None,
success_codes=None,
log=True,
external=False
):
"""Run a command-line program."""

if success_codes is None:
Expand All @@ -80,6 +89,24 @@ def run(args, *, env=None, silent=False, path=None, success_codes=None, log=True
if log:
logger.info(full_cmd)

is_external_tool = path is not None and not cmd_path.startswith(path)
if is_external_tool:
if external == "error":
logger.error(
"Error: {} is not installed into the virtualenv, it located at {}. "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it located -> it is located

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, 1646e52.

"Pass external=True into run() to explicitly allow this.".format(
cmd, cmd_path
)
)
raise CommandFailed("External program disallowed.")
elif external is False:
logger.warning(
"Warning: {} is not installed into the virtualenv, it located at {}. This might cause issues! "
"Pass external=True into run() to silence this message.".format(
cmd, cmd_path
)
)

env = _clean_env(env)

try:
Expand Down
24 changes: 23 additions & 1 deletion nox/sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,12 @@ def run(self, *args, env=None, **kwargs):
:param success_codes: A list of return codes that are considered
successful. By default, only ``0`` is considered success.
:type success_codes: list, tuple, or None
:param external: If False (the default) then programs not in the
virtualenv path will cause a warning. If True, no warning will be
emitted. These warnings can be turned into errors using
``--error-on-external-run``. This has no effect for sessions that
do not have a virtualenv.
:type external: bool
"""
if not args:
raise ValueError("At least one argument required to run().")
Expand All @@ -183,6 +189,14 @@ def run(self, *args, env=None, **kwargs):
else:
env = self.env

# If --error-on-external-run is specified, error on external programs.
if self._runner.global_config.error_on_external_run:
kwargs.setdefault("external", "error")

# If we aren't using a virtualenv allow all external programs.
if not isinstance(self.virtualenv, VirtualEnv):
kwargs["external"] = True

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

Expand Down Expand Up @@ -218,7 +232,15 @@ def install(self, *args, **kwargs):
if not args:
raise ValueError("At least one argument required to install().")

self.run("pip", "install", "--upgrade", *args, silent=True, **kwargs)
self.run(
"pip",
"install",
"--upgrade",
*args,
silent=True,
external="error",
**kwargs
)

def notify(self, target):
"""Place the given session at the end of the queue.
Expand Down
2 changes: 1 addition & 1 deletion noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def lint(session):
@nox.session(python="3.6")
def docs(session):
"""Build the documentation."""
session.run("rm", "-rf", "docs/_build")
session.run("rm", "-rf", "docs/_build", external=True)
session.install("-r", "requirements-test.txt")
session.install(".")
session.cd("docs")
Expand Down
30 changes: 30 additions & 0 deletions tests/test_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import logging
import os
import sys
from unittest import mock
Expand Down Expand Up @@ -94,6 +95,35 @@ def test_run_path_existent(tmpdir, monkeypatch):
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)

assert "external=True" in caplog.text


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

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

assert "external=True" not in caplog.text


def test_run_external_raises(tmpdir, caplog):
caplog.set_level(logging.ERROR)

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

assert "external=True" in caplog.text


def test_exit_codes():
assert nox.command.run([PYTHON, "-c", "import sys; sys.exit(0)"])

Expand Down
2 changes: 2 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ def make_args(self):
no_stop_on_first_error=False,
error_on_missing_interpreters=False,
no_error_on_missing_interpreters=False,
error_on_external_run=False,
no_error_on_external_run=True,
posargs=["a", "b", "c"],
report=None,
)
Expand Down
34 changes: 32 additions & 2 deletions tests/test_sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,14 @@ def make_session_and_runner(self):
name="test",
signature="test",
func=func,
global_config=argparse.Namespace(posargs=mock.sentinel.posargs),
global_config=argparse.Namespace(
posargs=mock.sentinel.posargs, error_on_external_run=False
),
manifest=mock.create_autospec(nox.manifest.Manifest),
)
runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv)
runner.venv.env = {}
runner.venv.bin = "/no/bin/for/you"
return nox.sessions.Session(runner=runner), runner

def test_properties(self):
Expand Down Expand Up @@ -133,6 +136,27 @@ def test_run_overly_env(self):
)
assert result.strip() == "1 3"

def test_run_external_not_a_virtualenv(self):
# Non-virtualenv sessions should always allow external programs.
session, runner = self.make_session_and_runner()

runner.venv = nox.virtualenv.ProcessEnv()

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

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

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

runner.global_config.error_on_external_run = True

with pytest.raises(nox.command.CommandFailed, match="External"):
session.run(sys.executable, "--version")

def test_install_bad_args(self):
session, _ = self.make_session_and_runner()

Expand Down Expand Up @@ -166,7 +190,13 @@ class SessionNoSlots(nox.sessions.Session):
with mock.patch.object(session, "run", autospec=True) as run:
session.install("requests", "urllib3")
run.assert_called_once_with(
"pip", "install", "--upgrade", "requests", "urllib3", silent=True
"pip",
"install",
"--upgrade",
"requests",
"urllib3",
silent=True,
external="error",
)

def test_notify(self):
Expand Down