Skip to content
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

cargo check --frozen requires the cargo home to be writable on nightly #6928

Closed
pietroalbini opened this issue May 10, 2019 · 4 comments · Fixed by #6940
Closed

cargo check --frozen requires the cargo home to be writable on nightly #6928

pietroalbini opened this issue May 10, 2019 · 4 comments · Fixed by #6940
Labels
C-bug Category: bug

Comments

@pietroalbini
Copy link
Member

Problem

On Rust 1.34.1, as long as all the dependencies were pre-fetched with cargo fetch, running cargo check --frozen never touched the ~/.cargo directory, allowing it to be read-only. This behavior is used by Crater to prevent misbehaving crates from messing up the cargo installation.

The latest nightly instead requires that directory to be writable:

error: failed to acquire package cache lock  

Caused by:
  failed to open: /home/pietro/tmp/repro/tmp-cargo-home/.package-cache

Caused by:
  Permission denied (os error 13)

Steps

#!/bin/bash
set +euo pipefail

if [[ $# -ne 1 ]]; then
    echo "usage: $0 <toolchain-name>"
    exit 1
fi
export RUSTUP_TOOLCHAIN="$1"

# Use a temporary cargo home so the user-wide one is not messed up
export CARGO_HOME=tmp-cargo-home

set -x

# Create a simple repro crate which depends on lazy_static
cargo init --bin repro
cd repro
echo 'lazy_static = "1"' >> Cargo.toml

# Generate the lockfile and fetch the dependencies using it
cargo generate-lockfile
cargo fetch --locked

# Make the cargo home read-only
chmod -w -R "${CARGO_HOME}"

cargo check --frozen
  • bash repro.sh stable for the correct behavior
  • bash repro.sh nightly for the wrong behavior

Possible Solution(s)

After talking to @Eh2406 the issue seems to be caused by #6880.

@pietroalbini pietroalbini added the C-bug Category: bug label May 10, 2019
@Eh2406
Copy link
Contributor

Eh2406 commented May 10, 2019

cc @alexcrichton

@pietroalbini
Copy link
Member Author

Other than ~/.cargo/.package-cache, ~/.cargo/registry/src also seems to be required to be writable from now on. My guess is that before crate unpacking was done during fetch and now during the actual build.

All of this (especially source unpacking, I can work around the package cache thingy) is bad for Crater: we want as much as possible in the containers to be read-only to prevent all the damage misbehaving or malicious crates can do during a run, and locking down {RUSTUP,CARGO}_HOME is one of the key steps of that isolation.

@ehuss
Copy link
Contributor

ehuss commented May 10, 2019

I think the read-only issue with extracting is due to this change: https:/rust-lang/cargo/pull/6880/files#diff-ce3ac564b3688633fe9c7ccb934e6253L470

It previously was careful to open the lock in read-only mode, but it no longer does that.

@pietroalbini
Copy link
Member Author

pietroalbini commented May 10, 2019

If it's needed I can work around the .package-cache file on the Crater side (by mounting only that file in rw mode). I'd really prefer not to mount the whole cargo home rw to allow extracting the source though.

alexcrichton added a commit to alexcrichton/cargo that referenced this issue May 14, 2019
Previously Cargo would attempt to work as much as possible with a
previously filled out CARGO_HOME, even if it was mounted as read-only.
In rust-lang#6880 this was regressed as a few global locks and files were always
attempted to be opened in writable mode.

This commit fixes these issues by correcting two locations:

* First the global package cache lock has error handling to allow
  acquiring the lock in read-only mode inaddition to read/write mode. If
  the read/write mode failed due to an error that looks like a readonly
  filesystem then we assume everything in the package cache is readonly
  and we switch to just acquiring any lock, this time a shared readonly
  one. We in theory aren't actually doing any synchronization at that
  point since it's all readonly anyway.

* Next when unpacking package we're careful to issue a `stat` call
  before opening a file in writable mode. This way our preexisting guard
  to return early if a package is unpacked will succeed before we open
  anything in writable mode.

Closes rust-lang#6928
bors added a commit that referenced this issue May 14, 2019
Re-enable compatibility with readonly CARGO_HOME

Previously Cargo would attempt to work as much as possible with a
previously filled out CARGO_HOME, even if it was mounted as read-only.
In #6880 this was regressed as a few global locks and files were always
attempted to be opened in writable mode.

This commit fixes these issues by correcting two locations:

* First the global package cache lock has error handling to allow
  acquiring the lock in read-only mode inaddition to read/write mode. If
  the read/write mode failed due to an error that looks like a readonly
  filesystem then we assume everything in the package cache is readonly
  and we switch to just acquiring any lock, this time a shared readonly
  one. We in theory aren't actually doing any synchronization at that
  point since it's all readonly anyway.

* Next when unpacking package we're careful to issue a `stat` call
  before opening a file in writable mode. This way our preexisting guard
  to return early if a package is unpacked will succeed before we open
  anything in writable mode.

Closes #6928
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants