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

Vendored crates should be checked for changes #5805

Closed
glandium opened this issue Jul 26, 2018 · 9 comments · Fixed by #13512
Closed

Vendored crates should be checked for changes #5805

glandium opened this issue Jul 26, 2018 · 9 comments · Fixed by #13512
Assignees
Labels
A-directory-source Area: directory sources (vendoring) A-documenting-cargo-itself Area: Cargo's documentation Command-vendor E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@glandium
Copy link
Contributor

glandium commented Jul 26, 2018

I was building Firefox, and tried to do some changes to some rust crate. Doing so made the build fail with something like

error: the listed checksum of ... has changed:
expected: ...
actual: ...

directory sources are not intended to be edited, if modifications are required then it is recommended that [replace] is used with a forked copy of the source

That's an IMO annoying feature of cargo, but this is not what this issue is about. The problem is the following: if I actually build the crate first, and then make the changes, rebuilding just goes on acting as if I had made no change, saying Fresh $crate_name.

It seems to me cargo should fail with the exact same error as if I hadn't already built.

Cc: @alexcrichton

@alexcrichton
Copy link
Member

This is currently expected behavior in that Cargo treats vendored crates like it does crates.io, it doesn't check any mtimes as it assumes it's all read-only. It is a performance optimization to not even look at the filesystem for these crates on incremental builds.

The correct way to modify a vendored crate is to use [patch] or a path dependency, and Cargo will then correctly handle the crate on incremental rebuilds as it knows that it's not a readonly dependency.

@ctrlcctrlv
Copy link

The correct way to modify a vendored crate is to use [patch] or a path dependency, and Cargo will then correctly handle the crate on incremental rebuilds as it knows that it's not a readonly dependency.

Fortunately GNU Guix can bypass this pointless check, as I today discovered.

https://fredrickbrennan.medium.com/how-to-bypass-rust-cargo-error-directory-sources-are-not-intended-to-be-edited-ab2f257b89da

ctrlcctrlv added a commit to ctrlcctrlv/revendor.guile that referenced this issue Oct 24, 2022
@k3d3
Copy link

k3d3 commented Jan 9, 2023

When I vendor code into my repository, it changes line endings (and nothing else) which triggers this check. Is there any way to disable or at least mitigate the error in this situation?

@ctrlcctrlv
Copy link

Does revendor.scm not work for you?

@epage
Copy link
Contributor

epage commented Oct 23, 2023

As this is expected behavior and is important for performance reasons, I'm going to propose to the cargo team that we close this.

@epage epage added Command-vendor S-propose-close Status: A team member has nominated this for closing, pending further input from the team labels Oct 23, 2023
@weihanglo
Copy link
Member

Instead of closing this now, I would like to see an improvement in the cargo-vendor doc. Could document that vendored sources are not intended to be modified, and [patch] is the correct way to do it.

@weihanglo weihanglo added A-documenting-cargo-itself Area: Cargo's documentation E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed S-propose-close Status: A team member has nominated this for closing, pending further input from the team labels Dec 22, 2023
@briansmith
Copy link

Instead of closing this now, I would like to see an improvement in the cargo-vendor doc. Could document that vendored sources are not intended to be modified, and [patch] is the correct way to do it.

cargo-vendor should have an option to generate the necessary [patch] for the vendored crates.

@LuuuXXX
Copy link
Contributor

LuuuXXX commented Feb 28, 2024

claim first, question it later~ @rustbot claim

bors added a commit that referenced this issue Mar 1, 2024
[docs]: Clarify vendored sources as read-only and way to modify

What does this PR try to resolve?

To document that vendored sources are not intended to be modified, and `[patch]` or a `path` dependency is the correct way to modify a vendored crate.

#5805 (comment)
@weihanglo
Copy link
Member

#13512 has improved the doc a bit, so I am fine closing this.

cargo-vendor should have an option to generate the necessary [patch] for the vendored crates.

@briansmith feel free to open an issue if you have an idea how it would look like. Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-directory-source Area: directory sources (vendoring) A-documenting-cargo-itself Area: Cargo's documentation Command-vendor E-easy Experience: Easy S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants