fix(nodeenv): let -p target a virtualenv nodeenv isn't installed in

CHANGES

@@ -16,6 +16,9 @@ Version [unreleased]
   virtualenv `#159 <https://github.com/ekalinin/nodeenv/issues/159>`_
 - Repeated `-p` runs no longer duplicate the `predeactivate` hook
   `#159 <https://github.com/ekalinin/nodeenv/issues/159>`_
+- `-p` accepts an optional virtualenv directory and prefers the activated
+  `VIRTUAL_ENV` over the virtualenv nodeenv itself is installed in
+  `#156 <https://github.com/ekalinin/nodeenv/issues/156>`_
 
 Version 1.3.1
 -------------

README.rst

@@ -276,9 +276,13 @@ Basic options
 ``-l, --list``
     Lists available node.js versions.
 
-``-p, --python-virtualenv``
-    Use current python virtualenv. Running it again with the same node
-    version does not reinstall node; pass ``--force`` to reinstall.
+``-p [VENV_DIR], --python-virtualenv [VENV_DIR]``
+    Use the given python virtualenv, or the current one if no directory
+    is given. Passing a directory is required when nodeenv lives in its
+    own virtualenv (``pipx``, ``pipsi``, ``uv tool``) and the activated
+    virtualenv doesn't export ``VIRTUAL_ENV``. Running it again with the
+    same node version does not reinstall node; pass ``--force`` to
+    reinstall.
 
 ``-r FILENAME, --requirements=FILENAME``
     Install all the packages listed in the given requirements file.

nodeenv.py

@@ -566,8 +566,9 @@ def make_parser():
 
     parser.add_argument(
         '--python-virtualenv', '-p', dest='python_virtualenv',
-        action='store_true', default=False,
-        help='Use current python virtualenv')
+        nargs='?', const=True, default=False, metavar='VENV_DIR',
+        help='Use the given python virtualenv, or the current one '
+        'if no directory is given')
 
     parser.add_argument(
         '--clean-src', '-c', dest='clean_src',
@@ -1381,14 +1382,27 @@ def resolve_node_version(spec):
 
 def get_env_dir(args):
     if args.python_virtualenv:
-        if hasattr(sys, 'real_prefix'):
-            res = sys.prefix
-        elif hasattr(sys, 'base_prefix') and sys.base_prefix != sys.prefix:
-            res = sys.prefix
-        elif 'CONDA_PREFIX' in os.environ:
-            res = sys.prefix
-        elif 'VIRTUAL_ENV' in os.environ:
+        # whether nodeenv itself is running inside a python virtualenv
+        in_virtualenv = (
+            hasattr(sys, 'real_prefix') or
+            (hasattr(sys, 'base_prefix') and sys.base_prefix != sys.prefix) or
+            'CONDA_PREFIX' in os.environ)
+        if args.python_virtualenv is not True:
+            res = args.python_virtualenv
+            if not os.path.isdir(res):
+                logger.error("Python virtualenv '%s' doesn't exist", res)
+                sys.exit(2)
+        # nodeenv itself can be installed into its own virtualenv
+        # (pipx, pipsi, uv tool), so the activated one wins over sys.prefix
+        elif os.environ.get('VIRTUAL_ENV'):
             res = os.environ['VIRTUAL_ENV']
+            if in_virtualenv and res != sys.prefix:
+                logger.warning(
+                    ' * Using activated virtualenv %s, not %s where nodeenv '
+                    'is installed, pass a directory to -p to override',
+                    res, sys.prefix)
+        elif in_virtualenv:
+            res = sys.prefix
         else:
             logger.error('No python virtualenv is available')
             sys.exit(2)

tests/nodeenv_test.py

@@ -698,6 +698,18 @@ def test_parse_args_prefer_system():
         assert nodeenv.parse_args().prefer_system is False
 
 
+def test_parse_args_python_virtualenv():
+    with mock.patch.object(sys, 'argv', ['nodeenv', '-p']):
+        assert nodeenv.parse_args().python_virtualenv is True
+    with mock.patch.object(sys, 'argv', ['nodeenv', '-p', 'venv']):
+        assert nodeenv.parse_args().python_virtualenv == 'venv'
+    with mock.patch.object(
+            sys, 'argv', ['nodeenv', '--python-virtualenv=venv']):
+        assert nodeenv.parse_args().python_virtualenv == 'venv'
+    with mock.patch.object(sys, 'argv', ['nodeenv', 'env']):
+        assert nodeenv.parse_args().python_virtualenv is False
+
+
 def test_isolate_npm_default():
     assert nodeenv.Config._default['isolate_npm'] is False
 
@@ -1622,7 +1634,8 @@ def test_with_python_virtualenv_real_prefix(self):
         test_prefix = '/path/to/virtualenv'
 
         with mock.patch.object(sys, 'real_prefix', test_prefix, create=True), \
-             mock.patch.object(sys, 'prefix', test_prefix):
+             mock.patch.object(sys, 'prefix', test_prefix), \
+             mock.patch.dict(os.environ, {}, clear=True):
             result = nodeenv.get_env_dir(args)
             assert result == test_prefix
 
@@ -1637,12 +1650,14 @@ def test_with_python_virtualenv_base_prefix(self):
         if hasattr(sys, 'real_prefix'):
             with mock.patch.object(sys, 'real_prefix', create=False):
                 with mock.patch.object(sys, 'prefix', test_prefix), \
-                     mock.patch.object(sys, 'base_prefix', test_base_prefix):
+                     mock.patch.object(sys, 'base_prefix', test_base_prefix), \
+                     mock.patch.dict(os.environ, {}, clear=True):
                     result = nodeenv.get_env_dir(args)
                     assert result == test_prefix
         else:
             with mock.patch.object(sys, 'prefix', test_prefix), \
-                 mock.patch.object(sys, 'base_prefix', test_base_prefix):
+                 mock.patch.object(sys, 'base_prefix', test_base_prefix), \
+                 mock.patch.dict(os.environ, {}, clear=True):
                 result = nodeenv.get_env_dir(args)
                 assert result == test_prefix
 
@@ -1658,14 +1673,14 @@ def test_with_python_virtualenv_conda_prefix(self):
                 env_dict = {'CONDA_PREFIX': test_prefix}
                 with mock.patch.object(sys, 'prefix', test_prefix), \
                      mock.patch.object(sys, 'base_prefix', test_prefix), \
-                     mock.patch.dict(os.environ, env_dict):
+                     mock.patch.dict(os.environ, env_dict, clear=True):
                     result = nodeenv.get_env_dir(args)
                     assert result == test_prefix
         else:
             env_dict = {'CONDA_PREFIX': test_prefix}
             with mock.patch.object(sys, 'prefix', test_prefix), \
                  mock.patch.object(sys, 'base_prefix', test_prefix), \
-                 mock.patch.dict(os.environ, env_dict):
+                 mock.patch.dict(os.environ, env_dict, clear=True):
                 result = nodeenv.get_env_dir(args)
                 assert result == test_prefix
 
@@ -1716,6 +1731,74 @@ def test_with_python_virtualenv_no_virtualenv_exits(self):
                 nodeenv.get_env_dir(args)
             assert exc_info.value.code == 2
 
+    def test_with_python_virtualenv_dir(self, tmpdir):
+        """Test get_env_dir when a virtualenv directory is given"""
+        args = mock.Mock()
+        args.python_virtualenv = str(tmpdir)
+
+        env_dict = {'VIRTUAL_ENV': '/path/to/other/venv'}
+        with mock.patch.dict(os.environ, env_dict, clear=True):
+            result = nodeenv.get_env_dir(args)
+            assert result == str(tmpdir)
+
+    def test_with_python_virtualenv_missing_dir_exits(self, tmpdir):
+        """Test get_env_dir exits when the given virtualenv doesn't exist"""
+        args = mock.Mock()
+        args.python_virtualenv = str(tmpdir.join('missing'))
+
+        with pytest.raises(SystemExit) as exc_info:
+            nodeenv.get_env_dir(args)
+        assert exc_info.value.code == 2
+
+    def test_with_python_virtualenv_prefers_virtual_env(self):
+        """Test get_env_dir prefers VIRTUAL_ENV over nodeenv's own venv"""
+        args = mock.Mock()
+        args.python_virtualenv = True
+        # nodeenv itself is installed into its own virtualenv
+        test_prefix = '/path/to/nodeenv/venv'
+        virtual_env = '/path/to/activated/venv'
+
+        env_dict = {'VIRTUAL_ENV': virtual_env}
+        with mock.patch.object(sys, 'real_prefix', test_prefix, create=True), \
+             mock.patch.object(sys, 'prefix', test_prefix), \
+             mock.patch.dict(os.environ, env_dict, clear=True), \
+             mock.patch.object(nodeenv.logger, 'warning') as mck:
+            result = nodeenv.get_env_dir(args)
+            assert result == virtual_env
+            # the ignored virtualenv is not silently dropped
+            assert mck.call_count == 1
+            assert mck.call_args[0][1:] == (virtual_env, test_prefix)
+
+    def test_with_python_virtualenv_same_venv_is_quiet(self):
+        """Test get_env_dir doesn't warn when both point to the same venv"""
+        args = mock.Mock()
+        args.python_virtualenv = True
+        test_prefix = '/path/to/venv'
+
+        env_dict = {'VIRTUAL_ENV': test_prefix}
+        with mock.patch.object(sys, 'real_prefix', test_prefix, create=True), \
+             mock.patch.object(sys, 'prefix', test_prefix), \
+             mock.patch.dict(os.environ, env_dict, clear=True), \
+             mock.patch.object(nodeenv.logger, 'warning') as mck:
+            result = nodeenv.get_env_dir(args)
+            assert result == test_prefix
+            mck.assert_not_called()
+
+    def test_with_python_virtualenv_system_python_is_quiet(self):
+        """Test get_env_dir doesn't warn when nodeenv runs system-wide"""
+        args = mock.Mock()
+        args.python_virtualenv = True
+        virtual_env = '/path/to/activated/venv'
+
+        env_dict = {'VIRTUAL_ENV': virtual_env}
+        with mock.patch.object(sys, 'prefix', '/usr'), \
+             mock.patch.object(sys, 'base_prefix', '/usr'), \
+             mock.patch.dict(os.environ, env_dict, clear=True), \
+             mock.patch.object(nodeenv.logger, 'warning') as mck:
+            result = nodeenv.get_env_dir(args)
+            assert result == virtual_env
+            mck.assert_not_called()
+
     def test_without_python_virtualenv(self):
         """Test get_env_dir when not using python virtualenv"""
         args = mock.Mock()