[dist] failure when build.rs or procmacro depend on externalities

[drahnr]

Having a workspace tree:

sub1/lvl2/Cargo.toml #depends on sub0 { version = .., path = ../../sub0 }
sub0/Cargo.toml

causes build failures where the Cargo.toml in sub0 can not be found

error: `/home/bernhard/Projects/substrate/primitives/wasm-interface/Cargo.toml` does not exist.
  --> /home/bernhard/Projects/substrate/primitives/wasm-interface/src/lib.rs:73:56
   |
73 | #[derive(PartialEq, Debug, Clone, Copy, codec::Encode, codec::Decode)]
   |                                                        ^^^^^^^^^^^^^

this assumes #666 is already applied / #87 is resolved to resolve to correct rustc versions, but is also present in latest master with a simpler setup.

[drahnr]

Again , compiling https://github.com/paritytech/substrate with cargo b

# <snip>
   Compiling trie-db v0.20.0
   Compiling frame-support-procedural v2.0.0-alpha.3 (/home/bernhard/Projects/substrate-perf-4876/frame/support/procedural)
   Compiling parity-scale-codec v1.2.0
   Compiling wasm-bindgen-futures v0.4.8
   Compiling impl-codec v0.4.2
   Compiling sp-wasm-interface v2.0.0-alpha.3 (/home/bernhard/Projects/substrate/primitives/wasm-interface)
error: `/home/bernhard/Projects/substrate/primitives/wasm-interface/Cargo.toml` does not exist.
  --> /home/bernhard/Projects/substrate/primitives/wasm-interface/src/lib.rs:73:56
   |
73 | #[derive(PartialEq, Debug, Clone, Copy, codec::Encode, codec::Decode)]
   |                                                        ^^^^^^^^^^^^^
   |
   = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)
# <snip>

there is a mix of stable native/host rust with nightly wasm .

@luser I could not find where the tree deps are included (are they) so I can iron this hopefully last pitfall out which I step into my daily usage :)

[drahnr]

Output of
RUST_LOG=sccache=trace SCCACHE_START_SERVER=1 SCCACHE_NO_DAEMON=1 sccache | rg -i -C 4 '(CWD|wasm|linux)'

TRACE 2020-03-05T09:04:45Z: sccache::compiler::rust: get_compiler_outputs: "libsp_wasm_interface-10e57696cdb0b389.rlib\n"
DEBUG 2020-03-05T09:04:45Z: sccache::compiler::compiler: [sp_wasm_interface]: generate_hash_key took 0.366 s
TRACE 2020-03-05T09:04:45Z: sccache::compiler::compiler: [sp_wasm_interface]: Hash key: 00ac1b7796ee2e381768a71a4aa5ae7b076bcdab8cfd0dbbcec694390ef0f6cb7bacb5e326c336e1127cc580ac96ae9964b77f806a74c0b9689e650e469a0819
DEBUG 2020-03-05T09:04:45Z: sccache::compiler::compiler: [sp_wasm_interface]: Cache miss in 0.001 s
TRACE 2020-03-05T09:04:45Z: sccache::compiler::rust: [sp_wasm_interface]: compile
DEBUG 2020-03-05T09:04:45Z: sccache::compiler::compiler: [sp_wasm_interface]: Attempting distributed compilation
DEBUG 2020-03-05T09:04:45Z: sccache::compiler::compiler: [sp_wasm_interface]: Creating distributed compile request
TRACE 2020-03-05T09:04:45Z: sccache::compiler::rust: Dist inputs: inputs=["/home/bernhard/Projects/substrate/primitives/wasm-interface/src/lib.rs", "/home/bernhard/Projects/substrate/target/debug/wbuild/target/release/deps/libimpl_trait_for_tuples-73c1f651d00a57ac.so", "/home/bernhard/Projects/substrate/target/debug/wbuild/target/wasm32-unknown-unknown/release/deps/libparity_scale_codec-ae6fd645b0ad7e6a.rmeta", "/home/bernhard/Projects/substrate/target/debug/wbuild/target/wasm32-unknown-unknown/release/deps/libsp_std-fe408af98f20a1d8.rmeta"] crate_link_paths=["/home/bernhard/Projects/substrate/target/debug/wbuild/target/wasm32-unknown-unknown/release/deps", "/home/bernhard/Projects/substrate/target/debug/wbuild/target/release/deps"]
DEBUG 2020-03-05T09:04:45Z: sccache::compiler::compiler: [sp_wasm_interface]: Identifying dist toolchain for "/home/bernhard/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustc"
DEBUG 2020-03-05T09:04:45Z: sccache::dist::cache::client: Using cached toolchain 1b938ab196be9665a3914d019a8e73565e621de792cbcd679de597d2e9d6ef455178f5a201b8c29703b88aab9ef43ebfa716b0616eed748eba7a503fb913ec45 -> 38c1b3a08da2bf48b2c1774dfe4aeb5db3131ddd6e6859de12bd774facdb695517a01270947450ab742b0622b5f1db98a46a1ebcfa35f7e3366d30c6a2c92919
DEBUG 2020-03-05T09:04:45Z: sccache::compiler::compiler: [sp_wasm_interface]: Requesting allocation
DEBUG 2020-03-05T09:04:45Z: sccache::compiler::compiler: [sp_wasm_interface]: Running job
DEBUG 2020-03-05T09:04:45Z: sccache::compiler::rust: Packaging compile inputs for compile
TRACE 2020-03-05T09:04:45Z: sccache::compiler::rust: Identified dependency crate names: {"core", "byte_slice_cast", "rustc_std_workspace_core", "compiler_builtins", "alloc", "arrayvec", "parity_scale_codec_derive"}
TRACE 2020-03-05T09:04:45Z: sccache::dist::http::client: Compressed inputs from 24642048 -> 8403629
 INFO 2020-03-05T09:04:46Z: sccache::compiler::compiler: fetched [("/home/bernhard/Projects/substrate-/target/debug/wbuild/target/wasm32-unknown-unknown/release/deps/sp_wasm_interface-10e57696cdb0b389.d", "Size: 773->180")]
TRACE 2020-03-05T09:04:46Z: sccache::compiler::rust: Pondering on rewriting dep file Some("/home/bernhard/Projects/substrate/target/debug/wbuild/target/wasm32-unknown-unknown/release/deps/sp_wasm_interface-10e57696cdb0b389.d")
TRACE 2020-03-05T09:04:46Z: sccache::compiler::rust: Comparing with /home/bernhard/Projects/substrate/target/debug/wbuild/target/wasm32-unknown-unknown/release/deps/sp_wasm_interface-10e57696cdb0b389.d
 INFO 2020-03-05T09:04:46Z: sccache::compiler::rust: Replacing using the transformer PathTransformer
DEBUG 2020-03-05T09:04:46Z: sccache::compiler::compiler: [sp_wasm_interface]: Compiled but failed, not storing in cache
TRACE 2020-03-05T09:04:46Z: sccache::server: CompileFinished retcode: signal: 1
qTRACE 2020-03-05T09:06:45Z: sccache::server: incoming connection
TRACE 2020-03-05T09:06:45Z: sccache::server: handle_client
DEBUG 2020-03-05T09:06:45Z: sccache::server: handle_client: compile
TRACE 2020-03-05T09:06:45Z: sccache::server: compiler_info Adjusting current working directory for rustup rustc check "/home/bernhard/Projects/substrate" -> "/home/bernhard/Projects/substrate"
TRACE 2020-03-05T09:06:45Z: sccache::compiler::rust: rustup which rustc produced: "/home/bernhard/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc"
TRACE 2020-03-05T09:06:45Z: sccache::server: compiler_info cache hit
DEBUG 2020-03-05T09:06:45Z: sccache::server: check_compiler: Supported compiler
DEBUG 2020-03-05T09:06:45Z: sccache::server: parse_arguments: CannotCache(-): ["-", "--crate-name", "___", "--print=file-names", "--target", "x86_64-unknown-linux-gnu", "--crate-type", "bin", "--crate-type", "rlib", "--crate-type", "dylib", "--crate-type", "cdylib", "--crate-type", "staticlib", "--crate-type", "proc-macro", "--print=sysroot", "--print=cfg"]

[luser]

What does that codec derive do? Is the proc macro looking for the Cargo.toml file? For a distributed compile sccache does not send the Cargo.toml to the remote builder AFAIK. The code that packages inputs for distributed Rust compilation is here:

sccache/src/compiler/rust.rs

Lines 1493 to 1683 in 41a584c

	fn write_inputs(self: Box<Self>, wtr: &mut dyn io::Write) -> Result<dist::PathTransformer> {
	debug!("Packaging compile inputs for compile");
	let RustInputsPackager {
	crate_link_paths,
	crate_types,
	inputs,
	mut path_transformer,
	rlib_dep_reader,
	env_vars,
	} = *{ self };
	// If this is a cargo build, we can assume all immediate `extern crate` dependencies
	// have been passed on the command line, allowing us to scan them all and find the
	// complete list of crates we might need.
	// If it's not a cargo build, we can't to extract the `extern crate` statements and
	// so have no way to build a list of necessary crates - send all rlibs.
	let is_cargo = env_vars.iter().any(|(k, _)| k == "CARGO_PKG_NAME");
	let mut rlib_dep_reader_and_names = if is_cargo {
	rlib_dep_reader.map(|r| (r, HashSet::new()))
	} else {
	None
	};
	let mut tar_inputs = vec![];
	for input_path in inputs.into_iter() {
	let input_path = pkg::simplify_path(&input_path)?;
	if let Some(ext) = input_path.extension() {
	if !super::CAN_DIST_DYLIBS && ext == DLL_EXTENSION {
	bail!(
	"Cannot distribute dylib input {} on this platform",
	input_path.display()
	)
	} else if ext == RLIB_EXTENSION || ext == RMETA_EXTENSION {
	if let Some((ref rlib_dep_reader, ref mut dep_crate_names)) =
	rlib_dep_reader_and_names
	{
	dep_crate_names.extend(
	rlib_dep_reader
	.discover_rlib_deps(&env_vars, &input_path)
	.chain_err(|| {
	format!("Failed to read deps of {}", input_path.display())
	})?,
	)
	}
	}
	}
	let dist_input_path = path_transformer
	.to_dist(&input_path)
	.chain_err(|| format!("unable to transform input path {}", input_path.display()))?;
	tar_inputs.push((input_path, dist_input_path))
	}
	if log_enabled!(Trace) {
	if let Some((_, ref dep_crate_names)) = rlib_dep_reader_and_names {
	trace!("Identified dependency crate names: {:?}", dep_crate_names)
	}
	}
	// Given the link paths, find the things we need to send over the wire to the remote machine. If
	// we've been able to use a dependency searcher then we can filter down just candidates for that
	// crate, otherwise we need to send everything.
	let mut tar_crate_libs = vec![];
	for crate_link_path in crate_link_paths.into_iter() {
	let crate_link_path = pkg::simplify_path(&crate_link_path)?;
	let dir_entries = match fs::read_dir(crate_link_path) {
	Ok(iter) => iter,
	Err(ref e) if e.kind() == io::ErrorKind::NotFound => continue,
	Err(e) => {
	return Err(Error::from(e)
	.chain_err(|| "Failed to read dir entries in crate link path"))
	}
	};
	for entry in dir_entries {
	let entry = match entry {
	Ok(entry) => entry,
	Err(e) => {
	return Err(Error::from(e)
	.chain_err(|| "Error during iteration over crate link path"))
	}
	};
	let path = entry.path();
	{
	// Take a look at the path and see if it's something we care about
	let libname: &str = match path.file_name().and_then(|s| s.to_str()) {
	Some(name) => {
	let mut rev_name_split = name.rsplitn(2, '-');
	let _extra_filename_and_ext = rev_name_split.next();
	let libname = if let Some(libname) = rev_name_split.next() {
	libname
	} else {
	continue;
	};
	assert!(rev_name_split.next().is_none());
	libname
	}
	None => continue,
	};
	let (crate_name, ext): (&str, _) = match path.extension() {
	Some(ext) if libname.starts_with(DLL_PREFIX) && ext == DLL_EXTENSION => {
	(&libname[DLL_PREFIX.len()..], ext)
	}
	Some(ext) if libname.starts_with(RLIB_PREFIX) && ext == RLIB_EXTENSION => {
	(&libname[RLIB_PREFIX.len()..], ext)
	}
	Some(ext) if libname.starts_with(RLIB_PREFIX) && ext == RMETA_EXTENSION => {
	(&libname[RLIB_PREFIX.len()..], ext)
	}
	_ => continue,
	};
	if let Some((_, ref dep_crate_names)) = rlib_dep_reader_and_names {
	// We have a list of crate names we care about, see if this lib is a candidate
	if !dep_crate_names.contains(crate_name) {
	continue;
	}
	}
	if !path.is_file() {
	continue;
	} else if !super::CAN_DIST_DYLIBS && ext == DLL_EXTENSION {
	bail!(
	"Cannot distribute dylib input {} on this platform",
	path.display()
	)
	}
	}
	// This is a lib that may be of interest during compilation
	let dist_path = path_transformer
	.to_dist(&path)
	.chain_err(|| format!("unable to transform lib path {}", path.display()))?;
	tar_crate_libs.push((path, dist_path))
	}
	}
	let mut all_tar_inputs: Vec<_> = tar_inputs.into_iter().chain(tar_crate_libs).collect();
	all_tar_inputs.sort();
	// There are almost certainly duplicates from explicit externs also within the lib search paths
	all_tar_inputs.dedup();
	// If we're just creating an rlib then the only thing inspected inside dependency rlibs is the
	// metadata, in which case we can create a trimmed rlib (which is actually a .a) with the metadata
	let can_trim_rlibs = if let CrateTypes {
	rlib: true,
	staticlib: false,
	} = crate_types
	{
	true
	} else {
	false
	};
	let mut builder = tar::Builder::new(wtr);
	for (input_path, dist_input_path) in all_tar_inputs.iter() {
	let mut file_header = pkg::make_tar_header(input_path, dist_input_path)?;
	let file = fs::File::open(input_path)?;
	if can_trim_rlibs
	&& input_path
	.extension()
	.map(|e| e == RLIB_EXTENSION)
	.unwrap_or(false)
	{
	let mut archive = ar::Archive::new(file);
	while let Some(entry_result) = archive.next_entry() {
	let mut entry = entry_result?;
	if entry.header().identifier() != b"rust.metadata.bin" {
	continue;
	}
	let mut metadata = vec![];
	io::copy(&mut entry, &mut metadata)?;
	let mut metadata_ar = vec![];
	{
	let mut ar_builder = ar::Builder::new(&mut metadata_ar);
	ar_builder.append(entry.header(), metadata.as_slice())?
	}
	file_header.set_size(metadata_ar.len() as u64);
	file_header.set_cksum();
	builder.append(&file_header, metadata_ar.as_slice())?;
	break;
	}
	} else {
	file_header.set_cksum();
	builder.append(&file_header, file)?
	}
	}
	// Finish archive
	let _ = builder.into_inner()?;
	Ok(path_transformer)
	}

[drahnr]

I think the root cause here is different.

A build.rs is triggering a cargo build from within that to create a wasm binary, to include in the final build via include!() macro.

Hence IIUC the easiest way would be to be able to prevent build.rs (or whatever is referenced in Cargo.toml) being excluded from dist compilation.

I would very happily implement that if that would be a desired change :)

[drahnr]

Possibly related to #675

[luser]

A build.rs is triggering a cargo build from within that to create a wasm binary, to include in the final build via include!() macro.

Oh, that's exciting. I would think that most build.rs compiles would not be distributed since they're usually producing a binary directly and sccache doesn't cache those compiles. Is the problem that it's distributing the compile driven by the build.rs and that is failing?

[drahnr]

I have yet to debug through this, the assumption currently is as you say: the build.rs is compiled via sccache-dist, the Cargo.toml it tries launch cargo for does not exist, hence it fails.
The environment is passed (as in not modified, besides the target dir IIRC) to the subprocess. So that cargo instance launched from the build.rs is also aware of the env var RUSTC_WRAPPER, but it never gets that far.
Hence my idea to exclude the build.rs by default, since it can contain all kind of mockery depending on the project setup and specifics (read git sha, inject things based on the current machine into the binary - while I do not recommend any of these, they are pretty common unfortunately).

[luser]

OK, I looked into this a little more. The codec derive is using this proc-macro-crate crate:
https://github.com/paritytech/parity-scale-codec/blob/5f668d00b20fd0ad61e891b6483b3eefcde5e232/derive/Cargo.toml#L16
https://github.com/paritytech/parity-scale-codec/blob/5f668d00b20fd0ad61e891b6483b3eefcde5e232/derive/src/lib.rs#L42

That crate does in fact attempt to read Cargo.toml:
https://github.com/bkchr/proc-macro-crate/blob/721d5a9dbdedbfbb6d160a3e4a482226d8edb178/src/lib.rs#L82

Since sccache does not send Cargo.toml to the build server this fails.

[drahnr]

So the issue that causes the failure is a different than the one I spotted:
https://github.com/paritytech/substrate/blob/master/bin/node/runtime/build.rs
https://github.com/paritytech/substrate/blob/master/utils/wasm-builder/src/prerequisites.rs#L63-L65
https://github.com/paritytech/substrate/blob/master/utils/wasm-builder/src/lib.rs#L188-L194
which would fail as I described before, but that is worth a separate ticket I guess

[DemiMarie]

I would prefer extra build requirements to be explicitly added.

[luser]

This likely needs upstream rustc changes to make it possible for sccache to know what's happening. There's rust-lang/rust#73921, which proposes adding an API that proc macros could use to indicate that they use external files. If that was added and proc macros like the one mentioned here used it then rustc could list these files in --output=dep-info. sccache already parses that output to use as inputs to the hash key so it could also use it to gather files for distributed compilation.

Side note: this made me realize that this is also currently a caching hazard in that sccache doesn't know to include files like this in the hash, so it could produce incorrect results from the cache.