Skip to content

Fix #10754 - std::run functions fail after io_error #10936

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 26 additions & 16 deletions src/compiletest/procsrv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,47 +46,57 @@ pub fn run(lib_path: &str,
prog: &str,
args: &[~str],
env: ~[(~str, ~str)],
input: Option<~str>) -> Result {
input: Option<~str>) -> Option<Result> {

let env = env + target_env(lib_path, prog);
let mut process = run::Process::new(prog, args, run::ProcessOptions {
let mut opt_process = run::Process::new(prog, args, run::ProcessOptions {
env: Some(env),
dir: None,
in_fd: None,
out_fd: None,
err_fd: None
});

for input in input.iter() {
process.input().write(input.as_bytes());
}
let run::ProcessOutput { status, output, error } = process.finish_with_output();
match opt_process {
Some(ref mut process) => {
for input in input.iter() {
process.input().write(input.as_bytes());
}
let run::ProcessOutput { status, output, error } = process.finish_with_output();

Result {
status: status,
out: str::from_utf8_owned(output),
err: str::from_utf8_owned(error)
Some(Result {
status: status,
out: str::from_utf8_owned(output),
err: str::from_utf8_owned(error)
})
},
None => None
}
}

pub fn run_background(lib_path: &str,
prog: &str,
args: &[~str],
env: ~[(~str, ~str)],
input: Option<~str>) -> run::Process {
input: Option<~str>) -> Option<run::Process> {

let env = env + target_env(lib_path, prog);
let mut process = run::Process::new(prog, args, run::ProcessOptions {
let opt_process = run::Process::new(prog, args, run::ProcessOptions {
env: Some(env),
dir: None,
in_fd: None,
out_fd: None,
err_fd: None
});

for input in input.iter() {
process.input().write(input.as_bytes());
}
match opt_process {
Some(mut process) => {
for input in input.iter() {
process.input().write(input.as_bytes());
}

return process;
Some(process)
},
None => None
}
}
40 changes: 26 additions & 14 deletions src/compiletest/runtest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,20 +289,23 @@ fn run_debuginfo_test(config: &config, props: &TestProps, testfile: &Path) {
dump_output_file(config, testfile, script_str, "debugger.script");


procsrv::run("", config.adb_path.clone(),
procsrv::run("", config.adb_path,
[~"push", exe_file.as_str().unwrap().to_owned(), config.adb_test_dir.clone()],
~[(~"",~"")], Some(~""));
~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

procsrv::run("", config.adb_path,
[~"forward", ~"tcp:5039", ~"tcp:5039"],
~[(~"",~"")], Some(~""));
~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

let adb_arg = format!("export LD_LIBRARY_PATH={}; gdbserver :5039 {}/{}",
config.adb_test_dir.clone(), config.adb_test_dir.clone(),
str::from_utf8(exe_file.filename().unwrap()));

let mut process = procsrv::run_background("", config.adb_path.clone(),
[~"shell",adb_arg.clone()],~[(~"",~"")], Some(~""));
let mut process = procsrv::run_background("", config.adb_path,
[~"shell",adb_arg.clone()],~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));
loop {
//waiting 1 second for gdbserver start
timer::sleep(1000);
Expand Down Expand Up @@ -334,10 +337,12 @@ fn run_debuginfo_test(config: &config, props: &TestProps, testfile: &Path) {
let debugger_opts = ~[~"-quiet", ~"-batch", ~"-nx",
"-command=" + debugger_script.as_str().unwrap().to_owned()];

let gdb_path = tool_path.append("/bin/arm-linux-androideabi-gdb");
let procsrv::Result{ out, err, status }=
procsrv::run("",
tool_path.append("/bin/arm-linux-androideabi-gdb"),
debugger_opts, ~[(~"",~"")], None);
gdb_path,
debugger_opts, ~[(~"",~"")], None)
.expect(format!("failed to exec `{}`", gdb_path));
let cmdline = {
let cmdline = make_cmdline("", "arm-linux-androideabi-gdb", debugger_opts);
logv(config, format!("executing {}", cmdline));
Expand Down Expand Up @@ -800,7 +805,8 @@ fn program_output(config: &config, testfile: &Path, lib_path: &str, prog: ~str,
cmdline
};
let procsrv::Result{ out, err, status } =
procsrv::run(lib_path, prog, args, env, input);
procsrv::run(lib_path, prog, args, env, input)
.expect(format!("failed to exec `{}`", prog));
dump_output(config, testfile, out, err);
return ProcRes {status: status,
stdout: out,
Expand Down Expand Up @@ -908,7 +914,8 @@ fn _arm_exec_compiled_test(config: &config, props: &TestProps,
// copy to target
let copy_result = procsrv::run("", config.adb_path,
[~"push", args.prog.clone(), config.adb_test_dir.clone()],
~[(~"",~"")], Some(~""));
~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

if config.verbose {
println!("push ({}) {} {} {}",
Expand All @@ -932,7 +939,8 @@ fn _arm_exec_compiled_test(config: &config, props: &TestProps,
for tv in args.args.iter() {
runargs.push(tv.to_owned());
}
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")], Some(~""));
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

// get exitcode of result
runargs = ~[];
Expand All @@ -942,7 +950,8 @@ fn _arm_exec_compiled_test(config: &config, props: &TestProps,

let procsrv::Result{ out: exitcode_out, err: _, status: _ } =
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")],
Some(~""));
Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

let mut exitcode : int = 0;
for c in exitcode_out.chars() {
Expand All @@ -960,7 +969,8 @@ fn _arm_exec_compiled_test(config: &config, props: &TestProps,
runargs.push(format!("{}/{}.stdout", config.adb_test_dir, prog_short));

let procsrv::Result{ out: stdout_out, err: _, status: _ } =
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")], Some(~""));
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

// get stderr of result
runargs = ~[];
Expand All @@ -969,7 +979,8 @@ fn _arm_exec_compiled_test(config: &config, props: &TestProps,
runargs.push(format!("{}/{}.stderr", config.adb_test_dir, prog_short));

let procsrv::Result{ out: stderr_out, err: _, status: _ } =
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")], Some(~""));
procsrv::run("", config.adb_path, runargs, ~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

dump_output(config, testfile, stdout_out, stderr_out);

Expand Down Expand Up @@ -1004,7 +1015,8 @@ fn _arm_push_aux_shared_library(config: &config, testfile: &Path) {
// FIXME (#9639): This needs to handle non-utf8 paths
let copy_result = procsrv::run("", config.adb_path,
[~"push", file.as_str().unwrap().to_owned(), config.adb_test_dir.clone()],
~[(~"",~"")], Some(~""));
~[(~"",~"")], Some(~""))
.expect(format!("failed to exec `{}`", config.adb_path));

if config.verbose {
println!("push ({}) {} {} {}",
Expand Down
26 changes: 18 additions & 8 deletions src/librustc/back/archive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,25 @@ fn run_ar(sess: Session, args: &str, cwd: Option<&Path>,
Some(p) => { debug!("inside {}", p.display()); }
None => {}
}
let o = Process::new(ar, args.as_slice(), opts).finish_with_output();
if !o.status.success() {
sess.err(format!("{} {} failed with: {}", ar, args.connect(" "),
o.status));
sess.note(format!("stdout ---\n{}", str::from_utf8(o.output)));
sess.note(format!("stderr ---\n{}", str::from_utf8(o.error)));
sess.abort_if_errors();
let mut opt_prog = Process::new(ar, args.as_slice(), opts);
match opt_prog {
Some(ref mut prog) => {
let o = prog.finish_with_output();
if !o.status.success() {
sess.err(format!("{} {} failed with: {}", ar, args.connect(" "),
o.status));
sess.note(format!("stdout ---\n{}", str::from_utf8(o.output)));
sess.note(format!("stderr ---\n{}", str::from_utf8(o.error)));
sess.abort_if_errors();
}
o
},
None => {
sess.err(format!("could not exec `{}`", ar));
sess.abort_if_errors();
fail!("rustc::back::archive::run_ar() should not reach this point");
}
}
o
}

impl Archive {
Expand Down
44 changes: 29 additions & 15 deletions src/librustc/back/link.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,13 +310,19 @@ pub mod write {
assembly.as_str().unwrap().to_owned()];

debug!("{} '{}'", cc, args.connect("' '"));
let prog = run::process_output(cc, args);

if !prog.status.success() {
sess.err(format!("linking with `{}` failed: {}", cc, prog.status));
sess.note(format!("{} arguments: '{}'", cc, args.connect("' '")));
sess.note(str::from_utf8_owned(prog.error + prog.output));
sess.abort_if_errors();
match run::process_output(cc, args) {
Some(prog) => {
if !prog.status.success() {
sess.err(format!("linking with `{}` failed: {}", cc, prog.status));
sess.note(format!("{} arguments: '{}'", cc, args.connect("' '")));
sess.note(str::from_utf8_owned(prog.error + prog.output));
sess.abort_if_errors();
}
},
None => {
sess.err(format!("could not exec `{}`", cc));
sess.abort_if_errors();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that this patch also fixes #10755. (hurray!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@klutzy That's interesting, although I don't know why this patch would fix the issue because the io_error condition is still raised.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cadencemarseille Oops, I misread your patch so my comment above is invalid. (I didn't actually build your patch so it's possible to not fix it.) Could you fix #10755 as well? I think it's simple to just catch io_error here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I will look into #10755 next.

}
}
}

Expand Down Expand Up @@ -949,14 +955,22 @@ fn link_natively(sess: Session, dylib: bool, obj_filename: &Path,

// Invoke the system linker
debug!("{} {}", cc_prog, cc_args.connect(" "));
let prog = time(sess.time_passes(), "running linker", (), |()|
run::process_output(cc_prog, cc_args));

if !prog.status.success() {
sess.err(format!("linking with `{}` failed: {}", cc_prog, prog.status));
sess.note(format!("{} arguments: '{}'", cc_prog, cc_args.connect("' '")));
sess.note(str::from_utf8_owned(prog.error + prog.output));
sess.abort_if_errors();
let opt_prog = time(sess.time_passes(), "running linker", (), |()|
run::process_output(cc_prog, cc_args));

match opt_prog {
Some(prog) => {
if !prog.status.success() {
sess.err(format!("linking with `{}` failed: {}", cc_prog, prog.status));
sess.note(format!("{} arguments: '{}'", cc_prog, cc_args.connect("' '")));
sess.note(str::from_utf8_owned(prog.error + prog.output));
sess.abort_if_errors();
}
},
None => {
sess.err(format!("could not exec `{}`", cc_prog));
sess.abort_if_errors();
}
}


Expand Down
28 changes: 16 additions & 12 deletions src/librustpkg/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,14 @@ pub fn install_pkg(cx: &BuildContext,
/// Builds an arbitrary library whose short name is `output`,
/// by invoking `tool` with arguments `args` plus "-o %s", where %s
/// is the platform-specific library name for `output`.
/// Returns that platform-specific name.
/// Returns that platform-specific name, or None if `tool` could not be started.
pub fn build_library_in_workspace(exec: &mut workcache::Exec,
context: &mut Context,
package_name: &str,
tool: &str,
flags: &[~str],
paths: &[~str],
output: &str) -> ~str {
output: &str) -> Option<~str> {
use command_failed = conditions::command_failed::cond;

let workspace = my_workspace(context, package_name);
Expand All @@ -169,16 +169,20 @@ pub fn build_library_in_workspace(exec: &mut workcache::Exec,

let all_args = flags + absolute_paths + cc_args +
~[~"-o", out_name.as_str().unwrap().to_owned()];
let exit_process = run::process_status(tool, all_args);
if exit_process.success() {
let out_name_str = out_name.as_str().unwrap().to_owned();
exec.discover_output("binary",
out_name_str,
digest_only_date(&out_name));
context.add_library_path(out_name.dir_path());
out_name_str
} else {
command_failed.raise((tool.to_owned(), all_args, exit_process))
match run::process_status(tool, all_args) {
Some(exit_process) => {
if exit_process.success() {
let out_name_str = out_name.as_str().unwrap().to_owned();
exec.discover_output("binary",
out_name_str,
digest_only_date(&out_name));
context.add_library_path(out_name.dir_path());
Some(out_name_str)
} else {
Some(command_failed.raise((tool.to_owned(), all_args, exit_process)))
}
},
None => None
}
}

Expand Down
Loading