Skip to content

Commit

Permalink
Only apply config patches on resolve
Browse files Browse the repository at this point in the history
  • Loading branch information
Jon Gjengset committed Feb 26, 2021
1 parent 8178f22 commit 140a770
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 19 deletions.
40 changes: 38 additions & 2 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::borrow::Cow;
use std::cell::RefCell;
use std::collections::hash_map::{Entry, HashMap};
use std::collections::{BTreeMap, BTreeSet, HashSet};
Expand Down Expand Up @@ -365,11 +366,46 @@ impl<'cfg> Workspace<'cfg> {
/// Returns the root `[patch]` section of this workspace.
///
/// This may be from a virtual crate or an actual crate.
pub fn root_patch(&self) -> &HashMap<Url, Vec<Dependency>> {
match self.root_maybe() {
pub fn root_patch(&self) -> Cow<'_, HashMap<Url, Vec<Dependency>>> {
let from_manifest = match self.root_maybe() {
MaybePackage::Package(p) => p.manifest().patch(),
MaybePackage::Virtual(vm) => vm.patch(),
};

let from_config = self
.config
.patch()
.expect("config [patch] was never parsed");
if from_config.is_empty() {
return Cow::Borrowed(from_manifest);
}
if from_manifest.is_empty() {
return Cow::Borrowed(from_config);
}

// We could just chain from_manifest and from_config,
// but that's not quite right as it won't deal with overlaps.
let mut combined = from_manifest.clone();
for (url, cdeps) in from_config {
if let Some(deps) = combined.get_mut(url) {
// We want from_manifest to take precedence for each patched name.
// NOTE: This is inefficient if the number of patches is large!
let mut left = cdeps.clone();
for dep in &mut *deps {
if let Some(i) = left.iter().position(|cdep| {
// XXX: should this also take into account version numbers?
dep.name_in_toml() == cdep.name_in_toml()
}) {
left.swap_remove(i);
}
}
// Whatever is left does not exist in manifest dependencies.
deps.extend(left);
} else {
combined.insert(url.clone(), cdeps.clone());
}
}
Cow::Owned(combined)
}

/// Returns an iterator over all packages in this workspace
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ pub fn resolve_with_previous<'cfg>(
// locked.
let mut avoid_patch_ids = HashSet::new();
if register_patches {
for (url, patches) in ws.root_patch() {
for (url, patches) in ws.root_patch().iter() {
let previous = match previous {
Some(r) => r,
None => {
Expand Down
16 changes: 15 additions & 1 deletion src/cargo/util/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ use url::Url;
use self::ConfigValue as CV;
use crate::core::compiler::rustdoc::RustdocExternMap;
use crate::core::shell::Verbosity;
use crate::core::{nightly_features_allowed, CliUnstable, Shell, SourceId, Workspace};
use crate::core::{nightly_features_allowed, CliUnstable, Dependency, Shell, SourceId, Workspace};
use crate::ops;
use crate::util::errors::{CargoResult, CargoResultExt};
use crate::util::toml as cargo_toml;
Expand Down Expand Up @@ -176,6 +176,8 @@ pub struct Config {
/// Lock, if held, of the global package cache along with the number of
/// acquisitions so far.
package_cache_lock: RefCell<Option<(Option<FileLock>, usize)>>,
/// `[patch]` section parsed from configuration.
patch: LazyCell<HashMap<Url, Vec<Dependency>>>,
/// Cached configuration parsed by Cargo
http_config: LazyCell<CargoHttpConfig>,
net_config: LazyCell<CargoNetConfig>,
Expand Down Expand Up @@ -261,6 +263,7 @@ impl Config {
upper_case_env,
updated_sources: LazyCell::new(),
package_cache_lock: RefCell::new(None),
patch: LazyCell::new(),
http_config: LazyCell::new(),
net_config: LazyCell::new(),
build_config: LazyCell::new(),
Expand Down Expand Up @@ -1291,6 +1294,17 @@ impl Config {
Ok(*(self.crates_io_source_id.try_borrow_with(f)?))
}

pub fn maybe_init_patch<F>(&self, f: F) -> CargoResult<&HashMap<Url, Vec<Dependency>>>
where
F: FnMut() -> CargoResult<HashMap<Url, Vec<Dependency>>>,
{
self.patch.try_borrow_with(f)
}

pub fn patch(&self) -> Option<&HashMap<Url, Vec<Dependency>>> {
self.patch.borrow()
}

pub fn creation_time(&self) -> Instant {
self.creation_time
}
Expand Down
28 changes: 13 additions & 15 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::path::{Path, PathBuf};
use std::rc::Rc;
use std::str;

use anyhow::{anyhow, bail};
use anyhow::{anyhow, bail, Context as _};
use cargo_platform::Platform;
use log::{debug, trace};
use semver::{self, VersionReq};
Expand Down Expand Up @@ -1558,22 +1558,20 @@ impl TomlManifest {
}

fn patch(&self, cx: &mut Context<'_, '_>) -> CargoResult<HashMap<Url, Vec<Dependency>>> {
let from_manifest = Self::patch_(self.patch.as_ref(), cx)?;

let config_patch: Option<BTreeMap<String, BTreeMap<String, TomlDependency>>> =
cx.config.get("patch")?;
let _ = cx.config.maybe_init_patch(|| {
// Parse out the patches from .config while we're at it.
let config_patch: Option<BTreeMap<String, BTreeMap<String, TomlDependency>>> =
cx.config.get("patch")?;

if config_patch.is_some() && !cx.config.cli_unstable().patch_in_config {
cx.warnings.push("`[patch]` in .cargo/config.toml ignored, the -Zpatch-in-config command-line flag is required".to_owned());
return Ok(HashMap::new());
}

if config_patch.is_some() && !cx.config.cli_unstable().patch_in_config {
cx.warnings.push("`[patch]` in .cargo/config.toml ignored, the -Zpatch-in-config command-line flag is required".to_owned());
return Ok(from_manifest);
}
Self::patch_(config_patch.as_ref(), cx)
}).context("parse `[patch]` from .cargo/config.toml")?;

let mut from_config = Self::patch_(config_patch.as_ref(), cx)?;
if from_config.is_empty() {
return Ok(from_manifest);
}
from_config.extend(from_manifest);
Ok(from_config)
Self::patch_(self.patch.as_ref(), cx)
}

/// Returns the path to the build script if one exists for this crate.
Expand Down
46 changes: 46 additions & 0 deletions tests/testsuite/patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ fn from_config_without_z() {
)
.run();
}

#[cargo_test]
fn from_config() {
Package::new("bar", "0.1.0").publish();
Expand Down Expand Up @@ -151,6 +152,51 @@ fn from_config() {
.run();
}

#[cargo_test]
fn from_config_precedence() {
Package::new("bar", "0.1.0").publish();

let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.0.1"
authors = []
[dependencies]
bar = "0.1.0"
[patch.crates-io]
bar = { path = 'bar' }
"#,
)
.file(
".cargo/config.toml",
r#"
[patch.crates-io]
bar = { path = 'no-such-path' }
"#,
)
.file("src/lib.rs", "")
.file("bar/Cargo.toml", &basic_manifest("bar", "0.1.1"))
.file("bar/src/lib.rs", r#""#)
.build();

p.cargo("build -Zpatch-in-config")
.masquerade_as_nightly_cargo()
.with_stderr(
"\
[UPDATING] `[ROOT][..]` index
[COMPILING] bar v0.1.1 ([..])
[COMPILING] foo v0.0.1 ([CWD])
[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]
",
)
.run();
}

#[cargo_test]
fn nonexistent() {
Package::new("baz", "0.1.0").publish();
Expand Down

0 comments on commit 140a770

Please sign in to comment.