Skip to content

Improved handling of non-writable rlib/rmeta files #140164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
j-baker opened this issue Sep 15, 2023 · 5 comments
Open

Improved handling of non-writable rlib/rmeta files #140164

j-baker opened this issue Sep 15, 2023 · 5 comments
Labels
A-metadata Area: Crate metadata C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@j-baker
Copy link

j-baker commented Sep 15, 2023

Problem

I use Crane to build my projects in a Nix environment. This pretty much ends up being vanilla cargo, but with one difference. Crane builds dependencies in one Nix build, storing the resulting target dir, and then moves it into place for the subsequent build. The advantage is that when subsequent builds run, they do not have to rebuild dependencies. This is convenient in a CI environment, as it provides for transparent caching.

Crane uses two mechanisms to handle this linking procedure. In the old one, it copies everything. This is fast compared to recompiling, but slow compared to a normal cargo workflow, as the data is voluminous. It additionally bloats the output size.
The other mechanism is symlinking, where it adds symlinks for each rlib/rmeta file into the Cargo target directory. These symlinks are read-only.

This mostly works, but fails when Cargo decides that a given library needs to rebuilt. Here, something like:

error: output file /private/tmp/nix-build-retina-clippy-0.0.0.drv-0/source/target/release/deps/libopencv-6a1a900b840d12b6.rmeta is not writeable -- check its permissions

might be printed by rustc. I believe this is because Cargo is telling rustc to emit to the symlink file, which is then resolved to the read-only path. Rebuilds are typically not required, but can happen due to a buggy build.rs (e.g. a build.rs which emits a non-existent file as a reason to rebuild).

Proposed Solution

I could see three potential solutions here.

  1. This naively seems like a failure in content-addressability, in that Cargo is deciding it needs to rewrite files into files with the same hash. Not sure what guarantees Cargo provides here, but given how well Rust tooling is thought through in general, this seems somewhat unlikely.
  2. If Cargo decides that a library needs to be rebuilt and the relevant rlib/rmeta files are not writeable/are a symlink, delete the symlink before continuing. The user's intentions are clear and a non-writeable symlink target is not the same as a non-writeable output.
  3. Cargo could allow reading from alternative sources of compiled data (as some kind of fallback). Essentially, allow the user to specify an extra target dir, and Cargo looks in here for compiled data if not otherwise present. This likely suffers from concurrency issues around build-dir locking.

There are additional crane-side solutions that could be applied (one simple one might be creating the cargo target dir as an overlayfs, for example). I'd note that I don't believe this problem is exclusively crane related. I've filed a similar tracking issue on Crane itself as ipetkov/crane#385.

Notes

It's very possible that I've misunderstood the separation of responsibilities between rustc and cargo - if so, my apologies. It's also very possible that I'm considering this issue too myopically - that there is some larger improvement that would solve my workflow - I tried to be minimalistic here because piecewise minimalist solutions to problems are usually faster to resolve if project maintainers are amenable to them.

Lastly, it's possible that this is considered a reasonable wontfix - I'm surfacing here because I quite like the Crane behaviour and would love it to be less error-prone.

@j-baker j-baker added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Sep 15, 2023
@ehuss ehuss added the A-layout Area: Memory layout of types label Sep 25, 2023
@joshtriplett
Copy link
Member

joshtriplett commented Apr 22, 2025

If Cargo decides that a library needs to be rebuilt and the relevant rlib/rmeta files are not writeable/are a symlink, delete the symlink before continuing. The user's intentions are clear and a non-writeable symlink target is not the same as a non-writeable output.

This seems reasonable, but cargo should mostly not be responsible for this, because rustc is the one writing these files. So this issue is the domain of rustc.

Cargo could allow reading from alternative sources of compiled data (as some kind of fallback). Essentially, allow the user to specify an extra target dir, and Cargo looks in here for compiled data if not otherwise present. This likely suffers from concurrency issues around build-dir locking.

We've talked about having a user-wide cache of built libraries. It's a challenge, since every possible configuration or input of a library needs to be taken into account, but it is something we'd love to do if possible. rust-lang/cargo#5931

@epage
Copy link
Contributor

epage commented Apr 22, 2025

This naively seems like a failure in content-addressability, in that Cargo is deciding it needs to rewrite files into files with the same hash. Not sure what guarantees Cargo provides here, but given how well Rust tooling is thought through in general, this seems somewhat unlikely.

You can see our documentation for what causes a rebuild (fingerprint) vs what causes a unique file (extra-filename) at https://doc.rust-lang.org/nightly/nightly-rustc/cargo/core/compiler/fingerprint/index.html#fingerprints-and-unithashs

That does not fully get into intent. One I can think of is having a low-effort GC. Source changes could rapidly expand the size of our cache if we tracked them in our hash. By only tracking them in our fingerprint, we clean up the previous versions automatically. Granted, this isn't ideal when back and forth between commits. We're just now starting to get a more feature rich GC for other parts of Cargo (#14287) and we have more work to do before we can consider a GC for intermediate artifacts and whether that can change how we generate our hash.

@epage
Copy link
Contributor

epage commented Apr 22, 2025

Cargo could allow reading from alternative sources of compiled data (as some kind of fallback). Essentially, allow the user to specify an extra target dir, and Cargo looks in here for compiled data if not otherwise present. This likely suffers from concurrency issues around build-dir locking.

You can share a target-dir today but there are several issues related to it. rust-lang/cargo#14125 adds build-dir which reduces the number of issues. After that is stabilized, we'll be looking at changing the layout o the build dir which will unblock exploration of changing our locking system which is being tracked in rust-lang/cargo#4282.

@epage
Copy link
Contributor

epage commented Apr 22, 2025

Since (1) is not likely to move forward (#140164) and (3) has other issues tracking it (#140164, #140164), that just leaves (2).

I'm going to transfer this so the compiler folks can evaluate whether they want to change how rlibs are written to allow for this scheme.

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Apr 22, 2025
@epage epage transferred this issue from rust-lang/cargo Apr 22, 2025
@jieyouxu jieyouxu added A-metadata Area: Crate metadata T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed A-layout Area: Memory layout of types needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Apr 30, 2025
@jieyouxu
Copy link
Member

Maybe related: rust-lang/compiler-team#790

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-metadata Area: Crate metadata C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants