Move from protobuf to bincode for a wire format #85
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -32,18 +32,8 @@ use number_prefix::{ | |
| Prefixed, | ||
| Standalone, | ||
| }; | ||
| use protocol::{Request, Response, CacheStats, CompileResponse, CompileFinished}; | ||
| use protocol::{Compile, CacheStat}; | ||
| use server; | ||
| use std::env; | ||
| use std::ffi::{OsStr,OsString}; | ||
|
|
@@ -69,14 +59,6 @@ pub const DEFAULT_PORT: u16 = 4226; | |
| /// The number of milliseconds to wait for server startup. | ||
| const SERVER_STARTUP_TIMEOUT_MS: u32 = 5000; | ||
|
|
||
| // Should this just be a Result? | ||
| /// Result of background server startup. | ||
| enum ServerStartup { | ||
|
|
@@ -105,7 +87,7 @@ fn run_server_process() -> Result<ServerStartup> { | |
| use futures::{Future, Stream}; | ||
| use std::time::Duration; | ||
| use tempdir::TempDir; | ||
| use tokio_io::io::read_exact; | ||
| use tokio_core::reactor::Timeout; | ||
|
|
||
| trace!("run_server_process"); | ||
|
|
@@ -384,14 +366,12 @@ fn connect_or_start_server(port: u16) -> Result<ServerConnection> { | |
| /// Send a `ZeroStats` request to the server, and return the `CacheStats` request if successful. | ||
| pub fn request_zero_stats(mut conn : ServerConnection) -> Result<CacheStats> { | ||
| debug!("request_stats"); | ||
| //TODO: better error mapping | ||
| let response = conn.request(Request::ZeroStats).chain_err(|| { | ||
| "failed to send zero statistics command to server or failed to receive respone" | ||
| })?; | ||
| if let Response::Stats(stats) = response { | ||
| Ok(stats) | ||
| } else { | ||
| bail!("Unexpected server response!") | ||
| } | ||
|
|
@@ -400,14 +380,12 @@ pub fn request_zero_stats(mut conn : ServerConnection) -> Result<CacheStats> { | |
| /// Send a `GetStats` request to the server, and return the `CacheStats` request if successful. | ||
| pub fn request_stats(mut conn : ServerConnection) -> Result<CacheStats> { | ||
| debug!("request_stats"); | ||
| //TODO: better error mapping | ||
| let response = conn.request(Request::GetStats).chain_err(|| { | ||
| "Failed to send data to or receive data from server" | ||
| })?; | ||
| if let Response::Stats(stats) = response { | ||
| Ok(stats) | ||
| } else { | ||
| bail!("Unexpected server response!") | ||
| } | ||
|
|
@@ -416,36 +394,34 @@ pub fn request_stats(mut conn : ServerConnection) -> Result<CacheStats> { | |
| /// Send a `Shutdown` request to the server, and return the `CacheStats` contained within the response if successful. | ||
| pub fn request_shutdown(mut conn : ServerConnection) -> Result<CacheStats> { | ||
| debug!("request_shutdown"); | ||
| //TODO: better error mapping | ||
| let response = conn.request(Request::Shutdown).chain_err(|| { | ||
| "Failed to send data to or receive data from server" | ||
| })?; | ||
| if let Response::ShuttingDown(stats) = response { | ||
| Ok(stats) | ||
| } else { | ||
| bail!("Unexpected server response!") | ||
| } | ||
| } | ||
|
|
||
| /// Print `stats` to stdout. | ||
| fn print_stats(stats: CacheStats) -> Result<()> { | ||
| let formatted = stats.stats.into_iter() | ||
| .map(|s| { | ||
| (s.name, match s.value { | ||
| CacheStat::Count(c) => c.to_string(), | ||
| CacheStat::String(s) => s, | ||
| CacheStat::Size(size) => { | ||
| match binary_prefix(size as f64) { | ||
| Standalone(bytes) => format!("{} bytes", bytes), | ||
| Prefixed(prefix, n) => format!("{:.0} {}B", n, prefix), | ||
| } | ||
| } | ||
| }) | ||
| }) | ||
| .collect::<Vec<_>>(); | ||
| let name_width = formatted.iter().map(|&(ref n, _)| n.len()).max().unwrap(); | ||
| let stat_width = formatted.iter().map(|&(_, ref s)| s.len()).max().unwrap(); | ||
| for (name, stat) in formatted { | ||
| println!("{:<name_width$} {:>stat_width$}", name, stat, name_width=name_width, stat_width=stat_width); | ||
|
|
@@ -455,28 +431,20 @@ fn print_stats(stats: CacheStats) -> Result<()> { | |
|
|
||
| /// Send a `Compile` request to the server, and return the server response if successful. | ||
| fn request_compile<W: AsRef<Path>, X: AsRef<OsStr>, Y: AsRef<Path>>(conn: &mut ServerConnection, exe: W, args: &Vec<X>, cwd: Y) -> Result<CompileResponse> { | ||
| let req = Request::Compile(Compile { | ||
| exe: exe.as_ref().to_str().ok_or("bad exe")?.to_owned(), | ||
|
There was a problem hiding this comment. as a follow-up, we could now just use There was a problem hiding this comment. Yeah I tried to do that but There was a problem hiding this comment. Actually... I'll send a PR! There was a problem hiding this comment. Ok this is now tracked at serde-rs/serde#823 |
||
| cwd: cwd.as_ref().to_str().ok_or("bad cwd")?.to_owned(), | ||
| args: args.iter().map(|a| { | ||
| a.as_ref().to_str().ok_or("bad cwd".into()).map(|s| s.to_owned()) | ||
| }).collect::<Result<_>>()?, | ||
| }); | ||
| trace!("request_compile: {:?}", req); | ||
| //TODO: better error mapping? | ||
| let response = conn.request(req).chain_err(|| { | ||
| "Failed to send data to or receive data from server" | ||
| })?; | ||
| if let Response::Compile(response) = response { | ||
| Ok(response) | ||
| } else { | ||
| bail!("Unexpected response from server") | ||
| } | ||
|
|
@@ -504,18 +472,14 @@ fn handle_compile_finished(response: CompileFinished, | |
| // It might be nice if the server sent stdout/stderr as the process | ||
| // ran, but then it would have to also save them in the cache as | ||
| // interleaved streams to really make it work. | ||
| stdout.write_all(&response.stdout)?; | ||
| stderr.write_all(&response.stderr)?; | ||
|
|
||
| if let Some(ret) = response.retcode { | ||
| trace!("compiler exited with status {}", ret); | ||
| Ok(ret) | ||
| } else if let Some(signal) = response.signal { | ||
| println!("Compiler killed by signal {}", signal); | ||
| Ok(-2) | ||
| } else { | ||
| println!("Missing compiler exit status!"); | ||
|
|
@@ -542,39 +506,28 @@ fn handle_compile_response<T>(mut creator: T, | |
| where T : CommandCreatorSync, | ||
| { | ||
| match response { | ||
| CompileResponse::CompileStarted => { | ||
| debug!("Server sent CompileStarted"); | ||
| // Wait for CompileFinished. | ||
| match conn.read_one_response() { | ||
| Ok(Response::CompileFinished(result)) => { | ||
| return handle_compile_finished(result, stdout, stderr) | ||
| } | ||
| Ok(_) => bail!("unexpected response from server"), | ||
| Err(Error(ErrorKind::Io(ref e), _)) | ||
| if e.kind() == io::ErrorKind::UnexpectedEof => | ||
| { | ||
| writeln!(io::stderr(), | ||
| "warning: sccache server looks like it shut down \ | ||
| unexpectedly, compiling locally instead").unwrap(); | ||
| } | ||
| Err(e) => return Err(e).chain_err(|| { | ||
| //TODO: something better here? | ||
| "error reading compile response from server" | ||
| }), | ||
| } | ||
| } | ||
| CompileResponse::UnhandledCompile => { | ||
| debug!("Server sent UnhandledCompile"); | ||
| } | ||
| }; | ||
|
|
@@ -586,12 +539,12 @@ fn handle_compile_response<T>(mut creator: T, | |
| if log_enabled!(Trace) { | ||
| trace!("running command: {:?}", cmd); | ||
| } | ||
| let output = core.run(run_input_output(cmd, None))?; | ||
| if !output.stdout.is_empty() { | ||
| stdout.write_all(&output.stdout)?; | ||
| } | ||
| if !output.stderr.is_empty() { | ||
| stderr.write_all(&output.stderr)?; | ||
| } | ||
| Ok(output.status.code().unwrap_or_else(|| { | ||
| if let Some(sig) = status_signal(output.status) { | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would things look nicer overall if we used tokio here instead of blocking IO? Originally I used blocking IO in the client because the mio stuff was so complicated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah I think they'd basically be equivalent. As soon as we'd want to do anything else, though, such as timeouts, then we'd definitely want to use Tokio as it'd look quite a bit better.
The tokio version would basically just do the futures equivalent of this, using
tokio_iocombinators, and a little less ergonomically.