Repository: theacodes/nox (python)

Issue:
`nox` uses `PATH` to find executables
This is a bit `tox`-like to suggest, so feel free to reject.

I was recently migrating my Pelican blog from a `Makefile` to a `noxfile`. I had `pelican` installed in a few of my base environments, but forgot to install it into the virtual environment. However, the sessions ran just fine because they referenced the `pelican` executable on my `${PATH}` even though none existed in the `virtualenv`.

Pull request description:
Warn when programs not in the virtualenv are used, allow erroring and silencing the warning.
Add --warn-on-external-run flag and the "external" keyword arg to session.run.

Closes #130

Patch:
diff --git a/docs/config.rst b/docs/config.rst
index b37b56e..4cbe227 100644
--- a/docs/config.rst
+++ b/docs/config.rst
@@ -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>`.
 
 
diff --git a/docs/usage.rst b/docs/usage.rst
index cb90c35..a30932d 100644
--- a/docs/usage.rst
+++ b/docs/usage.rst
@@ -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
 -----------------------------------------
 
diff --git a/nox/__main__.py b/nox/__main__.py
index a4f99a2..041e41b 100644
--- a/nox/__main__.py
+++ b/nox/__main__.py
@@ -52,6 +52,8 @@ class GlobalConfig:
         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
 
@@ -89,6 +91,11 @@ class GlobalConfig:
             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
 
 
@@ -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."
     )
diff --git a/nox/_options.py b/nox/_options.py
index 7856fa8..c22af87 100644
--- a/nox/_options.py
+++ b/nox/_options.py
@@ -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
diff --git a/nox/command.py b/nox/command.py
index d08e088..f2ab301 100644
--- a/nox/command.py
+++ b/nox/command.py
@@ -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:
@@ -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 {}. "
+                    "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:
diff --git a/nox/sessions.py b/nox/sessions.py
index 93d8d15..4f6b82b 100644
--- a/nox/sessions.py
+++ b/nox/sessions.py
@@ -167,6 +167,12 @@ class Session:
         :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().")
@@ -183,6 +189,14 @@ class Session:
         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)
 
@@ -218,7 +232,15 @@ class Session:
         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.
diff --git a/noxfile.py b/noxfile.py
index 2eaae92..9e9b47f 100644
--- a/noxfile.py
+++ b/noxfile.py
@@ -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")


Test patch:
diff --git a/tests/test_command.py b/tests/test_command.py
index 802b46c..1f9f1bf 100644
--- a/tests/test_command.py
+++ b/tests/test_command.py
@@ -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
@@ -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)"])
 
diff --git a/tests/test_main.py b/tests/test_main.py
index 9283622..42ab088 100644
--- a/tests/test_main.py
+++ b/tests/test_main.py
@@ -46,6 +46,8 @@ class TestGlobalConfig:
             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,
         )
diff --git a/tests/test_sessions.py b/tests/test_sessions.py
index 3d9d40f..3fa9c0e 100644
--- a/tests/test_sessions.py
+++ b/tests/test_sessions.py
@@ -62,11 +62,14 @@ class TestSession:
             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):
@@ -133,6 +136,27 @@ class TestSession:
         )
         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()
 
@@ -166,7 +190,13 @@ class TestSession:
         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):