Skip to content

Add a description of Cargo.lock conflicts in the Cargo FAQ #12092

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

Closed
wants to merge 6 commits into from

Conversation

heisen-li
Copy link
Contributor

What does this PR try to resolve?

Add a message for issue #12079.

I haven't figured out how to add a new entry to the Cargo FAQ yet. If it does need to be added, I will add it later.

@rustbot
Copy link
Collaborator

rustbot commented May 6, 2023

r? @weihanglo

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added A-dependency-resolution Area: dependency resolution and the resolver S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 6, 2023
Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I am not certain if we can add that message unconditionally. Most of the common dependency conflicts cannot be resolved by cargo generate-lockfile. I didn't think much when writing down the fix. Can you help me think of any scenario where the suggestion is considered harmful or useless?

@weihanglo
Copy link
Member

BTW I am more looking forward to an FAQ though it might be wordy and not easy to write one.

@heisen-li
Copy link
Contributor Author

Thank you for your guidance.

For now, I've modified some of the test cases to make it pass. Perhaps, as you say, for some scenarios that require more in-depth analysis, I'll go ahead with the work.

Oh, I'm going to try to add answers to the frequently asked questions, which may be a bit difficult for me to satisfy everyone, but I'll do my best.

@heisen-li heisen-li changed the title Add message for Cargo.lock conflicts [WIP]Add message for Cargo.lock conflicts May 6, 2023
heisen-li added 2 commits May 18, 2023 19:35
…o lock-conflict

* 'master' of https://github.com/QiangHeisenberg/cargo: (148 commits)
  doc: intra-doc links and doc comments for build script
  Adding a member should check with moderation.
  Leader selection with no objections.
  Remove useless drop of copy type
  Update comment to add more context.
  Fix dep/feat syntax with hidden implicit optional dependencies
  changelog: add entries of some behavior changes
  Update the semver-check script to be able to run in any directory.
  Clarify unsafe to safe function qualifier change.
  Fix typo
  Do not take a stance on #![deny(warnings)]
  Note that it is not a breaking change to make an `unsafe` function safe
  Change to <bin-name>
  Update libc to 0.2.144
  Add more documentation for artifact-dependencies.
  Add missing period.
  Disallow RUSTUP_TOOLCHAIN in the [env] table.
  Add a comment describing why these are disallowed.
  Disallow RUSTUP_HOME in the [env] table.
  Update git2
  ...
@rustbot rustbot added the A-documenting-cargo-itself Area: Cargo's documentation label May 18, 2023
@heisen-li
Copy link
Contributor Author

Allow me to explain a little bit.

As you said, it may not be appropriate to use the order aaaa suggested in the error message. I described some scenarios in PR, hope my understanding is correct. If I understand something wrong or explain it not clearly enough, I hope you can point it out.

@heisen-li heisen-li changed the title [WIP]Add message for Cargo.lock conflicts Add a description of Cargo.lock conflicts in the Cargo FAQ May 18, 2023
Comment on lines 268 to 269
or merging branches, it is recommended to try to run `cargo generate-lockfile` to
solve it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally wouldn't recommend this. cargo generate-lockfile will update all dependencies, which is usually not what is wanted.

I think most conflicts can be trivially resolved manually. The format is not that complicated, and editing manually shouldn't be such a concern.

I would say that if there are non-trivial conflicts, you can reset all "yours" changes, fix all other conflicts in the branch, and then run some cargo command (like cargo tree or cargo check), which should re-update the lockfile with your own local changes. If you previously ran some cargo update commands in your branch, you can re-run them that this time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think most conflicts can be trivially resolved manually. The format is not that complicated, and editing manually shouldn't be such a concern.

Even fairly familiar with cargo, I do not attempt to resolve conflicts on my own

I would say that if there are non-trivial conflicts, you can reset all "yours" changes, fix all other conflicts in the branch, and then run some cargo command (like cargo tree or cargo check), which should re-update the lockfile with your own local change

This is what I do to resolve conflicts

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I think it's pretty good enough. Just some minor issues need to resolve and then we can move forward. We can always polish it afterward.

@heisen-li
Copy link
Contributor Author

Thank you very much for your guidance. If I still have mistakes, please feel free to point them out, and I will take the time to modify them:)

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before merging this, @qiangheisenberg could you do a squash or else to make the git history more meaningful?

@weihanglo
Copy link
Member

Okay. I meant there are 6 commits in the history and some of them seem irrelevant or misleading, e.g., "Merge branch master". It is a good practice to keep the history clean. Anyhow, I'll merge this sooner or later if you haven't got time to cleanup git commits. Thank you!

@heisen-li
Copy link
Contributor Author

Okay. I meant there are 6 commits in the history and some of them seem irrelevant or misleading, e.g., "Merge branch master". It is a good practice to keep the history clean. Anyhow, I'll merge this sooner or later if you haven't got time to cleanup git commits. Thank you!

Very sorry for my unprofessional manner. I'm not very good at dealing with this problem, so I had to re-open a PR.

@heisen-li heisen-li closed this May 26, 2023
@heisen-li heisen-li deleted the lock-conflict branch May 27, 2023 01:43
bors added a commit that referenced this pull request May 27, 2023
Add a description of `Cargo.lock` conflicts in the Cargo FAQ

### What does this PR try to resolve?

Add a message for issue #12079.

For detailed discussion, please refer to:#12092
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-dependency-resolution Area: dependency resolution and the resolver A-documenting-cargo-itself Area: Cargo's documentation S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants