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

--frozen is not equivalent to specifying both --locked and --offline #15239

Closed
wegylexy opened this issue Feb 27, 2025 · 3 comments · Fixed by #15263
Closed

--frozen is not equivalent to specifying both --locked and --offline #15239

wegylexy opened this issue Feb 27, 2025 · 3 comments · Fixed by #15263
Labels
A-offline Area: offline mode C-bug Category: bug S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.

Comments

@wegylexy
Copy link

Problem

This fails:

cargo install --path . --frozen
0.389   Installing myapp v0.1.0 (/app)
0.472     Updating crates.io index
0.493 error: attempting to make an HTTP request, but --frozen was specified

This works:

cargo install --path . --locked --offline

Steps

FROM rust:alpine AS deps
WORKDIR /app
RUN apk add --no-cache build-base openssl-dev
COPY Cargo.toml Cargo.lock ./
RUN mkdir src && \
    echo "fn main() {}" > src/main.rs && \
    cargo build --release && \
    rm -rf src

FROM deps AS build
COPY src src
RUN cargo install --path . --frozen

FROM alpine:latest AS publish
COPY --from=build /usr/local/cargo/bin/myapp /usr/local/bin/
ENTRYPOINT [ "myapp" ]
docker build -t myapp .

Possible Solution(s)

Specify both --locked and --offline instead of --frozen.

Notes

Environment is Docker Desktop on WSL2

Version

cargo 1.85.0 (d73d2caf9 2024-12-31)
release: 1.85.0
commit-hash: d73d2caf9e41a39daf2a8d6ce60ec80bf354d2a7
commit-date: 2024-12-31
host: x86_64-unknown-linux-musl
libgit2: 1.8.1 (sys:0.19.0 vendored)
libcurl: 8.9.0-DEV (sys:0.4.74+curl-8.9.0 vendored ssl:OpenSSL/1.1.1w)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Alpine Linux 3.21.3 [64-bit]
@wegylexy wegylexy added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Feb 27, 2025
@epage
Copy link
Contributor

epage commented Feb 27, 2025

Hmm, some places call offline() and frozen() directly while others call network_allowed. Similar for locked() and frozen() vs lock_update_allowed.

Sometimes, this is for more specific error messages,

Other places, we don't have them paired

@epage epage added A-offline Area: offline mode S-needs-team-input Status: Needs input from team on whether/how to proceed. and removed Command-install S-triage Status: This issue is waiting on initial triage. labels Feb 27, 2025
@epage
Copy link
Contributor

epage commented Feb 27, 2025

From my naive perspective, I would assume each of those calls should be updated for handling both --offline and --frozen. To ensure correctness going forward, I'd go so far as removing GlobalContext::{frozen, offline, locked} and instead have a GobalContext::{offline_flag, locked_flag} as the only time to differentiate seems like it should be when reporting errors.

That so many of these missed calls exists makes me wonder if something there is intentional and would be interested in hearing from someone with more background on when these were added for why they diverge.

@ehuss
Copy link
Contributor

ehuss commented Mar 4, 2025

I would agree there is some messiness here due to evolution, I don't think it is intentional.

The network_allowed and lock_update_allowed methods were intended to try to abstract it a little to avoid "oh, we forget to check for flag X", but I think in practice it is still a bit messy since different places are checking different things. It's also an issue with error messages, since they often want to say which flag caused the problem, which is a leaky abstraction for those methods. If we got rid of the frozen method, it could be difficult to provide a good error message.

I suspect there is some way we can clean this up, though I'm not sure exactly how. It might be to audit the usages of the specific calls (like offline()) that Ed did above, and to switch those to network_allowed or lock_update_allowed as appropriate, and then make sure the error messages provide the correct hints. I don't know if there's some way to make that easier or less error-prone, though (maybe the offline/frozen/locked methods could be renamed or moved or abstracted so that it is clear they should only be used for error messages?).

@ehuss ehuss added S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing. and removed S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Mar 4, 2025
github-merge-queue bot pushed a commit that referenced this issue Mar 4, 2025
…15263)

### What does this PR try to resolve?

Fixes #15239

This also makes `--frozen` be respected in more situations, including

* `cargo add`
* `cargo install`
* Git fetches
* HTTP registry access

### How should we test and review this PR?

To prevent this from happening again, I removed `offline()` and
`locked()` accessors. To maintain the quality of error messages, I added
`offline_flag()` and `locked_flag()` that will pick up `--frozen`, if
present.

### Additional information
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-offline Area: offline mode C-bug Category: bug S-needs-mentor Status: Issue or feature is accepted, but needs a team member to commit to helping and reviewing.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants