Skip to content

Commit

Permalink
Elaborate registry names to index URLs when publishing
Browse files Browse the repository at this point in the history 
This avoids introducing a dependency on the publisher's name for the
registry.

Closes rust-lang#4880
  • Loading branch information
@sfackler
sfackler committed Jan 24, 2018
1 parent b1c3e78 commit f6dcdb5
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 39 deletions.
4 changes: 2 additions & 2 deletions src/cargo/core/package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ impl Package {
}
}

pub fn to_registry_toml(&self) -> CargoResult<String> {
let manifest = self.manifest().original().prepare_for_publish();
pub fn to_registry_toml(&self, config: &Config) -> CargoResult<String> {
let manifest = self.manifest().original().prepare_for_publish(config)?;
let toml = toml::to_string(&manifest)?;
Ok(format!("\
# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO\n\
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ fn tar(ws: &Workspace,
})?;

let mut header = Header::new_ustar();
let toml = pkg.to_registry_toml()?;
let toml = pkg.to_registry_toml(ws.config())?;
header.set_path(&path)?;
header.set_entry_type(EntryType::file());
header.set_mode(0o644);
Expand Down
87 changes: 56 additions & 31 deletions src/cargo/util/toml/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ impl<'de> de::Deserialize<'de> for TomlDependency {
pub struct DetailedTomlDependency {
version: Option<String>,
registry: Option<String>,
registry_index: Option<String>,
path: Option<String>,
git: Option<String>,
branch: Option<String>,
Expand Down Expand Up @@ -471,13 +472,13 @@ struct Context<'a, 'b> {
}

impl TomlManifest {
pub fn prepare_for_publish(&self) -> TomlManifest {
pub fn prepare_for_publish(&self, config: &Config) -> CargoResult<TomlManifest> {
let mut package = self.package.as_ref()
.or_else(|| self.project.as_ref())
.unwrap()
.clone();
package.workspace = None;
return TomlManifest {
return Ok(TomlManifest {
package: Some(package),
project: None,
profile: self.profile.clone(),
Expand All @@ -486,56 +487,68 @@ impl TomlManifest {
example: self.example.clone(),
test: self.test.clone(),
bench: self.bench.clone(),
dependencies: map_deps(self.dependencies.as_ref()),
dev_dependencies: map_deps(self.dev_dependencies.as_ref()
.or_else(|| self.dev_dependencies2.as_ref())),
dependencies: map_deps(config, self.dependencies.as_ref())?,
dev_dependencies: map_deps(config, self.dev_dependencies.as_ref()
.or_else(|| self.dev_dependencies2.as_ref()))?,
dev_dependencies2: None,
build_dependencies: map_deps(self.build_dependencies.as_ref()
.or_else(|| self.build_dependencies2.as_ref())),
build_dependencies: map_deps(config, self.build_dependencies.as_ref()
.or_else(|| self.build_dependencies2.as_ref()))?,
build_dependencies2: None,
features: self.features.clone(),
target: self.target.as_ref().map(|target_map| {
target: match self.target.as_ref().map(|target_map| {
target_map.iter().map(|(k, v)| {
(k.clone(), TomlPlatform {
dependencies: map_deps(v.dependencies.as_ref()),
dev_dependencies: map_deps(v.dev_dependencies.as_ref()
.or_else(|| v.dev_dependencies2.as_ref())),
Ok((k.clone(), TomlPlatform {
dependencies: map_deps(config, v.dependencies.as_ref())?,
dev_dependencies: map_deps(config, v.dev_dependencies.as_ref()
.or_else(|| v.dev_dependencies2.as_ref()))?,
dev_dependencies2: None,
build_dependencies: map_deps(v.build_dependencies.as_ref()
.or_else(|| v.build_dependencies2.as_ref())),
build_dependencies: map_deps(config, v.build_dependencies.as_ref()
.or_else(|| v.build_dependencies2.as_ref()))?,
build_dependencies2: None,
})
}))
}).collect()
}),
}) {
Some(Ok(v)) => Some(v),
Some(Err(e)) => return Err(e),
None => None,
},
replace: None,
patch: None,
workspace: None,
badges: self.badges.clone(),
cargo_features: self.cargo_features.clone(),
};
});

fn map_deps(deps: Option<&BTreeMap<String, TomlDependency>>)
-> Option<BTreeMap<String, TomlDependency>>
fn map_deps(config: &Config, deps: Option<&BTreeMap<String, TomlDependency>>)
-> CargoResult<Option<BTreeMap<String, TomlDependency>>>
{
let deps = match deps {
Some(deps) => deps,
None => return None
None => return Ok(None),
};
Some(deps.iter().map(|(k, v)| (k.clone(), map_dependency(v))).collect())
let deps = deps.iter()
.map(|(k, v)| Ok((k.clone(), map_dependency(config, v)?)))
.collect::<CargoResult<BTreeMap<_, _>>>()?;
Ok(Some(deps))
}

fn map_dependency(dep: &TomlDependency) -> TomlDependency {
fn map_dependency(config: &Config, dep: &TomlDependency) -> CargoResult<TomlDependency> {
match *dep {
TomlDependency::Detailed(ref d) => {
let mut d = d.clone();
d.path.take(); // path dependencies become crates.io deps
TomlDependency::Detailed(d)
// 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) => {
TomlDependency::Detailed(DetailedTomlDependency {
Ok(TomlDependency::Detailed(DetailedTomlDependency {
version: Some(s.clone()),
..Default::default()
})
}))
}
}
}
Expand Down Expand Up @@ -933,10 +946,18 @@ impl TomlDependency {
None => SourceId::crates_io(cx.config)?
};

let new_source_id = match (details.git.as_ref(), details.path.as_ref(), details.registry.as_ref()) {
(Some(_), _, Some(_)) => bail!("dependency ({}) specification is ambiguous. \
let new_source_id = match (
details.git.as_ref(),
details.path.as_ref(),
details.registry.as_ref(),
details.registry_index.as_ref(),
) {
(Some(_), _, Some(_), _) |
(Some(_), _, _, Some(_))=> bail!("dependency ({}) specification is ambiguous. \
Only one of `git` or `registry` is allowed.", name),
(Some(git), maybe_path, _) => {
(_, _, Some(_), Some(_)) => bail!("dependency ({}) specification is ambiguous. \
Only one of `registry` or `registry-index` is allowed.", name),
(Some(git), maybe_path, _, _) => {
if maybe_path.is_some() {
let msg = format!("dependency ({}) specification is ambiguous. \
Only one of `git` or `path` is allowed. \
Expand All @@ -963,7 +984,7 @@ impl TomlDependency {
let loc = git.to_url()?;
SourceId::for_git(&loc, reference)?
},
(None, Some(path), _) => {
(None, Some(path), _, _) => {
cx.nested_paths.push(PathBuf::from(path));
// If the source id for the package we're parsing is a path
// source, then we normalize the path here to get rid of
Expand All @@ -981,8 +1002,12 @@ impl TomlDependency {
cx.source_id.clone()
}
},
(None, None, Some(registry)) => SourceId::alt_registry(cx.config, registry)?,
(None, None, None) => SourceId::crates_io(cx.config)?,
(None, None, Some(registry), None) => SourceId::alt_registry(cx.config, registry)?,
(None, None, None, Some(registry_index)) => {
let url = registry_index.to_url()?;
SourceId::for_registry(&url)?
}
(None, None, None, None) => SourceId::crates_io(cx.config)?,
};

let version = details.version.as_ref().map(|v| &v[..]);
Expand Down
19 changes: 14 additions & 5 deletions tests/package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use std::fs::File;
use std::io::prelude::*;
use std::path::{Path, PathBuf};

use cargotest::{cargo_process, process};
use cargotest::support::{project, execs, paths, git, path2url, cargo_exe};
use cargotest::{cargo_process, process, ChannelChanger};
use cargotest::support::{project, execs, paths, git, path2url, cargo_exe, registry};
use cargotest::support::registry::Package;
use flate2::read::GzDecoder;
use hamcrest::{assert_that, existing_file, contains, equal_to};
Expand Down Expand Up @@ -715,6 +715,8 @@ fn generated_manifest() {
Package::new("ghi", "1.0.0").publish();
let p = project("foo")
.file("Cargo.toml", r#"
cargo-features = ["alternative-registries"]
[project]
name = "foo"
version = "0.0.1"
Expand All @@ -730,7 +732,7 @@ fn generated_manifest() {
[dependencies]
bar = { path = "bar", version = "0.1" }
def = "1.0"
def = { version = "1.0", registry = "alternative" }
ghi = "1.0"
abc = "1.0"
"#)
Expand All @@ -744,7 +746,9 @@ fn generated_manifest() {
.file("bar/src/lib.rs", "")
.build();

assert_that(p.cargo("package").arg("--no-verify"),
assert_that(p.cargo("package")
.masquerade_as_nightly_cargo()
.arg("--no-verify"),
execs().with_status(0));

let f = File::open(&p.root().join("target/package/foo-0.0.1.crate")).unwrap();
Expand All @@ -761,6 +765,7 @@ fn generated_manifest() {
// BTreeMap makes the order of dependencies in the generated file deterministic
// by sorting alphabetically
assert_that(&contents[..], equal_to(
&*format!(
r#"# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
#
# When uploading crates to the registry Cargo will automatically
Expand All @@ -773,6 +778,8 @@ r#"# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
# editing this file be aware that the upstream Cargo.toml
# will likely look very different (and much more reasonable)
cargo-features = ["alternative-registries"]
[package]
name = "foo"
version = "0.0.1"
Expand All @@ -791,10 +798,12 @@ version = "0.1"
[dependencies.def]
version = "1.0"
registry_index = "{}"
[dependencies.ghi]
version = "1.0"
"#));
"#,
registry::alt_registry())));
}

#[test]
Expand Down

0 comments on commit f6dcdb5

Please sign in to comment.