From 5b08b8fe1019147fe489db17a9a8ae7ebe97f9e9 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 3 May 2017 20:33:28 -0700 Subject: [PATCH] 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. --- src/cargo/core/registry.rs | 34 ++++++-- src/cargo/sources/config.rs | 52 +++++++++--- src/cargo/sources/directory.rs | 13 ++- src/cargo/sources/git/source.rs | 8 ++ src/cargo/sources/path.rs | 8 ++ src/cargo/sources/registry/mod.rs | 4 + src/cargo/sources/replaced.rs | 8 ++ src/doc/source-replacement.md | 6 ++ tests/directory.rs | 135 +++++++++++++++++++++++++++++- tests/resolve.rs | 2 + 10 files changed, 248 insertions(+), 22 deletions(-) diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index 1edaa305f..70efc66df 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -26,11 +26,11 @@ pub trait Registry { /// Returns whether or not this registry will return summaries with /// checksums listed. - /// - /// By default, registries do not support checksums. - fn supports_checksums(&self) -> bool { - false - } + fn supports_checksums(&self) -> bool; + + /// Returns whether or not this registry will return summaries with + /// the `precise` field in the source id listed. + fn requires_precise(&self) -> bool; } impl<'a, T: ?Sized + Registry + 'a> Registry for Box { @@ -39,6 +39,14 @@ impl<'a, T: ?Sized + Registry + 'a> Registry for Box { f: &mut FnMut(Summary)) -> CargoResult<()> { (**self).query(dep, f) } + + fn supports_checksums(&self) -> bool { + (**self).supports_checksums() + } + + fn requires_precise(&self) -> bool { + (**self).requires_precise() + } } /// This structure represents a registry of known packages. It internally @@ -415,6 +423,14 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { f(self.lock(override_summary)); Ok(()) } + + fn supports_checksums(&self) -> bool { + false + } + + fn requires_precise(&self) -> bool { + false + } } fn lock(locked: &LockedMap, @@ -579,5 +595,13 @@ pub mod test { Ok(()) } } + + fn supports_checksums(&self) -> bool { + false + } + + fn requires_precise(&self) -> bool { + false + } } } diff --git a/src/cargo/sources/config.rs b/src/cargo/sources/config.rs index 12e9c07e8..5aa44110a 100644 --- a/src/cargo/sources/config.rs +++ b/src/cargo/sources/config.rs @@ -9,7 +9,7 @@ use std::path::{Path, PathBuf}; use url::Url; -use core::{Source, SourceId}; +use core::{Source, SourceId, GitReference}; use sources::ReplacedSource; use util::{Config, ToUrl}; use util::config::ConfigValue; @@ -107,19 +107,25 @@ impl<'cfg> SourceConfigMap<'cfg> { } let new_src = new_id.load(self.config)?; let old_src = id.load(self.config)?; - if new_src.supports_checksums() != old_src.supports_checksums() { - let (supports, no_support) = if new_src.supports_checksums() { - (name, orig_name) - } else { - (orig_name, name) - }; + if !new_src.supports_checksums() && old_src.supports_checksums() { bail!("\ -cannot replace `{orig}` with `{name}`, the source `{supports}` supports \ -checksums, but `{no_support}` does not +cannot replace `{orig}` with `{name}`, the source `{orig}` supports \ +checksums, but `{name}` 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); + } + + if old_src.requires_precise() && id.precise().is_none() { + bail!("\ +the source {orig} requires a lock file to be present first before it can be +used against vendored source code + +remove the source replacement configuration, generate a lock file, and then +restore the source replacement configuration to continue the build +", orig = orig_name); } + Ok(Box::new(ReplacedSource::new(id, &new_id, new_src))) } @@ -153,6 +159,32 @@ a lock file compatible with `{orig}` cannot be generated in this situation path.push(s); srcs.push(SourceId::for_directory(&path)?); } + if let Some(val) = table.get("git") { + let url = url(val, &format!("source.{}.git", name))?; + let try = |s: &str| { + let val = match table.get(s) { + Some(s) => s, + None => return Ok(None), + }; + let key = format!("source.{}.{}", name, s); + val.string(&key).map(Some) + }; + let reference = match try("branch")? { + Some(b) => GitReference::Branch(b.0.to_string()), + None => { + match try("tag")? { + Some(b) => GitReference::Tag(b.0.to_string()), + None => { + match try("rev")? { + Some(b) => GitReference::Rev(b.0.to_string()), + None => GitReference::Branch("master".to_string()), + } + } + } + } + }; + srcs.push(SourceId::for_git(&url, reference)?); + } if name == "crates-io" && srcs.is_empty() { srcs.push(SourceId::crates_io(self.config)?); } diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index bf0251778..711676574 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -23,7 +23,7 @@ pub struct DirectorySource<'cfg> { #[derive(Deserialize)] struct Checksum { - package: String, + package: Option, files: HashMap, } @@ -60,6 +60,10 @@ impl<'cfg> Registry for DirectorySource<'cfg> { fn supports_checksums(&self) -> bool { true } + + fn requires_precise(&self) -> bool { + true + } } impl<'cfg> Source for DirectorySource<'cfg> { @@ -133,8 +137,11 @@ impl<'cfg> Source for DirectorySource<'cfg> { })?; let mut manifest = pkg.manifest().clone(); - let summary = manifest.summary().clone(); - manifest.set_summary(summary.set_checksum(cksum.package.clone())); + let mut summary = manifest.summary().clone(); + if let Some(ref package) = cksum.package { + summary = summary.set_checksum(package.clone()); + } + manifest.set_summary(summary); let pkg = Package::new(manifest, pkg.manifest_path()); self.packages.insert(pkg.package_id().clone(), (pkg, cksum)); } diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 6a5154b2e..13e266b04 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -130,6 +130,14 @@ impl<'cfg> Registry for GitSource<'cfg> { .expect("BUG: update() must be called before query()"); src.query(dep, f) } + + fn supports_checksums(&self) -> bool { + false + } + + fn requires_precise(&self) -> bool { + true + } } impl<'cfg> Source for GitSource<'cfg> { diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 76b274b7a..16995ba76 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -484,6 +484,14 @@ impl<'cfg> Registry for PathSource<'cfg> { } Ok(()) } + + fn supports_checksums(&self) -> bool { + false + } + + fn requires_precise(&self) -> bool { + false + } } impl<'cfg> Source for PathSource<'cfg> { diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index beb768759..e35d99568 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -356,6 +356,10 @@ impl<'cfg> Registry for RegistrySource<'cfg> { fn supports_checksums(&self) -> bool { true } + + fn requires_precise(&self) -> bool { + false + } } impl<'cfg> Source for RegistrySource<'cfg> { diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index d2d65e542..5048f6186 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -33,6 +33,14 @@ impl<'cfg> Registry for ReplacedSource<'cfg> { self.to_replace) }) } + + fn supports_checksums(&self) -> bool { + self.inner.supports_checksums() + } + + fn requires_precise(&self) -> bool { + self.inner.requires_precise() + } } impl<'cfg> Source for ReplacedSource<'cfg> { diff --git a/src/doc/source-replacement.md b/src/doc/source-replacement.md index 5f5150aff..2222f27ba 100644 --- a/src/doc/source-replacement.md +++ b/src/doc/source-replacement.md @@ -68,6 +68,12 @@ replace-with = "another-source" registry = "https://example.com/path/to/index" local-registry = "path/to/registry" directory = "path/to/vendor" + +# Git sources can optionally specify a branch/tag/rev as well +git = "https://example.com/path/to/repo" +# branch = "master" +# tag = "v1.0.1" +# rev = "313f44e8" ``` The `crates-io` represents the crates.io online registry (default source of diff --git a/tests/directory.rs b/tests/directory.rs index 96c520f5f..dab2dcdfb 100644 --- a/tests/directory.rs +++ b/tests/directory.rs @@ -11,9 +11,10 @@ use std::io::prelude::*; use std::str; use cargotest::cargo_process; -use cargotest::support::{project, execs, ProjectBuilder}; +use cargotest::support::git; use cargotest::support::paths; use cargotest::support::registry::{Package, cksum}; +use cargotest::support::{project, execs, ProjectBuilder}; use hamcrest::assert_that; fn setup() { @@ -35,7 +36,7 @@ struct VendorPackage { #[derive(Serialize)] struct Checksum { - package: String, + package: Option, files: HashMap, } @@ -44,7 +45,7 @@ impl VendorPackage { VendorPackage { p: Some(project(&format!("index/{}", name))), cksum: Checksum { - package: String::new(), + package: Some(String::new()), files: HashMap::new(), }, } @@ -56,6 +57,11 @@ impl VendorPackage { self } + fn disable_checksum(&mut self) -> &mut VendorPackage { + self.cksum.package = None; + self + } + fn build(&mut self) { let p = self.p.take().unwrap(); let json = serde_json::to_string(&self.cksum).unwrap(); @@ -373,7 +379,7 @@ fn crates_io_then_directory() { authors = [] "#); v.file("src/lib.rs", "pub fn foo() -> u32 { 1 }"); - v.cksum.package = cksum; + v.cksum.package = Some(cksum); v.build(); assert_that(p.cargo("build"), @@ -504,3 +510,124 @@ fn only_dot_files_ok() { assert_that(p.cargo("build"), execs().with_status(0)); } + +#[test] +fn git_lock_file_doesnt_change() { + + let git = git::new("git", |p| { + p.file("Cargo.toml", r#" + [project] + name = "git" + version = "0.5.0" + authors = [] + "#) + .file("src/lib.rs", "") + }).unwrap(); + + VendorPackage::new("git") + .file("Cargo.toml", r#" + [package] + name = "git" + version = "0.5.0" + authors = [] + "#) + .file("src/lib.rs", "") + .disable_checksum() + .build(); + + let p = project("bar") + .file("Cargo.toml", &format!(r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + git = {{ git = '{0}' }} + "#, git.url())) + .file("src/lib.rs", ""); + p.build(); + + assert_that(p.cargo("build"), execs().with_status(0)); + + let mut lock1 = String::new(); + t!(t!(File::open(p.root().join("Cargo.lock"))).read_to_string(&mut lock1)); + + let root = paths::root(); + t!(fs::create_dir(&root.join(".cargo"))); + t!(t!(File::create(root.join(".cargo/config"))).write_all(&format!(r#" + [source.my-git-repo] + git = '{}' + replace-with = 'my-awesome-local-registry' + + [source.my-awesome-local-registry] + directory = 'index' + "#, git.url()).as_bytes())); + + assert_that(p.cargo("build"), + execs().with_status(0) + .with_stderr("\ +[COMPILING] [..] +[COMPILING] [..] +[FINISHED] [..] +")); + + let mut lock2 = String::new(); + t!(t!(File::open(p.root().join("Cargo.lock"))).read_to_string(&mut lock2)); + assert!(lock1 == lock2, "lock files changed"); +} + +#[test] +fn git_override_requires_lockfile() { + VendorPackage::new("git") + .file("Cargo.toml", r#" + [package] + name = "git" + version = "0.5.0" + authors = [] + "#) + .file("src/lib.rs", "") + .disable_checksum() + .build(); + + let p = project("bar") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [dependencies] + git = { git = 'https://example.com/' } + "#) + .file("src/lib.rs", ""); + p.build(); + + let root = paths::root(); + t!(fs::create_dir(&root.join(".cargo"))); + t!(t!(File::create(root.join(".cargo/config"))).write_all(br#" + [source.my-git-repo] + git = 'https://example.com/' + replace-with = 'my-awesome-local-registry' + + [source.my-awesome-local-registry] + directory = 'index' + "#)); + + assert_that(p.cargo("build"), + execs().with_status(101) + .with_stderr("\ +error: failed to load source for a dependency on `git` + +Caused by: + Unable to update [..] + +Caused by: + the source my-git-repo requires a lock file to be present first before it can be +used against vendored source code + +remove the source replacement configuration, generate a lock file, and then +restore the source replacement configuration to continue the build + +")); +} diff --git a/tests/resolve.rs b/tests/resolve.rs index be9931171..61541b747 100644 --- a/tests/resolve.rs +++ b/tests/resolve.rs @@ -28,6 +28,8 @@ fn resolve(pkg: PackageId, deps: Vec, registry: &[Summary]) } Ok(()) } + fn supports_checksums(&self) -> bool { false } + fn requires_precise(&self) -> bool { false } } let mut registry = MyRegistry(registry); let summary = Summary::new(pkg.clone(), deps, HashMap::new()).unwrap(); -- 2.39.5