From d682439b71870cc8ecd61ffe416badd57b0a2dd6 Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Sun, 11 Aug 2019 16:50:13 -0700 Subject: [PATCH] Allow git+version dependency to be published. --- src/cargo/ops/registry.rs | 66 ++++++++------- src/cargo/util/toml/mod.rs | 24 +++--- tests/testsuite/publish.rs | 124 +++++++++++++++++++++++++++-- tests/testsuite/support/publish.rs | 40 +++++++++- 4 files changed, 203 insertions(+), 51 deletions(-) diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index d03d589f089..2b33abd70a8 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -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()); } } } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index e2232827885..5d9e000b807 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -801,23 +801,27 @@ impl TomlManifest { } fn map_dependency(config: &Config, dep: &TomlDependency) -> CargoResult { - 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, ®istry)?; 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() + })), } } } diff --git a/tests/testsuite/publish.rs b/tests/testsuite/publish.rs index 5e3a29a4603..bb9c1edb752 100644 --- a/tests/testsuite/publish.rs +++ b/tests/testsuite/publish.rs @@ -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}; @@ -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(); @@ -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(); @@ -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#" @@ -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\ + [..]", + ), + ], + ); +} diff --git a/tests/testsuite/support/publish.rs b/tests/testsuite/support/publish.rs index 0230aa63833..545007515db 100644 --- a/tests/testsuite/support/publish.rs +++ b/tests/testsuite/support/publish.rs @@ -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; @@ -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, ); } @@ -41,6 +59,7 @@ pub fn validate_alt_upload( expected_json, expected_crate_name, expected_files, + &[], ); } @@ -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. @@ -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. @@ -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 + ); + } } } }