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 chef appears to ignore Cargo.lock #252

Open
thomaseizinger opened this issue Jan 24, 2024 · 9 comments
Open

cargo chef appears to ignore Cargo.lock #252

thomaseizinger opened this issue Jan 24, 2024 · 9 comments

Comments

@thomaseizinger
Copy link

thomaseizinger commented Jan 24, 2024

I have the following Dockerfile: https://github.com/libp2p/rust-libp2p/blob/v0.52/interop-tests/Dockerfile.native.

Recently, we released a new version of libp2p-identity, version 0.2.8. That one raises the MSRV in a patch-version. Not necessarily best practice but that is not the point.

If you download the source code above for the v0.52 tag and try to build the Dockerfile using:

sudo docker build -f interop-tests/Dockerfile.native .

It will fail with:

124.1 error: package `libp2p-identity v0.2.8` cannot be built because it requires rustc 1.73.0 or newer, while the currently active rustc version is 1.67.0
124.1 Either upgrade to rustc 1.73.0 or newer, or use
124.1 cargo update -p [email protected] --precise ver
124.1 where `ver` is the latest version of `libp2p-identity` supporting rustc 1.67.0
124.1 thread 'main' panicked at 'Exited with status code: 101', src/recipe.rs:204:27
124.1 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
------
Dockerfile.native:13
--------------------
  11 |     COPY --from=planner /app/recipe.json recipe.json
  12 |     # Build dependencies - this is the caching Docker layer!
  13 | >>> RUN cargo chef cook --release --package interop-tests --bin native_ping --recipe-path recipe.json
  14 |     # Build application
  15 |     COPY . .
--------------------
ERROR: failed to solve: process "/bin/sh -c cargo chef cook --release --package interop-tests --bin native_ping --recipe-path recipe.json" did not complete successfully: exit code: 101

If you change the Dockerfile to the following (i.e. comment out cargo chef):

# syntax=docker/dockerfile:1.5-labs
FROM rust:1.67.0 as chef
RUN wget -q -O- https://github.com/LukeMathWalker/cargo-chef/releases/download/v0.1.62/cargo-chef-x86_64-unknown-linux-gnu.tar.gz | tar -zx -C /usr/local/bin
WORKDIR /app

# FROM chef AS planner
# COPY . .
# RUN cargo chef prepare --recipe-path recipe.json

FROM chef AS builder
# COPY --from=planner /app/recipe.json recipe.json
# Build dependencies - this is the caching Docker layer!
# RUN cargo chef cook --release --package interop-tests --bin native_ping --recipe-path recipe.json
# Build application
COPY . .
RUN cargo build --release --package interop-tests --bin native_ping

FROM gcr.io/distroless/cc
COPY --from=builder /app/target/release/native_ping /usr/local/bin/testplan
ENV RUST_BACKTRACE=1
ENTRYPOINT ["testplan"]

it suddenly works.

The Cargo.lock for the v0.52 tag points to libp2p-identity 0.2.7 which has an MSRV of 1.63 and therefore compiles with the old Dockerfile. It appears that cargo chef ignores Cargo.lock. Is that intended?

@thomaseizinger thomaseizinger changed the title Why does cargo chef ignore Cargo.lock? cargo chef appears to ignore Cargo.lock Jan 24, 2024
thomaseizinger added a commit to libp2p/rust-libp2p that referenced this issue Jan 24, 2024
This is to workaround a bug(?) in `cargo chef` where the lockfile isn't taken into account. This is currently blocking the interop tests.

Related: #5118.
Related: LukeMathWalker/cargo-chef#252.
@mladedav
Copy link
Contributor

mladedav commented Feb 6, 2024

cargo chef stores the contents of Cargo.lock inside the recipe.json file, it's not ignored.

I've tried to reproduce what you're describing but with no luck. I've tried to do this to create a lock file with your dependency at 0.2.7 and the lock file has been correctly recreated in the second directory.

cargo new --lib foo
cd foo
cargo add [email protected]
cargo update -p libp2p-identity --precise 0.2.7
cargo chef prepare --recipe-path recipe.json
mkdir ../bar
cd ../bar
cargo chef cook --recipe-path ../foo/recipe.json

Your project also seems to handle the identity crate specially with a crates.io override in the workspace Cargo.toml and the project is not part of said workspace so cargo build which is invoked by cargo chef might be confused because of that and tries to resolve Cargo.lock again. You can try passing --locked (and --verbose) to cargo chef cook, it might tell you why it decided it needs to be updated by emitting a different error.

When you run the build after COPY . . the local dependency is again reachable so cargo build does not need to update anything. You might want to try copying the identity crate before cargo chef cook if it's not updated often which would invalidate the docker cache.

@muhammad-asn
Copy link

Yes i got this issue too, I try to pass --locked but I got new error instead

 > [builder  3/10] RUN cargo chef cook --release --recipe-path recipe.json --locked:
0.471     Updating crates.io index
5.371 error: the lock file /usr/src/Cargo.lock needs to be updated but --locked was passed to prevent this
5.371 If you want to try to generate the lock file without accessing the network, remove the --locked flag and use --offline instead.
5.384 thread 'main' panicked at 'Exited with status code: 101', /usr/local/cargo/registry/src/index.crates.io-6f17d22bba15001f/cargo-chef-0.1.62/src/recipe.rs:204:27
5.384 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@thomaseizinger
Copy link
Author

Your project also seems to handle the identity crate specially with a crates.io override in the workspace Cargo.toml and the project is not part of said workspace

The identity crate is part of the workspace. It is a bit messy and you might be right that it is the cause of the issue. What might happen is that because it is patched without a version = , cargo wants to download it again.

I don't have to bandwidth to debug this further unfortunately and we've solved the problem in a different way for now.

Feel free to close this issue from my end.

@mladedav
Copy link
Contributor

I didn't understand the comment regarding the identity package the first time I read it, sorry for the confusion!

Now I think the issue is with cargo-chef not handling the override correctly. When minimal version of your workspace is generated by the cook command and cargo tries to build it, it finds the package as a transitive dependency with a version that cannot be satisfied by the saved (and edited) Cargo.lock so it tries to update it, but that fails.

So I think this issue is about crates.io overrides.

@muhammad-asn do you also use these?

@markovalexander
Copy link

markovalexander commented Feb 13, 2024

I've also got the issue trying to build docker image for https://github.com/predibase/lorax from main branch:

4.007 error: package `clap_derive v4.5.0` cannot be built because it requires rustc 1.74 or newer, while the currently active rustc version is 1.70.0
and
4.738 error: package `ahash v0.8.8` cannot be built because it requires rustc 1.72.0 or newer, while the currently active rustc version is 1.70.0

@mladedav
Copy link
Contributor

@markovalexander I think your issue is #231, cargo-chef currently does not handle rust-toolchain file so the cook command uses the installed toolchain, which in your case is 1.70.

You can either update the base docker image to be the same version as the toolchain you use (this will also save you some time in CI because then cargo and rustup don't have to download the correct toolchain) or you can just copy the toolchain file into the container before running the cook command.

@muhammad-asn
Copy link

muhammad-asn commented Feb 13, 2024

Somehow I update my rust tool-chain with version 1.74 also in my use case I use multi stage Dockerfile, so I need to COPY again the Cargo.toml and Cargo.lock. This solved the issue.

@markovalexander
Copy link

markovalexander commented Feb 14, 2024

@mladedav I am not sure that this is the case

cargo-chef currently does not handle rust-toolchain file so the cook command uses the installed toolchain, which in your case is 1.70

yes, it is an expected behaviour. What is not expected is that clap_derive is fixed to be 4.3.2 in cargo.lock and ahash version is fixed at 0.8.3. However, cargo chef cook ... tries to install different versions: 4.5.0 and 0.8.8 respectively.

Updating rust to 1.74 is not a solution: it helps to build an image but when running it you'll get an instant crash because of this.

@mladedav
Copy link
Contributor

@markovalexander I've tried copying the rust-toolchain fail into the container before running cargo chef cook and the compilation succeeded. What's strange is when I rewrite the file to use 1.70.0 locally, the compilation succeeds. So I'm not sure why cargo decides that it needs to update the dependencies during the docker build.

In any case you might also want to just use a rust image with the correct toolchain and glibc (such as 1.74-bullseye probably) and install cargo chef in there. It's going to be faster than installing the correct toolchain twice during the docker build.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants