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

Add warning when cargo install --locked used, but there is no Cargo.lock available #9106

Closed
dekellum opened this issue Jan 26, 2021 · 5 comments · Fixed by #9108
Closed

Add warning when cargo install --locked used, but there is no Cargo.lock available #9106

dekellum opened this issue Jan 26, 2021 · 5 comments · Fixed by #9108
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`

Comments

@dekellum
Copy link
Contributor

dekellum commented Jan 26, 2021

This warning would help understanding the cause of install failures, as in this URLO Thread.

So for example, cargo-graph 0.3.1 was published without a Cargo.lock (prior to that feature being stabilized in cargo 1.37), and so using the --locked flag has no effect (the build still fails due to a yanked dependency). It would be much easier to understand why --locked doesn't help, if the following warning was introduced in the output:

cargo install cargo-graph --locked 
    Updating crates.io index
  Installing cargo-graph v0.3.1
+warning: no Cargo.lock file published in cargo-graph v0.3.1
error: failed to compile `cargo-graph v0.3.1`, intermediate artifacts can be found at `/tmp/cargo-installdryLBw`

Caused by:
  failed to select a version for the requirement `clap = "~2.11.3"`
  candidate versions found which didn't match: 2.33.3, 2.33.2, 2.33.1, ...
  location searched: crates.io index
  required by package `cargo-graph v0.3.1`
@dekellum dekellum added the C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` label Jan 26, 2021
@CPerezz
Copy link
Contributor

CPerezz commented Jan 28, 2021

As @alexcrichton correctly stated in #9108 (comment) I made a PR to change this when indeed it was decided some time ago to specifically not warn about this.

Therefore, my question is whether it should still be like this, or instead, a warning should be added as suggested in the Rust forum thread linked above?

@dekellum
Copy link
Contributor Author

Can you (or cc, @alexcrichton) summarize the historic rationale (or current advantage) of not warning? I'm very surprised by that, but then again, I'm surpised by a lot of things. The comment you link just states that there was a historic reason. In the scenario of the above linked URLO thread, it seems entirely obvious that warning should be output!.

@CPerezz
Copy link
Contributor

CPerezz commented Jan 28, 2021

Hi @dekellum I'm completely unaware of the rationale behind. I was told by Alex in the PR that this was decided some time ago. but maybe @Eh2406 or @ehuss can bring some light here and decide whether it's worth or not to go for this.

@alexcrichton
Copy link
Member

Gah my apologies, I completely misinterpreted this issue and the associated PR. I basically didn't read an inversion of logic and I thought this issue was about warning if --locked isn't used when a Cargo.lock is present. I see now that is not the case.

That being said I typically find it more helpful if the issue description isn't simply "Do it!" but rather puts some more effort in to explaining what the issue is about.

@dekellum
Copy link
Contributor Author

@alexcrichton my bad, sorry. I felt the URLO comment made it very clear (inline diff format) and neglected to reproduce it here. I'll edit above shortly.

bors added a commit that referenced this issue Feb 1, 2021
Impl warn for locked install without Cargo.lock

If we're installing in --locked mode and there's no `Cargo.lock` published
ie. the bin was published before #7026
the cargo install errors were not stating that it was due to the lack of
the `Cargo.lock` file. Instead, the error seemed completely unrelated.

Therefore, this tries to address this by adding a warn in the stderr
output.

Closes #9106

I will need some help on the testing side (assuming the code I added for the warning is correct).
It looks to me that the publish function implemented for testing purposes does not publish `Cargo.lock` which is the actual convention. Should this be updated too? See  #7026
@bors bors closed this as completed in b526fad Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted`
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants