Skip to content

Commit

Permalink
Auto merge of #6840 - ehuss:publish-lockfile-updates, r=alexcrichton
Browse files Browse the repository at this point in the history
publish-lockfile: Various updates

This is a collection of changes to the `publish-lockfile` feature. They are separated by commit (see each for more detailed commit messages), and I can separate them into separate PRs if necessary.

- Move publish-lockfile tests to a dedicated file.
- Regenerate `Cargo.lock` when creating a package.
- Ignore `Cargo.lock` in `cargo install` by default (requires `--locked` to use).
- Add warnings for yanked dependencies.
- Change default of publish-lockfile to true.
  • Loading branch information
bors committed Apr 18, 2019
2 parents f2ea95a + 7d20230 commit 96e9496
Show file tree
Hide file tree
Showing 28 changed files with 895 additions and 272 deletions.
12 changes: 11 additions & 1 deletion src/cargo/core/package.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::cell::{Cell, Ref, RefCell};
use std::cell::{Cell, Ref, RefCell, RefMut};
use std::cmp::Ordering;
use std::collections::{HashMap, HashSet};
use std::fmt;
Expand Down Expand Up @@ -236,6 +236,12 @@ impl Package {
toml
))
}

/// Returns if package should include `Cargo.lock`.
pub fn include_lockfile(&self) -> bool {
self.manifest().publish_lockfile()
&& self.targets().iter().any(|t| t.is_example() || t.is_bin())
}
}

impl fmt::Display for Package {
Expand Down Expand Up @@ -454,6 +460,10 @@ impl<'cfg> PackageSet<'cfg> {
pub fn sources(&self) -> Ref<'_, SourceMap<'cfg>> {
self.sources.borrow()
}

pub fn sources_mut(&self) -> RefMut<'_, SourceMap<'cfg>> {
self.sources.borrow_mut()
}
}

// When dynamically linked against libcurl, we want to ignore some failures
Expand Down
12 changes: 12 additions & 0 deletions src/cargo/core/source/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ pub trait Source {
/// queries, even if they are yanked. Currently only applies to registry
/// sources.
fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]);

/// Query if a package is yanked. Only registry sources can mark packages
/// as yanked. This ignores the yanked whitelist.
fn is_yanked(&mut self, _pkg: PackageId) -> CargoResult<bool>;
}

pub enum MaybePackage {
Expand Down Expand Up @@ -169,6 +173,10 @@ impl<'a, T: Source + ?Sized + 'a> Source for Box<T> {
fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) {
(**self).add_to_yanked_whitelist(pkgs);
}

fn is_yanked(&mut self, pkg: PackageId) -> CargoResult<bool> {
(**self).is_yanked(pkg)
}
}

impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T {
Expand Down Expand Up @@ -227,6 +235,10 @@ impl<'a, T: Source + ?Sized + 'a> Source for &'a mut T {
fn add_to_yanked_whitelist(&mut self, pkgs: &[PackageId]) {
(**self).add_to_yanked_whitelist(pkgs);
}

fn is_yanked(&mut self, pkg: PackageId) -> CargoResult<bool> {
(**self).is_yanked(pkg)
}
}

/// A `HashMap` of `SourceId` -> `Box<Source>`.
Expand Down
12 changes: 11 additions & 1 deletion src/cargo/core/source/source_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,14 +228,24 @@ impl SourceId {
&self.inner.url
}

pub fn display_registry(self) -> String {
pub fn display_index(self) -> String {
if self.is_default_registry() {
"crates.io index".to_string()
} else {
format!("`{}` index", url_display(self.url()))
}
}

pub fn display_registry_name(self) -> String {
if self.is_default_registry() {
"crates.io".to_string()
} else if let Some(name) = &self.inner.name {
name.clone()
} else {
url_display(self.url())
}
}

/// Returns `true` if this source is from a filesystem path.
pub fn is_path(self) -> bool {
self.inner.kind == Kind::Path
Expand Down
19 changes: 17 additions & 2 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ pub struct Workspace<'cfg> {
// A cache of loaded packages for particular paths which is disjoint from
// `packages` up above, used in the `load` method down below.
loaded_packages: RefCell<HashMap<PathBuf, Package>>,

// If `true`, then the resolver will ignore any existing `Cargo.lock`
// file. This is set for `cargo install` without `--locked`.
ignore_lock: bool,
}

// Separate structure for tracking loaded packages (to avoid loading anything
Expand Down Expand Up @@ -148,6 +152,7 @@ impl<'cfg> Workspace<'cfg> {
is_ephemeral: false,
require_optional_deps: true,
loaded_packages: RefCell::new(HashMap::new()),
ignore_lock: false,
};
ws.root_manifest = ws.find_root(manifest_path)?;
ws.find_members()?;
Expand Down Expand Up @@ -184,6 +189,7 @@ impl<'cfg> Workspace<'cfg> {
is_ephemeral: true,
require_optional_deps,
loaded_packages: RefCell::new(HashMap::new()),
ignore_lock: false,
};
{
let key = ws.current_manifest.parent().unwrap();
Expand Down Expand Up @@ -320,14 +326,23 @@ impl<'cfg> Workspace<'cfg> {
self.require_optional_deps
}

pub fn set_require_optional_deps<'a>(
&'a mut self,
pub fn set_require_optional_deps(
&mut self,
require_optional_deps: bool,
) -> &mut Workspace<'cfg> {
self.require_optional_deps = require_optional_deps;
self
}

pub fn ignore_lock(&self) -> bool {
self.ignore_lock
}

pub fn set_ignore_lock(&mut self, ignore_lock: bool) -> &mut Workspace<'cfg> {
self.ignore_lock = ignore_lock;
self
}

/// Finds the root of a workspace for the crate whose manifest is located
/// at `manifest_path`.
///
Expand Down
7 changes: 3 additions & 4 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use crate::core::compiler::{
use crate::core::compiler::{CompileMode, Kind, Unit};
use crate::core::profiles::{Profiles, UnitFor};
use crate::core::resolver::{Method, Resolve};
use crate::core::{Package, Source, Target};
use crate::core::{Package, Target};
use crate::core::{PackageId, PackageIdSpec, TargetKind, Workspace};
use crate::ops;
use crate::util::config::Config;
Expand Down Expand Up @@ -247,12 +247,11 @@ pub fn compile_with_exec<'a>(
exec: &Arc<dyn Executor>,
) -> CargoResult<Compilation<'a>> {
ws.emit_warnings()?;
compile_ws(ws, None, options, exec)
compile_ws(ws, options, exec)
}

pub fn compile_ws<'a>(
ws: &Workspace<'a>,
source: Option<Box<dyn Source + 'a>>,
options: &CompileOptions<'a>,
exec: &Arc<dyn Executor>,
) -> CargoResult<Compilation<'a>> {
Expand Down Expand Up @@ -305,7 +304,7 @@ pub fn compile_ws<'a>(
all_features,
uses_default_features: !no_default_features,
};
let resolve = ops::resolve_ws_with_method(ws, source, method, &specs)?;
let resolve = ops::resolve_ws_with_method(ws, method, &specs)?;
let (packages, resolve_with_overrides) = resolve;

let to_build_ids = specs
Expand Down
1 change: 0 additions & 1 deletion src/cargo/ops/cargo_doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ pub fn doc(ws: &Workspace<'_>, options: &DocOptions<'_>) -> CargoResult<()> {
let specs = options.compile_opts.spec.to_package_id_specs(ws)?;
let resolve = ops::resolve_ws_precisely(
ws,
None,
&options.compile_opts.features,
options.compile_opts.all_features,
options.compile_opts.no_default_features,
Expand Down
38 changes: 34 additions & 4 deletions src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use tempfile::Builder as TempFileBuilder;

use crate::core::compiler::Freshness;
use crate::core::compiler::{DefaultExecutor, Executor};
use crate::core::{Edition, PackageId, Source, SourceId, Workspace};
use crate::core::resolver::Method;
use crate::core::{Edition, PackageId, PackageIdSpec, Source, SourceId, Workspace};
use crate::ops;
use crate::ops::common_for_install_and_uninstall::*;
use crate::sources::{GitSource, SourceConfigMap};
Expand Down Expand Up @@ -144,7 +145,7 @@ fn install_one(
) -> CargoResult<()> {
let config = opts.config;

let (pkg, source) = if source_id.is_git() {
let pkg = if source_id.is_git() {
select_pkg(
GitSource::new(source_id, config)?,
krate,
Expand Down Expand Up @@ -214,14 +215,15 @@ fn install_one(
Some(Filesystem::new(config.cwd().join("target-install")))
};

let ws = match overidden_target_dir {
let mut ws = match overidden_target_dir {
Some(dir) => Workspace::ephemeral(pkg, config, Some(dir), false)?,
None => {
let mut ws = Workspace::new(pkg.manifest_path(), config)?;
ws.set_require_optional_deps(false);
ws
}
};
ws.set_ignore_lock(config.lock_update_allowed());
let pkg = ws.current()?;

if from_cwd {
Expand Down Expand Up @@ -302,8 +304,10 @@ fn install_one(

config.shell().status("Installing", pkg)?;

check_yanked_install(&ws)?;

let exec: Arc<dyn Executor> = Arc::new(DefaultExecutor);
let compile = ops::compile_ws(&ws, Some(source), opts, &exec).chain_err(|| {
let compile = ops::compile_ws(&ws, opts, &exec).chain_err(|| {
if let Some(td) = td_opt.take() {
// preserve the temporary directory, so the user can inspect it
td.into_path();
Expand Down Expand Up @@ -477,6 +481,32 @@ fn install_one(
}
}

fn check_yanked_install(ws: &Workspace<'_>) -> CargoResult<()> {
if ws.ignore_lock() || !ws.root().join("Cargo.lock").exists() {
return Ok(());
}
let specs = vec![PackageIdSpec::from_package_id(ws.current()?.package_id())];
// 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_method(ws, Method::Everything, &specs)?;
let mut sources = pkg_set.sources_mut();
for pkg_id in resolve.iter() {
if let Some(source) = sources.get_mut(pkg_id.source_id()) {
if source.is_yanked(pkg_id)? {
ws.config().shell().warn(format!(
"package `{}` in Cargo.lock is yanked in registry `{}`, \
consider running without --locked",
pkg_id,
pkg_id.source_id().display_registry_name()
))?;
}
}
}

Ok(())
}

/// Display a list of installed binaries.
pub fn install_list(dst: Option<&str>, config: &Config) -> CargoResult<()> {
let root = resolve_root(dst, config)?;
Expand Down
1 change: 0 additions & 1 deletion src/cargo/ops/cargo_output_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ fn metadata_full(ws: &Workspace<'_>, opt: &OutputMetadataOptions) -> CargoResult
let specs = Packages::All.to_package_id_specs(ws)?;
let (package_set, resolve) = ops::resolve_ws_precisely(
ws,
None,
&opt.features,
opt.all_features,
opt.no_default_features,
Expand Down
Loading

0 comments on commit 96e9496

Please sign in to comment.