Support rustup overrides properly

[luser]

For the initial cut at Rust support we're not handling rustup overrides, since we're caching compiler info purely by compiler path. With rustup, the same compiler binary can invoke different Rust toolchains depending on the cwd, so we'll have to handle that somehow.

I was thinking of using a two-tier cache where we cache the "is this a Rust compiler?" result in the server code as we currently do, but then internally check for the presence of a rustup binary next to rustc and run rustup which rustc or something in the compile cwd to locate the actual compiler in use, and then have a cache of compilers using that path.

[alexcrichton]

cc @brson, you may have thoughts here as well

[drahnr]

Could it be possibly easier to resolve the compiler path, but instead hash the compiler binary itself and use that as key rather than the path? This would also remove all needs to duplicate the logic of https://github.com/rust-lang/rustup/blob/master/src/config.rs. How often would the hash need to be calculated?
The path is also not a good key, since it will differ from user to user.

[luser]

The path is also not a good key, since it will differ from user to user.

The cache I'm discussing here is only in memory on the local machine where you're running sccache:

sccache/src/server.rs

Lines 825 to 865 in 24f4306

	/// Look up compiler info from the cache for the compiler `path`.
	/// If not cached, determine the compiler type and cache the result.
	fn compiler_info(
	&self,
	path: PathBuf,
	env: &[(OsString, OsString)],
	) -> SFuture<Result<Box<dyn Compiler<C>>>> {
	trace!("compiler_info");
	let mtime = ftry!(metadata(&path).map(|attr| FileTime::from_last_modification_time(&attr)));
	//TODO: properly handle rustup overrides. Currently this will
	// cache based on the rustup rustc path, ignoring overrides.
	// https://github.com/mozilla/sccache/issues/87
	let result = match self.compilers.borrow().get(&path) {
	// It's a hit only if the mtime matches.
	Some(&Some((ref c, ref cached_mtime))) if *cached_mtime == mtime => Some(c.clone()),
	_ => None,
	};
	match result {
	Some(info) => {
	trace!("compiler_info cache hit");
	f_ok(Ok(info))
	}
	None => {
	trace!("compiler_info cache miss");
	// Check the compiler type and return the result when
	// finished. This generally involves invoking the compiler,
	// so do it asynchronously.
	let me = self.clone();
	let info = get_compiler_info(&self.creator, &path, env, &self.pool);
	Box::new(info.then(move |info| {
	let map_info = match info {
	Ok(ref c) => Some((c.clone(), mtime)),
	Err(_) => None,
	};
	me.compilers.borrow_mut().insert(path, map_info);
	Ok(info)
	}))
	}
	}
	}

For the hashes for compiled objects themselves we include hashes of the full contents of the Rust compiler's sysroot. Unfortunately we only do this the first time we locate a new compiler so this could result in invalid cache entries being stored when this situation is encountered.

[drahnr]

So the simplest version would be to track changes of the compiler sysroot (inotify, is there something similiar on windows?) and additionally check on launch of the server and invalidate the cache on each of those events, causing a full checksum of the sysroot to be done?
That should be enough?

[luser]

I think the best solution would still be something similar to what I proposed in my initial comment: make the Rust compiler cache two-tiered. The first tier would just be keyed by path+mtime, the second tier would check if this is a rustup rustc and if so use rustup which rustc in the cwd for the compile request to get the path to the actual compiler:

luser@eye7:/build/rust-minidump$ rustup which rustc
/home/luser/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc

Then the second tier could use that path+mtime to cache compiler info since that's the real compiler binary and will change if it gets updated. Additionally this would fix the original problem as stated where /src/a might compile with stable rust, /src/b might have an override for nightly rust, but they are both using the rustup shim ~/.cargo/bin/rustc so sccache doesn't differentiate in the internal cache currently.

[drahnr]

@luser what's the preferred way of detecting rustup, currently only which is employed combined with a check of rustup --version

[luser]

I would start with something simple: take the path to rustc and see if a rustup executable exists alongside it. Something like:

fn has_rustup(rustc: &Path) -> bool {
  rustc.with_file_name("rustup").with_extension(std::env::consts::EXE_EXTENSION).is_file()
}

(you might want something slightly fancier, the which crate sccache is already using does check that the file is executable etc.)

[drahnr]

I am currently using which, so that's all good then, thanks!

[drahnr]

It would be awesome if you could review #666 :)

[luser]

Fixed by #666.

[drahnr]

@luser there seems to be an uncovered edgecase which can still trigger this behaviour under certain unknown circumstances. I did not get around to investigate the issue yet, though it's been a few months (I can't seem to find the initial comments right now pointing this out, I have a reproducible test case).
Another point might be to bake a release and make the changes of #666 available via cargo.