Make host_root return host.root(), not host.dest()

[khyperia]

Also create host_dest function to let other callsites retain their old functionality. Fixes #8817, verified it works on the original problem reported in the rust-gpu repo.

I did two things here:

1. Rename host_root (which returns self.host.dest()) to be host_dest. This has three callsites. I did this to make it more clear that it returns dest, not root.
2. For the callsite that's relevant in fingerprint.rs uses different target roots when serializing/deserializing #8817, I created a "new" host_root function (that returns self.host.root()). This means that the callsite that this PR is actually fixing doesn't show up in this diff :/ - but I thought it was more clear this way.

(Also copied the example path docs over from layout.rs to hopefully avoid this mistake again in the future)

I tried to look into if the other two callsites should actually be calling host.root() instead of dest, because I imagine the same mistake could have been made again, but it quickly grew out of my understanding (this is my first time in the cargo codebase). Feel free to let me know if they should also call host.root() too, and I can update them.

Thanks! (oh gosh I have no idea what I'm doing, I hope this is right)

r? @alexcrichton

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[alexcrichton]

Nah this looks great, thanks! Would it be possible to add a test of some form for this as well?

[ehuss]

Thanks!

Do you think you could add a test for this? It would probably go in freshness.rs. There is some documentation on running and writing tests at https://doc.crates.io/contrib/tests/index.html. I can also help answer questions or help with writing it.

I'm a little uncertain what exactly the issue is, though. Is there a command I can run in the rust-gpu repo to reproduce the problem?

[khyperia]

I'm a little uncertain what exactly the issue is, though. Is there a command I can run in the rust-gpu repo to reproduce the problem?

There's a bit of setup before you can build rust-gpu, overviewed in the getting started guide - tl;dr install the pinned nightly version, add rust-src rustc-dev llvm-tools-preview components, install SPIRV-Tools binaries to PATH, then cargo build.

There's a few fiddly dependency bugs, but theoretically, from a clean build, running cargo build followed by cargo build will not be a no-op, but rather rebuild a couple crates. This is because the dep file in target/spirv-unknown-unknown/release/example_shader.d has the nonexistent path /.../target/release/debug/deps/librustc_codegen_spirv.so (it should be target/debug/deps, not target/release/debug/deps), and so cargo says "oh, missing file in the deps, better rebuild".

Then, follow the logic of #8817 to see how that path in that file leads to this change.

It certainly is a massive and bizzarely-set-up project, though (building a compiler and using it requires shenanigans), which obscures things a bit.

Do you think you could add a test for this?

Yeah!

[khyperia]

Oh, I just realized, cargo itself doesn't consume the incorrect .d file, that's probably why this bug has gone unnoticed - it's intended to only be used by external build systems (which is technically what's happening in rust-gpu, even though the "external build system" is actually another cargo)

cargo/src/cargo/core/compiler/output_depinfo.rs

Lines 16 to 23 in becb4c2

	//! On top of all this, Cargo emits its own dep-info files in the output
	//! directory. This is done for every "uplifted" artifact. These are intended
	//! to be used with external build systems so that they can detect if Cargo
	//! needs to be re-executed. It includes all the entries from the `rustc`
	//! dep-info file, and extends it with any `rerun-if-changed` entries from
	//! build scripts. It also includes sources from any path dependencies. Registry
	//! dependencies are not included under the assumption that changes to them can
	//! be detected via changes to `Cargo.lock`.

In any case, test written! fails before this commit, succeeds afterwards.

---

edit: also, if you'd like, feel free to ping me on Embark discord, rust-gpu channel, or the rust-lang zulip (I'm either "Ashley Hauck" or khyperia, I forget), if you'd like to poke around what utterly cursed things we're doing to cargo in rust-gpu, and perhaps ways we could do things better. (I'm guessing there might be questions around why we're consuming cargo dep files into another cargo...). It's very similar to the wasm problem of "an x86 crate wants to reference a wasm crate and embed the binary in itself to use it", except spir-v, not wasm, and we need to build the compiler backend for spir-v too.

[ehuss]

Looks good, thanks!

@bors r+

[bors]

📌 Commit 628f701 has been approved by ehuss

[bors]

⌛ Testing commit 628f701 with merge 7197c66...

[bors]

☀️ Test successful - checks-actions
Approved by: ehuss
Pushing 7197c66 to master...