Skip to content

Send environment variables from client, hash CARGO_ env vars in Rust #92

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.

Sign up for GitHub

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
luser merged 2 commits into from
Mar 28, 2017
Merged

Send environment variables from client, hash CARGO_ env vars in Rust #92

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
27d7198
Send environment variables from client for compilation requests. Fixe…
luser Mar 27, 2017
473b026
Include CARGO_ environment variables in Rust hash generation. Fixes #89
luser Mar 27, 2017
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
3 changes: 3 additions & 0 deletions src/cmdline.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ pub enum Command {
cmdline: Vec<OsString>,
/// The directory in which to execute the command.
cwd: PathBuf,
/// The environment variables to use for execution.
env_vars: Vec<(OsString, OsString)>,
},
}

Expand Down Expand Up @@ -146,6 +148,7 @@ pub fn parse() -> Result<Command> {
exe: exe.to_owned(),
cmdline: cmdline,
cwd: cwd,
env_vars: env::vars_os().collect(),
})
} else {
bail!("No compile command");
Expand Down
18 changes: 13 additions & 5 deletions src/commands.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,13 +430,19 @@ 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> {
fn request_compile<W, X, Y>(conn: &mut ServerConnection, exe: W, args: &Vec<X>, cwd: Y,
env_vars: Vec<(OsString, OsString)>) -> Result<CompileResponse>
where W: AsRef<Path>,
X: AsRef<OsStr>,
Y: AsRef<Path>,
{
let req = Request::Compile(Compile {
exe: exe.as_ref().to_str().ok_or("bad exe")?.to_owned(),
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())
a.as_ref().to_str().ok_or("bad arg".into()).map(|s| s.to_owned())
}).collect::<Result<_>>()?,
env_vars: env_vars,
});
trace!("request_compile: {:?}", req);
//TODO: better error mapping?
Expand Down Expand Up @@ -567,13 +573,14 @@ pub fn do_compile<T>(creator: T,
cmdline: Vec<OsString>,
cwd: &Path,
path: Option<OsString>,
env_vars: Vec<(OsString, OsString)>,
stdout: &mut Write,
stderr: &mut Write) -> Result<i32>
where T : CommandCreatorSync,
where T: CommandCreatorSync,
{
trace!("do_compile");
let exe_path = which_in(exe, path, &cwd)?;
let res = request_compile(&mut conn, &exe_path, &cmdline, &cwd)?;
let res = request_compile(&mut conn, &exe_path, &cmdline, &cwd, env_vars)?;
handle_compile_response(creator, core, &mut conn, res, &exe_path, cmdline, cwd, stdout, stderr)
}

Expand Down Expand Up @@ -620,7 +627,7 @@ pub fn run_command(cmd: Command) -> Result<i32> {
let stats = request_shutdown(server)?;
print_stats(stats)?
}
Command::Compile { exe, cmdline, cwd } => {
Command::Compile { exe, cmdline, cwd, env_vars } => {
trace!("Command::Compile {{ {:?}, {:?}, {:?} }}", exe, cmdline, cwd);
let conn = connect_or_start_server(get_port())?;
let mut core = Core::new()?;
Expand All @@ -631,6 +638,7 @@ pub fn run_command(cmd: Command) -> Result<i32> {
cmdline,
&cwd,
env::var_os("PATH"),
env_vars,
&mut io::stdout(),
&mut io::stderr());
return res.chain_err(|| {
Expand Down
71 changes: 36 additions & 35 deletions src/compiler/c.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ use futures_cpupool::CpuPool;
use mock_command::CommandCreatorSync;
use sha1;
use std::borrow::Cow;
use std::collections::HashMap;
use std::env;
use std::collections::{HashMap, HashSet};
use std::ffi::{OsStr, OsString};
use std::fmt;
use std::path::Path;
use std::process;
use util::sha1_digest;
use util::{os_str_bytes, sha1_digest};

use errors::*;

Expand Down Expand Up @@ -106,6 +106,7 @@ pub trait CCompilerImpl: Clone + fmt::Debug + Send + 'static {
executable: &str,
parsed_args: &ParsedArguments,
cwd: &str,
env_vars: &[(OsString, OsString)],
pool: &CpuPool)
-> SFuture<process::Output> where T: CommandCreatorSync;
/// Run the C compiler with the specified set of arguments, using the
Expand All @@ -116,6 +117,7 @@ pub trait CCompilerImpl: Clone + fmt::Debug + Send + 'static {
preprocessor_output: Vec<u8>,
parsed_args: &ParsedArguments,
cwd: &str,
env_vars: &[(OsString, OsString)],
pool: &CpuPool)
-> SFuture<(Cacheable, process::Output)>
where T: CommandCreatorSync;
Expand Down Expand Up @@ -167,13 +169,15 @@ impl<T, I> CompilerHasher<T> for CCompilerHasher<I>
fn generate_hash_key(self: Box<Self>,
creator: &T,
cwd: &str,
env_vars: &[(OsString, OsString)],
pool: &CpuPool)
-> SFuture<HashResult<T>>
{
let me = *self;
let CCompilerHasher { parsed_args, executable, executable_digest, compiler } = me;
let result = compiler.preprocess(creator, &executable, &parsed_args, cwd, pool);
let result = compiler.preprocess(creator, &executable, &parsed_args, cwd, env_vars, pool);
let out_file = parsed_args.output_file().into_owned();
let env_vars = env_vars.to_vec();
let result = result.map_err(move |e| {
debug!("[{}]: preprocessor failed: {:?}", out_file, e);
e
Expand Down Expand Up @@ -202,7 +206,7 @@ impl<T, I> CompilerHasher<T> for CCompilerHasher<I>
.filter(|a| **a != out_file)
.map(|a| a.as_str())
.collect::<String>();
hash_key(&executable_digest, &arguments, &preprocessor_result.stdout)
hash_key(&executable_digest, &arguments, &env_vars, &preprocessor_result.stdout)
};
HashResult::Ok {
key: key,
Expand Down Expand Up @@ -231,12 +235,14 @@ impl<T: CommandCreatorSync, I: CCompilerImpl> Compilation<T> for CCompilation<I>
fn compile(self: Box<Self>,
creator: &T,
cwd: &str,
env_vars: &[(OsString, OsString)],
pool: &CpuPool)
-> SFuture<(Cacheable, process::Output)>
{
let me = *self;
let CCompilation { parsed_args, executable, preprocessor_output, compiler } = me;
compiler.compile(creator, &executable, preprocessor_output, &parsed_args, cwd, pool)
compiler.compile(creator, &executable, preprocessor_output, &parsed_args, cwd, env_vars,
pool)
}

fn outputs<'a>(&'a self) -> Box<Iterator<Item=(&'a str, &'a String)> + 'a>
Expand All @@ -255,19 +261,21 @@ pub const CACHED_ENV_VARS : &'static [&'static str] = &[
];

/// Compute the hash key of `compiler` compiling `preprocessor_output` with `args`.
pub fn hash_key(compiler_digest: &str, arguments: &str, preprocessor_output: &[u8]) -> String {
pub fn hash_key(compiler_digest: &str, arguments: &str, env_vars: &[(OsString, OsString)],
preprocessor_output: &[u8]) -> String
{
// If you change any of the inputs to the hash, you should change `CACHE_VERSION`.
let mut m = sha1::Sha1::new();
m.update(compiler_digest.as_bytes());
m.update(CACHE_VERSION);
m.update(arguments.as_bytes());
//TODO: should propogate these over from the client.
// https://github.com/glandium/sccache/issues/5
for var in CACHED_ENV_VARS.iter() {
if let Ok(val) = env::var(var) {
m.update(var.as_bytes());
//TODO: use lazy_static.
let cached_env_vars: HashSet<OsString> = CACHED_ENV_VARS.iter().map(|v| OsStr::new(v).to_os_string()).collect();
for &(ref var, ref val) in env_vars.iter() {
if cached_env_vars.contains(var) {
m.update(os_str_bytes(var));

@alexcrichton alexcrichton Mar 29, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This runs a risk of breaking in future rust versions, but could Hash for OsStr be used here instead?

m.update(&b"="[..]);
m.update(val.as_bytes());
m.update(os_str_bytes(val));
}
}
m.update(preprocessor_output);
Expand All @@ -277,35 +285,34 @@ pub fn hash_key(compiler_digest: &str, arguments: &str, preprocessor_output: &[u
#[cfg(test)]
mod test {
use super::*;
use std::env;

#[test]
fn test_hash_key_executable_contents_differs() {
let args = "a b c";
const PREPROCESSED : &'static [u8] = b"hello world";
assert_neq!(hash_key("abcd", &args, &PREPROCESSED),
hash_key("wxyz", &args, &PREPROCESSED));
assert_neq!(hash_key("abcd", &args, &[], &PREPROCESSED),
hash_key("wxyz", &args, &[], &PREPROCESSED));
}

#[test]
fn test_hash_key_args_differs() {
let digest = "abcd";
const PREPROCESSED: &'static [u8] = b"hello world";
assert_neq!(hash_key(digest, "a b c", &PREPROCESSED),
hash_key(digest, "x y z", &PREPROCESSED));
assert_neq!(hash_key(digest, "a b c", &[], &PREPROCESSED),
hash_key(digest, "x y z", &[], &PREPROCESSED));

assert_neq!(hash_key(digest, "a b c", &PREPROCESSED),
hash_key(digest, "a b", &PREPROCESSED));
assert_neq!(hash_key(digest, "a b c", &[], &PREPROCESSED),
hash_key(digest, "a b", &[], &PREPROCESSED));

assert_neq!(hash_key(digest, "a b c", &PREPROCESSED),
hash_key(digest, "a", &PREPROCESSED));
assert_neq!(hash_key(digest, "a b c", &[], &PREPROCESSED),
hash_key(digest, "a", &[], &PREPROCESSED));
}

#[test]
fn test_hash_key_preprocessed_content_differs() {
let args = "a b c";
assert_neq!(hash_key("abcd", &args, &b"hello world"[..]),
hash_key("abcd", &args, &b"goodbye"[..]));
assert_neq!(hash_key("abcd", &args, &[], &b"hello world"[..]),
hash_key("abcd", &args, &[], &b"goodbye"[..]));
}

#[test]
Expand All @@ -314,17 +321,11 @@ mod test {
let digest = "abcd";
const PREPROCESSED: &'static [u8] = b"hello world";
for var in CACHED_ENV_VARS.iter() {
let old = env::var_os(var);
env::remove_var(var);
let h1 = hash_key(digest, &args, &PREPROCESSED);
env::set_var(var, "something");
let h2 = hash_key(digest, &args, &PREPROCESSED);
env::set_var(var, "something else");
let h3 = hash_key(digest, &args, &PREPROCESSED);
match old {
Some(val) => env::set_var(var, val),
None => env::remove_var(var),
}
let h1 = hash_key(digest, &args, &[], &PREPROCESSED);
let vars = vec![(OsString::from(var), OsString::from("something"))];
let h2 = hash_key(digest, &args, &vars, &PREPROCESSED);
let vars = vec![(OsString::from(var), OsString::from("something else"))];
let h3 = hash_key(digest, &args, &vars, &PREPROCESSED);
assert_neq!(h1, h2);
assert_neq!(h2, h3);
}
Expand Down
14 changes: 12 additions & 2 deletions src/compiler/clang.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use mock_command::{
CommandCreatorSync,
RunCommand,
};
use std::ffi::OsString;
use std::fs::File;
use std::io::{
self,
Expand Down Expand Up @@ -57,10 +58,11 @@ impl CCompilerImpl for Clang {
executable: &str,
parsed_args: &ParsedArguments,
cwd: &str,
env_vars: &[(OsString, OsString)],
pool: &CpuPool)
-> SFuture<process::Output> where T: CommandCreatorSync
{
gcc::preprocess(creator, executable, parsed_args, cwd, pool)
gcc::preprocess(creator, executable, parsed_args, cwd, env_vars, pool)
}

fn compile<T>(&self,
Expand All @@ -69,11 +71,12 @@ impl CCompilerImpl for Clang {
preprocessor_output: Vec<u8>,
parsed_args: &ParsedArguments,
cwd: &str,
env_vars: &[(OsString, OsString)],
pool: &CpuPool)
-> SFuture<(Cacheable, process::Output)>
where T: CommandCreatorSync
{
compile(creator, executable, preprocessor_output, parsed_args, cwd, pool)
compile(creator, executable, preprocessor_output, parsed_args, cwd, env_vars, pool)
}
}

Expand All @@ -90,6 +93,7 @@ fn compile<T>(creator: &T,
preprocessor_output: Vec<u8>,
parsed_args: &ParsedArguments,
cwd: &str,
env_vars: &[(OsString, OsString)],
pool: &CpuPool)
-> SFuture<(Cacheable, process::Output)>
where T: CommandCreatorSync,
Expand Down Expand Up @@ -117,6 +121,8 @@ fn compile<T>(creator: &T,
.arg("-o")
.arg(&out_file)
.args(&parsed_args.common_args)
.env_clear()
.envs(env_vars.iter().map(|&(ref k, ref v)| (k, v)))
.current_dir(&cwd);
let output = write.and_then(move |(tempdir, input)| {
attempt.arg(&input);
Expand All @@ -141,6 +147,8 @@ fn compile<T>(creator: &T,
.arg("-o")
.arg(&out_file)
.args(&parsed_args.common_args)
.env_clear()
.envs(env_vars.iter().map(|&(ref k, ref v)| (k, v)))
.current_dir(&cwd);
Box::new(output.and_then(move |output| -> SFuture<_> {
if !output.status.success() {
Expand Down Expand Up @@ -223,6 +231,7 @@ mod test {
vec!(),
&parsed_args,
f.tempdir.path().to_str().unwrap(),
&[],
&pool).wait().unwrap();
assert_eq!(Cacheable::Yes, cacheable);
// Ensure that we ran all processes.
Expand Down Expand Up @@ -252,6 +261,7 @@ mod test {
vec!(),
&parsed_args,
f.tempdir.path().to_str().unwrap(),
&[],
&pool).wait().unwrap();
assert_eq!(Cacheable::Yes, cacheable);
assert_eq!(exit_status(0), output.status);
Expand Down
Loading