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

Fix profile override warning in a workspace. #7536

Merged
merged 1 commit into from
Oct 24, 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
2 changes: 1 addition & 1 deletion src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ pub fn resolve_std<'cfg>(
/*uses_default_features*/ true,
);
let resolve = ops::resolve_ws_with_opts(&std_ws, opts, &specs)?;
Ok(resolve)
Ok((resolve.pkg_set, resolve.targeted_resolve))
}

/// Generate a list of root `Unit`s for the standard library.
Expand Down
27 changes: 11 additions & 16 deletions src/cargo/core/profiles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use serde::Deserialize;

use crate::core::compiler::{CompileMode, ProfileKind};
use crate::core::interning::InternedString;
use crate::core::{Feature, Features, PackageId, PackageIdSpec, PackageSet, Shell};
use crate::core::{Feature, Features, PackageId, PackageIdSpec, Resolve, Shell};
use crate::util::errors::CargoResultExt;
use crate::util::toml::{ProfilePackageSpec, StringOrBool, TomlProfile, TomlProfiles, U32OrBool};
use crate::util::{closest_msg, CargoResult, Config};
Expand Down Expand Up @@ -429,13 +429,9 @@ impl Profiles {
}

/// Used to check for overrides for non-existing packages.
pub fn validate_packages(
&self,
shell: &mut Shell,
packages: &PackageSet<'_>,
) -> CargoResult<()> {
pub fn validate_packages(&self, shell: &mut Shell, resolve: &Resolve) -> CargoResult<()> {
for profile in self.by_name.values() {
profile.validate_packages(shell, packages)?;
profile.validate_packages(shell, resolve)?;
}
Ok(())
}
Expand Down Expand Up @@ -502,16 +498,16 @@ impl ProfileMaker {
profile
}

fn validate_packages(&self, shell: &mut Shell, packages: &PackageSet<'_>) -> CargoResult<()> {
self.validate_packages_toml(shell, packages, &self.toml, true)?;
self.validate_packages_toml(shell, packages, &self.config, false)?;
fn validate_packages(&self, shell: &mut Shell, resolve: &Resolve) -> CargoResult<()> {
self.validate_packages_toml(shell, resolve, &self.toml, true)?;
self.validate_packages_toml(shell, resolve, &self.config, false)?;
Ok(())
}

fn validate_packages_toml(
&self,
shell: &mut Shell,
packages: &PackageSet<'_>,
resolve: &Resolve,
toml: &Option<TomlProfile>,
warn_unmatched: bool,
) -> CargoResult<()> {
Expand All @@ -525,7 +521,7 @@ impl ProfileMaker {
};
// Verify that a package doesn't match multiple spec overrides.
let mut found = HashSet::new();
for pkg_id in packages.package_ids() {
for pkg_id in resolve.iter() {
let matches: Vec<&PackageIdSpec> = overrides
.keys()
.filter_map(|key| match *key {
Expand Down Expand Up @@ -575,8 +571,8 @@ impl ProfileMaker {
});
for spec in missing_specs {
// See if there is an exact name match.
let name_matches: Vec<String> = packages
.package_ids()
let name_matches: Vec<String> = resolve
.iter()
.filter_map(|pkg_id| {
if pkg_id.name() == spec.name() {
Some(pkg_id.to_string())
Expand All @@ -586,8 +582,7 @@ impl ProfileMaker {
})
.collect();
if name_matches.is_empty() {
let suggestion =
closest_msg(&spec.name(), packages.package_ids(), |p| p.name().as_str());
let suggestion = closest_msg(&spec.name(), resolve.iter(), |p| p.name().as_str());
shell.warn(format!(
"package profile spec `{}` did not match any packages{}",
spec, suggestion
Expand Down
31 changes: 17 additions & 14 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use crate::core::resolver::{Resolve, ResolveOpts};
use crate::core::{LibKind, Package, PackageSet, Target};
use crate::core::{PackageId, PackageIdSpec, TargetKind, Workspace};
use crate::ops;
use crate::ops::resolve::WorkspaceResolve;
use crate::util::config::Config;
use crate::util::{closest_msg, profile, CargoResult};

Expand Down Expand Up @@ -303,7 +304,11 @@ pub fn compile_ws<'a>(
let dev_deps = ws.require_optional_deps() || filter.need_dev_deps(build_config.mode);
let opts = ResolveOpts::new(dev_deps, features, all_features, !no_default_features);
let resolve = ops::resolve_ws_with_opts(ws, opts, &specs)?;
let (mut packages, resolve_with_overrides) = resolve;
let WorkspaceResolve {
mut pkg_set,
workspace_resolve,
targeted_resolve: resolve,
} = resolve;

let std_resolve = if let Some(crates) = &config.cli_unstable().build_std {
if build_config.build_plan {
Expand All @@ -319,7 +324,7 @@ pub fn compile_ws<'a>(
}
let (mut std_package_set, std_resolve) = standard_lib::resolve_std(ws, crates)?;
remove_dylib_crate_type(&mut std_package_set)?;
packages.add_set(std_package_set);
pkg_set.add_set(std_package_set);
Some(std_resolve)
} else {
None
Expand All @@ -330,12 +335,12 @@ pub fn compile_ws<'a>(
// Vec<PackageIdSpec> to a Vec<&PackageId>.
let to_build_ids = specs
.iter()
.map(|s| s.query(resolve_with_overrides.iter()))
.map(|s| s.query(resolve.iter()))
.collect::<CargoResult<Vec<_>>>()?;
// Now get the `Package` for each `PackageId`. This may trigger a download
// if the user specified `-p` for a dependency that is not downloaded.
// Dependencies will be downloaded during build_unit_dependencies.
let mut to_builds = packages.get_many(to_build_ids)?;
let mut to_builds = pkg_set.get_many(to_build_ids)?;

// The ordering here affects some error messages coming out of cargo, so
// let's be test and CLI friendly by always printing in the same order if
Expand Down Expand Up @@ -370,12 +375,15 @@ pub fn compile_ws<'a>(
);
}

profiles.validate_packages(&mut config.shell(), &packages)?;
profiles.validate_packages(
&mut config.shell(),
workspace_resolve.as_ref().unwrap_or(&resolve),
)?;

let interner = UnitInterner::new();
let mut bcx = BuildContext::new(
ws,
&packages,
&pkg_set,
config,
build_config,
profiles,
Expand All @@ -388,7 +396,7 @@ pub fn compile_ws<'a>(
&to_builds,
filter,
build_config.requested_kind,
&resolve_with_overrides,
&resolve,
&bcx,
)?;

Expand Down Expand Up @@ -434,13 +442,8 @@ pub fn compile_ws<'a>(
}
}

let unit_dependencies = build_unit_dependencies(
&bcx,
&resolve_with_overrides,
std_resolve.as_ref(),
&units,
&std_roots,
)?;
let unit_dependencies =
build_unit_dependencies(&bcx, &resolve, std_resolve.as_ref(), &units, &std_roots)?;

let ret = {
let _p = profile::start("compiling");
Expand Down
7 changes: 3 additions & 4 deletions src/cargo/ops/cargo_doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,13 @@ pub fn doc(ws: &Workspace<'_>, options: &DocOptions<'_>) -> CargoResult<()> {
options.compile_opts.all_features,
!options.compile_opts.no_default_features,
);
let resolve = ops::resolve_ws_with_opts(ws, opts, &specs)?;
let (packages, resolve_with_overrides) = resolve;
let ws_resolve = ops::resolve_ws_with_opts(ws, opts, &specs)?;

let ids = specs
.iter()
.map(|s| s.query(resolve_with_overrides.iter()))
.map(|s| s.query(ws_resolve.targeted_resolve.iter()))
.collect::<CargoResult<Vec<_>>>()?;
let pkgs = packages.get_many(ids)?;
let pkgs = ws_resolve.pkg_set.get_many(ids)?;

let mut lib_names = HashMap::new();
let mut bin_names = HashMap::new();
Expand Down
6 changes: 3 additions & 3 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,14 +491,14 @@ fn check_yanked_install(ws: &Workspace<'_>) -> CargoResult<()> {
// It would be best if `source` could be passed in here to avoid a
// duplicate "Updating", but since `source` is taken by value, then it
// wouldn't be available for `compile_ws`.
let (pkg_set, resolve) = ops::resolve_ws_with_opts(ws, ResolveOpts::everything(), &specs)?;
let mut sources = pkg_set.sources_mut();
let ws_resolve = ops::resolve_ws_with_opts(ws, ResolveOpts::everything(), &specs)?;
let mut sources = ws_resolve.pkg_set.sources_mut();

// Checking the yanked status involves taking a look at the registry and
// maybe updating files, so be sure to lock it here.
let _lock = ws.config().acquire_package_cache_lock()?;

for pkg_id in resolve.iter() {
for pkg_id in ws_resolve.targeted_resolve.iter() {
if let Some(source) = sources.get_mut(pkg_id.source_id()) {
if source.is_yanked(pkg_id)? {
ws.config().shell().warn(format!(
Expand Down
9 changes: 6 additions & 3 deletions src/cargo/ops/cargo_output_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,20 @@ fn metadata_full(ws: &Workspace<'_>, opt: &OutputMetadataOptions) -> CargoResult
opt.all_features,
!opt.no_default_features,
);
let (package_set, resolve) = ops::resolve_ws_with_opts(ws, opts, &specs)?;
let ws_resolve = ops::resolve_ws_with_opts(ws, opts, &specs)?;
let mut packages = HashMap::new();
for pkg in package_set.get_many(package_set.package_ids())? {
for pkg in ws_resolve
.pkg_set
.get_many(ws_resolve.pkg_set.package_ids())?
{
packages.insert(pkg.package_id(), pkg.clone());
}

Ok(ExportInfo {
packages: packages.values().map(|p| (*p).clone()).collect(),
workspace_members: ws.members().map(|pkg| pkg.package_id()).collect(),
resolve: Some(MetadataResolve {
resolve: (packages, resolve),
resolve: (packages, ws_resolve.targeted_resolve),
root: ws.current_opt().map(|pkg| pkg.package_id()),
}),
target_directory: ws.target_dir().into_path_unlocked(),
Expand Down
14 changes: 9 additions & 5 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,15 +155,19 @@ fn build_lock(ws: &Workspace<'_>) -> CargoResult<String> {
// Regenerate Cargo.lock using the old one as a guide.
let specs = vec![PackageIdSpec::from_package_id(new_pkg.package_id())];
let tmp_ws = Workspace::ephemeral(new_pkg, ws.config(), None, true)?;
let (pkg_set, new_resolve) =
ops::resolve_ws_with_opts(&tmp_ws, ResolveOpts::everything(), &specs)?;
let new_resolve = ops::resolve_ws_with_opts(&tmp_ws, ResolveOpts::everything(), &specs)?;

if let Some(orig_resolve) = orig_resolve {
compare_resolve(config, tmp_ws.current()?, &orig_resolve, &new_resolve)?;
compare_resolve(
config,
tmp_ws.current()?,
&orig_resolve,
&new_resolve.targeted_resolve,
)?;
}
check_yanked(config, &pkg_set, &new_resolve)?;
check_yanked(config, &new_resolve.pkg_set, &new_resolve.targeted_resolve)?;

ops::resolve_to_string(&tmp_ws, &new_resolve)
ops::resolve_to_string(&tmp_ws, &new_resolve.targeted_resolve)
}

// Checks that the package has some piece of metadata that a human can
Expand Down
24 changes: 21 additions & 3 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,20 @@ use crate::sources::PathSource;
use crate::util::errors::{CargoResult, CargoResultExt};
use crate::util::profile;

/// Result for `resolve_ws_with_opts`.
pub struct WorkspaceResolve<'a> {
/// Packages to be downloaded.
pub pkg_set: PackageSet<'a>,
/// The resolve for the entire workspace.
///
/// This may be `None` for things like `cargo install` and `-Zavoid-dev-deps`.
/// This does not include `paths` overrides.
pub workspace_resolve: Option<Resolve>,
/// The narrowed resolve, with the specific features enabled, and only the
/// given package specs requested.
pub targeted_resolve: Resolve,
}

const UNUSED_PATCH_WARNING: &str = "\
Check that the patched package version and available features are compatible
with the dependency requirements. If the patch has a different version from
Expand Down Expand Up @@ -60,7 +74,7 @@ pub fn resolve_ws_with_opts<'a>(
ws: &Workspace<'a>,
opts: ResolveOpts,
specs: &[PackageIdSpec],
) -> CargoResult<(PackageSet<'a>, Resolve)> {
) -> CargoResult<WorkspaceResolve<'a>> {
let mut registry = PackageRegistry::new(ws.config())?;
let mut add_patches = true;

Expand Down Expand Up @@ -106,9 +120,13 @@ pub fn resolve_ws_with_opts<'a>(
add_patches,
)?;

let packages = get_resolved_packages(&resolved_with_overrides, registry)?;
let pkg_set = get_resolved_packages(&resolved_with_overrides, registry)?;

Ok((packages, resolved_with_overrides))
Ok(WorkspaceResolve {
pkg_set,
workspace_resolve: resolve,
targeted_resolve: resolved_with_overrides,
})
}

fn resolve_with_registry<'cfg>(
Expand Down
32 changes: 32 additions & 0 deletions tests/testsuite/profile_overrides.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,3 +512,35 @@ fn override_package_rename() {
")
.run();
}

#[cargo_test]
fn no_warning_ws() {
// https://github.com/rust-lang/cargo/issues/7378, avoid warnings in a workspace.
let p = project()
.file(
"Cargo.toml",
r#"
cargo-features = ["profile-overrides"]
[workspace]
members = ["a", "b"]

[profile.dev.package.a]
codegen-units = 3
"#,
)
.file("a/Cargo.toml", &basic_manifest("a", "0.1.0"))
.file("a/src/lib.rs", "")
.file("b/Cargo.toml", &basic_manifest("b", "0.1.0"))
.file("b/src/lib.rs", "")
.build();

p.cargo("build -p b")
.masquerade_as_nightly_cargo()
.with_stderr(
"\
[COMPILING] b [..]
[FINISHED] [..]
",
)
.run();
}