Skip to content

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

Closed
lambdageek opened this issue Feb 28, 2025 · 4 comments · Fixed by #15260
Closed

cargo vendor --respect-source-config deletes previously vendored sources #15244

lambdageek opened this issue Feb 28, 2025 · 4 comments · Fixed by #15260
Labels
C-bug Category: bug 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

@lambdageek
Copy link

Problem

Running cargo vendor > .cargo/config.toml followed by running cargo vendor --respect-source-config results in a failure. Moreover cargo vendor deletes the previous contents of the vendor folder.

% cargo vendor --versioned-dirs --respect-source-config --no-delete
error: failed to sync

Caused by:
  failed to load pkg lockfile

Caused by:
  no matching package named `bytemuck` found
  location searched: directory source `/Users/alklig/work/bad_vendor/vendor` (which is replacing registry `crates-io`)
  required by package `bad_vendor v0.1.0 (/Users/alklig/work/bad_vendor)`

Steps

  1. clone https://github.com/lambdageek/bad-vendor
  2. cargo vendor --versioned-dirs --respect-source-config --no-delete > .cargo/config.toml
  3. 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

drop(fs::remove_dir_all(pkg.root()));

Notes

No response

Version

cargo 1.84.1
release: 1.84.1
host: aarch64-apple-darwin
libgit2: 1.8.1 (sys:0.19.0 vendored)
libcurl: 8.7.1 (sys:0.4.74+curl-8.9.0 system ssl:(SecureTransport) LibreSSL/3.3.6)
ssl: OpenSSL 1.1.1w  11 Sep 2023
os: Mac OS 15.3.1 [64-bit]
@lambdageek lambdageek added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Feb 28, 2025
@weihanglo
Copy link
Member

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

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.

@weihanglo weihanglo added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review E-easy Experience: Easy and removed S-triage Status: This issue is waiting on initial triage. labels Mar 3, 2025
@lambdageek
Copy link
Author

It tried to vendor crates from a source location that is exactly the same as the destination.

There's something more going on. The following also deletes files from vendor_dist1:

$ 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 cargo vendor ... vendor_dist2 the directory cargo_dist1 is empty

@weihanglo
Copy link
Member

Yeah it has pretty much the same root cause. As I said,

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.

Cargo doesn't check that, so adding a check for it should suffice.

@weihanglo
Copy link
Member

Hmm… maybe not as easy as I thought. Might also need to check if it is a source replacement.

github-merge-queue bot pushed a commit that referenced this issue Mar 4, 2025
### 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug 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.

3 participants