From a3ad97c786615a958c124a95677f10f1982523a8 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Fri, 9 Aug 2019 13:19:39 +0300 Subject: [PATCH 1/8] Add Popen.terminate --- tests/snippets/stdlib_subprocess.py | 5 +++++ vm/src/stdlib/os.rs | 2 +- vm/src/stdlib/subprocess.rs | 10 +++++++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/tests/snippets/stdlib_subprocess.py b/tests/snippets/stdlib_subprocess.py index fa1f3c30cd5..9e7d7fbfce1 100644 --- a/tests/snippets/stdlib_subprocess.py +++ b/tests/snippets/stdlib_subprocess.py @@ -1,6 +1,7 @@ import subprocess import time import sys +import signal from testutils import assertRaises @@ -33,3 +34,7 @@ else: # windows assert p.stdout.read() == b"test\r\n" + +p = subprocess.Popen(["sleep", "2"]) +p.terminate() +assert p.poll() == -signal.SIGTERM diff --git a/vm/src/stdlib/os.rs b/vm/src/stdlib/os.rs index bb8bdec6607..c2fff8c2e25 100644 --- a/vm/src/stdlib/os.rs +++ b/vm/src/stdlib/os.rs @@ -159,7 +159,7 @@ pub fn os_open(vm: &VirtualMachine, args: PyFuncArgs) -> PyResult { Ok(vm.ctx.new_int(raw_file_number(handle))) } -fn convert_io_error(vm: &VirtualMachine, err: io::Error) -> PyObjectRef { +pub fn convert_io_error(vm: &VirtualMachine, err: io::Error) -> PyObjectRef { let os_error = match err.kind() { ErrorKind::NotFound => { let exc_type = vm.ctx.exceptions.file_not_found_error.clone(); diff --git a/vm/src/stdlib/subprocess.rs b/vm/src/stdlib/subprocess.rs index dd49e247ecf..168a54e4f58 100644 --- a/vm/src/stdlib/subprocess.rs +++ b/vm/src/stdlib/subprocess.rs @@ -11,7 +11,7 @@ use crate::obj::objstr::{self, PyStringRef}; use crate::obj::objtype::PyClassRef; use crate::pyobject::{Either, IntoPyObject, PyObjectRef, PyRef, PyResult, PyValue}; use crate::stdlib::io::io_open; -use crate::stdlib::os::{raw_file_number, rust_file}; +use crate::stdlib::os::{convert_io_error, raw_file_number, rust_file}; use crate::vm::VirtualMachine; #[derive(Debug)] @@ -149,6 +149,13 @@ impl PopenRef { fn stderr(self, vm: &VirtualMachine) -> PyResult { convert_to_file_io(&self.process.borrow().stderr, "rb".to_string(), vm) } + + fn terminate(self, vm: &VirtualMachine) -> PyResult<()> { + self.process + .borrow_mut() + .terminate() + .map_err(|err| convert_io_error(vm, err)) + } } pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { @@ -165,6 +172,7 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { "stdin" => ctx.new_property(PopenRef::stdin), "stdout" => ctx.new_property(PopenRef::stdout), "stderr" => ctx.new_property(PopenRef::stderr), + "terminate" => ctx.new_rustfunc(PopenRef::terminate), }); let module = py_module!(vm, "subprocess", { From 572e3b6e9322e9d3267d8e6ea7f2e6adea2f3a10 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Fri, 9 Aug 2019 13:21:51 +0300 Subject: [PATCH 2/8] Add Popen.kill --- tests/snippets/stdlib_subprocess.py | 4 ++++ vm/src/stdlib/subprocess.rs | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/tests/snippets/stdlib_subprocess.py b/tests/snippets/stdlib_subprocess.py index 9e7d7fbfce1..0421610cc9b 100644 --- a/tests/snippets/stdlib_subprocess.py +++ b/tests/snippets/stdlib_subprocess.py @@ -38,3 +38,7 @@ p = subprocess.Popen(["sleep", "2"]) p.terminate() assert p.poll() == -signal.SIGTERM + +p = subprocess.Popen(["sleep", "2"]) +p.kill() +assert p.poll() == -signal.SIGKILL diff --git a/vm/src/stdlib/subprocess.rs b/vm/src/stdlib/subprocess.rs index 168a54e4f58..e214fc9fa44 100644 --- a/vm/src/stdlib/subprocess.rs +++ b/vm/src/stdlib/subprocess.rs @@ -156,6 +156,13 @@ impl PopenRef { .terminate() .map_err(|err| convert_io_error(vm, err)) } + + fn kill(self, vm: &VirtualMachine) -> PyResult<()> { + self.process + .borrow_mut() + .kill() + .map_err(|err| convert_io_error(vm, err)) + } } pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { @@ -173,6 +180,7 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { "stdout" => ctx.new_property(PopenRef::stdout), "stderr" => ctx.new_property(PopenRef::stderr), "terminate" => ctx.new_rustfunc(PopenRef::terminate), + "kill" => ctx.new_rustfunc(PopenRef::kill), }); let module = py_module!(vm, "subprocess", { From 93701098f94b4d19600158226be14c71fe239556 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Fri, 9 Aug 2019 14:27:28 +0300 Subject: [PATCH 3/8] Add Popen.communicate --- tests/snippets/stdlib_subprocess.py | 10 ++++++++-- vm/src/obj/objbytes.rs | 9 ++++++++- vm/src/obj/objtuple.rs | 25 ++++++++++++++++++++++++- vm/src/stdlib/subprocess.rs | 13 +++++++++++++ 4 files changed, 53 insertions(+), 4 deletions(-) diff --git a/tests/snippets/stdlib_subprocess.py b/tests/snippets/stdlib_subprocess.py index 0421610cc9b..f80a3775d66 100644 --- a/tests/snippets/stdlib_subprocess.py +++ b/tests/snippets/stdlib_subprocess.py @@ -30,10 +30,12 @@ if "win" not in sys.platform: # unix - assert p.stdout.read() == b"test\n" + test_output = b"test\n" else: # windows - assert p.stdout.read() == b"test\r\n" + test_output = b"test\r\n" + +assert p.stdout.read() == test_output p = subprocess.Popen(["sleep", "2"]) p.terminate() @@ -42,3 +44,7 @@ p = subprocess.Popen(["sleep", "2"]) p.kill() assert p.poll() == -signal.SIGKILL + +p = subprocess.Popen(["echo", "test"], stdout=subprocess.PIPE) +(stdout, stderr) = p.communicate() +assert stdout == test_output diff --git a/vm/src/obj/objbytes.rs b/vm/src/obj/objbytes.rs index 8e3149b17f2..c67a044b2b9 100644 --- a/vm/src/obj/objbytes.rs +++ b/vm/src/obj/objbytes.rs @@ -11,7 +11,8 @@ use std::ops::Deref; use crate::function::OptionalArg; use crate::pyobject::{ - PyClassImpl, PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, TryFromObject, + IntoPyObject, PyClassImpl, PyContext, PyIterable, PyObjectRef, PyRef, PyResult, PyValue, + TryFromObject, }; use super::objbyteinner::{ @@ -59,6 +60,12 @@ impl PyBytes { } } +impl IntoPyObject for Vec { + fn into_pyobject(self, vm: &VirtualMachine) -> PyResult { + Ok(vm.ctx.new_bytes(self)) + } +} + impl Deref for PyBytes { type Target = [u8]; diff --git a/vm/src/obj/objtuple.rs b/vm/src/obj/objtuple.rs index e13d3160fe4..5be1232c04e 100644 --- a/vm/src/obj/objtuple.rs +++ b/vm/src/obj/objtuple.rs @@ -3,7 +3,9 @@ use std::fmt; use crate::function::OptionalArg; use crate::pyhash; -use crate::pyobject::{IdProtocol, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue}; +use crate::pyobject::{ + IdProtocol, IntoPyObject, PyClassImpl, PyContext, PyObjectRef, PyRef, PyResult, PyValue, +}; use crate::vm::{ReprGuard, VirtualMachine}; use super::objbool; @@ -37,6 +39,27 @@ impl PyValue for PyTuple { } } +impl IntoPyObject for (A,) +where + A: IntoPyObject, +{ + fn into_pyobject(self, vm: &VirtualMachine) -> PyResult { + Ok(vm.ctx.new_tuple(vec![self.0.into_pyobject(vm)?])) + } +} + +impl IntoPyObject for (A, B) +where + A: IntoPyObject, + B: IntoPyObject, +{ + fn into_pyobject(self, vm: &VirtualMachine) -> PyResult { + Ok(vm + .ctx + .new_tuple(vec![self.0.into_pyobject(vm)?, self.1.into_pyobject(vm)?])) + } +} + impl PyTuple { pub fn fast_getitem(&self, idx: usize) -> PyObjectRef { self.elements[idx].clone() diff --git a/vm/src/stdlib/subprocess.rs b/vm/src/stdlib/subprocess.rs index e214fc9fa44..1e7b1e2624e 100644 --- a/vm/src/stdlib/subprocess.rs +++ b/vm/src/stdlib/subprocess.rs @@ -5,6 +5,7 @@ use std::time::Duration; use subprocess; use crate::function::OptionalArg; +use crate::obj::objbytes::PyBytesRef; use crate::obj::objlist::PyListRef; use crate::obj::objsequence; use crate::obj::objstr::{self, PyStringRef}; @@ -163,6 +164,17 @@ impl PopenRef { .kill() .map_err(|err| convert_io_error(vm, err)) } + + fn communicate( + self, + stdin: OptionalArg, + vm: &VirtualMachine, + ) -> PyResult<(Option>, Option>)> { + self.process + .borrow_mut() + .communicate_bytes(stdin.into_option().as_ref().map(|bytes| bytes.get_value())) + .map_err(|err| convert_io_error(vm, err)) + } } pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { @@ -181,6 +193,7 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { "stderr" => ctx.new_property(PopenRef::stderr), "terminate" => ctx.new_rustfunc(PopenRef::terminate), "kill" => ctx.new_rustfunc(PopenRef::kill), + "communicate" => ctx.new_rustfunc(PopenRef::communicate), }); let module = py_module!(vm, "subprocess", { From c44b1c843f59c8b5301b1c9529c05bdef7c207b8 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 10 Aug 2019 09:53:39 +0300 Subject: [PATCH 4/8] Make test less flaky --- tests/snippets/stdlib_subprocess.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/snippets/stdlib_subprocess.py b/tests/snippets/stdlib_subprocess.py index f80a3775d66..8d0dfe9205f 100644 --- a/tests/snippets/stdlib_subprocess.py +++ b/tests/snippets/stdlib_subprocess.py @@ -39,11 +39,13 @@ p = subprocess.Popen(["sleep", "2"]) p.terminate() -assert p.poll() == -signal.SIGTERM +p.wait() +assert p.returncode == -signal.SIGTERM p = subprocess.Popen(["sleep", "2"]) p.kill() -assert p.poll() == -signal.SIGKILL +p.wait() +assert p.returncode == -signal.SIGKILL p = subprocess.Popen(["echo", "test"], stdout=subprocess.PIPE) (stdout, stderr) = p.communicate() From 608572b2b527fc5a89daf2d4903a85689baabbbc Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 10 Aug 2019 09:58:40 +0300 Subject: [PATCH 5/8] Add Popen.pid --- vm/src/stdlib/subprocess.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/vm/src/stdlib/subprocess.rs b/vm/src/stdlib/subprocess.rs index 1e7b1e2624e..006d463a490 100644 --- a/vm/src/stdlib/subprocess.rs +++ b/vm/src/stdlib/subprocess.rs @@ -175,6 +175,10 @@ impl PopenRef { .communicate_bytes(stdin.into_option().as_ref().map(|bytes| bytes.get_value())) .map_err(|err| convert_io_error(vm, err)) } + + fn pid(self, _vm: &VirtualMachine) -> Option { + self.process.borrow().pid() + } } pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { @@ -194,6 +198,7 @@ pub fn make_module(vm: &VirtualMachine) -> PyObjectRef { "terminate" => ctx.new_rustfunc(PopenRef::terminate), "kill" => ctx.new_rustfunc(PopenRef::kill), "communicate" => ctx.new_rustfunc(PopenRef::communicate), + "pid" => ctx.new_property(PopenRef::pid), }); let module = py_module!(vm, "subprocess", { From 12c59dacfb77aafd13713593cfc2082366155020 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 10 Aug 2019 10:13:58 +0300 Subject: [PATCH 6/8] Add Popen cwd support --- vm/src/stdlib/subprocess.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/vm/src/stdlib/subprocess.rs b/vm/src/stdlib/subprocess.rs index 006d463a490..82cd42a5e1f 100644 --- a/vm/src/stdlib/subprocess.rs +++ b/vm/src/stdlib/subprocess.rs @@ -1,4 +1,5 @@ use std::cell::RefCell; +use std::ffi::OsString; use std::fs::File; use std::time::Duration; @@ -38,6 +39,8 @@ struct PopenArgs { stdout: Option, #[pyarg(positional_or_keyword, default = "None")] stderr: Option, + #[pyarg(positional_or_keyword, default = "None")] + cwd: Option, } impl IntoPyObject for subprocess::ExitStatus { @@ -96,6 +99,7 @@ impl PopenRef { .map(|x| objstr::get_value(x)) .collect(), }; + let cwd = args.cwd.map(|x| OsString::from(x.as_str())); let process = subprocess::Popen::create( &command_list, @@ -103,6 +107,7 @@ impl PopenRef { stdin, stdout, stderr, + cwd, ..Default::default() }, ) From ba31f4fde73ba53bb0bab34f28e128e508727b87 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 10 Aug 2019 19:40:52 +0300 Subject: [PATCH 7/8] Ignore clippy warning --- vm/src/stdlib/subprocess.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/vm/src/stdlib/subprocess.rs b/vm/src/stdlib/subprocess.rs index 82cd42a5e1f..3af308f7c5d 100644 --- a/vm/src/stdlib/subprocess.rs +++ b/vm/src/stdlib/subprocess.rs @@ -170,6 +170,7 @@ impl PopenRef { .map_err(|err| convert_io_error(vm, err)) } + #[allow(clippy::type_complexity)] fn communicate( self, stdin: OptionalArg, From f790de24018efe98738c9de9d4a7d47ba284edc7 Mon Sep 17 00:00:00 2001 From: Aviv Palivoda Date: Sat, 10 Aug 2019 22:28:33 +0300 Subject: [PATCH 8/8] Fix windows test --- tests/snippets/stdlib_subprocess.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/snippets/stdlib_subprocess.py b/tests/snippets/stdlib_subprocess.py index 8d0dfe9205f..1202d8ef71f 100644 --- a/tests/snippets/stdlib_subprocess.py +++ b/tests/snippets/stdlib_subprocess.py @@ -40,12 +40,18 @@ p = subprocess.Popen(["sleep", "2"]) p.terminate() p.wait() -assert p.returncode == -signal.SIGTERM +if "win" not in sys.platform: + assert p.returncode == -signal.SIGTERM +else: + assert p.returncode == 1 p = subprocess.Popen(["sleep", "2"]) p.kill() p.wait() -assert p.returncode == -signal.SIGKILL +if "win" not in sys.platform: + assert p.returncode == -signal.SIGKILL +else: + assert p.returncode == 1 p = subprocess.Popen(["echo", "test"], stdout=subprocess.PIPE) (stdout, stderr) = p.communicate()