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

Allow git+version dependency to be published. #7237

Merged
merged 1 commit into from
Aug 19, 2019
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
66 changes: 36 additions & 30 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,41 +114,47 @@ fn verify_dependencies(
registry_src: SourceId,
) -> CargoResult<()> {
for dep in pkg.dependencies().iter() {
if dep.source_id().is_path() {
if dep.source_id().is_path() || dep.source_id().is_git() {
if !dep.specified_req() {
let which = if dep.source_id().is_path() {
"path"
} else {
"git"
};
let dep_version_source = dep.registry_id().map_or_else(
|| "crates.io".to_string(),
|registry_id| registry_id.display_registry_name(),
);
bail!(
"all path dependencies must have a version specified \
when publishing.\ndependency `{}` does not specify \
a version",
dep.package_name()
"all dependencies must have a version specified when publishing.\n\
dependency `{}` does not specify a version\n\
Note: The published dependency will use the version from {},\n\
the `{}` specification will be removed from the dependency declaration.",
dep.package_name(),
dep_version_source,
which,
)
}
// TomlManifest::prepare_for_publish will rewrite the dependency
// to be just the `version` field.
} else if dep.source_id() != registry_src {
if dep.source_id().is_registry() {
// Block requests to send to crates.io with alt-registry deps.
// This extra hostname check is mostly to assist with testing,
// but also prevents someone using `--index` to specify
// something that points to crates.io.
if registry_src.is_default_registry() || registry.host_is_crates_io() {
bail!("crates cannot be published to crates.io with dependencies sourced from other\n\
registries either publish `{}` on crates.io or pull it into this repository\n\
and specify it with a path and version\n\
(crate `{}` is pulled from {})",
dep.package_name(),
dep.package_name(),
dep.source_id());
}
} else {
bail!(
"crates cannot be published with dependencies sourced from \
a repository\neither publish `{}` as its own crate and \
specify a version as a dependency or pull it into this \
repository and specify it with a path and version\n(crate `{}` has \
repository path `{}`)",
dep.package_name(),
dep.package_name(),
dep.source_id()
);
if !dep.source_id().is_registry() {
// Consider making SourceId::kind a public type that we can
// exhaustively match on. Using match can help ensure that
// every kind is properly handled.
panic!("unexpected source kind for dependency {:?}", dep);
}
// Block requests to send to crates.io with alt-registry deps.
// This extra hostname check is mostly to assist with testing,
// but also prevents someone using `--index` to specify
// something that points to crates.io.
if registry_src.is_default_registry() || registry.host_is_crates_io() {
bail!("crates cannot be published to crates.io with dependencies sourced from other\n\
registries. `{}` needs to be published to crates.io before publishing this crate.\n\
(crate `{}` is pulled from {})",
dep.package_name(),
dep.package_name(),
dep.source_id());
}
}
}
Expand Down
24 changes: 14 additions & 10 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -801,23 +801,27 @@ impl TomlManifest {
}

fn map_dependency(config: &Config, dep: &TomlDependency) -> CargoResult<TomlDependency> {
match *dep {
TomlDependency::Detailed(ref d) => {
match dep {
TomlDependency::Detailed(d) => {
let mut d = d.clone();
d.path.take(); // path dependencies become crates.io deps
// registry specifications are elaborated to the index URL
// Path dependencies become crates.io deps.
d.path.take();
// Same with git dependencies.
d.git.take();
d.branch.take();
d.tag.take();
d.rev.take();
// registry specifications are elaborated to the index URL
if let Some(registry) = d.registry.take() {
let src = SourceId::alt_registry(config, &registry)?;
d.registry_index = Some(src.url().to_string());
}
Ok(TomlDependency::Detailed(d))
}
TomlDependency::Simple(ref s) => {
Ok(TomlDependency::Detailed(DetailedTomlDependency {
version: Some(s.clone()),
..Default::default()
}))
}
TomlDependency::Simple(s) => Ok(TomlDependency::Detailed(DetailedTomlDependency {
version: Some(s.clone()),
..Default::default()
})),
}
}
}
Expand Down
124 changes: 116 additions & 8 deletions tests/testsuite/publish.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::fs::{self, File};
use std::io::prelude::*;

use crate::support::git::repo;
use crate::support::git::{self, repo};
use crate::support::paths;
use crate::support::registry::{self, registry_path, registry_url, Package};
use crate::support::{basic_manifest, project, publish};
Expand Down Expand Up @@ -283,11 +283,10 @@ fn git_deps() {
.with_stderr(
"\
[UPDATING] [..] index
[ERROR] crates cannot be published with dependencies sourced from \
a repository\neither publish `foo` as its own crate and \
specify a version as a dependency or pull it into this \
repository and specify it with a path and version\n\
(crate `foo` has repository path `git://path/to/nowhere`)\
[ERROR] all dependencies must have a version specified when publishing.
dependency `foo` does not specify a version
Note: The published dependency will use the version from crates.io,
the `git` specification will be removed from the dependency declaration.
",
)
.run();
Expand Down Expand Up @@ -323,8 +322,10 @@ fn path_dependency_no_version() {
.with_stderr(
"\
[UPDATING] [..] index
[ERROR] all path dependencies must have a version specified when publishing.
[ERROR] all dependencies must have a version specified when publishing.
dependency `bar` does not specify a version
Note: The published dependency will use the version from crates.io,
the `path` specification will be removed from the dependency declaration.
",
)
.run();
Expand Down Expand Up @@ -367,7 +368,7 @@ fn dont_publish_dirty() {
registry::init();
let p = project().file("bar", "").build();

let _ = repo(&paths::root().join("foo"))
let _ = git::repo(&paths::root().join("foo"))
.file(
"Cargo.toml",
r#"
Expand Down Expand Up @@ -1070,3 +1071,110 @@ Check for a source-replacement in .cargo/config.
)
.run();
}

#[cargo_test]
fn publish_git_with_version() {
// A dependency with both `git` and `version`.
Package::new("dep1", "1.0.1")
.file("src/lib.rs", "pub fn f() -> i32 {1}")
.publish();

let git_project = git::new("dep1", |project| {
project
.file("Cargo.toml", &basic_manifest("dep1", "1.0.0"))
.file("src/lib.rs", "pub fn f() -> i32 {2}")
});

let p = project()
.file(
"Cargo.toml",
&format!(
r#"
[package]
name = "foo"
version = "0.1.0"
authors = []
edition = "2018"
license = "MIT"
description = "foo"

[dependencies]
dep1 = {{version = "1.0", git="{}"}}
"#,
git_project.url()
),
)
.file(
"src/main.rs",
r#"
pub fn main() {
println!("{}", dep1::f());
}
"#,
)
.build();

p.cargo("run").with_stdout("2").run();
p.cargo("publish --no-verify --index")
.arg(registry_url().to_string())
.run();

publish::validate_upload_with_contents(
r#"
{
"authors": [],
"badges": {},
"categories": [],
"deps": [
{
"default_features": true,
"features": [],
"kind": "normal",
"name": "dep1",
"optional": false,
"registry": "https://github.com/rust-lang/crates.io-index",
"target": null,
"version_req": "^1.0"
}
],
"description": "foo",
"documentation": null,
"features": {},
"homepage": null,
"keywords": [],
"license": "MIT",
"license_file": null,
"links": null,
"name": "foo",
"readme": null,
"readme_file": null,
"repository": null,
"vers": "0.1.0"
}
"#,
"foo-0.1.0.crate",
&["Cargo.lock", "Cargo.toml", "Cargo.toml.orig", "src/main.rs"],
&[
(
"Cargo.toml",
// Check that only `version` is included in Cargo.toml.
"[..]\n\
[dependencies.dep1]\n\
version = \"1.0\"\n\
",
),
(
"Cargo.lock",
// The important check here is that it is 1.0.1 in the registry.
"[..]\n\
[[package]]\n\
name = \"foo\"\n\
version = \"0.1.0\"\n\
dependencies = [\n\
\x20\"dep1 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)\",\n\
]\n\
[..]",
),
],
);
}
40 changes: 37 additions & 3 deletions tests/testsuite/support/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use std::fs::File;
use std::io::{self, prelude::*, SeekFrom};
use std::path::{Path, PathBuf};

use crate::support::find_json_mismatch;
use crate::support::registry::{self, alt_api_path};
use crate::support::{find_json_mismatch, lines_match};

use flate2::read::GzDecoder;
use tar::Archive;
Expand All @@ -26,6 +26,24 @@ pub fn validate_upload(expected_json: &str, expected_crate_name: &str, expected_
expected_json,
expected_crate_name,
expected_files,
&[],
);
}

/// Checks the result of a crate publish, along with the contents of the files.
pub fn validate_upload_with_contents(
expected_json: &str,
expected_crate_name: &str,
expected_files: &[&str],
expected_contents: &[(&str, &str)],
) {
let new_path = registry::api_path().join("api/v1/crates/new");
_validate_upload(
&new_path,
expected_json,
expected_crate_name,
expected_files,
expected_contents,
);
}

Expand All @@ -41,6 +59,7 @@ pub fn validate_alt_upload(
expected_json,
expected_crate_name,
expected_files,
&[],
);
}

Expand All @@ -49,6 +68,7 @@ fn _validate_upload(
expected_json: &str,
expected_crate_name: &str,
expected_files: &[&str],
expected_contents: &[(&str, &str)],
) {
let mut f = File::open(new_path).unwrap();
// 32-bit little-endian integer of length of JSON data.
Expand All @@ -69,7 +89,12 @@ fn _validate_upload(
assert_eq!(f.seek(SeekFrom::End(0)).unwrap(), current);

// Verify the tarball.
validate_crate_contents(&krate_bytes[..], expected_crate_name, expected_files, &[]);
validate_crate_contents(
&krate_bytes[..],
expected_crate_name,
expected_files,
expected_contents,
);
}

/// Checks the contents of a `.crate` file.
Expand Down Expand Up @@ -126,7 +151,16 @@ pub fn validate_crate_contents(
let actual_contents = files
.get(&full_e_name)
.unwrap_or_else(|| panic!("file `{}` missing in archive", e_file_name));
assert_eq!(actual_contents, e_file_contents);
if !lines_match(e_file_contents, actual_contents) {
panic!(
"Crate contents mismatch for {:?}:\n\
--- expected\n\
{}\n\
--- actual \n\
{}\n",
e_file_name, e_file_contents, actual_contents
);
}
}
}
}