From b02dcf697de1ef53228e30c3e8bfec8e61f31cff Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Thu, 30 Jan 2025 18:32:57 +0100 Subject: [PATCH] Move from python2 compliant IOError and EnvironmentError to python3-only OSError (#48764) * IOError -> OSError * also do EnvironmentError --- lib/spack/llnl/util/filesystem.py | 12 ++++++------ lib/spack/llnl/util/link_tree.py | 2 +- lib/spack/llnl/util/lock.py | 2 +- lib/spack/llnl/util/tty/log.py | 4 ++-- lib/spack/spack/cmd/external.py | 5 +---- lib/spack/spack/environment/environment.py | 6 +++--- lib/spack/spack/extensions.py | 2 +- lib/spack/spack/filesystem_view.py | 4 ++-- lib/spack/spack/hooks/absolutify_elf_sonames.py | 2 +- lib/spack/spack/hooks/sbang.py | 2 +- lib/spack/spack/repo.py | 4 ++-- lib/spack/spack/test/install.py | 2 +- lib/spack/spack/test/installer.py | 2 +- lib/spack/spack/test/llnl/util/filesystem.py | 8 ++++---- lib/spack/spack/test/llnl/util/lock.py | 4 ++-- lib/spack/spack/test/modules/tcl.py | 5 +---- lib/spack/spack/test/util/editor.py | 6 +++--- lib/spack/spack/util/editor.py | 4 ++-- lib/spack/spack/util/ld_so_conf.py | 4 ++-- var/spack/repos/builtin/packages/charmpp/package.py | 2 +- var/spack/repos/builtin/packages/cp2k/package.py | 4 ++-- var/spack/repos/builtin/packages/perl/package.py | 2 +- 22 files changed, 41 insertions(+), 47 deletions(-) diff --git a/lib/spack/llnl/util/filesystem.py b/lib/spack/llnl/util/filesystem.py index 9d6f7e80640..cd4d8484c67 100644 --- a/lib/spack/llnl/util/filesystem.py +++ b/lib/spack/llnl/util/filesystem.py @@ -668,7 +668,7 @@ def copy(src, dest, _permissions=False): _permissions (bool): for internal use only Raises: - IOError: if *src* does not match any files or directories + OSError: if *src* does not match any files or directories ValueError: if *src* matches multiple files but *dest* is not a directory """ @@ -679,7 +679,7 @@ def copy(src, dest, _permissions=False): files = glob.glob(src) if not files: - raise IOError("No such file or directory: '{0}'".format(src)) + raise OSError("No such file or directory: '{0}'".format(src)) if len(files) > 1 and not os.path.isdir(dest): raise ValueError( "'{0}' matches multiple files but '{1}' is not a directory".format(src, dest) @@ -710,7 +710,7 @@ def install(src, dest): dest (str): the destination file or directory Raises: - IOError: if *src* does not match any files or directories + OSError: if *src* does not match any files or directories ValueError: if *src* matches multiple files but *dest* is not a directory """ @@ -748,7 +748,7 @@ def copy_tree( _permissions (bool): for internal use only Raises: - IOError: if *src* does not match any files or directories + OSError: if *src* does not match any files or directories ValueError: if *src* is a parent directory of *dest* """ if _permissions: @@ -762,7 +762,7 @@ def copy_tree( files = glob.glob(src) if not files: - raise IOError("No such file or directory: '{0}'".format(src)) + raise OSError("No such file or directory: '{0}'".format(src)) # For Windows hard-links and junctions, the source path must exist to make a symlink. Add # all symlinks to this list while traversing the tree, then when finished, make all @@ -843,7 +843,7 @@ def install_tree(src, dest, symlinks=True, ignore=None): ignore (typing.Callable): function indicating which files to ignore Raises: - IOError: if *src* does not match any files or directories + OSError: if *src* does not match any files or directories ValueError: if *src* is a parent directory of *dest* """ copy_tree(src, dest, symlinks=symlinks, ignore=ignore, _permissions=True) diff --git a/lib/spack/llnl/util/link_tree.py b/lib/spack/llnl/util/link_tree.py index 0d17d866b82..81c3aa09ad4 100644 --- a/lib/spack/llnl/util/link_tree.py +++ b/lib/spack/llnl/util/link_tree.py @@ -308,7 +308,7 @@ class LinkTree: def __init__(self, source_root): if not os.path.exists(source_root): - raise IOError("No such file or directory: '%s'", source_root) + raise OSError("No such file or directory: '%s'", source_root) self._root = source_root diff --git a/lib/spack/llnl/util/lock.py b/lib/spack/llnl/util/lock.py index 081e88912c7..9c45b55b4d0 100644 --- a/lib/spack/llnl/util/lock.py +++ b/lib/spack/llnl/util/lock.py @@ -391,7 +391,7 @@ def _poll_lock(self, op: int) -> bool: return True - except IOError as e: + except OSError as e: # EAGAIN and EACCES == locked by another process (so try again) if e.errno not in (errno.EAGAIN, errno.EACCES): raise diff --git a/lib/spack/llnl/util/tty/log.py b/lib/spack/llnl/util/tty/log.py index fbdbc9bff4f..d05bb750115 100644 --- a/lib/spack/llnl/util/tty/log.py +++ b/lib/spack/llnl/util/tty/log.py @@ -918,7 +918,7 @@ def _writer_daemon( try: if stdin_file.read(1) == "v": echo = not echo - except IOError as e: + except OSError as e: # If SIGTTIN is ignored, the system gives EIO # to let the caller know the read failed b/c it # was in the bg. Ignore that too. @@ -1013,7 +1013,7 @@ def wrapped(*args, **kwargs): while True: try: return function(*args, **kwargs) - except IOError as e: + except OSError as e: if e.errno == errno.EINTR: continue raise diff --git a/lib/spack/spack/cmd/external.py b/lib/spack/spack/cmd/external.py index d481933b283..a877e71f981 100644 --- a/lib/spack/spack/cmd/external.py +++ b/lib/spack/spack/cmd/external.py @@ -110,10 +110,7 @@ def external_find(args): # Note that KeyboardInterrupt does not subclass Exception # (so CTRL-C will terminate the program as expected). skip_msg = "Skipping manifest and continuing with other external checks" - if (isinstance(e, IOError) or isinstance(e, OSError)) and e.errno in [ - errno.EPERM, - errno.EACCES, - ]: + if isinstance(e, OSError) and e.errno in (errno.EPERM, errno.EACCES): # The manifest file does not have sufficient permissions enabled: # print a warning and keep going tty.warn("Unable to read manifest due to insufficient permissions.", skip_msg) diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py index 7877a847645..b32c7b3e5fb 100644 --- a/lib/spack/spack/environment/environment.py +++ b/lib/spack/spack/environment/environment.py @@ -581,7 +581,7 @@ def _error_on_nonempty_view_dir(new_root): # Check if the target path lexists try: st = os.lstat(new_root) - except (IOError, OSError): + except OSError: return # Empty directories are fine @@ -861,7 +861,7 @@ def regenerate(self, concrete_roots: List[Spec]) -> None: ): try: shutil.rmtree(old_root) - except (IOError, OSError) as e: + except OSError as e: msg = "Failed to remove old view at %s\n" % old_root msg += str(e) tty.warn(msg) @@ -2554,7 +2554,7 @@ def is_latest_format(manifest): try: with open(manifest, encoding="utf-8") as f: data = syaml.load(f) - except (OSError, IOError): + except OSError: return True top_level_key = _top_level_key(data) changed = spack.schema.env.update(data[top_level_key]) diff --git a/lib/spack/spack/extensions.py b/lib/spack/spack/extensions.py index 4cface72f88..76626d93a97 100644 --- a/lib/spack/spack/extensions.py +++ b/lib/spack/spack/extensions.py @@ -187,7 +187,7 @@ def path_for_extension(target_name: str, *, paths: List[str]) -> str: if name == target_name: return path else: - raise IOError('extension "{0}" not found'.format(target_name)) + raise OSError('extension "{0}" not found'.format(target_name)) def get_module(cmd_name): diff --git a/lib/spack/spack/filesystem_view.py b/lib/spack/spack/filesystem_view.py index 74b01190068..86efd8e2f56 100644 --- a/lib/spack/spack/filesystem_view.py +++ b/lib/spack/spack/filesystem_view.py @@ -427,7 +427,7 @@ def needs_file(spec, file): try: with open(manifest_file, "r", encoding="utf-8") as f: manifest = s_json.load(f) - except (OSError, IOError): + except OSError: # if we can't load it, assume it doesn't know about the file. manifest = {} return test_path in manifest @@ -831,7 +831,7 @@ def get_spec_from_file(filename): try: with open(filename, "r", encoding="utf-8") as f: return spack.spec.Spec.from_yaml(f) - except IOError: + except OSError: return None diff --git a/lib/spack/spack/hooks/absolutify_elf_sonames.py b/lib/spack/spack/hooks/absolutify_elf_sonames.py index 32277bfc44c..aaf8194510c 100644 --- a/lib/spack/spack/hooks/absolutify_elf_sonames.py +++ b/lib/spack/spack/hooks/absolutify_elf_sonames.py @@ -26,7 +26,7 @@ def is_shared_library_elf(filepath): with open(filepath, "rb") as f: elf = parse_elf(f, interpreter=True, dynamic_section=True) return elf.has_pt_dynamic and (elf.has_soname or not elf.has_pt_interp) - except (IOError, OSError, ElfParsingError): + except (OSError, ElfParsingError): return False diff --git a/lib/spack/spack/hooks/sbang.py b/lib/spack/spack/hooks/sbang.py index b8b3683d8b2..5c0a3ac8561 100644 --- a/lib/spack/spack/hooks/sbang.py +++ b/lib/spack/spack/hooks/sbang.py @@ -166,7 +166,7 @@ def filter_shebangs_in_directory(directory, filenames=None): # Only look at executable, non-symlink files. try: st = os.lstat(path) - except (IOError, OSError): + except OSError: continue if stat.S_ISLNK(st.st_mode) or stat.S_ISDIR(st.st_mode) or not st.st_mode & is_exe: diff --git a/lib/spack/spack/repo.py b/lib/spack/spack/repo.py index 62aaafaf638..e4a945f0bfe 100644 --- a/lib/spack/spack/repo.py +++ b/lib/spack/spack/repo.py @@ -1041,7 +1041,7 @@ def _read_config(self) -> Dict[str, str]: return yaml_data["repo"] - except IOError: + except OSError: tty.die(f"Error reading {self.config_file} when opening {self.root}") def get(self, spec: "spack.spec.Spec") -> "spack.package_base.PackageBase": @@ -1369,7 +1369,7 @@ def create_repo(root, namespace=None, subdir=packages_dir_name): if subdir != packages_dir_name: config.write(f" subdirectory: '{subdir}'\n") - except (IOError, OSError) as e: + except OSError as e: # try to clean up. if existed: shutil.rmtree(config_path, ignore_errors=True) diff --git a/lib/spack/spack/test/install.py b/lib/spack/spack/test/install.py index be80fdfaa79..7060bb26934 100644 --- a/lib/spack/spack/test/install.py +++ b/lib/spack/spack/test/install.py @@ -458,7 +458,7 @@ def test_log_install_without_build_files(install_mockery): spec = spack.concretize.concretize_one("trivial-install-test-package") # Attempt installing log without the build log file - with pytest.raises(IOError, match="No such file or directory"): + with pytest.raises(OSError, match="No such file or directory"): spack.installer.log(spec.package) diff --git a/lib/spack/spack/test/installer.py b/lib/spack/spack/test/installer.py index 776d27873e9..ee716069f65 100644 --- a/lib/spack/spack/test/installer.py +++ b/lib/spack/spack/test/installer.py @@ -470,7 +470,7 @@ def _repoerr(repo, name): # The call to install_tree will raise the exception since not mocking # creation of dependency package files within *install* directories. - with pytest.raises(IOError, match=path if sys.platform != "win32" else ""): + with pytest.raises(OSError, match=path if sys.platform != "win32" else ""): inst.dump_packages(spec, path) # Now try the error path, which requires the mock directory structure diff --git a/lib/spack/spack/test/llnl/util/filesystem.py b/lib/spack/spack/test/llnl/util/filesystem.py index 593f7c75c74..14709ec2852 100644 --- a/lib/spack/spack/test/llnl/util/filesystem.py +++ b/lib/spack/spack/test/llnl/util/filesystem.py @@ -82,7 +82,7 @@ def test_non_existing_src(self, stage): """Test using a non-existing source.""" with fs.working_dir(str(stage)): - with pytest.raises(IOError, match="No such file or directory"): + with pytest.raises(OSError, match="No such file or directory"): fs.copy("source/none", "dest") def test_multiple_src_file_dest(self, stage): @@ -139,7 +139,7 @@ def test_non_existing_src(self, stage): """Test using a non-existing source.""" with fs.working_dir(str(stage)): - with pytest.raises(IOError, match="No such file or directory"): + with pytest.raises(OSError, match="No such file or directory"): fs.install("source/none", "dest") def test_multiple_src_file_dest(self, stage): @@ -220,7 +220,7 @@ def test_non_existing_src(self, stage): """Test using a non-existing source.""" with fs.working_dir(str(stage)): - with pytest.raises(IOError, match="No such file or directory"): + with pytest.raises(OSError, match="No such file or directory"): fs.copy_tree("source/none", "dest") def test_parent_dir(self, stage): @@ -301,7 +301,7 @@ def test_non_existing_src(self, stage): """Test using a non-existing source.""" with fs.working_dir(str(stage)): - with pytest.raises(IOError, match="No such file or directory"): + with pytest.raises(OSError, match="No such file or directory"): fs.install_tree("source/none", "dest") def test_parent_dir(self, stage): diff --git a/lib/spack/spack/test/llnl/util/lock.py b/lib/spack/spack/test/llnl/util/lock.py index 8e666583423..8e737b2bfde 100644 --- a/lib/spack/spack/test/llnl/util/lock.py +++ b/lib/spack/spack/test/llnl/util/lock.py @@ -1339,7 +1339,7 @@ def test_poll_lock_exception(tmpdir, monkeypatch, err_num, err_msg): """Test poll lock exception handling.""" def _lockf(fd, cmd, len, start, whence): - raise IOError(err_num, err_msg) + raise OSError(err_num, err_msg) with tmpdir.as_cwd(): lockfile = "lockfile" @@ -1351,7 +1351,7 @@ def _lockf(fd, cmd, len, start, whence): if err_num in [errno.EAGAIN, errno.EACCES]: assert not lock._poll_lock(fcntl.LOCK_EX) else: - with pytest.raises(IOError, match=err_msg): + with pytest.raises(OSError, match=err_msg): lock._poll_lock(fcntl.LOCK_EX) monkeypatch.undo() diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py index ada1d789aae..33cb66b333e 100644 --- a/lib/spack/spack/test/modules/tcl.py +++ b/lib/spack/spack/test/modules/tcl.py @@ -238,10 +238,7 @@ def test_exclude(self, modulefile_content, module_configuration, host_architectu assert len([x for x in content if "module load " in x]) == 1 - # Catch "Exception" to avoid using FileNotFoundError on Python 3 - # and IOError on Python 2 or common bases like EnvironmentError - # which are not officially documented - with pytest.raises(Exception): + with pytest.raises(FileNotFoundError): modulefile_content(f"callpath target={host_architecture_str}") content = modulefile_content(f"zmpi target={host_architecture_str}") diff --git a/lib/spack/spack/test/util/editor.py b/lib/spack/spack/test/util/editor.py index d2d2a5402f7..f244d4bd8cf 100644 --- a/lib/spack/spack/test/util/editor.py +++ b/lib/spack/spack/test/util/editor.py @@ -204,13 +204,13 @@ def test_no_editor(): def assert_exec(exe, args): assert False - with pytest.raises(EnvironmentError, match=r"No text editor found.*"): + with pytest.raises(OSError, match=r"No text editor found.*"): 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.*"): + with pytest.raises(OSError, match=r"No text editor found.*"): ed.editor("/path/to/file", exec_fn=assert_exec) @@ -220,5 +220,5 @@ def test_exec_fn_executable(editor_var, good_exe, bad_exe): assert ed.editor(exec_fn=ed.executable) os.environ[editor_var] = bad_exe - with pytest.raises(EnvironmentError, match=r"No text editor found.*"): + with pytest.raises(OSError, 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 9702e31f003..11b40ec356b 100644 --- a/lib/spack/spack/util/editor.py +++ b/lib/spack/spack/util/editor.py @@ -9,7 +9,7 @@ defined by the EDITOR environment variable if VISUAL is not set or the specified editor fails (e.g. no DISPLAY for a graphical editor). If neither variable is set, we fall back to one of several common editors, -raising an EnvironmentError if we are unable to find one. +raising an OSError if we are unable to find one. """ import os import shlex @@ -141,7 +141,7 @@ def try_env_var(var): return True # Fail if nothing could be found - raise EnvironmentError( + raise OSError( "No text editor found! Please set the VISUAL and/or EDITOR " "environment variable(s) to your preferred text editor." ) diff --git a/lib/spack/spack/util/ld_so_conf.py b/lib/spack/spack/util/ld_so_conf.py index ca26f895b15..7198deeba99 100644 --- a/lib/spack/spack/util/ld_so_conf.py +++ b/lib/spack/spack/util/ld_so_conf.py @@ -46,7 +46,7 @@ def _process_ld_so_conf_queue(queue): try: with open(p, "rb") as f: lines = f.readlines() - except (IOError, OSError): + except OSError: continue for line in lines: @@ -132,7 +132,7 @@ def host_dynamic_linker_search_paths(): if os.path.exists(possible_conf): conf_file = possible_conf - except (IOError, OSError, elf_utils.ElfParsingError): + except (OSError, elf_utils.ElfParsingError): pass # Note: ld_so_conf doesn't error if the file does not exist. diff --git a/var/spack/repos/builtin/packages/charmpp/package.py b/var/spack/repos/builtin/packages/charmpp/package.py index 075c3279d00..56d7503d906 100644 --- a/var/spack/repos/builtin/packages/charmpp/package.py +++ b/var/spack/repos/builtin/packages/charmpp/package.py @@ -396,7 +396,7 @@ def install(self, spec, prefix): copy(filepath, tmppath) os.remove(filepath) os.rename(tmppath, filepath) - except (IOError, OSError): + except OSError: pass tmp_path = join_path(builddir, "tmp") diff --git a/var/spack/repos/builtin/packages/cp2k/package.py b/var/spack/repos/builtin/packages/cp2k/package.py index 135635736f6..0a0e336578c 100644 --- a/var/spack/repos/builtin/packages/cp2k/package.py +++ b/var/spack/repos/builtin/packages/cp2k/package.py @@ -763,8 +763,8 @@ def edit(self, pkg, spec, prefix): "Point environment variable LIBSMM_PATH to " "the absolute path of the libsmm.a file" ) - except IOError: - raise IOError( + except OSError: + raise OSError( "The file LIBSMM_PATH pointed to does not " "exist. Note that it must be absolute path." ) diff --git a/var/spack/repos/builtin/packages/perl/package.py b/var/spack/repos/builtin/packages/perl/package.py index 7eefd4dbb5a..3f8e1605826 100644 --- a/var/spack/repos/builtin/packages/perl/package.py +++ b/var/spack/repos/builtin/packages/perl/package.py @@ -331,7 +331,7 @@ def do_stage(self, mirror_only=False): try: perm = os.stat(filename).st_mode os.chmod(filename, perm | 0o200) - except IOError: + except OSError: continue def nmake_arguments(self):