From a59556ad2590fa7d034f0715c299dde2e7e66c17 Mon Sep 17 00:00:00 2001 From: Dean Li Date: Sat, 30 Oct 2021 13:30:40 +0800 Subject: [PATCH] Refactor OSError Implement `raw_os_error_to_exc_type` to reduce duplication of errno to exception type. --- Lib/test/test_importlib/source/test_finder.py | 2 - Lib/test/test_shutil.py | 2 - Lib/test/test_subprocess.py | 12 +-- Lib/test/test_tempfile.py | 8 -- vm/src/exceptions.rs | 76 +++++++++---------- vm/src/stdlib/os.rs | 22 +++--- 6 files changed, 50 insertions(+), 72 deletions(-) diff --git a/Lib/test/test_importlib/source/test_finder.py b/Lib/test/test_importlib/source/test_finder.py index 03993da62..f372b850d 100644 --- a/Lib/test/test_importlib/source/test_finder.py +++ b/Lib/test/test_importlib/source/test_finder.py @@ -178,8 +178,6 @@ class FinderTests(abc.FinderTests): found = self._find(finder, 'doesnotexist') self.assertEqual(found, self.NOT_FOUND) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_ignore_file(self): # If a directory got changed to a file from underneath us, then don't # worry about looking for submodules. diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 330584586..d6b9fc876 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -734,8 +734,6 @@ class TestShutil(unittest.TestCase): self.assertFalse(shutil._use_fd_functions) self.assertFalse(shutil.rmtree.avoids_symlink_attacks) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_rmtree_dont_delete_file(self): # When called on a file instead of a directory, don't delete it. handle, path = tempfile.mkstemp() diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 73ffbb97e..07c4bee78 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -1395,8 +1395,6 @@ class ProcessTestCase(BaseTestCase): self.assertFalse(os.path.exists(ofname)) self.assertFalse(os.path.exists(efname)) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_communicate_epipe(self): # Issue 10963: communicate() should hide EPIPE p = subprocess.Popen(ZERO_RETURN_CMD, @@ -1409,7 +1407,9 @@ class ProcessTestCase(BaseTestCase): p.communicate(b"x" * 2**20) # TODO: RUSTPYTHON - @unittest.expectedFailure + if sys.platform == "win32": + test_communicate_epipe = unittest.expectedFailure(test_communicate_epipe) + def test_communicate_epipe_only_stdin(self): # Issue 10963: communicate() should hide EPIPE p = subprocess.Popen(ZERO_RETURN_CMD, @@ -1418,6 +1418,10 @@ class ProcessTestCase(BaseTestCase): p.wait() p.communicate(b"x" * 2**20) + # TODO: RUSTPYTHON + if sys.platform == "win32": + test_communicate_epipe_only_stdin = unittest.expectedFailure(test_communicate_epipe_only_stdin) + # TODO: RUSTPYTHON @unittest.expectedFailure @unittest.skipUnless(hasattr(signal, 'SIGUSR1'), @@ -2753,8 +2757,6 @@ class POSIXProcessTestCase(BaseTestCase): stderr=inout, stdin=inout) p.wait() - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_wait_when_sigchild_ignored(self): # NOTE: sigchild_ignore.py may not be an effective test on all OSes. sigchild_ignore = support.findfile("sigchild_ignore.py", diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 3c2abcde1..3f8a6f0fd 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -358,10 +358,6 @@ class TestBadTempdir: with self.assertRaises((NotADirectoryError, FileNotFoundError)): self.make_temp() - # TODO: RUSTPYTHON - if sys.platform != "win32": - test_non_directory = unittest.expectedFailure(test_non_directory) - class TestMkstempInner(TestBadTempdir, BaseTestCase): """Test the internal function _mkstemp_inner.""" @@ -1503,10 +1499,6 @@ class TestTemporaryDirectory(BaseTestCase): d.cleanup() self.assertFalse(os.path.exists(d.name)) - # TODO: RUSTPYTHON - if sys.platform not in {"darwin", "win32"}: - test_modes = unittest.expectedFailure(test_modes) - @unittest.skipUnless(hasattr(os, 'chflags'), 'requires os.lchflags') def test_flags(self): flags = stat.UF_IMMUTABLE | stat.UF_NOUNLINK diff --git a/vm/src/exceptions.rs b/vm/src/exceptions.rs index 3307eb63b..ed34f050f 100644 --- a/vm/src/exceptions.rs +++ b/vm/src/exceptions.rs @@ -958,6 +958,33 @@ impl IntoPyException for widestring::NulError { } } +#[cfg(not(target_arch = "wasm32"))] +pub(crate) fn raw_os_error_to_exc_type(errno: i32, vm: &VirtualMachine) -> Option { + use crate::stdlib::errno::errors; + let excs = &vm.ctx.exceptions; + match errno { + errors::EWOULDBLOCK => Some(excs.blocking_io_error.clone()), + errors::EALREADY => Some(excs.blocking_io_error.clone()), + errors::EINPROGRESS => Some(excs.blocking_io_error.clone()), + errors::EPIPE => Some(excs.broken_pipe_error.clone()), + errors::ESHUTDOWN => Some(excs.broken_pipe_error.clone()), + errors::ECHILD => Some(excs.child_process_error.clone()), + errors::ECONNABORTED => Some(excs.connection_aborted_error.clone()), + errors::ECONNREFUSED => Some(excs.connection_refused_error.clone()), + errors::ECONNRESET => Some(excs.connection_reset_error.clone()), + errors::EEXIST => Some(excs.file_exists_error.clone()), + errors::ENOENT => Some(excs.file_not_found_error.clone()), + errors::EISDIR => Some(excs.is_a_directory_error.clone()), + errors::ENOTDIR => Some(excs.not_a_directory_error.clone()), + errors::EINTR => Some(excs.interrupted_error.clone()), + errors::EACCES => Some(excs.permission_error.clone()), + errors::EPERM => Some(excs.permission_error.clone()), + errors::ESRCH => Some(excs.process_lookup_error.clone()), + errors::ETIMEDOUT => Some(excs.timeout_error.clone()), + _ => None, + } +} + pub(super) mod types { use crate::common::lock::PyRwLock; use crate::{ @@ -1166,51 +1193,16 @@ pub(super) mod types { fn os_error_optional_new( args: Vec, vm: &VirtualMachine, - ) -> Option> { - use crate::stdlib::errno::errors; - + ) -> Option { let len = args.len(); if len >= 2 { let args = args.as_slice(); let errno = &args[0]; - let error = match errno.payload_if_subclass::(vm) { - Some(errno) => match errno.try_to_primitive::(vm) { - Ok(errno) => { - let excs = &vm.ctx.exceptions; - let error = match errno { - errors::EWOULDBLOCK => Some(excs.blocking_io_error.clone()), - errors::EALREADY => Some(excs.blocking_io_error.clone()), - errors::EINPROGRESS => Some(excs.blocking_io_error.clone()), - errors::EPIPE => Some(excs.broken_pipe_error.clone()), - errors::ESHUTDOWN => Some(excs.broken_pipe_error.clone()), - errors::ECHILD => Some(excs.child_process_error.clone()), - errors::ECONNABORTED => Some(excs.connection_aborted_error.clone()), - errors::ECONNREFUSED => Some(excs.connection_refused_error.clone()), - errors::ECONNRESET => Some(excs.connection_reset_error.clone()), - errors::EEXIST => Some(excs.file_exists_error.clone()), - errors::ENOENT => Some(excs.file_not_found_error.clone()), - errors::EISDIR => Some(excs.is_a_directory_error.clone()), - errors::ENOTDIR => Some(excs.not_a_directory_error.clone()), - errors::EINTR => Some(excs.interrupted_error.clone()), - errors::EACCES => Some(excs.permission_error.clone()), - errors::EPERM => Some(excs.permission_error.clone()), - errors::ESRCH => Some(excs.process_lookup_error.clone()), - errors::ETIMEDOUT => Some(excs.timeout_error.clone()), - _ => None, - }; - - if error.is_some() { - Some(vm.invoke_exception(error?, args.to_vec())) - } else { - None - } - } - Err(_) => None, - }, - None => None, - }; - - error + errno + .payload_if_subclass::(vm) + .and_then(|errno| errno.try_to_primitive::(vm).ok()) + .and_then(|errno| super::raw_os_error_to_exc_type(errno, vm)) + .and_then(|typ| vm.invoke_exception(typ, args.to_vec()).ok()) } else { None } @@ -1222,7 +1214,7 @@ pub(super) mod types { // See: `BaseException_new` if cls.name().deref() == vm.ctx.exceptions.os_error.name().deref() { match os_error_optional_new(args.args.to_vec(), vm) { - Some(error) => error.unwrap().into_pyresult(vm), + Some(error) => error.into_pyresult(vm), None => PyBaseException::slot_new(cls, args, vm), } } else { diff --git a/vm/src/stdlib/os.rs b/vm/src/stdlib/os.rs index 444eb1636..fef36ed50 100644 --- a/vm/src/stdlib/os.rs +++ b/vm/src/stdlib/os.rs @@ -1,4 +1,3 @@ -use super::errno::errors; use crate::crt_fd::Fd; use crate::{ builtins::{PyBaseExceptionRef, PyBytes, PyBytesRef, PyInt, PySet, PyStr, PyStrRef}, @@ -238,20 +237,17 @@ impl IntoPyException for io::Error { } impl IntoPyException for &'_ io::Error { fn into_pyexception(self, vm: &VirtualMachine) -> PyBaseExceptionRef { + let excs = &vm.ctx.exceptions; #[allow(unreachable_patterns)] // some errors are just aliases of each other let exc_type = match self.kind() { - ErrorKind::NotFound => vm.ctx.exceptions.file_not_found_error.clone(), - ErrorKind::PermissionDenied => vm.ctx.exceptions.permission_error.clone(), - ErrorKind::AlreadyExists => vm.ctx.exceptions.file_exists_error.clone(), - ErrorKind::WouldBlock => vm.ctx.exceptions.blocking_io_error.clone(), - _ => match self.raw_os_error() { - Some(errors::EAGAIN) - | Some(errors::EALREADY) - | Some(errors::EWOULDBLOCK) - | Some(errors::EINPROGRESS) => vm.ctx.exceptions.blocking_io_error.clone(), - Some(errors::ESRCH) => vm.ctx.exceptions.process_lookup_error.clone(), - _ => vm.ctx.exceptions.os_error.clone(), - }, + ErrorKind::NotFound => excs.file_not_found_error.clone(), + ErrorKind::PermissionDenied => excs.permission_error.clone(), + ErrorKind::AlreadyExists => excs.file_exists_error.clone(), + ErrorKind::WouldBlock => excs.blocking_io_error.clone(), + _ => self + .raw_os_error() + .and_then(|errno| crate::exceptions::raw_os_error_to_exc_type(errno, vm)) + .unwrap_or_else(|| excs.os_error.clone()), }; let errno = self.raw_os_error().into_pyobject(vm); let msg = vm.ctx.new_str(self.to_string()).into();