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: Use the right channels when upgrading global packages #1326

Merged
merged 4 commits into from
May 19, 2024
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
6 changes: 3 additions & 3 deletions src/cli/global/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,13 @@ pub(super) fn channel_name_from_prefix(
/// # Returns
///
/// The package records (with dependencies records) for the given package MatchSpec
pub fn load_package_records(
pub fn load_package_records<'a>(
package_matchspec: MatchSpec,
sparse_repodata: &IndexMap<(Channel, Platform), SparseRepoData>,
sparse_repodata: impl IntoIterator<Item = &'a SparseRepoData>,
) -> miette::Result<Vec<RepoDataRecord>> {
let package_name = package_name(&package_matchspec)?;
let available_packages =
SparseRepoData::load_records_recursive(sparse_repodata.values(), vec![package_name], None)
SparseRepoData::load_records_recursive(sparse_repodata, vec![package_name], None)
.into_diagnostic()?;
let virtual_packages = rattler_virtual_packages::VirtualPackage::current()
.into_diagnostic()?
Expand Down
2 changes: 1 addition & 1 deletion src/cli/global/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ pub async fn execute(args: Args) -> miette::Result<()> {
// Install the package(s)
let mut executables = vec![];
for (package_name, package_matchspec) in args.specs()? {
let records = load_package_records(package_matchspec, &sparse_repodata)?;
let records = load_package_records(package_matchspec, sparse_repodata.values())?;

let (prefix_package, scripts, _) = globally_install_package(
&package_name,
Expand Down
95 changes: 56 additions & 39 deletions src/cli/global/upgrade.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
use std::collections::HashMap;
use std::sync::Arc;
use std::time::Duration;

use clap::Parser;
use indexmap::IndexMap;
use indicatif::ProgressBar;
use itertools::Itertools;
use miette::IntoDiagnostic;
use miette::{IntoDiagnostic, Report};
use rattler_conda_types::{Channel, MatchSpec, PackageName, Platform};
use tokio::task::JoinSet;

use crate::config::Config;
use crate::progress::{global_multi_progress, long_running_progress_style};
Expand Down Expand Up @@ -59,56 +61,77 @@ pub(super) async fn upgrade_packages(
cli_channels: &[String],
platform: &Platform,
) -> miette::Result<()> {
// Get channels and versions of globally installed packages
let mut installed_versions = HashMap::with_capacity(specs.len());
let mut channels = config.compute_channels(cli_channels).into_diagnostic()?;

for package_name in specs.keys() {
let prefix_record = find_installed_package(package_name).await?;
let last_installed_channel = Channel::from_str(
prefix_record.repodata_record.channel.clone(),
config.channel_config(),
)
.into_diagnostic()?;

channels.push(last_installed_channel);

let installed_version = prefix_record
.repodata_record
.package_record
.version
.into_version();
installed_versions.insert(package_name.clone(), installed_version);
let channel_cli = config.compute_channels(cli_channels).into_diagnostic()?;

// Get channels and version of globally installed packages in parallel
let mut channels = HashMap::with_capacity(specs.len());
let mut versions = HashMap::with_capacity(specs.len());
let mut set: JoinSet<Result<_, Report>> = JoinSet::new();
for package_name in specs.keys().cloned() {
let channel_config = config.channel_config().clone();
set.spawn(async move {
let p = find_installed_package(&package_name).await?;
let channel =
Channel::from_str(p.repodata_record.channel, &channel_config).into_diagnostic()?;
let version = p.repodata_record.package_record.version.into_version();
Ok((package_name, channel, version))
});
}
while let Some(data) = set.join_next().await {
let (package_name, channel, version) = data.into_diagnostic()??;
channels.insert(package_name.clone(), channel);
versions.insert(package_name, version);
}
channels = channels.into_iter().unique().collect();

// Fetch sparse repodata
let (authenticated_client, sparse_repodata) =
get_client_and_sparse_repodata(&channels, *platform, &config).await?;
// Fetch sparse repodata across all channels
let all_channels = channels.values().chain(channel_cli.iter()).unique();
let (client, repodata) =
get_client_and_sparse_repodata(all_channels, *platform, &config).await?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this hasn't changed, but I just noticed if you installed a package from a channel that isn't available anymore, this fails. And it doesn't update any environment.

Might be nice to filter these and just list which where skipped in the end.

I'm not avoiding this to merge because of it but just picking your brain.

The case for me was that I installed a locally build package which I had push to /tmp and installed from there.


// Resolve environments in parallel
let mut set: JoinSet<Result<_, Report>> = JoinSet::new();
let repodata = Arc::new(repodata);
let channels = Arc::new(channels);
for (package_name, package_matchspec) in specs {
let repodata = Arc::clone(&repodata);
let channels = Arc::clone(&channels);
let channel_cli = channel_cli.clone();
set.spawn_blocking(move || {
// Filter repodata based on channels specific to the package (and from the CLI)
let specific_repodata = repodata.iter().filter_map(|((c, _), v)| {
if channel_cli.contains(c) || channels.get(&package_name).unwrap() == c {
Some(v)
} else {
None
}
});
let records = load_package_records(package_matchspec.clone(), specific_repodata)?;
Ok((package_name, package_matchspec, records))
});
}

// Upgrade each package when relevant
let mut upgraded = false;
for (package_name, package_matchspec) in specs {
let matchspec_has_version = package_matchspec.version.is_some();
let records = load_package_records(package_matchspec, &sparse_repodata)?;
let package_record = records
while let Some(data) = set.join_next().await {
let (package_name, package_matchspec, records) = data.into_diagnostic()??;
let toinstall_version = records
.iter()
.find(|r| r.package_record.name == package_name)
.map(|p| p.package_record.version.version().to_owned())
.ok_or_else(|| {
miette::miette!(
"Package {} not found in the specified channels",
package_name.as_normalized()
)
})?;
let toinstall_version = package_record.package_record.version.version().to_owned();
let installed_version = installed_versions
let installed_version = versions
.get(&package_name)
.expect("should have the installed version")
.to_owned();

// Perform upgrade if a specific version was requested
// OR if a more recent version is available
if matchspec_has_version || toinstall_version > installed_version {
if package_matchspec.version.is_some() || toinstall_version > installed_version {
let message = format!(
"{} v{} -> v{}",
package_name.as_normalized(),
Expand All @@ -124,13 +147,7 @@ pub(super) async fn upgrade_packages(
console::style("Updating").green(),
message
));
globally_install_package(
&package_name,
records,
authenticated_client.clone(),
platform,
)
.await?;
globally_install_package(&package_name, records, client.clone(), platform).await?;
pb.finish_with_message(format!("{} {}", console::style("Updated").green(), message));
upgraded = true;
}
Expand Down
Loading