From bb92ea59a26a5f4f6765c89e1c73398ba73d64d3 Mon Sep 17 00:00:00 2001 From: Cory Bloor Date: Thu, 7 Jul 2022 09:08:15 -0600 Subject: [PATCH] Fix spack edit message when no editor installed (#31412) When no default editor is installed and no environment variable is set, which_string would return None and this would be passed to os.execv resulting in a TypeError. The message presented to the user would be: Error: execv: path should be string, bytes or os.PathLike, not NoneType This change checks that which_string has returned successfully before attempting to execute the result, resulting in a new error message: Error: No text editor found! Please set the VISUAL and/or EDITOR environment variable(s) to your preferred text editor. It's not strictly necessary, but I've also changed try_exec to catch all errors rather than just OSErrors. This would have provided slightly more context for the original error message. --- lib/spack/spack/test/util/editor.py | 14 ++++++++++++++ lib/spack/spack/util/editor.py | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/spack/spack/test/util/editor.py b/lib/spack/spack/test/util/editor.py index e7c8bf2364..54b1c251fe 100644 --- a/lib/spack/spack/test/util/editor.py +++ b/lib/spack/spack/test/util/editor.py @@ -132,3 +132,17 @@ def assert_exec(exe, args): assert args == [vim_exe, '/path/to/file'] ed.editor('/path/to/file', _exec_func=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) diff --git a/lib/spack/spack/util/editor.py b/lib/spack/spack/util/editor.py index e3f740adac..58198907af 100644 --- a/lib/spack/spack/util/editor.py +++ b/lib/spack/spack/util/editor.py @@ -122,7 +122,7 @@ def try_env_var(var): # 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) - if try_exec(exe, [exe] + list(args)): + if exe and try_exec(exe, [exe] + list(args)): return # Fail if nothing could be found