-
Notifications
You must be signed in to change notification settings - Fork 2.6k
cargo vendor --respect-source-config
deletes previously vendored sources
#15244
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
Comments
The observation is mostly correct, but I don't think it is intended to use like that. It tried to vendor crates from a source location that is exactly the same as the destination. It sounds pretty awkward. However I do think it is a bug. Cargo shouldn't delete anything that is not a cache. That means Cargo should only delete crates from registry sources but not other kinds of sources. |
There's something more going on. The following also deletes files from $ cargo vendor --versioned-dirs --respect-source-config --no-delete vendor_dist1 > .cargo/config.toml
# ... success ...
$ ls vendor_dist1
bytemuck-1.21
$ cargo vendor --versioned-dirs --respect-source-config --no-delete vendor_dist2
rror: failed to sync
Caused by:
failed to load pkg lockfile
Caused by:
no matching package named `bytemuck` found
location searched: directory source `/tmp/bad-vendor/vendor_dist1` (which is replacing registry `crates-io`)
required by package `bad_vendor v0.1.0 (/tmp/bad-vendor)` after |
Yeah it has pretty much the same root cause. As I said,
Cargo doesn't check that, so adding a check for it should suffice. |
Hmm… maybe not as easy as I thought. Might also need to check if it is a source replacement. |
### What does this PR try to resolve? Fixes #15244 With this fix, `cargo vendor` will not delete original sources, if you want to vendor things from one directory sources to the other #### Background cargo-vendor has a workaround that to mitigate #5956: it removes all cached sources in order to trigger a re-unpack. It was meant for dealing with registry sources only, but accidentally applied to directory source kind. While directory source kind was invented for vendoring, and vendoring from one vendored directory to the other seems unusual, Cargo IMO should not delete any real sources. It does not mean that registry sources are okay to delete, In long term, we should explore a way that unpacks `.crate` files directly, without any removal. See #12509 (comment) ### How should we test and review this PR? The added test should suffice. Also, although this is for fixing #15244, `cargo vendor` still doesn't support vendor from and to the same location. Unless we figure out an `rsync`-like solutin to update vendor sources, it is not going to support in short term. (And I also doubt the real world use case of it) ### Additional information
Problem
Running
cargo vendor > .cargo/config.toml
followed by runningcargo vendor --respect-source-config
results in a failure. Moreovercargo vendor
deletes the previous contents of thevendor
folder.Steps
cargo vendor --versioned-dirs --respect-source-config --no-delete > .cargo/config.toml
cargo vendor --versioned-dirs --respect-source-config --no-delete
Possible Solution(s)
I suspect something in the loop trying to deal with #5956 is not expecting to see replaced sources. In particular, I wonder if this line is deleting files in the "vendor" directory
cargo/src/cargo/ops/vendor.rs
Line 137 in 3fe24ab
Notes
No response
Version
The text was updated successfully, but these errors were encountered: