Cargo 1.91.0 doesn't lock the build directory on illumos

[pietroalbini]

Cargo 1.91.0 (in #15935) switched from implementing its own build directory locking code to the recent File::lock family of functions in the standard library. It did so while maintaining the code (#4116) that ignores file locking if the API returns "unsupported".

Unfortunately the standard library's implementation of locking was not fully equivalent to Cargo's. Cargo used flock on every platform except for Solaris (which used a custom implementation), while the standard library uses flock on a set of allowlisted targets, the same custom implementation in Solaris, and returns "unsupported" for other platforms.

Turns out that due to an oversight illumos was not included in the allowlist of platforms supporting flock. This means build directory locking is ignored by Cargo 1.91.0 on illumos even if the underlying filesystem supports it.

The standard library missing illumos is tracked in rust-lang/rust#146312, and a fix adding illumos to the standard library's allowlist is in rust-lang/rust#148322.

[weihanglo]

I am not sure what the best way forwards

• Revert the use of std flock in Cargo
• Patch up flock in std

I would love to see the latter getting better coverage though.

[epage]

Ah, it was Solaris that we were aware of and blocking on (rust-lang/rust#146269)

I am not sure what the best way forwards

My interpretation is this is more of a heads up to officially track the impact to Cargo users since rust-lang/rust#148322 already exists.

[weihanglo]

Ah I see rust-lang/rust#148322. Do people think it is worthy we revert #15935 in beta only, and keep nightly with std flock? That would give us one less broken version on illumos.

[epage]

Would be good to know if std gets a backport

[pietroalbini]

Apart from the specific problem affecting illumos in this release, I could see this affecting future targets gaining Cargo support.

Before the code that swallows "unsupported" errors would only be triggered if the underlying filesystem didn't support file locking (for example on some NFSes). Now it can be triggered both if the filesystem is not compatible, or if the target is not allowlisted in std. I'm not sure what the best way to notice that would be though (maybe a test failing if locking is not supported, so that target maintainers notice it during porting?).

My interpretation is this is more of a heads up to officially track the impact to Cargo users since rust-lang/rust#148322 already exists.

Yep!

Do people think it is worthy we revert #15935 in beta only, and keep nightly with std flock?

I don't think it's worth it, I fully expect the std change to get backported into beta.