'cargo metadata' fails on read-only file system

[RalfJung]

Problem

I first reported this with rust-analyzer at rust-lang/rust-analyzer#10792: when running cargo metadata on a read-only file system, it fails saying

rust-analyzer failed to load workspace: Failed to read Cargo metadata for Rust sources: Failed to run `cargo metadata --manifest-path /home/r/.rustup/toolchains/miri/lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.toml`: `cargo metadata` exited with an error:     Updating crates.io index
error: failed to write /home/r/.rustup/toolchains/miri/lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.lock

Caused by:
  failed to open: /home/r/.rustup/toolchains/miri/lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.lock

Caused by:
  Read-only file system (os error 30)
rust-analyzer failed to load workspace: Failed to read Cargo metadata for Rust sources: Failed to run `cargo metadata --manifest-path /home/r/.rustup/toolchains/miri/lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.toml`: `cargo metadata` exited with an error:     Updating crates.io index
error: failed to write /home/r/.rustup/toolchains/miri/lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.lock

Caused by:
  failed to open: /home/r/.rustup/toolchains/miri/lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.lock

Caused by:
  Read-only file system (os error 30)

However, the action RA is trying to perform is fundamentally a read-only operation (querying for information about the rustc compiler crates).

I don't think the currently available flags provide any way to make this work on a read-only file system either (I tried --frozen but as expected it refuses to download required crates from the network). Even if my toolchain was on a read-write file system I would not want RA's cargo metadata to change toolchain files, after all.

Proposed Solution

cargo metadata should either handle read-only file systems by falling back to not creating/updating the lock file -- or (IMO the right fix) it should not even attempt to mutate the workspace when what it is doing is a read-only operation like just determining the metadata.

Notes

No response

[ehuss]

Can you say more about your use case about why the Cargo.lock is out of date? You wouldn't be able to run any other cargo command, either, so just allowing cargo metadata to work with an out-of-date lockfile may not help much.

[bjorn3]

The rustc-src rustup component doesn't have the root Cargo.toml so the root Cargo.lock isn't used. Rust-analyzer runs cargo metadata for the compiler/rustc crate which normally doesn't need it's own Cargo.lock as it uses the workspace's version, but because the root Cargo.toml is missing, this isn't possible.

[RalfJung]

You wouldn't be able to run any other cargo command, either, so just allowing cargo metadata to work with an out-of-date lockfile may not help much.

AFAIK cargo metadata is the only command that needs to work for RA to be able to proceed here -- but I am not sure and I wouldn't know how to test this.

[aos]

Hi all - I ran into this issue while loading stdlib from a read-only location (attempting to access it from Nix store). I dug into this a bit and we can trace it down to resolve_with_registry writing the lock file:

cargo/src/cargo/ops/resolve.rs

Lines 206 to 208 in b816d82

	if !ws.is_ephemeral() && ws.require_optional_deps() {
	ops::write_pkg_lockfile(ws, &mut resolve)?;
	}

Since we don't write the lock file if the workspace is already marked as ephemeral, can we make the assumption that a read-only file system is inherently "ephemeral" and mark the workspace as such when creating it? The only other place where we check for is_ephemeral is when preloading a provided registry here:

cargo/src/cargo/core/workspace.rs

Lines 1029 to 1037 in b816d82

	pub fn preload(&self, registry: &mut PackageRegistry<'cfg>) {
	// These can get weird as this generally represents a workspace during
	// `cargo install`. Things like git repositories will actually have a
	// `PathSource` with multiple entries in it, so the logic below is
	// mostly just an optimization for normal `cargo build` in workspaces
	// during development.
	if self.is_ephemeral {
	return;
	}

Perhaps the metadata command can be instantiated with an ephemeral workspace? Or checking to see if the manifest_path is read-only, and then doing some operation to not allow the lock file to be written. Unfortunately, this is my only usage of this command, so I don't know other use cases. Either way, I'm happy to work on this one!

[bjorn3]

I don't think a read-only fs ahould be considered ephemeral. It can be compiled (or get metadata retrieved) twice in whoch case the same dependency versions should be used. I see rust-analyzer only reloading cargo metadata when Cargo.* changes as an optimization. cargo metadata should be an idempotent operation. That is running it twice will result in the same result. If there is no Cargo.lock that will not be true. The real fix would be to ship a correct Cargo.lock file as part of the rustc-src component.

[aos]

That makes sense - thanks for the insight.

[RalfJung]

I would say even more important than being idempotent is it being a read-only operation. It is merely querying some information and state from a crate. If that information depends on things like the crate registry, I am not surprised by multiple queries returning different results. (I would not expect curl to be idempotent, either.) However, I am severely surprised by such a command changing the on-disk state of my project, and even more surprised when there is no fallback for when performing that unexpected mutation fails.

[leoluk]

I would say even more important than being idempotent is it being a read-only operation.

Most cargo commands will mutate the lockfile by default - you need to specify --frozen or --locked to prevent that.

[RalfJung]

Yeah but those will just fail in that case, so that is not really helpful either.

I honestly don't think there is a good reason that a query like cargo metadata requires read-write access to the project files, no matter whether the lockfile is somehow out-of-date or not. Sure we could work around this issue by shipping a slightly different lockfile, but IMO that's a hack, no a solution.

[bjorn3]

Sure we could work around this issue by shipping a slightly different lockfile, but IMO that's a hack, no a solution.

The problem is that the lockfile is outdated. It is the solution. Ignoring lockfile write errors is a hack IMO. cargo metadata --frozen is the only mode of operation I would expect to be entirely side effect free. Anything else may need to download crates from crates.io and store them in ~/.cargo. In addition cargo build is conceptually just cargo metadata followed by deriving a list of build commands from the result. If cargo metadata didn't write the lockfile, cargo build wouldn't either.

[RalfJung]

In addition cargo build is conceptually just cargo metadata followed by deriving a list of build commands from the result. If cargo metadata didn't write the lockfile, cargo build wouldn't either.

I see no good reason why it has to be like that. It certainly does not match my mental model at all.

cargo build has good reasons to fail when called on a crate stored on a read-only FS. cargo metadata IMO does not.

Anything else may need to download crates from crates.io and store them in ~/.cargo.

Populating a cache is (conceptually) not a side-effect (formally speaking: a memoized function is observably pure). So I don't mind it changing ~/.cargo. That is categorically different from mutating the crate it is working on.

[RalfJung]

Not sure if that is related, but there are also cases where cargo metadata fails because it cannot update the index, but doing a build actually works fine -- see rust-lang/rust-analyzer#12499.

[gilescope]

It seems that this can trigger unnecessary rebuilds if a rerun_if_changed is issued on Cargo.lock.

[RalfJung]

Unfortunately it seems that even if a lockfile exists that has all the required information to make the metadata call idempotent, cargo will still insist on writing to that lockfile if the lockfile also contains information for other creates that are not present in the workspace. Even if one buys into the idempotency argument (which I don't), that should still be considered a bug.

This bug means that just copying the lockfile from lib/rustlib/rustc-src/rust/Cargo.lock to lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.lock is not enough to make RA work with a read-only rustup directory.

[GenericNerdyUsername]

This seems to have stalled and I need it to be fixed. Whats needed for progress?

[epage]

@GenericNerdyUsername generally a good first step is to summarize the existing discussion and the different angles.

For example, this thread has people with a couple different perspectives

• cargo metadata should be consistent in its output which can only happen if we can write a Cargo.lock to the filesystem
• cargo metadata should always let me query information, even it means the information changes due to outside changes (e.g. a new version of a dependency is released so it gets selected)

So not much so far. Now, putting on my cargo hat, the problem I see with the latter is user intention. Today, you can run cargo metadata --no-deps and no Cargo.lock gets generated. Once you ask for dependencies, you are asking for dependencies of that instance of the project and not some ephemeral state. There might be cases, like in this thread, where something else is needed, but that likely runs counter to other uses. To move this forward, we'd need to find a way to balance these two needs.

Some potential ideas

• If we had RFE: add option to specify path to Cargo.lock #5707, then the caller can decide that they don't care about the exact state of the dependencies, check if its read-only, and specify a lockfile from a temp location. In this case, a caller like rust-analyzer could even make that temp location keyed based off of the project so it stays consistent
• With Cargo time machine (generate lock files based on old registry state) #5221 and cargo script, we've talked about the idea of locking to timestamps, rather than to specific dependencies. This is a lot more radical idea with a lot of steps in the middle to be figured out.
• If a workaround is to allow reusing lockfiles between projects, then we could limit what project information we put in (e.g. Cargo.lock contains project's own version, unnecessarily #5979) and make it so we skip writing the lockfile if the one on-disk is a superset of what we've resolved. A problem with that is removing dependencies won't change your lockfile which might surprise and upset people (particularly if they are relying on the lockfile for audits). Maybe we could check if the lockfile is a superset of what was resolved and skip the write if it errored?

[GenericNerdyUsername]

the solution seems pretty clear to me, if the user wants idempotent behavior but readonly is implemented, they can just update the lockfile beforehand. If the user wants RO behavior but idempotent is whats implemented, they're screwed (afaict).

[epage]

Another potential way of addressing this is error recovery. We report all errors that happened along the way and we report the metadata result (with a bad exit code).

However, if someone wants to only ignore read-only file systems but error for the rest (like bad toml syntax), it might be difficult.

[GenericNerdyUsername]

What would be the downsides of not writing to the lockfile, other than needing another command beforehand if you want consistent output?

[GenericNerdyUsername]

any change on this?

[Ifropc]

Just my 5 cents:
For the mentioned above cases, I believe the first mentioned option (#5707) would be the best solution. It feels like the easiest approach to write something like a lock file on a modifiable FS while keeping original folder read-only with underlying .toml file. The caller in this case should be responsible for managing the lockfile. (it's also possible to add a root directory storing lockfiles in Cargo config file in the future)
As for the #5979 suggestion, I would imagine the issue would still happen if the user would want to index a freshly downloaded nightly on a read-only FS (as no superset lockfile would exists), or if the user simply has a fresh install on read-only FS. (Please correct me if I'm wrong)
And as mentioned before, #5221 could be a bit too complex in a scope of this issue.

That being said, it looks like there was an attempt to make an alternative solution with introducing a new flag in #10716 which was previously rejected.

I'm not sure that is something that would warrant supporting or adding a new flag for, at least not without significant justification.

@epage do you know by any change, if let's say #5707 was implemented, this issue is a strong enough argument for adding a new flag? I'd imagine the team wouldn't want to add new flags on a whim, but this issue seem to be impactful enough (e.g. people are experiencing Jetbrains IDEs such as rust-rover not working on read-only installations of rust)

I'd love to take on #5707 implementation (as it seems to not be too complicated, at least at the moment), but as a first-time committer would be happy to hear opinion of someone's experienced.

[GenericNerdyUsername]

you could implement the ro mechanism without adding another flag by adding a special case for --lockfile-path

[Ifropc]

you could implement the ro mechanism without adding another flag by adding a special case for --lockfile-path

I think as long lockfile is written in another directory, original toml path can be RO (assuming we adding a --lockfile-path). I don't think any other flag would be necessary.

[epage]

We discussed #5707 in today's Cargo team meeting and are good with it moving forward. I've marked it as "Accepted" and laid out some high level information. I would recommend checking out https://doc.crates.io/contrib/ for more details on processes (both PR processes and "how to add unstable features")

[Ifropc]

Thanks for the follow-up and providing the design @epage!
I'm going to scope out the work to be done and follow up with the questions (if any) in the #5707
Let's keep the conversation there and I'll try to get the implementation out ASAP as this issue been lingering for a while

[Ifropc]

FYI: lockfile path PR has been merged. We can now call cargo metadata --lockfile-path /my/write/path/Cargo.lock
@GenericNerdyUsername @RalfJung you might be interested in it ^

[weihanglo]

Just FYI, it will be available in next one or two nightly release rust-lang/rust#129178.

[RalfJung]

Awesome, thanks. :)

I guess now the question is whether RA is interested in using this feature to support running RA in situations where the sysroot is in a read-only location. I reopened rust-lang/rust-analyzer#10792 for that.