Skip to content

Commit d682439

Browse files
ehussehuss
committed
Allow git+version dependency to be published.
1 parent f42275c commit d682439

File tree

4 files changed

+203
-51
lines changed

4 files changed

+203
-51
lines changed

‎src/cargo/ops/registry.rs

+36-30
Original file line numberDiff line numberDiff line change
@@ -114,41 +114,47 @@ fn verify_dependencies(
114114
registry_src: SourceId,
115115
) -> CargoResult<()> {
116116
for dep in pkg.dependencies().iter() {
117-
if dep.source_id().is_path() {
117+
if dep.source_id().is_path() || dep.source_id().is_git() {
118118
if !dep.specified_req() {
119+
let which = if dep.source_id().is_path() {
120+
"path"
121+
} else {
122+
"git"
123+
};
124+
let dep_version_source = dep.registry_id().map_or_else(
125+
|| "crates.io".to_string(),
126+
|registry_id| registry_id.display_registry_name(),
127+
);
119128
bail!(
120-
"all path dependencies must have a version specified \
121-
when publishing.\ndependency `{}` does not specify \
122-
a version",
123-
dep.package_name()
129+
"all dependencies must have a version specified when publishing.\n\
130+
dependency `{}` does not specify a version\n\
131+
Note: The published dependency will use the version from {},\n\
132+
the `{}` specification will be removed from the dependency declaration.",
133+
dep.package_name(),
134+
dep_version_source,
135+
which,
124136
)
125137
}
138+
// TomlManifest::prepare_for_publish will rewrite the dependency
139+
// to be just the `version` field.
126140
} else if dep.source_id() != registry_src {
127-
if dep.source_id().is_registry() {
128-
// Block requests to send to crates.io with alt-registry deps.
129-
// This extra hostname check is mostly to assist with testing,
130-
// but also prevents someone using `--index` to specify
131-
// something that points to crates.io.
132-
if registry_src.is_default_registry() || registry.host_is_crates_io() {
133-
bail!("crates cannot be published to crates.io with dependencies sourced from other\n\
134-
registries either publish `{}` on crates.io or pull it into this repository\n\
135-
and specify it with a path and version\n\
136-
(crate `{}` is pulled from {})",
137-
dep.package_name(),
138-
dep.package_name(),
139-
dep.source_id());
140-
}
141-
} else {
142-
bail!(
143-
"crates cannot be published with dependencies sourced from \
144-
a repository\neither publish `{}` as its own crate and \
145-
specify a version as a dependency or pull it into this \
146-
repository and specify it with a path and version\n(crate `{}` has \
147-
repository path `{}`)",
148-
dep.package_name(),
149-
dep.package_name(),
150-
dep.source_id()
151-
);
141+
if !dep.source_id().is_registry() {
142+
// Consider making SourceId::kind a public type that we can
143+
// exhaustively match on. Using match can help ensure that
144+
// every kind is properly handled.
145+
panic!("unexpected source kind for dependency {:?}", dep);
146+
}
147+
// Block requests to send to crates.io with alt-registry deps.
148+
// This extra hostname check is mostly to assist with testing,
149+
// but also prevents someone using `--index` to specify
150+
// something that points to crates.io.
151+
if registry_src.is_default_registry() || registry.host_is_crates_io() {
152+
bail!("crates cannot be published to crates.io with dependencies sourced from other\n\
153+
registries. `{}` needs to be published to crates.io before publishing this crate.\n\
154+
(crate `{}` is pulled from {})",
155+
dep.package_name(),
156+
dep.package_name(),
157+
dep.source_id());
152158
}
153159
}
154160
}

‎src/cargo/util/toml/mod.rs

+14-10
Original file line numberDiff line numberDiff line change
@@ -801,23 +801,27 @@ impl TomlManifest {
801801
}
802802

803803
fn map_dependency(config: &Config, dep: &TomlDependency) -> CargoResult<TomlDependency> {
804-
match *dep {
805-
TomlDependency::Detailed(ref d) => {
804+
match dep {
805+
TomlDependency::Detailed(d) => {
806806
let mut d = d.clone();
807-
d.path.take(); // path dependencies become crates.io deps
808-
// registry specifications are elaborated to the index URL
807+
// Path dependencies become crates.io deps.
808+
d.path.take();
809+
// Same with git dependencies.
810+
d.git.take();
811+
d.branch.take();
812+
d.tag.take();
813+
d.rev.take();
814+
// registry specifications are elaborated to the index URL
809815
if let Some(registry) = d.registry.take() {
810816
let src = SourceId::alt_registry(config, &registry)?;
811817
d.registry_index = Some(src.url().to_string());
812818
}
813819
Ok(TomlDependency::Detailed(d))
814820
}
815-
TomlDependency::Simple(ref s) => {
816-
Ok(TomlDependency::Detailed(DetailedTomlDependency {
817-
version: Some(s.clone()),
818-
..Default::default()
819-
}))
820-
}
821+
TomlDependency::Simple(s) => Ok(TomlDependency::Detailed(DetailedTomlDependency {
822+
version: Some(s.clone()),
823+
..Default::default()
824+
})),
821825
}
822826
}
823827
}

‎tests/testsuite/publish.rs

+116-8
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::fs::{self, File};
22
use std::io::prelude::*;
33

4-
use crate::support::git::repo;
4+
use crate::support::git::{self, repo};
55
use crate::support::paths;
66
use crate::support::registry::{self, registry_path, registry_url, Package};
77
use crate::support::{basic_manifest, project, publish};
@@ -283,11 +283,10 @@ fn git_deps() {
283283
.with_stderr(
284284
"\
285285
[UPDATING] [..] index
286-
[ERROR] crates cannot be published with dependencies sourced from \
287-
a repository\neither publish `foo` as its own crate and \
288-
specify a version as a dependency or pull it into this \
289-
repository and specify it with a path and version\n\
290-
(crate `foo` has repository path `git://path/to/nowhere`)\
286+
[ERROR] all dependencies must have a version specified when publishing.
287+
dependency `foo` does not specify a version
288+
Note: The published dependency will use the version from crates.io,
289+
the `git` specification will be removed from the dependency declaration.
291290
",
292291
)
293292
.run();
@@ -323,8 +322,10 @@ fn path_dependency_no_version() {
323322
.with_stderr(
324323
"\
325324
[UPDATING] [..] index
326-
[ERROR] all path dependencies must have a version specified when publishing.
325+
[ERROR] all dependencies must have a version specified when publishing.
327326
dependency `bar` does not specify a version
327+
Note: The published dependency will use the version from crates.io,
328+
the `path` specification will be removed from the dependency declaration.
328329
",
329330
)
330331
.run();
@@ -367,7 +368,7 @@ fn dont_publish_dirty() {
367368
registry::init();
368369
let p = project().file("bar", "").build();
369370

370-
let _ = repo(&paths::root().join("foo"))
371+
let _ = git::repo(&paths::root().join("foo"))
371372
.file(
372373
"Cargo.toml",
373374
r#"
@@ -1070,3 +1071,110 @@ Check for a source-replacement in .cargo/config.
10701071
)
10711072
.run();
10721073
}
1074+
1075+
#[cargo_test]
1076+
fn publish_git_with_version() {
1077+
// A dependency with both `git` and `version`.
1078+
Package::new("dep1", "1.0.1")
1079+
.file("src/lib.rs", "pub fn f() -> i32 {1}")
1080+
.publish();
1081+
1082+
let git_project = git::new("dep1", |project| {
1083+
project
1084+
.file("Cargo.toml", &basic_manifest("dep1", "1.0.0"))
1085+
.file("src/lib.rs", "pub fn f() -> i32 {2}")
1086+
});
1087+
1088+
let p = project()
1089+
.file(
1090+
"Cargo.toml",
1091+
&format!(
1092+
r#"
1093+
[package]
1094+
name = "foo"
1095+
version = "0.1.0"
1096+
authors = []
1097+
edition = "2018"
1098+
license = "MIT"
1099+
description = "foo"
1100+
1101+
[dependencies]
1102+
dep1 = {{version = "1.0", git="{}"}}
1103+
"#,
1104+
git_project.url()
1105+
),
1106+
)
1107+
.file(
1108+
"src/main.rs",
1109+
r#"
1110+
pub fn main() {
1111+
println!("{}", dep1::f());
1112+
}
1113+
"#,
1114+
)
1115+
.build();
1116+
1117+
p.cargo("run").with_stdout("2").run();
1118+
p.cargo("publish --no-verify --index")
1119+
.arg(registry_url().to_string())
1120+
.run();
1121+
1122+
publish::validate_upload_with_contents(
1123+
r#"
1124+
{
1125+
"authors": [],
1126+
"badges": {},
1127+
"categories": [],
1128+
"deps": [
1129+
{
1130+
"default_features": true,
1131+
"features": [],
1132+
"kind": "normal",
1133+
"name": "dep1",
1134+
"optional": false,
1135+
"registry": "https://github.com/rust-lang/crates.io-index",
1136+
"target": null,
1137+
"version_req": "^1.0"
1138+
}
1139+
],
1140+
"description": "foo",
1141+
"documentation": null,
1142+
"features": {},
1143+
"homepage": null,
1144+
"keywords": [],
1145+
"license": "MIT",
1146+
"license_file": null,
1147+
"links": null,
1148+
"name": "foo",
1149+
"readme": null,
1150+
"readme_file": null,
1151+
"repository": null,
1152+
"vers": "0.1.0"
1153+
}
1154+
"#,
1155+
"foo-0.1.0.crate",
1156+
&["Cargo.lock", "Cargo.toml", "Cargo.toml.orig", "src/main.rs"],
1157+
&[
1158+
(
1159+
"Cargo.toml",
1160+
// Check that only `version` is included in Cargo.toml.
1161+
"[..]\n\
1162+
[dependencies.dep1]\n\
1163+
version = \"1.0\"\n\
1164+
",
1165+
),
1166+
(
1167+
"Cargo.lock",
1168+
// The important check here is that it is 1.0.1 in the registry.
1169+
"[..]\n\
1170+
[[package]]\n\
1171+
name = \"foo\"\n\
1172+
version = \"0.1.0\"\n\
1173+
dependencies = [\n\
1174+
\x20\"dep1 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)\",\n\
1175+
]\n\
1176+
[..]",
1177+
),
1178+
],
1179+
);
1180+
}

‎tests/testsuite/support/publish.rs

+37-3
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ use std::fs::File;
33
use std::io::{self, prelude::*, SeekFrom};
44
use std::path::{Path, PathBuf};
55

6-
use crate::support::find_json_mismatch;
76
use crate::support::registry::{self, alt_api_path};
7+
use crate::support::{find_json_mismatch, lines_match};
88

99
use flate2::read::GzDecoder;
1010
use tar::Archive;
@@ -26,6 +26,24 @@ pub fn validate_upload(expected_json: &str, expected_crate_name: &str, expected_
2626
expected_json,
2727
expected_crate_name,
2828
expected_files,
29+
&[],
30+
);
31+
}
32+
33+
/// Checks the result of a crate publish, along with the contents of the files.
34+
pub fn validate_upload_with_contents(
35+
expected_json: &str,
36+
expected_crate_name: &str,
37+
expected_files: &[&str],
38+
expected_contents: &[(&str, &str)],
39+
) {
40+
let new_path = registry::api_path().join("api/v1/crates/new");
41+
_validate_upload(
42+
&new_path,
43+
expected_json,
44+
expected_crate_name,
45+
expected_files,
46+
expected_contents,
2947
);
3048
}
3149

@@ -41,6 +59,7 @@ pub fn validate_alt_upload(
4159
expected_json,
4260
expected_crate_name,
4361
expected_files,
62+
&[],
4463
);
4564
}
4665

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

7191
// Verify the tarball.
72-
validate_crate_contents(&krate_bytes[..], expected_crate_name, expected_files, &[]);
92+
validate_crate_contents(
93+
&krate_bytes[..],
94+
expected_crate_name,
95+
expected_files,
96+
expected_contents,
97+
);
7398
}
7499

75100
/// Checks the contents of a `.crate` file.
@@ -126,7 +151,16 @@ pub fn validate_crate_contents(
126151
let actual_contents = files
127152
.get(&full_e_name)
128153
.unwrap_or_else(|| panic!("file `{}` missing in archive", e_file_name));
129-
assert_eq!(actual_contents, e_file_contents);
154+
if !lines_match(e_file_contents, actual_contents) {
155+
panic!(
156+
"Crate contents mismatch for {:?}:\n\
157+
--- expected\n\
158+
{}\n\
159+
--- actual \n\
160+
{}\n",
161+
e_file_name, e_file_contents, actual_contents
162+
);
163+
}
130164
}
131165
}
132166
}

0 commit comments

Comments
 (0)