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 add: update Cargo.lock #10901

Closed
ehuss opened this issue Jul 25, 2022 · 0 comments · Fixed by #10902
Closed

cargo add: update Cargo.lock #10901

ehuss opened this issue Jul 25, 2022 · 0 comments · Fixed by #10902
Labels
C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-add

Comments

@ehuss
Copy link
Contributor

ehuss commented Jul 25, 2022

Problem

cargo add current does not update Cargo.lock after updating Cargo.toml. That means that the user needs to run some other command to update it. In most cases, this is fine, as most users will then run build or check to use their new dependency. However, some commands do not work (such as cargo pkgid, cargo update -p NEW_DEP, etc.). Additionally, it may be possible that the new dependency doesn't resolve at all (such as a conflict with another dependency).

Proposed Solution

Update Cargo.lock for just the new dependency (essentially, the equivalent of what cargo build and friends do when Cargo.toml changes).

If the update fails, I'm not sure what it should do, but I would lean towards backing out any changes made.

Notes

cargo 1.64.0-nightly (d8d30a753 2022-07-19)

@ehuss ehuss added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` Command-add labels Jul 25, 2022
epage added a commit to epage/cargo that referenced this issue Jul 25, 2022
This is prep for rust-lang#10901 to avoid the most common failure case for the
lock file.  We are assuming if the users action caused a change in the
manifes, then it will cause a change in the lock file.  This isn't
entirely true but close enough and I think these semantics can make
sense.
epage added a commit to epage/cargo that referenced this issue Jul 25, 2022
This is done in the command, rather than in the op,
- To consistently construct the `Workspace`
- It is more composable as an API

A downside is we update the git dependencies a second time.

We are not rolling back on error.
- For some errors, the user might want to debug what went wrong
- Rollback adds its own complications and risks, including since its
  non-atomic

Fixes rust-lang#10901
epage added a commit to epage/cargo that referenced this issue Jul 26, 2022
This is done in the command, rather than in the op,
- To consistently construct the `Workspace`
- It is more composable as an API

A downside is we update the git dependencies a second time.

We are not rolling back on error.
- For some errors, the user might want to debug what went wrong
- Rollback adds its own complications and risks, including since its
  non-atomic

Fixes rust-lang#10901
epage added a commit to epage/cargo that referenced this issue Jul 26, 2022
This is done in the command, rather than in the op,
- To consistently construct the `Workspace`
- It is more composable as an API

A downside is we update the git dependencies a second time.

We are not rolling back on error.
- For some errors, the user might want to debug what went wrong
- Rollback adds its own complications and risks, including since its
  non-atomic

Fixes rust-lang#10901
bors added a commit that referenced this issue Jul 27, 2022
fix(add): Update the lock file

This is done in the command, rather than in the op,
- To consistently construct the `Workspace`
- It is more composable as an API

A downside is we update the git dependencies a second time and any sources we didn't initially update.

Unlike the proposal in the attached issue, this does not roll back on error.
- For some errors, the user might want to debug what went wrong.  Losing the intermediate state makes that difficult
- Rollback adds its own complications and risks, including since its
  non-atomic

We've already tried to address most potential errors during `cargo add`s processing.  To meet this desire, we now error if `--locked` is passed in and we would change the manifest.  See that individual commit's message for more details.

Fixes #10901
@bors bors closed this as completed in 5789c12 Jul 27, 2022
Hezuikn pushed a commit to Hezuikn/cargo that referenced this issue Sep 22, 2022
This is prep for rust-lang#10901 to avoid the most common failure case for the
lock file.  We are assuming if the users action caused a change in the
manifes, then it will cause a change in the lock file.  This isn't
entirely true but close enough and I think these semantics can make
sense.
Hezuikn pushed a commit to Hezuikn/cargo that referenced this issue Sep 22, 2022
This is done in the command, rather than in the op,
- To consistently construct the `Workspace`
- It is more composable as an API

A downside is we update the git dependencies a second time.

We are not rolling back on error.
- For some errors, the user might want to debug what went wrong
- Rollback adds its own complications and risks, including since its
  non-atomic

Fixes rust-lang#10901
Hezuikn pushed a commit to Hezuikn/cargo that referenced this issue Sep 22, 2022
This is prep for rust-lang#10901 to avoid the most common failure case for the
lock file.  We are assuming if the users action caused a change in the
manifes, then it will cause a change in the lock file.  This isn't
entirely true but close enough and I think these semantics can make
sense.
Hezuikn pushed a commit to Hezuikn/cargo that referenced this issue Sep 22, 2022
This is done in the command, rather than in the op,
- To consistently construct the `Workspace`
- It is more composable as an API

A downside is we update the git dependencies a second time.

We are not rolling back on error.
- For some errors, the user might want to debug what went wrong
- Rollback adds its own complications and risks, including since its
  non-atomic

Fixes rust-lang#10901
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` Command-add
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant