Don't panic when failing to initialize incremental directory.

[ehuss]

This removes a panic when rustc fails to initialize the incremental directory. This can commonly happen on various filesystems that don't support locking (often various network filesystems). Panics can be confusing and scary, and there are already plenty of issues reporting this.

This has been panicking since 1.22 due to I think #44502 which was a major rework of how things work. Previously, things were simpler and the load_dep_graph function would emit an error and then continue on without panicking. With 1.22, load_dep_graph was changed so that it assumes it can load the data without errors. Today, the problem is that it calls prepare_session_directory and then immediately calls garbage_collect_session_directories which will panic since the session is IncrCompSession::NotInitialized.

The solution here is to have prepare_session_directory return an error that must be handled so that compilation stops if it fails.

Some other options:

• Ignore directory lock failures.
• Print a warning on directory lock failure, but otherwise continue with incremental enabled.
• Print a warning on directory lock failure, and disable incremental.
• Provide a different locking mechanism.

Cargo ignores lock errors if locking is not supported, so that would be a precedent for the first option. These options would require quite a bit more changes, but I'm happy to entertain any of them, as I think they all have valid justifications.

There is more discussion on the many issues where this is reported: #49773, #59224, #66513, #76251. I'm not sure if this can be considered closing any of those, though, since I think there is some value in discussing if there is a way to avoid the error altogether. But I think it would make sense to at least close all but one to consolidate them.

[rust-highfive]

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

[estebank]

Is there any way to create a regression test to fake a locking error on an incremental compilation?

I'm ok with the approach but am unclear on what the user will see if they hit this case going forward.

As for the options, my preference would be a warning if the user can actually do anything about the problem (even if it is just letting them know that maybe they want to disable incremental compilation), and if not and we can "gracefully" continue as if we had locked the file and plow forward without adverse effects, be silent like cargo is. That being said, I figure that we can't recover gracefully, so an error instead of an ICE is fine by me.

[ehuss]

I don't think there is a reasonable way to simulate a lock failure on CI. I tested on macOS (using an smb share) and on Windows (using a WSL2 share), and it worked as expected. I couldn't get an error on Linux, but I didn't try too hard (smb, NFS, or ramdisks seemed like good candidates, but I couldn't get them to fail).

The error looks something like this:

Compiling foo v0.1.0 (/Volumes/home/Temp/foo)
error: incremental compilation: could not create session directory lock file: Operation not supported (os error 45)
|
= note: the filesystem for the incremental path at /Volumes/home/Temp/foo/target/debug/incremental/foo-i5li2empboxb/s-fyxs59ufkp-rv543z-working does not appear to support locking, consider changing the incremental path to a filesystem that supports locking or disable incremental compilation

error: aborting due to previous error

error: could not compile foo

I added a test that verifies the general behavior when it fails to initialize the session directory by making a read-only directory. I'm not sure if that is worth it, but it should cover the general issue of it failing with ICE.

I changed my mind about possibly ignoring lock failures. bjorn3 pointed out on zulip that concurrent access could lead to silent miscompilation. I'd like to stay conservative for now and leave it as an error.

[estebank]

I'm ready to r+, but...

consider changing the incremental path to a filesystem that supports locking or disable incremental compilation

Could we change this to include an explanation or link on how to do either of those things? We could link to https://doc.rust-lang.org/cargo/reference/profiles.html#incremental and/or mention setting the env variable CARGO_INCREMENTAL=0.

Either way, if you don't want to change the wording, r=me at your convenience.

[ehuss]

I added some extra text on how to resolve the error. It is a little wordy, but hopefully it should help (and hopefully this error is rare). It looks like this:

@bors r=estebank

[bors]

📌 Commit e7411a2 has been approved by estebank

[bors]

⌛ Testing commit e7411a2 with merge 29b3ce92b75637d52cd8b037bc03b0428e99e229...

[bors]

💔 Test failed - checks-actions

[ehuss]

@bors r=estebank

Fixed the test so that it works correctly when run as root.

[bors]

📌 Commit 4c550bc has been approved by estebank

[bors]

⌛ Testing commit 4c550bc with merge b663c0f...

[bors]

☀️ Test successful - checks-actions
Approved by: estebank
Pushing b663c0f to master...