0.1.17 breaks vendoring when you have replacement sources from git checkouts

[froydnj]

Firefox uses a custom fork of serde; its .cargo/config.in contains:

[source.crates-io]
registry = 'https://github.com/rust-lang/crates.io-index'
replace-with = 'vendored-sources'

[source."https://github.com/servo/serde"]
git = "https://github.com/servo/serde"
branch = "deserialize_from_enums8"
replace-with = "vendored-sources"

My local cargo git cache (is that what it's called?) for serde looks like:

froydnj@hawkeye:~/src/gecko-dev.git$ ls -l /home/froydnj/.cargo/git/checkouts/serde-7c389ead256ba659/c4457d8/
total 64
-rw-r--r-- 1 froydnj froydnj   499 Nov  1 12:00 appveyor.yml
-rw-r--r-- 1 froydnj froydnj   125 Nov  1 12:00 Cargo.toml
-rw-r--r-- 1 froydnj froydnj  2334 Nov  1 12:00 CONTRIBUTING.md
-rw-r--r-- 1 froydnj froydnj  1606 Nov  1 12:00 crates-io.md
-rw-r--r-- 1 froydnj froydnj 10847 Nov  1 12:00 LICENSE-APACHE
-rw-r--r-- 1 froydnj froydnj  1071 Nov  1 12:00 LICENSE-MIT
-rw-r--r-- 1 froydnj froydnj  3183 Nov  1 12:00 README.md
-rw-r--r-- 1 froydnj froydnj    31 Nov  1 12:00 rustfmt.toml
drwxr-xr-x 3 froydnj froydnj  4096 Nov  1 12:00 serde
drwxr-xr-x 3 froydnj froydnj  4096 Nov  1 12:00 serde_derive
drwxr-xr-x 2 froydnj froydnj  4096 Nov  1 12:00 serde_derive_internals
drwxr-xr-x 3 froydnj froydnj  4096 Nov  1 12:00 serde_test
drwxr-xr-x 5 froydnj froydnj  4096 Nov  1 12:00 test_suite
-rwxr-xr-x 1 froydnj froydnj  2354 Nov  1 12:00 travis.sh

When I use cargo-vendor 0.1.17 or above to try to re-vendor some changes, I get peculiar error messages:

froydnj@hawkeye:~/src/gecko-dev.git$ ./mach vendor rust --ignore-modified
    Updating registry `https://github.com/rust-lang/crates.io-index`

Caused by:
  failed to load pkg lockfile

Caused by:
  failed to resolve patches for `https://github.com/rust-lang/crates.io-index`

Caused by:
  failed to load source for a dependency on `serde_derive`

Caused by:
  Unable to update https://github.com/servo/serde?branch=deserialize_from_enums8#c4457d80

Caused by:
  Could not find `Cargo.toml` in `/home/froydnj/.cargo/git/checkouts/serde-7c389ead256ba659/c4457d8/serde_derive`
...
  File "/home/froydnj/src/gecko-dev.git/python/mozbuild/mozbuild/vendor_rust.py", line 320, in vendor
    subprocess.check_call([cargo, 'vendor', '--quiet', '--sync', 'Cargo.lock'] + [vendor_dir], cwd=self.topsrcdir)

There's some Firefox-specific stuff in there, but the core idea is that we invoked cargo vendor --quiet --sync Cargo.lock $VENDOR_DIR and we get the above. My cached serde checkout now looks like:

froydnj@hawkeye:~/src/gecko-dev.git$ ls -l /home/froydnj/.cargo/git/checkouts/serde-7c389ead256ba659/c4457d8/
total 60
-rw-r--r-- 1 froydnj froydnj   499 Nov  1 12:00 appveyor.yml
-rw-r--r-- 1 froydnj froydnj   125 Nov  1 12:00 Cargo.toml
-rw-r--r-- 1 froydnj froydnj  2334 Nov  1 12:00 CONTRIBUTING.md
-rw-r--r-- 1 froydnj froydnj  1606 Nov  1 12:00 crates-io.md
-rw-r--r-- 1 froydnj froydnj 10847 Nov  1 12:00 LICENSE-APACHE
-rw-r--r-- 1 froydnj froydnj  1071 Nov  1 12:00 LICENSE-MIT
-rw-r--r-- 1 froydnj froydnj  3183 Nov  1 12:00 README.md
-rw-r--r-- 1 froydnj froydnj    31 Nov  1 12:00 rustfmt.toml
drwxr-xr-x 3 froydnj froydnj  4096 Nov  1 12:00 serde
drwxr-xr-x 2 froydnj froydnj  4096 Nov  1 12:00 serde_derive_internals
drwxr-xr-x 3 froydnj froydnj  4096 Nov  1 12:00 serde_test
drwxr-xr-x 5 froydnj froydnj  4096 Nov  1 12:00 test_suite
-rwxr-xr-x 1 froydnj froydnj  2354 Nov  1 12:00 travis.sh

Somehow, the serde_derive directory from the checkout has gotten deleted. I don't exactly understand how the fix for #130 is responsible for this, but this seems like a bad thing. My suspicion is that cargo doesn't try very hard to re-download git checkouts, or something?

/cc @luser @alexcrichton

[froydnj]

Adding an additional check for a git checkout here:

cargo-vendor/src/main.rs

Lines 235 to 238 in b7eaef8

	// Don't delete actual source code!
	if pkg.source_id().is_path() {
	continue
	}

seems to fix things. I'm not sure if that's the right fix, but it certainly seems in the spirit of the comment!

[froydnj]

Some debugging output to indicate what cargo-vendor thinks it's doing looks like:

Deleting Package { id: PackageId { name: "serde_derive", version: "1.0.66", source: "https://github.com/servo/serde?branch=deserialize_from_enums8#c4457d80" }, ..: ".." }
in dir "/home/froydnj/.cargo/git/checkouts/serde-7c389ead256ba659/c4457d8/serde_derive"
Deleting Package { id: PackageId { name: "serde", version: "1.0.66", source: "registry `https://github.com/rust-lang/crates.io-index`" }, ..: ".." }
in dir "/home/froydnj/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.66"

What I don't really understand in this case is why it thinks serde_derive only is from the git checkout, while serde itself is from the crates.io registry. Maybe that's part of the problem? (Or maybe the logic for determining what directory to delete should be different between git checkouts and registry checkouts?)

[froydnj]

Ah, because we have:

libudev-sys = { path = "dom/webauthn/libudev-sys" }
serde_derive = { git = "https://github.com/servo/serde", branch = "deserialize_from_enums8" }

in Cargo.toml, so we're only replacing serde_derive with our custom checkout, not all of serde?

[alexcrichton]

Ah ok I see what's happening here. I think yeah it's fine to just skip git repos for now, and we can always fix those later and add full support at a later date if necessary