Rebuild crate dependencies when the rust compiler version changes

[ekump]

I've noticed that every time a new version of Rust is released my Heroku deployments fail and I have to purge the build cache using the Heroku CLI. It turns out that there is one particular crate that has a build script that fails. I intend on working with the maintainer of the crate to make their build script more resilient. But, I thought it might be nice to also have the option to rebuild all crates when a new version of Rust is released.

[emk]

I'm sorry to hear about this issue!

I haven't looked at this problem in detail, but I feel like one of two things is true (at least in an ideal world):

1. Clearing the build cache is properly the responsibility of cargo, but it's failing. The ideal fix here would be to fix either cargo or the library that is causing problems.
2. We can't count on cargo to clear things, in which case we should always clear it manually after a compiler upgrade. In this case, we don't really need a configuration variable which controls this.

Since you've looked at this problem more closely than I have, what do you think?

[ekump]

Clearing the build cache is properly the responsibility of cargo, but it's failing.

I agree. I decided to dig deeper into what is going on and I don't believe the buildpack should have to worry about the build cache. When the compiler is upgraded it triggers the project to rebuild its dependencies. But it doesn't clean the target folders for the crates in ~/.cargo/registry/src (or in the buildpack's case cache/cargo/registry/src). The same can be said for running cargo clean on a project. It doesn't actually clean the contents in cargo's registry, just the target/ folder for the current workspace.

The ideal fix here would be to fix either cargo or the library that is causing problems.

I think you're correct. It looks like not cleaning what's in cargo's registry is a known missing feature.

Fixing the library that is a problem is a good idea. I incorrectly assumed it was an issue with the buildpack because the problem only occurs on linux, and I primarily develop on macOS and the only time I was having an issue was when building on Heroku. I just spun up a linux VM and created a simple project with the library as a dependency and was able to recreate the failure by performing cargo build && cargo clean && cargo build. So, it's definitely not the buildpack's fault!

On linux, the custom build script for the library creates a symlink in its target/release/deps folder in cargo/registry/src. If the symlink already exists (because a previous build put it there) it will error out. Clearly this build script should be fixed. But I wonder if the buildpack cleaning cache/cargo/registry/src would be useful until Cargo handles this correctly?

We can't count on cargo to clear things, in which case we should always clear it manually after a compiler upgrade.

As I've investigated this more I've concluded this isn't an issue specific to the compiler updating. It just so happens that the only time the buildpack is rebuilding existing dependencies is when the compiler version is changed.

If you agree that it is a good idea to clean cache/cargo/registry/src then I think the correct thing to do is always perform an rm -rf cache/cargo/registry/src after cargo build. This will achieve the same effect with simpler code. It would also mean we don't have to maintain the rustc version file in the cache. It looks like cargo only references what is in cargo/registry/src when it's rebuilding a crate, which will theoretically only happen on builds with a new version of the compiler so it shouldn't adversely affect build times.

we don't really need a configuration variable which controls this

For my use case I'm ok with not making this a configuration variable. I made it configurable because I was not sure if there are other users who have a longer or more complex build process who don't want to clear the cargo registry folder.

Let me know what you think and I can update or close this PR.

[emk]

This sounds like an upstream bug that should be fixed upstream, if possible. But since that hasn't happened, let's go with a plan where we clear the entire build cache whenever the compiler version changes. (But don't clear anything if the compiler version stays the same.) Sound reasonable?

[ekump]

Sounds good to me. Should I remove the config var?

[ekump]

I fixed the issue with the crate that was causing the error.