From 66bb59016d3541f5501015110ea63943df2dbcd9 Mon Sep 17 00:00:00 2001 From: Dean Li Date: Tue, 28 Sep 2021 22:06:14 +0800 Subject: [PATCH] generalize IOErrorWithFilename to use PathOrFd --- Lib/test/test_fileio.py | 15 +++++++++++++-- Lib/test/test_urllib.py | 2 -- vm/src/stdlib/os.rs | 35 +++++++++++++++++++++++++---------- vm/src/stdlib/posix.rs | 15 +++++++++++---- 4 files changed, 49 insertions(+), 18 deletions(-) diff --git a/Lib/test/test_fileio.py b/Lib/test/test_fileio.py index 8b3151b1e..d923c9351 100644 --- a/Lib/test/test_fileio.py +++ b/Lib/test/test_fileio.py @@ -219,9 +219,10 @@ class AutoFileTests: self.assertRaises(TypeError, self.f.writelines) self.assertRaises(ValueError, self.f.writelines, b'') - # TODO: RUSTPYTHON - @unittest.expectedFailure def testOpendir(self): + # TODO: RUSTPYTHON + if sys.platform == "win32": + testOpendir = unittest.expectedFailure(testOpendir) # Issue 3703: opening a directory should fill the errno # Windows always returns "[Errno 13]: Permission denied # Unix uses fstat and returns "[Errno 21]: Is a directory" @@ -379,11 +380,21 @@ class CAutoFileTests(AutoFileTests, unittest.TestCase): def testOpenDirFD(self): super().testOpenDirFD() + # TODO: RUSTPYTHON + @unittest.expectedFailure + def testOpendir(self): + super().testOpendir() + @unittest.skipIf(sys.platform == "win32", "TODO: RUSTPYTHON, test setUp errors on Windows") class PyAutoFileTests(AutoFileTests, unittest.TestCase): FileIO = _pyio.FileIO modulename = '_pyio' + # TODO: RUSTPYTHON + @unittest.expectedFailure + def testOpendir(self): + super().testOpendir() + class OtherFileTests: diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py index 1a82a9d30..46f7c1075 100644 --- a/Lib/test/test_urllib.py +++ b/Lib/test/test_urllib.py @@ -516,8 +516,6 @@ Connection: close finally: self.unfakehttp() - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_missing_localfile(self): # Test for #10836 with self.assertRaises(urllib.error.URLError) as e: diff --git a/vm/src/stdlib/os.rs b/vm/src/stdlib/os.rs index aeb8362e3..c1e046749 100644 --- a/vm/src/stdlib/os.rs +++ b/vm/src/stdlib/os.rs @@ -53,6 +53,7 @@ impl OutputMode { } } +#[derive(Clone)] pub struct PyPathLike { pub path: PathBuf, pub(super) mode: OutputMode, @@ -205,6 +206,7 @@ impl TryFromObject for PyPathLike { } } +#[derive(Clone)] pub(crate) enum PathOrFd { Path(PyPathLike), Fd(i32), @@ -219,6 +221,12 @@ impl TryFromObject for PathOrFd { } } +impl From for PathOrFd { + fn from(path: PyPathLike) -> Self { + Self::Path(path) + } +} + impl IntoPyException for io::Error { fn into_pyexception(self, vm: &VirtualMachine) -> PyBaseExceptionRef { (&self).into_pyexception(vm) @@ -260,8 +268,11 @@ pub struct IOErrorWithFilename { } impl IOErrorWithFilename { - pub fn new(error: io::Error, path: &PyPathLike, vm: &VirtualMachine) -> Self { - let filename = path.filename(vm).unwrap_or_else(|_| vm.ctx.none()); + pub(crate) fn new(error: io::Error, path: impl Into, vm: &VirtualMachine) -> Self { + let filename = match path.into() { + PathOrFd::Path(path) => path.filename(vm).unwrap_or_else(|_| vm.ctx.none()), + PathOrFd::Fd(fd) => vm.ctx.new_int(fd), + }; IOErrorWithFilename { error, filename } } } @@ -486,7 +497,7 @@ pub(super) mod _os { }; #[cfg(not(windows))] let fd = { - let name = name.into_cstring(vm)?; + let name = name.clone().into_cstring(vm)?; #[cfg(not(target_os = "wasi"))] let flags = flags | libc::O_CLOEXEC; #[cfg(not(target_os = "redox"))] @@ -501,7 +512,8 @@ pub(super) mod _os { Fd::open(&name, flags, mode) } }; - fd.map(|fd| fd.0).map_err(|e| e.into_pyexception(vm)) + fd.map(|fd| fd.0) + .map_err(|e| IOErrorWithFilename::new(e, name, vm).into_pyexception(vm)) } #[pyfunction] @@ -543,7 +555,7 @@ pub(super) mod _os { } else { fs::remove_file(&path) }; - res.map_err(|err| IOErrorWithFilename::new(err, &path, vm).into_pyexception(vm)) + res.map_err(|err| IOErrorWithFilename::new(err, path, vm).into_pyexception(vm)) } #[cfg(not(windows))] @@ -580,7 +592,8 @@ pub(super) mod _os { #[pyfunction] fn rmdir(path: PyPathLike, dir_fd: DirFd<0>, vm: &VirtualMachine) -> PyResult<()> { let [] = dir_fd.0; - fs::remove_dir(path).map_err(|err| err.into_pyexception(vm)) + fs::remove_dir(&path) + .map_err(|err| IOErrorWithFilename::new(err, path, vm).into_pyexception(vm)) } const LISTDIR_FD: bool = cfg!(all(unix, not(target_os = "redox"))); @@ -594,7 +607,9 @@ pub(super) mod _os { dir_iter .map(|entry| match entry { Ok(entry_path) => path.mode.process_path(entry_path.file_name(), vm), - Err(e) => Err(IOErrorWithFilename::new(e, &path, vm).into_pyexception(vm)), + Err(e) => { + Err(IOErrorWithFilename::new(e, path.clone(), vm).into_pyexception(vm)) + } }) .collect::>()? } @@ -1146,8 +1161,8 @@ pub(super) mod _os { follow_symlinks: FollowSymlinks, vm: &VirtualMachine, ) -> PyResult { - let stat = stat_inner(file, dir_fd, follow_symlinks) - .map_err(|e| e.into_pyexception(vm))? + let stat = stat_inner(file.clone(), dir_fd, follow_symlinks) + .map_err(|e| IOErrorWithFilename::new(e, file, vm).into_pyexception(vm))? .ok_or_else(|| crate::exceptions::cstring_error(vm))?; Ok(StatResult::from_stat(&stat).into_pyobject(vm)) } @@ -1186,7 +1201,7 @@ pub(super) mod _os { #[pyfunction] fn chdir(path: PyPathLike, vm: &VirtualMachine) -> PyResult<()> { env::set_current_dir(&path.path) - .map_err(|err| IOErrorWithFilename::new(err, &path, vm).into_pyexception(vm)) + .map_err(|err| IOErrorWithFilename::new(err, path, vm).into_pyexception(vm)) } #[pyfunction] diff --git a/vm/src/stdlib/posix.rs b/vm/src/stdlib/posix.rs index 94d218a2d..6e57c3617 100644 --- a/vm/src/stdlib/posix.rs +++ b/vm/src/stdlib/posix.rs @@ -36,7 +36,7 @@ pub mod module { slots::SlotConstructor, stdlib::os::{ errno_err, DirFd, FollowSymlinks, PathOrFd, PyPathLike, SupportFunc, TargetIsDirectory, - _os, fs_metadata, + _os, fs_metadata, IOErrorWithFilename, }, utils::{Either, ToCString}, IntoPyObject, ItemProtocol, PyObjectRef, PyResult, PyValue, TryFromObject, VirtualMachine, @@ -369,10 +369,16 @@ pub mod module { let dir_fd = dir_fd.get_opt(); match path { - PathOrFd::Path(p) => nix::unistd::fchownat(dir_fd, p.path.as_os_str(), uid, gid, flag), + PathOrFd::Path(ref p) => { + nix::unistd::fchownat(dir_fd, p.path.as_os_str(), uid, gid, flag) + } PathOrFd::Fd(fd) => nix::unistd::fchown(fd, uid, gid), } - .map_err(|err| err.into_pyexception(vm)) + .map_err(|err| { + // Use `From for io::Error` when it is available + IOErrorWithFilename::new(io::Error::from_raw_os_error(err as i32), path, vm) + .into_pyexception(vm) + }) } #[cfg(not(target_os = "redox"))] @@ -694,6 +700,7 @@ pub mod module { vm: &VirtualMachine, ) -> PyResult<()> { let [] = dir_fd.0; + let err_path = path.clone(); let body = move || { use std::os::unix::fs::PermissionsExt; let meta = fs_metadata(&path, follow_symlinks.0)?; @@ -701,7 +708,7 @@ pub mod module { permissions.set_mode(mode); fs::set_permissions(&path, permissions) }; - body().map_err(|err| err.into_pyexception(vm)) + body().map_err(|err| IOErrorWithFilename::new(err, err_path, vm).into_pyexception(vm)) } #[pyfunction]