Skip to content
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

Support vendoring git repositories #3992

Merged
merged 1 commit into from
Sep 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions src/cargo/core/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
Expand All @@ -39,6 +39,14 @@ impl<'a, T: ?Sized + Registry + 'a> Registry for Box<T> {
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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -579,5 +595,13 @@ pub mod test {
Ok(())
}
}

fn supports_checksums(&self) -> bool {
false
}

fn requires_precise(&self) -> bool {
false
}
}
}
52 changes: 42 additions & 10 deletions src/cargo/sources/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)))
}

Expand Down Expand Up @@ -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()),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}
}
}
};
srcs.push(SourceId::for_git(&url, reference)?);
}
if name == "crates-io" && srcs.is_empty() {
srcs.push(SourceId::crates_io(self.config)?);
}
Expand Down
13 changes: 10 additions & 3 deletions src/cargo/sources/directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub struct DirectorySource<'cfg> {

#[derive(Deserialize)]
struct Checksum {
package: String,
package: Option<String>,
files: HashMap<String, String>,
}

Expand Down Expand Up @@ -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> {
Expand Down Expand Up @@ -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));
}
Expand Down
8 changes: 8 additions & 0 deletions src/cargo/sources/git/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
8 changes: 8 additions & 0 deletions src/cargo/sources/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
4 changes: 4 additions & 0 deletions src/cargo/sources/registry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
8 changes: 8 additions & 0 deletions src/cargo/sources/replaced.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
6 changes: 6 additions & 0 deletions src/doc/source-replacement.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading