diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/spack/spack/hooks/licensing.py | 29 | ||||
| -rw-r--r-- | lib/spack/spack/test/util/editor.py | 89 | ||||
| -rw-r--r-- | lib/spack/spack/util/editor.py | 48 |
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( |