Skip to content

Commit 5d681e0

Browse files
HeliozoaHeliozoa
authored
Merge pull request #45 from rage/subprocess
change TmcCommand to use file IO and use subprocess to timeout properly
2 parents f0e99d7 + 1a425ff commit 5d681e0

File tree

15 files changed

+328
-383
lines changed

15 files changed

+328
-383
lines changed

‎plugins/csharp/src/plugin.rs‎

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ use std::io::{BufReader, Cursor, Read, Seek};
1111
use std::path::{Path, PathBuf};
1212
use std::time::Duration;
1313
use tmc_langs_framework::{
14-
command::{OutputWithTimeout, TmcCommand},
14+
command::TmcCommand,
1515
domain::{
1616
ExerciseDesc, RunResult, RunStatus, Strategy, TestDesc, TestResult, ValidationResult,
1717
},
18-
error::FileIo,
18+
error::{CommandError, FileIo},
1919
io::file_util,
2020
nom::{bytes, character, combinator, sequence, IResult},
2121
plugin::Language,
@@ -160,12 +160,14 @@ impl LanguagePlugin for CSharpPlugin {
160160
file_util::remove_file(&exercise_desc_json_path)?;
161161
}
162162

163-
let mut command = TmcCommand::new("dotnet".to_string());
164-
command
165-
.current_dir(path)
166-
.arg(Self::get_bootstrap_path()?)
167-
.arg("--generate-points-file");
168-
command.output_checked()?;
163+
let bootstrap_path = Self::get_bootstrap_path()?;
164+
let _output = TmcCommand::new_with_file_io("dotnet")?
165+
.with(|e| {
166+
e.cwd(path)
167+
.arg(bootstrap_path)
168+
.arg("--generate-points-file")
169+
})
170+
.output_checked()?;
169171

170172
let exercise_desc_json = file_util::open_file(&exercise_desc_json_path)?;
171173
let json: HashMap<String, Vec<String>> =
@@ -193,30 +195,25 @@ impl LanguagePlugin for CSharpPlugin {
193195
file_util::remove_file(&test_results_path)?;
194196
}
195197

196-
let mut command = TmcCommand::new("dotnet".to_string());
197-
command
198-
.current_dir(path)
199-
.arg(Self::get_bootstrap_path()?)
200-
.arg("--run-tests");
198+
let bootstrap_path = Self::get_bootstrap_path()?;
199+
let command = TmcCommand::new_with_file_io("dotnet")?
200+
.with(|e| e.cwd(path).arg(bootstrap_path).arg("--run-tests"));
201201
let output = if let Some(timeout) = timeout {
202-
command.wait_with_timeout(timeout)?
202+
command.output_with_timeout(timeout)
203203
} else {
204-
OutputWithTimeout::Output(command.output()?)
204+
command.output()
205205
};
206-
log::trace!("stdout: {}", String::from_utf8_lossy(output.stdout()));
207-
log::debug!("stderr: {}", String::from_utf8_lossy(output.stderr()));
208-
let mut logs = HashMap::new();
209-
logs.insert(
210-
"stdout".to_string(),
211-
String::from_utf8_lossy(&output.stdout()).into_owned(),
212-
);
213-
logs.insert(
214-
"stderr".to_string(),
215-
String::from_utf8_lossy(&output.stderr()).into_owned(),
216-
);
217206

218207
match output {
219-
OutputWithTimeout::Output(output) => {
208+
Ok(output) => {
209+
let stdout = String::from_utf8_lossy(&output.stdout);
210+
let stderr = String::from_utf8_lossy(&output.stderr);
211+
log::trace!("stdout: {}", stdout);
212+
log::debug!("stderr: {}", stderr);
213+
let mut logs = HashMap::new();
214+
logs.insert("stdout".to_string(), stdout.into_owned());
215+
logs.insert("stderr".to_string(), stderr.into_owned());
216+
220217
if !output.status.success() {
221218
return Ok(RunResult {
222219
status: RunStatus::CompileFailed,
@@ -226,9 +223,13 @@ impl LanguagePlugin for CSharpPlugin {
226223
}
227224
Self::parse_test_results(&test_results_path, logs).map_err(|e| e.into())
228225
}
229-
OutputWithTimeout::Timeout { .. } => Ok(RunResult {
230-
status: RunStatus::TestsFailed,
231-
test_results: vec![TestResult {
226+
Err(TmcError::Command(CommandError::TimeOut { stdout, stderr, .. })) => {
227+
let mut logs = HashMap::new();
228+
logs.insert("stdout".to_string(), stdout);
229+
logs.insert("stderr".to_string(), stderr);
230+
Ok(RunResult {
231+
status: RunStatus::TestsFailed,
232+
test_results: vec![TestResult {
232233
name: "Timeout test".to_string(),
233234
successful: false,
234235
points: vec![],
@@ -237,8 +238,10 @@ impl LanguagePlugin for CSharpPlugin {
237238
.to_string(),
238239
exception: vec![],
239240
}],
240-
logs,
241-
}),
241+
logs,
242+
})
243+
}
244+
Err(error) => Err(error),
242245
}
243246
}
244247

‎plugins/java/src/ant.rs‎

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ use policy::AntStudentFilePolicy;
88
use std::env;
99
use std::ffi::OsStr;
1010
use std::path::{Path, PathBuf};
11-
use std::process::Stdio;
1211
use std::time::Duration;
1312
use tmc_langs_framework::{
1413
command::TmcCommand,
@@ -32,14 +31,11 @@ impl AntPlugin {
3231

3332
fn get_ant_executable(&self) -> &'static str {
3433
if cfg!(windows) {
35-
if let Ok(status) = TmcCommand::new("ant".to_string())
36-
.arg("-version")
37-
.stdout(Stdio::piped())
38-
.stderr(Stdio::piped())
39-
.status()
40-
{
41-
if status.success() {
42-
return "ant";
34+
if let Ok(command) = TmcCommand::new_with_file_io("ant") {
35+
if let Ok(status) = command.with(|e| e.arg("-version")).status() {
36+
if status.success() {
37+
return "ant";
38+
}
4339
}
4440
}
4541
// if ant not found on windows, try ant.bat
@@ -121,13 +117,9 @@ impl LanguagePlugin for AntPlugin {
121117
let stderr = file_util::create_file(&stderr_path)?;
122118

123119
let ant_exec = self.get_ant_executable();
124-
let mut command = TmcCommand::new(ant_exec.to_string());
125-
command
126-
.arg("clean")
127-
.stdout(stdout)
128-
.stderr(stderr)
129-
.current_dir(path);
130-
command.output_checked()?;
120+
let _output = TmcCommand::new(ant_exec.to_string())
121+
.with(|e| e.arg("clean").stdout(stdout).stderr(stderr).cwd(path))
122+
.output_checked()?;
131123
file_util::remove_file(&stdout_path)?;
132124
file_util::remove_file(&stderr_path)?;
133125
Ok(())
@@ -192,9 +184,9 @@ impl JavaPlugin for AntPlugin {
192184

193185
// TODO: don't require ant in path?
194186
let ant_exec = self.get_ant_executable();
195-
let mut command = TmcCommand::new(ant_exec.to_string());
196-
command.arg("compile-test").current_dir(project_root_path);
197-
let output = command.output()?;
187+
let output = TmcCommand::new_with_file_io(ant_exec)?
188+
.with(|e| e.arg("compile-test").cwd(project_root_path))
189+
.output()?;
198190

199191
log::debug!("stdout: {}", String::from_utf8_lossy(&output.stdout));
200192
log::debug!("stderr: {}", String::from_utf8_lossy(&output.stderr));
@@ -259,9 +251,9 @@ impl JavaPlugin for AntPlugin {
259251
}
260252

261253
log::debug!("java args {} in {}", arguments.join(" "), path.display());
262-
let mut command = TmcCommand::new("java".to_string());
263-
command.current_dir(path).args(arguments);
264-
let output = command.output()?;
254+
let output = TmcCommand::new_with_file_io("java")?
255+
.with(|e| e.cwd(path).args(&arguments))
256+
.output()?;
265257

266258
Ok(TestRun {
267259
test_results: result_file,

‎plugins/java/src/lib.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@ use j4rs::{ClasspathEntry, Jvm, JvmBuilder};
1313
use serde::Deserialize;
1414
use std::fmt::Display;
1515
use std::path::PathBuf;
16-
use std::process::ExitStatus;
17-
use tmc_langs_framework::io::file_util;
16+
use tmc_langs_framework::{io::file_util, subprocess::ExitStatus};
1817

1918
#[cfg(windows)]
2019
const SEPARATOR: &str = ";";

‎plugins/java/src/maven.rs‎

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use policy::MavenStudentFilePolicy;
99
use std::ffi::OsString;
1010
use std::io::Cursor;
1111
use std::path::{Path, PathBuf};
12-
use std::process::Stdio;
1312
use std::time::Duration;
1413
use tar::Archive;
1514
use tmc_langs_framework::{
@@ -39,10 +38,8 @@ impl MavenPlugin {
3938
// finally, return the path to the extracted executable
4039
fn get_mvn_command() -> Result<OsString, JavaError> {
4140
// check if mvn is in PATH
42-
if let Ok(status) = TmcCommand::new("mvn".to_string())
43-
.arg("--version")
44-
.stdout(Stdio::piped())
45-
.stderr(Stdio::piped())
41+
if let Ok(status) = TmcCommand::new_with_file_io("mvn")?
42+
.with(|e| e.arg("--version"))
4643
.status()
4744
{
4845
if status.success() {
@@ -116,9 +113,9 @@ impl LanguagePlugin for MavenPlugin {
116113
log::info!("Cleaning maven project at {}", path.display());
117114

118115
let mvn_command = Self::get_mvn_command()?;
119-
let mut command = TmcCommand::named("maven", mvn_command);
120-
command.current_dir(path).arg("clean");
121-
command.output_checked()?;
116+
let _output = TmcCommand::new_with_file_io(mvn_command)?
117+
.with(|e| e.cwd(path).arg("clean"))
118+
.output_checked()?;
122119

123120
Ok(())
124121
}
@@ -151,12 +148,13 @@ impl JavaPlugin for MavenPlugin {
151148

152149
let output_arg = format!("-Dmdep.outputFile={}", class_path_file.display());
153150
let mvn_path = Self::get_mvn_command()?;
154-
let mut command = TmcCommand::named("maven", &mvn_path);
155-
command
156-
.current_dir(path)
157-
.arg("dependency:build-classpath")
158-
.arg(output_arg);
159-
command.output_checked()?;
151+
let _output = TmcCommand::new_with_file_io(mvn_path)?
152+
.with(|e| {
153+
e.cwd(path)
154+
.arg("dependency:build-classpath")
155+
.arg(output_arg)
156+
})
157+
.output_checked()?;
160158

161159
let class_path = file_util::read_file_to_string(&class_path_file)?;
162160
if class_path.is_empty() {
@@ -178,13 +176,14 @@ impl JavaPlugin for MavenPlugin {
178176
log::info!("Building maven project at {}", project_root_path.display());
179177

180178
let mvn_path = Self::get_mvn_command()?;
181-
let mut command = TmcCommand::named("maven", &mvn_path);
182-
command
183-
.current_dir(project_root_path)
184-
.arg("clean")
185-
.arg("compile")
186-
.arg("test-compile");
187-
let output = command.output_checked()?;
179+
let output = TmcCommand::new_with_file_io(mvn_path)?
180+
.with(|e| {
181+
e.cwd(project_root_path)
182+
.arg("clean")
183+
.arg("compile")
184+
.arg("test-compile")
185+
})
186+
.output_checked()?;
188187

189188
Ok(CompileResult {
190189
status_code: output.status,
@@ -201,11 +200,12 @@ impl JavaPlugin for MavenPlugin {
201200
log::info!("Running tests for maven project at {}", path.display());
202201

203202
let mvn_path = Self::get_mvn_command()?;
204-
let mut command = TmcCommand::named("maven", &mvn_path);
205-
command
206-
.current_dir(path)
207-
.arg("fi.helsinki.cs.tmc:tmc-maven-plugin:1.12:test");
208-
let output = command.output_checked()?;
203+
let output = TmcCommand::new_with_file_io(mvn_path)?
204+
.with(|e| {
205+
e.cwd(path)
206+
.arg("fi.helsinki.cs.tmc:tmc-maven-plugin:1.12:test")
207+
})
208+
.output_checked()?;
209209

210210
Ok(TestRun {
211211
test_results: path.join("target/test_output.txt"),

‎plugins/java/src/plugin.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use std::collections::HashMap;
66
use std::convert::TryFrom;
77
use std::ffi::OsStr;
88
use std::path::{Path, PathBuf};
9+
use std::time::Duration;
910
use tmc_langs_framework::{
1011
command::TmcCommand,
1112
domain::{ExerciseDesc, RunResult, RunStatus, TestDesc, TestResult, ValidationResult},
@@ -118,9 +119,9 @@ pub(crate) trait JavaPlugin: LanguagePlugin {
118119

119120
/// Tries to find the java.home property.
120121
fn get_java_home() -> Result<PathBuf, JavaError> {
121-
let mut command = TmcCommand::new("java".to_string());
122-
command.arg("-XshowSettings:properties").arg("-version");
123-
let output = command.output_checked()?;
122+
let output = TmcCommand::new_with_file_io("java")?
123+
.with(|e| e.arg("-XshowSettings:properties").arg("-version"))
124+
.output()?;
124125

125126
// information is printed to stderr
126127
let stderr = String::from_utf8_lossy(&output.stderr);

‎plugins/make/src/error.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,18 @@
22
33
use std::num::ParseIntError;
44
use std::path::PathBuf;
5-
use std::process::ExitStatus;
65
use thiserror::Error;
7-
use tmc_langs_framework::{error::FileIo, TmcError};
6+
use tmc_langs_framework::{error::FileIo, subprocess::ExitStatus, TmcError};
87

98
#[derive(Error, Debug)]
109
pub enum MakeError {
1110
#[error("No exercise found at {0}")]
1211
NoExerciseFound(PathBuf),
1312
#[error("Can't parse exercise description: could not find {0}")]
1413
CantFindAvailablePoints(PathBuf),
15-
#[error("Failed to run tests without valgrind. Exit code: {0}, stderr: {1}")]
14+
#[error("Failed to run tests without valgrind. Exit code: {0:?}, stderr: {1}")]
1615
RunningTests(ExitStatus, String),
17-
#[error("Failed to run tests with valgrind. Exit code: {0}, stderr: {1}")]
16+
#[error("Failed to run tests with valgrind. Exit code: {0:?}, stderr: {1}")]
1817
RunningTestsWithValgrind(ExitStatus, String),
1918
#[error("Failed to parse valgrind logs: could not find pids")]
2019
NoPidsInValgrindLogs,

‎plugins/make/src/plugin.rs‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,15 @@ use regex::Regex;
99
use std::collections::HashMap;
1010
use std::io::{self, BufRead, BufReader};
1111
use std::path::{Path, PathBuf};
12-
use std::process::Output;
1312
use std::time::Duration;
1413
use tmc_langs_framework::{
15-
command::TmcCommand,
14+
command::{Output, TmcCommand},
1615
domain::{ExerciseDesc, RunResult, RunStatus, TestDesc, TmcProjectYml},
1716
error::{CommandError, FileIo},
1817
io::file_util,
1918
nom::{bytes, character, combinator, sequence, IResult},
2019
plugin::LanguagePlugin,
20+
subprocess::PopenError,
2121
TmcError,
2222
};
2323

@@ -82,9 +82,9 @@ impl MakePlugin {
8282
};
8383
log::info!("Running make {}", arg);
8484

85-
let mut command = TmcCommand::new("make".to_string());
86-
command.current_dir(path).arg(arg);
87-
let output = command.output()?;
85+
let output = TmcCommand::new_with_file_io("make")?
86+
.with(|e| e.cwd(path).arg(arg))
87+
.output()?;
8888

8989
log::trace!("stdout: {}", String::from_utf8_lossy(&output.stdout));
9090
let stderr = String::from_utf8_lossy(&output.stderr);
@@ -108,9 +108,9 @@ impl MakePlugin {
108108
/// the process finished successfully or not.
109109
fn build(&self, dir: &Path) -> Result<Output, MakeError> {
110110
log::debug!("building {}", dir.display());
111-
let mut command = TmcCommand::new("make".to_string());
112-
command.current_dir(dir).arg("test");
113-
let output = command.output()?;
111+
let output = TmcCommand::new_with_file_io("make")?
112+
.with(|e| e.cwd(dir).arg("test"))
113+
.output()?;
114114

115115
log::trace!("stdout:\n{}", String::from_utf8_lossy(&output.stdout));
116116
log::debug!("stderr:\n{}", String::from_utf8_lossy(&output.stderr));
@@ -172,8 +172,8 @@ impl LanguagePlugin for MakePlugin {
172172
Err(error) => match error {
173173
MakeError::Tmc(TmcError::Command(command_error)) => {
174174
match command_error {
175-
CommandError::Spawn(_, io_error)
176-
| CommandError::FailedToRun(_, io_error)
175+
CommandError::Popen(_, PopenError::IoError(io_error))
176+
| CommandError::FailedToRun(_, PopenError::IoError(io_error))
177177
if io_error.kind() == io::ErrorKind::PermissionDenied =>
178178
{
179179
// failed due to lacking permissions, try to clean and rerun
@@ -290,9 +290,9 @@ impl LanguagePlugin for MakePlugin {
290290

291291
// does not check for success
292292
fn clean(&self, path: &Path) -> Result<(), TmcError> {
293-
let mut command = TmcCommand::new("make".to_string());
294-
command.current_dir(path).arg("clean");
295-
let output = command.output()?;
293+
let output = TmcCommand::new_with_file_io("make")?
294+
.with(|e| e.cwd(path).arg("clean"))
295+
.output()?;
296296

297297
if output.status.success() {
298298
log::info!("Cleaned make project");

0 commit comments

Comments
 (0)