Support vendoring git repositories #3992

alexcrichton · 2017-05-04T03:49:26Z

Currently the vendoring support in Cargo primarily only allows replacing
registry sources, e.g. crates.io. Other networked sources of code, such as git
repositories, cannot currently be replaced. The purpose of this commit is to
support vendoring of git dependencies to eventually have support implemented in
the cargo-vendor subcommand.

Support for vendoring git repositories required a few subtle changes:

First and foremost, configuration for source replacement of a git repository
was added. This looks similar to the Cargo.toml configuration of a git
source.
The restriction around checksum providing sources was relaxed. If a
replacement source provides checksums but the replaced source doesn't then
that's now considered ok unlike it being an error before.
Lock files can be generated for crates.io crates against vendored sources, but
lock files cannot be generated against git sources. A lock file must
previously exist to make use of a vendored git source.
The package field of .cargo-checksum.json is now optional, and it is
intended to be omitted for git sources that are vendored.

alexcrichton · 2017-05-04T03:49:34Z

r? @matklad

cc @wycats

rust-highfive · 2017-05-04T03:49:36Z

r? @brson

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

matklad · 2017-05-04T08:58:17Z

src/cargo/sources/config.rs

+                        None => {
+                            match try("rev")? {
+                                Some(b) => GitReference::Rev(b.0.to_string()),
+                                None => GitReference::Branch("master".to_string()),


Wasn't there a bug somewhere that we shouldn't hardcode master, because it might not be the default branch?

Hm I can't seem to locate that isue but it sounds familiar. Do you think it's better to fix the bug here or differ from git sources in the manifest? I'd lean a bit towards mirroring git sources ...

#3517

It's not fixed yet (I was not sure about it), so it's better to be consistent with our behavior elsewhere.

Looks like the proper fix is to add a new HEAD variant to the GitRefernce enum, and it's to much work for this PR.

Ah yeah thanks for the link! And yeah falling back to HEAD seems like a reasonable implementation to me.

matklad · 2017-05-04T09:06:23Z

src/cargo/sources/config.rs

            bail!("\
 cannot replace `{orig}` with `{name}`, the source `{supports}` supports \
 checksums, but `{no_support}` does not

 a lock file compatible with `{orig}` cannot be generated in this situation
-", orig = orig_name, name = name, supports = supports, no_support = no_support);
+", orig = orig_name, name = name, supports = orig_name, no_support = name);


We don't need supports and no_support keys any more, may just use orig and name (or replaced)

matklad · 2017-05-04T09:24:53Z

@alexcrichton could you explain how cheksums work (they are created in cargo-vendor as I understand, but I don't see where they are checked on the Cargo side) and why we need more relaxed rules for them for git dependencies? Am I correct that it would still be not possible to accidentally modify vendored git dependency, because we still checksum all the files?

The package field of .cargo-checksum.json is now optional, and it is intended to be omitted for git sources that are vendored.

So the package is sha256 of .crate file? Perhaps we can remove this field altogether and just put it into the files map?

alexcrichton · 2017-05-04T14:34:28Z

Sure yeah, so right now every source in Cargo can report whether it supports checksums or not. This basically means that the Summary instances it generates may have the checksum field filled out. Checksums are then verified when managing lock files.

There were two pieces that needed to be changed to allow this implementation:

First Cargo previously disallowed source replacement if the two sources differed in how they provided checksums. This mean you could replace the registry with a directory source (they both support checksums) but you couldn't replace the registry with a git repo (git repos don't have checksums). The intention here was that you shouldn't modify the source code if you're using source replacement, and checksums are a verifiable way to do that. The checksum listed in the manifest allows verification that you're always working with the same source.

So even though directory sources support checksums we want the ability to override with a directory source. This basically means that it's ok to override a git source with a directory source. The implementation of the vendoring of a git source ends up meaning that we won't list a checksum for the git package to ensure it matches the original git crate.
Even if sources without checksums can be replaced by sources with checksums we still have a check when merging lock files to prevent this. That means that we need the directory source to be able to produce crates without checksums so git can be the same. The purpose here is that when you generate a lock file against vendored sources it should be the same as a lock file against the actual sources.

You're correct in that it's still intended that vendored sources are not allowed to have modifications. The .cargo-checksum.json file must still be present and list checksums for the relevant files in the crate, it just omits the package top-level hash field.

So the package is sha256 of .crate file? Perhaps we can remove this field altogether and just put it into the files map?

Oh but the .crate file isn't actually in the directory, (that's why we can't calculate it at runtime).

matklad · 2017-05-10T15:03:31Z

Looks good to me! (modulo the fact that I have not yet fully wrapped my mind around Registry, Source, RegistrySource and all other awesome traits and structs :) )

alexcrichton · 2017-05-10T22:54:27Z

FWIW I'd like to hold off on merging this just yet, I'd ideally prefer to implement support in cargo-vendor first and make sure that works out well.

bors · 2017-05-12T00:38:33Z

☔ The latest upstream changes (presumably #4026) made this pull request unmergeable. Please resolve the merge conflicts.

Currently the vendoring support in Cargo primarily only allows replacing registry sources, e.g. crates.io. Other networked sources of code, such as git repositories, cannot currently be replaced. The purpose of this commit is to support vendoring of git dependencies to eventually have support implemented in the `cargo-vendor` subcommand. Support for vendoring git repositories required a few subtle changes: * First and foremost, configuration for source replacement of a git repository was added. This looks similar to the `Cargo.toml` configuration of a git source. * The restriction around checksum providing sources was relaxed. If a replacement source provides checksums but the replaced source doesn't then that's now considered ok unlike it being an error before. * Lock files can be generated for crates.io crates against vendored sources, but lock files cannot be generated against git sources. A lock file must previously exist to make use of a vendored git source. * The `package` field of `.cargo-checksum.json` is now optional, and it is intended to be omitted for git sources that are vendored.

alexcrichton · 2017-09-08T18:32:10Z

Ok this is rebased now and cargo-vendor is ready to go, so I'm going to r+..

@bors: r=matklad

bors · 2017-09-08T18:32:11Z

📌 Commit 5b08b8f has been approved by matklad

bors · 2017-09-09T00:42:19Z

⌛ Testing commit 5b08b8f with merge b9e7500...

Support vendoring git repositories Currently the vendoring support in Cargo primarily only allows replacing registry sources, e.g. crates.io. Other networked sources of code, such as git repositories, cannot currently be replaced. The purpose of this commit is to support vendoring of git dependencies to eventually have support implemented in the `cargo-vendor` subcommand. Support for vendoring git repositories required a few subtle changes: * First and foremost, configuration for source replacement of a git repository was added. This looks similar to the `Cargo.toml` configuration of a git source. * The restriction around checksum providing sources was relaxed. If a replacement source provides checksums but the replaced source doesn't then that's now considered ok unlike it being an error before. * Lock files can be generated for crates.io crates against vendored sources, but lock files cannot be generated against git sources. A lock file must previously exist to make use of a vendored git source. * The `package` field of `.cargo-checksum.json` is now optional, and it is intended to be omitted for git sources that are vendored.

bors · 2017-09-09T05:29:38Z

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing b9e7500 to master...

rust-highfive assigned brson May 4, 2017

rust-highfive assigned matklad and unassigned brson May 4, 2017

alexcrichton added the relnotes Release-note worthy label May 4, 2017

matklad reviewed May 4, 2017

View reviewed changes

alexcrichton force-pushed the replace-git branch from e3b7c4a to c975b37 Compare May 4, 2017 14:37

bennofs mentioned this pull request May 22, 2017

Current "master" version does not compile nix-community/nix-index#10

Closed

bennofs mentioned this pull request Jun 2, 2017

WIP: implement vendoring for git dependencies alexcrichton/cargo-vendor#42

Merged

est31 mentioned this pull request Jul 16, 2017

building beta with vendor=true fail due to network dependencies rust-lang/rust#42719

Closed

bennofs mentioned this pull request Aug 9, 2017

Does this support vendoring git dependencies? alexcrichton/cargo-vendor#30

Closed

alexcrichton mentioned this pull request Aug 11, 2017

Fails with replace git dependency. dhovart/cargo-local-registry#9

Open

alexcrichton force-pushed the replace-git branch from c975b37 to 5b08b8f Compare September 5, 2017 22:18

bors merged commit 5b08b8f into rust-lang:master Sep 9, 2017

alexcrichton deleted the replace-git branch September 11, 2017 14:38

est31 mentioned this pull request Oct 22, 2017

Updated Release notes for 1.22.0 rust-lang/rust#45454

Merged

ehuss added this to the 1.22.0 milestone Feb 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support vendoring git repositories #3992

Support vendoring git repositories #3992

alexcrichton commented May 4, 2017

alexcrichton commented May 4, 2017

rust-highfive commented May 4, 2017

matklad May 4, 2017

alexcrichton May 4, 2017

matklad May 4, 2017

alexcrichton May 4, 2017

matklad May 4, 2017

matklad commented May 4, 2017

alexcrichton commented May 4, 2017

matklad commented May 10, 2017

alexcrichton commented May 10, 2017

bors commented May 12, 2017

alexcrichton commented Sep 8, 2017

bors commented Sep 8, 2017

bors commented Sep 9, 2017

bors commented Sep 9, 2017

Support vendoring git repositories #3992

Support vendoring git repositories #3992

Conversation

alexcrichton commented May 4, 2017

alexcrichton commented May 4, 2017

rust-highfive commented May 4, 2017

matklad May 4, 2017

Choose a reason for hiding this comment

alexcrichton May 4, 2017

Choose a reason for hiding this comment

matklad May 4, 2017

Choose a reason for hiding this comment

alexcrichton May 4, 2017

Choose a reason for hiding this comment

matklad May 4, 2017

Choose a reason for hiding this comment

matklad commented May 4, 2017

alexcrichton commented May 4, 2017

matklad commented May 10, 2017

alexcrichton commented May 10, 2017

bors commented May 12, 2017

alexcrichton commented Sep 8, 2017

bors commented Sep 8, 2017

bors commented Sep 9, 2017

bors commented Sep 9, 2017