summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
Diffstat (limited to 'lib')
-rw-r--r--lib/spack/spack/hooks/licensing.py29
-rw-r--r--lib/spack/spack/test/util/editor.py89
-rw-r--r--lib/spack/spack/util/editor.py48
3 files changed, 107 insertions, 59 deletions
diff --git a/lib/spack/spack/hooks/licensing.py b/lib/spack/spack/hooks/licensing.py
index 27628776f2..e21445c302 100644
--- a/lib/spack/spack/hooks/licensing.py
+++ b/lib/spack/spack/hooks/licensing.py
@@ -9,8 +9,7 @@ import llnl.util.tty as tty
from llnl.util.filesystem import mkdirp
from llnl.util.symlink import symlink
-from spack.util.editor import editor
-from spack.util.executable import Executable, which
+import spack.util.editor as ed
def pre_install(spec):
@@ -38,29 +37,9 @@ def set_up_license(pkg):
if not os.path.exists(license_path):
# Create a new license file
write_license_file(pkg, license_path)
- # Open up file in user's favorite $EDITOR for editing
- editor_exe = None
- if "VISUAL" in os.environ:
- editor_exe = Executable(os.environ["VISUAL"])
- # gvim runs in the background by default so we force it to run
- # in the foreground to make sure the license file is updated
- # before we try to install
- if "gvim" in os.environ["VISUAL"]:
- editor_exe.add_default_arg("-f")
- elif "EDITOR" in os.environ:
- editor_exe = Executable(os.environ["EDITOR"])
- else:
- editor_exe = which("vim", "vi", "emacs", "nano")
- if editor_exe is None:
- raise EnvironmentError(
- "No text editor found! Please set the VISUAL and/or EDITOR"
- " environment variable(s) to your preferred text editor."
- )
-
- def editor_wrapper(exe, args):
- editor_exe(license_path)
-
- editor(license_path, _exec_func=editor_wrapper)
+
+ # use spack.util.executable so the editor does not hang on return here
+ ed.editor(license_path, exec_fn=ed.executable)
else:
# Use already existing license file
tty.msg("Found already existing license %s" % license_path)
diff --git a/lib/spack/spack/test/util/editor.py b/lib/spack/spack/test/util/editor.py
index 613185537a..18d19ff1d6 100644
--- a/lib/spack/spack/test/util/editor.py
+++ b/lib/spack/spack/test/util/editor.py
@@ -18,6 +18,30 @@ pytestmark = [
]
+# env vars that control the editor
+EDITOR_VARS = ["VISUAL", "EDITOR"]
+
+
+@pytest.fixture(scope="module", autouse=True)
+def clean_env_vars():
+ """Unset all editor env vars before tests."""
+ for var in EDITOR_VARS:
+ if var in os.environ:
+ del os.environ[var]
+
+
+@pytest.fixture(autouse=True)
+def working_editor_test_env(working_env):
+ """Don't leak environent variables between functions here."""
+ pass
+
+
+# parameterized fixture for editor var names
+@pytest.fixture(params=EDITOR_VARS)
+def editor_var(request):
+ return request.param
+
+
def _make_exe(tmpdir_factory, name, contents=None):
if sys.platform == "win32":
name += ".exe"
@@ -49,6 +73,11 @@ def vim_exe(tmpdir_factory):
return _make_exe(tmpdir_factory, "vim", "exit 0")
+@pytest.fixture(scope="session")
+def gvim_exe(tmpdir_factory):
+ return _make_exe(tmpdir_factory, "gvim", "exit 0")
+
+
def test_find_exe_from_env_var(good_exe):
os.environ["EDITOR"] = good_exe
assert ed._find_exe_from_env_var("EDITOR") == (good_exe, [good_exe])
@@ -64,20 +93,35 @@ def test_find_exe_from_env_var_bad_path(nosuch_exe):
assert ed._find_exe_from_env_var("FOO") == (None, [])
+def test_editor_gvim_special_case(gvim_exe):
+ os.environ["EDITOR"] = gvim_exe
+
+ def assert_exec(exe, args):
+ assert exe == gvim_exe
+ assert args == [gvim_exe, "-f", "/path/to/file"]
+ return 0
+
+ assert ed.editor("/path/to/file", exec_fn=assert_exec)
+
+ os.environ["EDITOR"] = gvim_exe + " -f"
+ assert ed.editor("/path/to/file", exec_fn=assert_exec)
+
+
def test_find_exe_from_env_var_no_editor():
if "FOO" in os.environ:
os.environ.unset("FOO")
assert ed._find_exe_from_env_var("FOO") == (None, [])
-def test_editor_visual(good_exe):
- os.environ["VISUAL"] = good_exe
+def test_editor(editor_var, good_exe):
+ os.environ[editor_var] = good_exe
def assert_exec(exe, args):
assert exe == good_exe
assert args == [good_exe, "/path/to/file"]
+ return 0
- ed.editor("/path/to/file", _exec_func=assert_exec)
+ ed.editor("/path/to/file", exec_fn=assert_exec)
def test_editor_visual_bad(good_exe, bad_exe):
@@ -90,34 +134,32 @@ def test_editor_visual_bad(good_exe, bad_exe):
assert exe == good_exe
assert args == [good_exe, "/path/to/file"]
+ return 0
- ed.editor("/path/to/file", _exec_func=assert_exec)
+ ed.editor("/path/to/file", exec_fn=assert_exec)
def test_editor_no_visual(good_exe):
- if "VISUAL" in os.environ:
- del os.environ["VISUAL"]
os.environ["EDITOR"] = good_exe
def assert_exec(exe, args):
assert exe == good_exe
assert args == [good_exe, "/path/to/file"]
+ return 0
- ed.editor("/path/to/file", _exec_func=assert_exec)
+ ed.editor("/path/to/file", exec_fn=assert_exec)
def test_editor_no_visual_with_args(good_exe):
- if "VISUAL" in os.environ:
- del os.environ["VISUAL"]
-
# editor has extra args in the var (e.g., emacs -nw)
os.environ["EDITOR"] = good_exe + " -nw --foo"
def assert_exec(exe, args):
assert exe == good_exe
assert args == [good_exe, "-nw", "--foo", "/path/to/file"]
+ return 0
- ed.editor("/path/to/file", _exec_func=assert_exec)
+ ed.editor("/path/to/file", exec_fn=assert_exec)
def test_editor_both_bad(nosuch_exe, vim_exe):
@@ -129,19 +171,32 @@ def test_editor_both_bad(nosuch_exe, vim_exe):
def assert_exec(exe, args):
assert exe == vim_exe
assert args == [vim_exe, "/path/to/file"]
+ return 0
- ed.editor("/path/to/file", _exec_func=assert_exec)
+ ed.editor("/path/to/file", exec_fn=assert_exec)
def test_no_editor():
- if "VISUAL" in os.environ:
- del os.environ["VISUAL"]
- if "EDITOR" in os.environ:
- del os.environ["EDITOR"]
os.environ["PATH"] = ""
def assert_exec(exe, args):
assert False
with pytest.raises(EnvironmentError, match=r"No text editor found.*"):
- ed.editor("/path/to/file", _exec_func=assert_exec)
+ ed.editor("/path/to/file", exec_fn=assert_exec)
+
+ def assert_exec(exe, args):
+ return False
+
+ with pytest.raises(EnvironmentError, match=r"No text editor found.*"):
+ ed.editor("/path/to/file", exec_fn=assert_exec)
+
+
+def test_exec_fn_executable(editor_var, good_exe, bad_exe):
+ """Make sure editor() works with ``ed.executable`` as well as execv"""
+ os.environ[editor_var] = good_exe
+ assert ed.editor(exec_fn=ed.executable)
+
+ os.environ[editor_var] = bad_exe
+ with pytest.raises(EnvironmentError, match=r"No text editor found.*"):
+ ed.editor(exec_fn=ed.executable)
diff --git a/lib/spack/spack/util/editor.py b/lib/spack/spack/util/editor.py
index 737a71d433..90644c7627 100644
--- a/lib/spack/spack/util/editor.py
+++ b/lib/spack/spack/util/editor.py
@@ -14,17 +14,18 @@ raising an EnvironmentError if we are unable to find one.
"""
import os
import shlex
+from typing import Callable, List
import llnl.util.tty as tty
import spack.config
-from spack.util.executable import which_string
+import spack.util.executable
#: editors to try if VISUAL and EDITOR are not set
_default_editors = ["vim", "vi", "emacs", "nano", "notepad"]
-def _find_exe_from_env_var(var):
+def _find_exe_from_env_var(var: str):
"""Find an executable from an environment variable.
Args:
@@ -45,12 +46,22 @@ def _find_exe_from_env_var(var):
if not args:
return None, []
- exe = which_string(args[0])
+ exe = spack.util.executable.which_string(args[0])
args = [exe] + args[1:]
return exe, args
-def editor(*args, **kwargs):
+def executable(exe: str, args: List[str]) -> int:
+ """Wrapper that makes ``spack.util.executable.Executable`` look like ``os.execv()``.
+
+ Use this with ``editor()`` if you want it to return instead of running ``execv``.
+ """
+ cmd = spack.util.executable.Executable(exe)
+ cmd(*args[1:], fail_on_error=False)
+ return cmd.returncode
+
+
+def editor(*args: List[str], exec_fn: Callable[[str, List[str]], int] = os.execv) -> bool:
"""Invoke the user's editor.
This will try to execute the following, in order:
@@ -65,27 +76,30 @@ def editor(*args, **kwargs):
searching the full list above, we'll raise an error.
Arguments:
- args (list): args to pass to editor
+ args: args to pass to editor
Optional Arguments:
- _exec_func (function): invoke this function instead of ``os.execv()``
-
+ exec_fn: invoke this function to run; use ``spack.util.editor.executable`` if you
+ want something that returns, instead of the default ``os.execv()``.
"""
- # allow this to be customized for testing
- _exec_func = kwargs.get("_exec_func", os.execv)
def try_exec(exe, args, var=None):
"""Try to execute an editor with execv, and warn if it fails.
Returns: (bool) False if the editor failed, ideally does not
return if ``execv`` succeeds, and ``True`` if the
- ``_exec_func`` does return successfully.
+ ``exec`` does return successfully.
"""
+ # gvim runs in the background by default so we force it to run
+ # in the foreground to ensure it gets attention.
+ if "gvim" in exe and "-f" not in args:
+ exe, *rest = args
+ args = [exe, "-f"] + rest
+
try:
- _exec_func(exe, args)
- return True
+ return exec_fn(exe, args) == 0
- except OSError as e:
+ except (OSError, spack.util.executable.ProcessError) as e:
if spack.config.get("config:debug"):
raise
@@ -114,16 +128,16 @@ def editor(*args, **kwargs):
# try standard environment variables
if try_env_var("VISUAL"):
- return
+ return True
if try_env_var("EDITOR"):
- return
+ return True
# nothing worked -- try the first default we can find don't bother
# trying them all -- if we get here and one fails, something is
# probably much more deeply wrong with the environment.
- exe = which_string(*_default_editors)
+ exe = spack.util.executable.which_string(*_default_editors)
if exe and try_exec(exe, [exe] + list(args)):
- return
+ return True
# Fail if nothing could be found
raise EnvironmentError(