Skip to content

Commit

Permalink
install-upgrade: Fix bugs, more comments, review updates.
Browse files Browse the repository at this point in the history
  • Loading branch information
@ehuss
ehuss committed Apr 4, 2019
1 parent e023a66 commit 9ecdfe0
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 42 deletions.
6 changes: 4 additions & 2 deletions src/cargo/ops/cargo_install.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,9 @@ fn install_one(
}
};

// For bare `cargo install` (no `--bin` or `--example`), check if there is
// *something* to install. Explicit `--bin` or `--example` flags will be
// checked at the start of `compile_ws`.
if !opts.filter.is_specific() && !pkg.targets().iter().any(|t| t.is_bin()) {
bail!("specified package `{}` has no binaries", pkg);
}
Expand Down Expand Up @@ -394,8 +397,7 @@ fn install_one(
try_install()
};

if !no_track {
let mut tracker = tracker.unwrap();
if let Some(mut tracker) = tracker {
tracker.mark_installed(
pkg,
&successful_bins,
Expand Down
123 changes: 86 additions & 37 deletions src/cargo/ops/common_for_install_and_uninstall.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ use crate::util::{FileLock, Filesystem, Freshness};
///
/// This maintains a filesystem lock, preventing other instances of Cargo from
/// modifying at the same time. Drop the value to unlock.
///
/// If/when v2 is stabilized, it is intended that v1 is retained for a while
/// during a longish transition period, and then v1 can be removed.
pub struct InstallTracker {
v1: CrateListingV1,
v2: CrateListingV2,
Expand All @@ -33,6 +36,10 @@ pub struct InstallTracker {
unstable_upgrade: bool,
}

/// Tracking information for the set of installed packages.
///
/// This v2 format is unstable and requires the `-Z unstable-upgrade` option
/// to enable.
#[derive(Default, Deserialize, Serialize)]
struct CrateListingV2 {
installs: BTreeMap<PackageId, InstallInfo>,
Expand All @@ -41,6 +48,14 @@ struct CrateListingV2 {
other: BTreeMap<String, serde_json::Value>,
}

/// Tracking information for the installation of a single package.
///
/// This tracks the settings that were used when the package was installed.
/// Future attempts to install the same package will check these settings to
/// determine if it needs to be rebuilt/reinstalled. If nothing has changed,
/// then Cargo will inform the user that it is "up to date".
///
/// This is only used for the (unstable) v2 format.
#[derive(Debug, Deserialize, Serialize)]
struct InstallInfo {
/// Version requested via `--version`.
Expand Down Expand Up @@ -68,6 +83,7 @@ struct InstallInfo {
other: BTreeMap<String, serde_json::Value>,
}

/// Tracking information for the set of installed packages.
#[derive(Default, Deserialize, Serialize)]
pub struct CrateListingV1 {
v1: BTreeMap<PackageId, BTreeSet<String>>,
Expand Down Expand Up @@ -102,23 +118,20 @@ impl InstallTracker {
})?;

let v2 = (|| -> CargoResult<_> {
if unstable_upgrade {
let mut contents = String::new();
v2_lock
.as_ref()
.unwrap()
.file()
.read_to_string(&mut contents)?;
let mut v2 = if contents.is_empty() {
CrateListingV2::default()
} else {
serde_json::from_str(&contents)
.chain_err(|| format_err!("invalid JSON found for metadata"))?
};
v2.sync_v1(&v1)?;
Ok(v2)
} else {
Ok(CrateListingV2::default())
match &v2_lock {
Some(lock) => {
let mut contents = String::new();
lock.file().read_to_string(&mut contents)?;
let mut v2 = if contents.is_empty() {
CrateListingV2::default()
} else {
serde_json::from_str(&contents)
.chain_err(|| format_err!("invalid JSON found for metadata"))?
};
v2.sync_v1(&v1)?;
Ok(v2)
}
None => Ok(CrateListingV2::default()),
}
})()
.chain_err(|| {
Expand All @@ -142,8 +155,14 @@ impl InstallTracker {
///
/// Returns a tuple `(freshness, map)`. `freshness` indicates if the
/// package should be built (`Dirty`) or if it is already up-to-date
/// (`Fresh`). The map maps binary names to the PackageId that installed
/// it (which is None if not known).
/// (`Fresh`) and should be skipped. The map maps binary names to the
/// PackageId that installed it (which is None if not known).
///
/// If there are no duplicates, then it will be considered `Dirty` (i.e.,
/// it is OK to build/install).
///
/// `force=true` will always be considered `Dirty` (i.e., it will always
/// be rebuilt/reinstalled).
///
/// Returns an error if there is a duplicate and `--force` is not used.
pub fn check_upgrade(
Expand All @@ -156,12 +175,17 @@ impl InstallTracker {
_rustc: &str,
) -> CargoResult<(Freshness, BTreeMap<String, Option<PackageId>>)> {
let exes = exe_names(pkg, &opts.filter);
// Check if any tracked exe's are already installed.
let duplicates = self.find_duplicates(dst, &exes);
if force || duplicates.is_empty() {
return Ok((Freshness::Dirty, duplicates));
}
// If any duplicates are not tracked, then --force is required.
// If any duplicates are from a package with a different name, --force is required.
// Check if all duplicates come from packages of the same name. If
// there are duplicates from other packages, then --force will be
// required.
//
// There may be multiple matching duplicates if different versions of
// the same package installed different binaries.
let matching_duplicates: Vec<PackageId> = duplicates
.values()
.filter_map(|v| match v {
Expand All @@ -170,9 +194,13 @@ impl InstallTracker {
})
.collect();

// If both sets are the same length, that means all duplicates come
// from packages with the same name.
if self.unstable_upgrade && matching_duplicates.len() == duplicates.len() {
// Determine if it is dirty or fresh.
let source_id = pkg.package_id().source_id();
if source_id.is_path() {
// `cargo install --path ...` is always rebuilt.
return Ok((Freshness::Dirty, duplicates));
}
if matching_duplicates.iter().all(|dupe_pkg_id| {
Expand All @@ -182,6 +210,8 @@ impl InstallTracker {
.get(dupe_pkg_id)
.expect("dupes must be in sync");
let precise_equal = if source_id.is_git() {
// Git sources must have the exact same hash to be
// considered "fresh".
dupe_pkg_id.source_id().precise() == source_id.precise()
} else {
true
Expand All @@ -190,13 +220,7 @@ impl InstallTracker {
dupe_pkg_id.version() == pkg.version()
&& dupe_pkg_id.source_id() == source_id
&& precise_equal
&& info.features == feature_set(&opts.features)
&& info.all_features == opts.all_features
&& info.no_default_features == opts.no_default_features
&& info.profile == profile_name(opts.build_config.release)
&& (info.target.is_none()
|| info.target.as_ref().map(|t| t.as_ref()) == Some(target))
&& info.bins == exes
&& info.is_up_to_date(opts, target, &exes)
}) {
Ok((Freshness::Fresh, duplicates))
} else {
Expand All @@ -218,6 +242,11 @@ impl InstallTracker {
}
}

/// Check if any executables are already installed.
///
/// Returns a map of duplicates, the key is the executable name and the
/// value is the PackageId that is already installed. The PackageId is
/// None if it is an untracked executable.
fn find_duplicates(
&self,
dst: &Path,
Expand Down Expand Up @@ -312,7 +341,7 @@ impl CrateListingV1 {
other_bins.remove(bin);
}
}
// Remove empty metadata lines. If only BTreeMap had `retain`.
// Remove entries where `bins` is empty.
let to_remove = self
.v1
.iter()
Expand All @@ -322,11 +351,10 @@ impl CrateListingV1 {
self.v1.remove(p);
}
// Add these bins.
let mut bins = bins.clone();
self.v1
.entry(pkg.package_id())
.and_modify(|set| set.append(&mut bins))
.or_insert(bins);
.or_insert_with(BTreeSet::new)
.append(&mut bins.clone());
}

fn remove(&mut self, pkg_id: PackageId, bins: &BTreeSet<String>) {
Expand Down Expand Up @@ -354,6 +382,11 @@ impl CrateListingV1 {
}

impl CrateListingV2 {
/// Incorporate any changes from v1 into self.
/// This handles the initial upgrade to v2, *and* handles the case
/// where v2 is in use, and a v1 update is made, then v2 is used again.
/// i.e., `cargo +new install foo ; cargo +old install bar ; cargo +new install bar`
/// For now, v1 is the source of truth, so its values are trusted over v2.
fn sync_v1(&mut self, v1: &CrateListingV1) -> CargoResult<()> {
// Make the `bins` entries the same.
for (pkg_id, bins) in &v1.v1 {
Expand Down Expand Up @@ -397,7 +430,7 @@ impl CrateListingV2 {
info.bins.remove(bin);
}
}
// Remove empty metadata lines. If only BTreeMap had `retain`.
// Remove entries where `bins` is empty.
let to_remove = self
.installs
.iter()
Expand All @@ -408,9 +441,7 @@ impl CrateListingV2 {
}
// Add these bins.
if let Some(info) = self.installs.get_mut(&pkg.package_id()) {
for bin in bins {
info.bins.remove(bin);
}
info.bins.append(&mut bins.clone());
info.version_req = version_req;
info.features = feature_set(&opts.features);
info.all_features = opts.all_features;
Expand Down Expand Up @@ -454,7 +485,8 @@ impl CrateListingV2 {
let mut file = lock.file();
file.seek(SeekFrom::Start(0))?;
file.set_len(0)?;
serde_json::to_writer(file, self)?;
let data = serde_json::to_string(self)?;
file.write_all(data.as_bytes())?;
Ok(())
}
}
Expand All @@ -473,6 +505,23 @@ impl InstallInfo {
other: BTreeMap::new(),
}
}

/// Determine if this installation is "up to date", or if it needs to be reinstalled.
///
/// This does not do Package/Source/Version checking.
fn is_up_to_date(
&self,
opts: &CompileOptions<'_>,
target: &str,
exes: &BTreeSet<String>,
) -> bool {
self.features == feature_set(&opts.features)
&& self.all_features == opts.all_features
&& self.no_default_features == opts.no_default_features
&& self.profile == profile_name(opts.build_config.release)
&& (self.target.is_none() || self.target.as_ref().map(|t| t.as_ref()) == Some(target))
&& &self.bins == exes
}
}

/// Determines the root directory where installation is done.
Expand Down
3 changes: 3 additions & 0 deletions src/doc/src/reference/unstable.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,9 @@ If any of these values change, then Cargo will reinstall the package.
Installation will still fail if a different package installs a binary of the
same name. `--force` may be used to unconditionally reinstall the package.

Installing with `--path` will always build and install, unless there are
conflicting binaries from another package.

Additionally, a new flag `--no-track` is available to prevent `cargo install`
from writing tracking information in `$CARGO_HOME` about which packages are
installed.
Loading

0 comments on commit 9ecdfe0

Please sign in to comment.