Skip to content

Commit 4b021f9

Browse files
HeliozoaHeliozoa
committed
further fixes
1 parent 7fb4b22 commit 4b021f9

File tree

9 files changed

+51
-85
lines changed

9 files changed

+51
-85
lines changed

‎plugins/csharp/src/plugin.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ impl LanguagePlugin for CSharpPlugin {
221221

222222
// run command
223223
let bootstrap_path = Self::get_bootstrap_path()?;
224-
let _output = TmcCommand::new_with_file_io("dotnet")?
224+
let _output = TmcCommand::piped("dotnet")
225225
.with(|e| {
226226
e.cwd(path)
227227
.arg(bootstrap_path)
@@ -261,7 +261,7 @@ impl LanguagePlugin for CSharpPlugin {
261261

262262
// run command
263263
let bootstrap_path = Self::get_bootstrap_path()?;
264-
let command = TmcCommand::new_with_file_io("dotnet")?
264+
let command = TmcCommand::piped("dotnet")
265265
.with(|e| e.cwd(path).arg(bootstrap_path).arg("--run-tests"));
266266
let output = if let Some(timeout) = timeout {
267267
command.output_with_timeout(timeout)

‎plugins/java/src/ant_plugin.rs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,10 @@ impl AntPlugin {
3232

3333
fn get_ant_executable(&self) -> &'static str {
3434
if cfg!(windows) {
35-
if let Ok(command) = TmcCommand::new_with_file_io("ant") {
36-
if let Ok(status) = command.with(|e| e.arg("-version")).status() {
37-
if status.success() {
38-
return "ant";
39-
}
35+
let command = TmcCommand::piped("ant");
36+
if let Ok(status) = command.with(|e| e.arg("-version")).status() {
37+
if status.success() {
38+
return "ant";
4039
}
4140
}
4241
// if ant not found on windows, try ant.bat
@@ -188,7 +187,7 @@ impl JavaPlugin for AntPlugin {
188187
log::info!("building project at {}", project_root_path.display());
189188

190189
let ant_exec = self.get_ant_executable();
191-
let output = TmcCommand::new_with_file_io(ant_exec)?
190+
let output = TmcCommand::piped(ant_exec)
192191
.with(|e| e.arg("compile-test").cwd(project_root_path))
193192
.output()?;
194193

@@ -263,7 +262,7 @@ impl JavaPlugin for AntPlugin {
263262
}
264263

265264
log::debug!("java args '{}' in {}", arguments.join(" "), path.display());
266-
let command = TmcCommand::new_with_file_io("java")?.with(|e| e.cwd(path).args(&arguments));
265+
let command = TmcCommand::piped("java").with(|e| e.cwd(path).args(&arguments));
267266
let output = if let Some(timeout) = timeout {
268267
command.output_with_timeout(timeout)?
269268
} else {

‎plugins/java/src/java_plugin.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ pub(crate) trait JavaPlugin: LanguagePlugin {
124124

125125
/// Tries to find the java.home property.
126126
fn get_java_home() -> Result<PathBuf, JavaError> {
127-
let output = TmcCommand::new_with_file_io("java")?
127+
let output = TmcCommand::piped("java")
128128
.with(|e| e.arg("-XshowSettings:properties").arg("-version"))
129129
.output()?;
130130

‎plugins/java/src/maven_plugin.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ impl MavenPlugin {
4040
// the executable used from within the extracted maven differs per platform
4141
fn get_mvn_command() -> Result<OsString, JavaError> {
4242
// check if mvn is in PATH
43-
if let Ok(status) = TmcCommand::new_with_file_io("mvn")?
43+
if let Ok(status) = TmcCommand::piped("mvn")
4444
.with(|e| e.arg("--batch-mode").arg("--version"))
4545
.status()
4646
{
@@ -118,7 +118,7 @@ impl LanguagePlugin for MavenPlugin {
118118
log::info!("Cleaning maven project at {}", path.display());
119119

120120
let mvn_command = Self::get_mvn_command()?;
121-
let _output = TmcCommand::new_with_file_io(mvn_command)?
121+
let _output = TmcCommand::piped(mvn_command)
122122
.with(|e| e.cwd(path).arg("--batch-mode").arg("clean"))
123123
.output_checked()?;
124124

@@ -153,7 +153,7 @@ impl JavaPlugin for MavenPlugin {
153153

154154
let output_arg = format!("-Dmdep.outputFile={}", class_path_file.display());
155155
let mvn_path = Self::get_mvn_command()?;
156-
let _output = TmcCommand::new_with_file_io(mvn_path)?
156+
let _output = TmcCommand::piped(mvn_path)
157157
.with(|e| {
158158
e.cwd(path)
159159
.arg("--batch-mode")
@@ -182,7 +182,7 @@ impl JavaPlugin for MavenPlugin {
182182
log::info!("Building maven project at {}", project_root_path.display());
183183

184184
let mvn_path = Self::get_mvn_command()?;
185-
let output = TmcCommand::new_with_file_io(mvn_path)?
185+
let output = TmcCommand::piped(mvn_path)
186186
.with(|e| {
187187
e.cwd(project_root_path)
188188
.arg("--batch-mode")
@@ -209,7 +209,7 @@ impl JavaPlugin for MavenPlugin {
209209
log::info!("Running tests for maven project at {}", path.display());
210210

211211
let mvn_path = Self::get_mvn_command()?;
212-
let command = TmcCommand::new_with_file_io(mvn_path)?.with(|e| {
212+
let command = TmcCommand::piped(mvn_path).with(|e| {
213213
e.cwd(path)
214214
.arg("--batch-mode")
215215
.arg("fi.helsinki.cs.tmc:tmc-maven-plugin:1.12:test")

‎plugins/make/src/plugin.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ impl MakePlugin {
8686
};
8787
log::info!("Running make {}", arg);
8888

89-
let output = TmcCommand::new_with_file_io("make")?
89+
let output = TmcCommand::piped("make")
9090
.with(|e| e.cwd(path).arg(arg))
9191
.output()?;
9292

@@ -112,7 +112,7 @@ impl MakePlugin {
112112
/// the process finished successfully or not.
113113
fn build(&self, dir: &Path) -> Result<Output, MakeError> {
114114
log::debug!("building {}", dir.display());
115-
let output = TmcCommand::new_with_file_io("make")?
115+
let output = TmcCommand::piped("make")
116116
.with(|e| e.cwd(dir).arg("test"))
117117
.output()?;
118118

@@ -327,7 +327,7 @@ impl LanguagePlugin for MakePlugin {
327327

328328
// does not check for success
329329
fn clean(&self, path: &Path) -> Result<(), TmcError> {
330-
let output = TmcCommand::new_with_file_io("make")?
330+
let output = TmcCommand::piped("make")
331331
.with(|e| e.cwd(path).arg("clean"))
332332
.output()?;
333333

‎plugins/python3/src/plugin.rs‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ impl Python3Plugin {
2828
Self {}
2929
}
3030

31-
fn get_local_python_command() -> Result<TmcCommand, PythonError> {
31+
fn get_local_python_command() -> TmcCommand {
3232
lazy_static! {
3333
// the correct python command is platform-dependent
3434
static ref LOCAL_PY: LocalPy = {
@@ -62,17 +62,16 @@ impl Python3Plugin {
6262
Custom { python_exec: String },
6363
}
6464

65-
let command = match &*LOCAL_PY {
66-
LocalPy::Unix => TmcCommand::new_with_file_io("python3")?,
67-
LocalPy::Windows => TmcCommand::new_with_file_io("py")?.with(|e| e.arg("-3")),
68-
LocalPy::WindowsConda { conda_path } => TmcCommand::new_with_file_io(conda_path)?,
69-
LocalPy::Custom { python_exec } => TmcCommand::new_with_file_io(python_exec)?,
70-
};
71-
Ok(command)
65+
match &*LOCAL_PY {
66+
LocalPy::Unix => TmcCommand::piped("python3"),
67+
LocalPy::Windows => TmcCommand::piped("py").with(|e| e.arg("-3")),
68+
LocalPy::WindowsConda { conda_path } => TmcCommand::piped(conda_path),
69+
LocalPy::Custom { python_exec } => TmcCommand::piped(python_exec),
70+
}
7271
}
7372

7473
fn get_local_python_ver() -> Result<(usize, usize, usize), PythonError> {
75-
let output = Self::get_local_python_command()?
74+
let output = Self::get_local_python_command()
7675
.with(|e| e.args(&["-c", "import sys; print(sys.version_info.major); print(sys.version_info.minor); print(sys.version_info.micro);"]))
7776
.output_checked()?;
7877
let stdout = String::from_utf8_lossy(&output.stdout);
@@ -139,7 +138,7 @@ impl Python3Plugin {
139138
log::debug!("running tmc command at {}", path.display());
140139
let common_args = ["-m", "tmc"];
141140

142-
let command = Self::get_local_python_command()?;
141+
let command = Self::get_local_python_command();
143142
let command = command.with(|e| e.args(&common_args).args(extra_args).cwd(path));
144143
let output = if let Some(timeout) = timeout {
145144
command.output_with_timeout(timeout)?
@@ -446,7 +445,7 @@ mod test {
446445
fn gets_local_python_command() {
447446
init();
448447

449-
let _cmd = Python3Plugin::get_local_python_command().unwrap();
448+
let _cmd = Python3Plugin::get_local_python_command();
450449
}
451450

452451
#[test]

‎plugins/r/src/plugin.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ impl LanguagePlugin for RPlugin {
4545
} else {
4646
&["-e", "library(tmcRtestrunner);run_available_points()"]
4747
};
48-
let _output = TmcCommand::new_with_file_io("Rscript")?
48+
let _output = TmcCommand::piped("Rscript")
4949
.with(|e| e.cwd(path).args(args))
5050
.output_checked()?;
5151

@@ -85,11 +85,11 @@ impl LanguagePlugin for RPlugin {
8585
};
8686

8787
if let Some(timeout) = timeout {
88-
let _command = TmcCommand::new_with_file_io("Rscript")?
88+
let _command = TmcCommand::piped("Rscript")
8989
.with(|e| e.cwd(path).args(args))
9090
.output_with_timeout_checked(timeout)?;
9191
} else {
92-
let _command = TmcCommand::new_with_file_io("Rscript")?
92+
let _command = TmcCommand::piped("Rscript")
9393
.with(|e| e.cwd(path).args(args))
9494
.output_checked()?;
9595
}

‎tmc-langs-framework/src/command.rs‎

Lines changed: 17 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,71 +1,52 @@
11
//! Custom wrapper for Command that supports timeouts and contains custom error handling.
22
3-
use crate::{
4-
error::{CommandError, FileIo},
5-
file_util, TmcError,
6-
};
3+
use crate::{error::CommandError, TmcError};
74
use std::fs::File;
85
use std::io::Read;
96
use std::time::Duration;
107
use std::{ffi::OsStr, thread::JoinHandle};
11-
use subprocess::{Exec, ExitStatus, PopenError};
8+
use subprocess::{Exec, ExitStatus, PopenError, Redirection};
129

1310
/// Wrapper around subprocess::Exec
1411
#[must_use]
1512
pub struct TmcCommand {
1613
exec: Exec,
17-
stdout: Option<File>,
18-
stderr: Option<File>,
1914
}
2015

2116
impl TmcCommand {
2217
/// Creates a new command
2318
pub fn new(cmd: impl AsRef<OsStr>) -> Self {
2419
Self {
2520
exec: Exec::cmd(cmd).env("LANG", "en_US.UTF-8"),
26-
stdout: None,
27-
stderr: None,
2821
}
2922
}
3023

31-
/// Creates a new command with stdout and stderr redirected to files
32-
// todo: remember why this is useful
33-
pub fn new_with_file_io(cmd: impl AsRef<OsStr>) -> Result<Self, TmcError> {
34-
let stdout = file_util::temp_file()?;
35-
let stderr = file_util::temp_file()?;
36-
Ok(Self {
24+
/// Creates a new command, defaults to piped stdout/stderr.
25+
pub fn piped(cmd: impl AsRef<OsStr>) -> Self {
26+
Self {
3727
exec: Exec::cmd(cmd)
38-
.stdout(stdout.try_clone().map_err(FileIo::FileHandleClone)?)
39-
.stderr(stderr.try_clone().map_err(FileIo::FileHandleClone)?)
40-
.env("LANG", "en_US.UTF-8"), // some languages may error on UTF-8 files if the LANG variable is unset or set to some non-UTF-8 value
41-
stdout: Some(stdout),
42-
stderr: Some(stderr),
43-
})
28+
.stdout(Redirection::Pipe)
29+
.stderr(Redirection::Pipe)
30+
.env("LANG", "en_US.UTF-8"),
31+
}
4432
}
4533

4634
/// Allows modification of the internal command without providing access to it.
4735
pub fn with(self, f: impl FnOnce(Exec) -> Exec) -> Self {
48-
Self {
49-
exec: f(self.exec),
50-
..self
51-
}
36+
Self { exec: f(self.exec) }
5237
}
5338

5439
// executes the given command and collects its output
5540
fn execute(self, timeout: Option<Duration>, checked: bool) -> Result<Output, TmcError> {
5641
let cmd = self.exec.to_cmdline_lossy();
5742
log::info!("executing {}", cmd);
5843

59-
let Self {
60-
exec,
61-
stdout,
62-
stderr,
63-
} = self;
44+
let Self { exec } = self;
6445

6546
// starts executing the command
6647
let mut popen = exec.popen().map_err(|e| popen_to_tmc_err(cmd.clone(), e))?;
67-
let stdout_handle = spawn_reader(stdout, popen.stdout.take());
68-
let stderr_handle = spawn_reader(stderr, popen.stderr.take());
48+
let stdout_handle = spawn_reader(popen.stdout.take());
49+
let stderr_handle = spawn_reader(popen.stderr.take());
6950

7051
let exit_status = if let Some(timeout) = timeout {
7152
// timeout set
@@ -164,13 +145,9 @@ impl TmcCommand {
164145
}
165146

166147
// it's assumed the thread will never panic
167-
fn spawn_reader(temp_file: Option<File>, file: Option<File>) -> JoinHandle<Vec<u8>> {
148+
fn spawn_reader(file: Option<File>) -> JoinHandle<Vec<u8>> {
168149
std::thread::spawn(move || {
169-
if let Some(mut temp_file) = temp_file {
170-
let mut buf = vec![];
171-
let _ = temp_file.read_to_end(&mut buf);
172-
buf
173-
} else if let Some(mut file) = file {
150+
if let Some(mut file) = file {
174151
let mut buf = vec![];
175152
let _ = file.read_to_end(&mut buf);
176153
buf
@@ -206,9 +183,7 @@ mod test {
206183

207184
#[test]
208185
fn timeout() {
209-
let cmd = TmcCommand::new_with_file_io("sleep")
210-
.unwrap()
211-
.with(|e| e.arg("1"));
186+
let cmd = TmcCommand::piped("sleep").with(|e| e.arg("1"));
212187
assert!(matches!(
213188
cmd.output_with_timeout(Duration::from_nanos(1)),
214189
Err(TmcError::Command(CommandError::TimeOut { .. }))
@@ -217,7 +192,7 @@ mod test {
217192

218193
#[test]
219194
fn not_found() {
220-
let cmd = TmcCommand::new_with_file_io("nonexistent command").unwrap();
195+
let cmd = TmcCommand::piped("nonexistent command");
221196
assert!(matches!(
222197
cmd.output(),
223198
Err(TmcError::Command(CommandError::NotFound { .. }))

‎tmc-langs-util/src/task_executor/course_refresher.rs‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use serde::{Deserialize, Serialize};
1010
use serde_yaml::Mapping;
1111
use std::path::{Path, PathBuf};
1212
use std::{io::Write, time::Duration};
13-
use tmc_langs_framework::{command::TmcCommand, file_util, subprocess::Redirection};
13+
use tmc_langs_framework::{command::TmcCommand, file_util};
1414
use walkdir::WalkDir;
1515

1616
#[cfg(unix)]
@@ -210,13 +210,8 @@ fn initialize_new_cache_clone(
210210
file_util::copy(old_clone_path, new_course_root)?;
211211

212212
let run_git = |args: &[&str]| {
213-
TmcCommand::new("git".to_string())
214-
.with(|e| {
215-
e.cwd(new_clone_path)
216-
.args(args)
217-
.stdout(Redirection::Pipe)
218-
.stderr(Redirection::Pipe)
219-
})
213+
TmcCommand::piped("git".to_string())
214+
.with(|e| e.cwd(new_clone_path).args(args))
220215
.output_with_timeout_checked(Duration::from_secs(60 * 2))
221216
};
222217

@@ -243,14 +238,12 @@ fn initialize_new_cache_clone(
243238
log::info!("could not copy from previous cache, cloning");
244239

245240
// clone_repository
246-
TmcCommand::new("git".to_string())
241+
TmcCommand::piped("git".to_string())
247242
.with(|e| {
248243
e.args(&["clone", "-q", "-b"])
249244
.arg(course_git_branch)
250245
.arg(course_source_url)
251246
.arg(new_clone_path)
252-
.stdout(Redirection::Pipe)
253-
.stderr(Redirection::Pipe)
254247
})
255248
.output_with_timeout_checked(Duration::from_secs(60 * 2))?;
256249
Ok(())

0 commit comments

Comments
 (0)