From 6a8dfe0ba31661ccda6dcf4e086e1cf8ea5ee079 Mon Sep 17 00:00:00 2001 From: daimond113 <72147841+daimond113@users.noreply.github.com> Date: Wed, 1 Jan 2025 00:34:21 +0100 Subject: [PATCH] feat: switch to flat graph handling --- CHANGELOG.md | 3 + registry/src/endpoints/publish_version.rs | 2 +- registry/src/storage/fs.rs | 2 +- registry/src/storage/mod.rs | 2 +- registry/src/storage/s3.rs | 2 +- src/cli/commands/outdated.rs | 141 +++++----- src/cli/commands/patch.rs | 11 +- src/cli/commands/patch_commit.rs | 22 +- src/cli/commands/publish.rs | 3 +- src/cli/commands/run.rs | 54 ++-- src/cli/install.rs | 47 ++-- src/cli/mod.rs | 31 ++- src/download.rs | 38 +-- src/download_and_link.rs | 42 ++- src/lib.rs | 13 +- src/linking/mod.rs | 297 +++++++++++----------- src/lockfile.rs | 139 +++++++++- src/manifest/mod.rs | 4 +- src/patches.rs | 28 +- src/resolver.rs | 213 ++++++---------- src/source/{version_id.rs => ids.rs} | 73 +++++- src/source/mod.rs | 8 +- src/source/path/mod.rs | 2 +- src/source/wally/mod.rs | 2 +- src/source/workspace/mod.rs | 2 +- 25 files changed, 644 insertions(+), 537 deletions(-) rename src/source/{version_id.rs => ids.rs} (60%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c0675b..4f8990e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Inherit pesde-managed scripts from workspace root by @daimond113 - Allow using binaries from workspace root in member packages by @daimond113 +### Changed +- Change handling of graphs to a flat structure by @daimond113 + ### Removed - Remove old includes format compatibility by @daimond113 - Remove data redundacy for workspace package references by @daimond113 diff --git a/registry/src/endpoints/publish_version.rs b/registry/src/endpoints/publish_version.rs index 58c24b9..b36ad96 100644 --- a/registry/src/endpoints/publish_version.rs +++ b/registry/src/endpoints/publish_version.rs @@ -16,10 +16,10 @@ use pesde::{ manifest::Manifest, source::{ git_index::{read_file, root_tree, GitBasedSource}, + ids::VersionId, pesde::{DocEntry, DocEntryKind, IndexFile, IndexFileEntry, ScopeInfo, SCOPE_INFO_FILE}, specifiers::DependencySpecifiers, traits::RefreshOptions, - version_id::VersionId, IGNORED_DIRS, IGNORED_FILES, }, MANIFEST_FILE_NAME, diff --git a/registry/src/storage/fs.rs b/registry/src/storage/fs.rs index 1cc5857..cf49e8e 100644 --- a/registry/src/storage/fs.rs +++ b/registry/src/storage/fs.rs @@ -4,7 +4,7 @@ use actix_web::{ HttpResponse, }; use fs_err::tokio as fs; -use pesde::{names::PackageName, source::version_id::VersionId}; +use pesde::{names::PackageName, source::ids::VersionId}; use std::{ fmt::Display, path::{Path, PathBuf}, diff --git a/registry/src/storage/mod.rs b/registry/src/storage/mod.rs index ff05962..25f26c9 100644 --- a/registry/src/storage/mod.rs +++ b/registry/src/storage/mod.rs @@ -1,6 +1,6 @@ use crate::{benv, error::Error, make_reqwest}; use actix_web::HttpResponse; -use pesde::{names::PackageName, source::version_id::VersionId}; +use pesde::{names::PackageName, source::ids::VersionId}; use rusty_s3::{Bucket, Credentials, UrlStyle}; use std::fmt::Display; diff --git a/registry/src/storage/s3.rs b/registry/src/storage/s3.rs index 7cc081f..369b536 100644 --- a/registry/src/storage/s3.rs +++ b/registry/src/storage/s3.rs @@ -3,7 +3,7 @@ use crate::{ storage::StorageImpl, }; use actix_web::{http::header::LOCATION, HttpResponse}; -use pesde::{names::PackageName, source::version_id::VersionId}; +use pesde::{names::PackageName, source::ids::VersionId}; use reqwest::header::{CONTENT_ENCODING, CONTENT_TYPE}; use rusty_s3::{ actions::{GetObject, PutObject}, diff --git a/src/cli/commands/outdated.rs b/src/cli/commands/outdated.rs index 854af5b..97207da 100644 --- a/src/cli/commands/outdated.rs +++ b/src/cli/commands/outdated.rs @@ -4,7 +4,6 @@ use clap::Args; use futures::future::try_join_all; use pesde::{ source::{ - refs::PackageRefs, specifiers::DependencySpecifiers, traits::{PackageRef, PackageSource, RefreshOptions, ResolveOptions}, }, @@ -39,89 +38,77 @@ impl OutdatedCommand { let refreshed_sources = RefreshedSources::new(); - if try_join_all( - graph - .into_iter() - .flat_map(|(_, versions)| versions.into_iter()) - .map(|(current_version_id, node)| { - let project = project.clone(); - let refreshed_sources = refreshed_sources.clone(); - async move { - let Some((alias, mut specifier, _)) = node.node.direct else { - return Ok::(true); - }; + if try_join_all(graph.into_iter().map(|(current_id, node)| { + let project = project.clone(); + let refreshed_sources = refreshed_sources.clone(); + async move { + let Some((alias, mut specifier, _)) = node.node.direct else { + return Ok::(true); + }; - if matches!( - specifier, - DependencySpecifiers::Git(_) - | DependencySpecifiers::Workspace(_) - | DependencySpecifiers::Path(_) - ) { - return Ok(true); + if matches!( + specifier, + DependencySpecifiers::Git(_) + | DependencySpecifiers::Workspace(_) + | DependencySpecifiers::Path(_) + ) { + return Ok(true); + } + + let source = node.node.pkg_ref.source(); + refreshed_sources + .refresh( + &source, + &RefreshOptions { + project: project.clone(), + }, + ) + .await?; + + if !self.strict { + match &mut specifier { + DependencySpecifiers::Pesde(spec) => { + spec.version = VersionReq::STAR; } - - let source = node.node.pkg_ref.source(); - refreshed_sources - .refresh( - &source, - &RefreshOptions { - project: project.clone(), - }, - ) - .await?; - - if !self.strict { - match &mut specifier { - DependencySpecifiers::Pesde(spec) => { - spec.version = VersionReq::STAR; - } - #[cfg(feature = "wally-compat")] - DependencySpecifiers::Wally(spec) => { - spec.version = VersionReq::STAR; - } - DependencySpecifiers::Git(_) => {} - DependencySpecifiers::Workspace(_) => {} - DependencySpecifiers::Path(_) => {} - }; + #[cfg(feature = "wally-compat")] + DependencySpecifiers::Wally(spec) => { + spec.version = VersionReq::STAR; } + DependencySpecifiers::Git(_) => {} + DependencySpecifiers::Workspace(_) => {} + DependencySpecifiers::Path(_) => {} + }; + } - let version_id = source - .resolve( - &specifier, - &ResolveOptions { - project: project.clone(), - target: manifest_target_kind, - refreshed_sources: refreshed_sources.clone(), - }, - ) - .await - .context("failed to resolve package versions")? - .1 - .pop_last() - .map(|(v_id, _)| v_id) - .with_context(|| format!("no versions of {specifier} found"))?; + let version_id = source + .resolve( + &specifier, + &ResolveOptions { + project: project.clone(), + target: manifest_target_kind, + refreshed_sources: refreshed_sources.clone(), + }, + ) + .await + .context("failed to resolve package versions")? + .1 + .pop_last() + .map(|(v_id, _)| v_id) + .with_context(|| format!("no versions of {specifier} found"))?; - if version_id != current_version_id { - println!( - "{} {} ({alias}) {} -> {}", - match node.node.pkg_ref { - PackageRefs::Pesde(pkg_ref) => pkg_ref.name.to_string(), - #[cfg(feature = "wally-compat")] - PackageRefs::Wally(pkg_ref) => pkg_ref.name.to_string(), - _ => unreachable!(), - }, - current_version_id.target(), - current_version_id.version(), - version_id.version() - ); + if version_id != *current_id.version_id() { + println!( + "{} ({alias}) {} -> {version_id}", + current_id.name(), + current_id.version_id(), + ); - return Ok(false); - } + return Ok(false); + } - Ok(true) - } - }), - ) + Ok(true) + } + })) .await? .into_iter() .all(|b| b) diff --git a/src/cli/commands/patch.rs b/src/cli/commands/patch.rs index 25a2d37..0a8b858 100644 --- a/src/cli/commands/patch.rs +++ b/src/cli/commands/patch.rs @@ -29,12 +29,9 @@ impl PatchCommand { anyhow::bail!("outdated lockfile, please run the install command first") }; - let (name, version_id) = self.package.get(&graph)?; + let id = self.package.get(&graph)?; - let node = graph - .get(&name) - .and_then(|versions| versions.get(&version_id)) - .context("package not found in graph")?; + let node = graph.get(&id).context("package not found in graph")?; if matches!(node.node.pkg_ref, PackageRefs::Workspace(_)) { anyhow::bail!("cannot patch a workspace package") @@ -45,8 +42,8 @@ impl PatchCommand { let directory = project .data_dir() .join("patches") - .join(name.escaped()) - .join(version_id.escaped()) + .join(id.name().escaped()) + .join(id.version_id().escaped()) .join(chrono::Utc::now().timestamp().to_string()); fs::create_dir_all(&directory).await?; diff --git a/src/cli/commands/patch_commit.rs b/src/cli/commands/patch_commit.rs index 763462f..2a12acb 100644 --- a/src/cli/commands/patch_commit.rs +++ b/src/cli/commands/patch_commit.rs @@ -2,7 +2,12 @@ use crate::cli::up_to_date_lockfile; use anyhow::Context; use clap::Args; use fs_err::tokio as fs; -use pesde::{names::PackageNames, patches::create_patch, source::version_id::VersionId, Project}; +use pesde::{ + names::PackageNames, + patches::create_patch, + source::ids::{PackageId, VersionId}, + Project, +}; use std::{path::PathBuf, str::FromStr}; #[derive(Debug, Args)] @@ -20,7 +25,7 @@ impl PatchCommitCommand { anyhow::bail!("outdated lockfile, please run the install command first") }; - let (name, version_id) = ( + let id = PackageId::new( PackageNames::from_escaped( self.directory .parent() @@ -43,10 +48,7 @@ impl PatchCommitCommand { )?, ); - graph - .get(&name) - .and_then(|versions| versions.get(&version_id)) - .context("package not found in graph")?; + graph.get(&id).context("package not found in graph")?; let mut manifest = toml_edit::DocumentMut::from_str( &project @@ -66,7 +68,11 @@ impl PatchCommitCommand { .await .context("failed to create patches directory")?; - let patch_file_name = format!("{}-{}.patch", name.escaped(), version_id.escaped()); + let patch_file_name = format!( + "{}-{}.patch", + id.name().escaped(), + id.version_id().escaped() + ); let patch_file = patches_dir.join(&patch_file_name); if patch_file.exists() { @@ -78,7 +84,7 @@ impl PatchCommitCommand { .context("failed to write patch file")?; manifest["patches"].or_insert(toml_edit::Item::Table(toml_edit::Table::new())) - [&name.to_string()][&version_id.to_string()] = + [&id.name().to_string()][&id.version_id().to_string()] = toml_edit::value(format!("patches/{patch_file_name}")); project diff --git a/src/cli/commands/publish.rs b/src/cli/commands/publish.rs index 8069f0a..8971b45 100644 --- a/src/cli/commands/publish.rs +++ b/src/cli/commands/publish.rs @@ -114,8 +114,7 @@ impl PublishCommand { if lockfile .graph .values() - .flatten() - .filter_map(|(_, node)| node.node.direct.as_ref().map(|_| node)) + .filter_map(|node| node.node.direct.as_ref().map(|_| node)) .any(|node| { node.target.build_files().is_none() && !matches!(node.node.resolved_ty, DependencyType::Dev) diff --git a/src/cli/commands/run.rs b/src/cli/commands/run.rs index 47191d3..39dc30e 100644 --- a/src/cli/commands/run.rs +++ b/src/cli/commands/run.rs @@ -73,35 +73,39 @@ impl RunCommand { let pkg_name = PackageNames::Pesde(pkg_name); - for (version_id, node) in graph.get(&pkg_name).context("package not found in graph")? { - if node.node.direct.is_none() { - continue; - } + let mut versions = graph + .into_iter() + .filter(|(id, node)| *id.name() == pkg_name && node.node.direct.is_some()) + .collect::>(); - let Some(bin_path) = node.target.bin_path() else { - anyhow::bail!("package has no bin path"); - }; + let (id, node) = match versions.len() { + 0 => anyhow::bail!("package not found"), + 1 => versions.pop().unwrap(), + _ => anyhow::bail!("multiple versions found. use the package's alias instead."), + }; - let base_folder = project - .deser_manifest() - .await? - .target - .kind() - .packages_folder(version_id.target()); - let container_folder = node.node.container_folder( - &project - .package_dir() - .join(base_folder) - .join(PACKAGES_CONTAINER_NAME), - &pkg_name, - version_id.version(), - ); + let Some(bin_path) = node.target.bin_path() else { + anyhow::bail!("package has no bin path"); + }; - let path = bin_path.to_path(&container_folder); + let base_folder = project + .deser_manifest() + .await? + .target + .kind() + .packages_folder(id.version_id().target()); + let container_folder = node.node.container_folder( + &project + .package_dir() + .join(base_folder) + .join(PACKAGES_CONTAINER_NAME), + &id, + ); - run(&path, &path); - return Ok(()); - } + let path = bin_path.to_path(&container_folder); + + run(&path, &path); + return Ok(()); } if let Ok(manifest) = project.deser_manifest().await { diff --git a/src/cli/install.rs b/src/cli/install.rs index 3eee534..653c1fa 100644 --- a/src/cli/install.rs +++ b/src/cli/install.rs @@ -64,11 +64,10 @@ impl DownloadAndLinkHooks for InstallHooks { async fn on_bins_downloaded( &self, - downloaded_graph: &pesde::lockfile::DownloadedGraph, + downloaded_graph: &DownloadedGraph, ) -> Result<(), Self::Error> { let mut tasks = downloaded_graph .values() - .flat_map(|versions| versions.values()) .filter(|node| node.target.bin_path().is_some()) .filter_map(|node| node.node.direct.as_ref()) .map(|(alias, _, _)| alias) @@ -242,15 +241,7 @@ pub async fn install( lockfile .graph .into_iter() - .map(|(name, versions)| { - ( - name, - versions - .into_iter() - .map(|(version, node)| (version, node.node)) - .collect(), - ) - }) + .map(|(id, node)| (id, node.node)) .collect() }); @@ -345,42 +336,38 @@ pub fn print_package_diff(prefix: &str, old_graph: DependencyGraph, new_graph: D let mut new_pkg_map = BTreeMap::new(); let mut new_direct_pkg_map = BTreeMap::new(); - for (name, versions) in &old_graph { - for (version, node) in versions { - old_pkg_map.insert((name.clone(), version), node); - if node.direct.is_some() { - old_direct_pkg_map.insert((name.clone(), version), node); - } + for (id, node) in &old_graph { + old_pkg_map.insert(id, node); + if node.direct.is_some() { + old_direct_pkg_map.insert(id, node); } } - for (name, versions) in &new_graph { - for (version, node) in versions { - new_pkg_map.insert((name.clone(), version), &node.node); - if node.node.direct.is_some() { - new_direct_pkg_map.insert((name.clone(), version), &node.node); - } + for (id, node) in &new_graph { + new_pkg_map.insert(id, &node.node); + if node.node.direct.is_some() { + new_direct_pkg_map.insert(id, &node.node); } } let added_pkgs = new_pkg_map .iter() - .filter(|(key, _)| !old_pkg_map.contains_key(key)) + .filter(|(key, _)| !old_pkg_map.contains_key(*key)) .map(|(key, &node)| (key, node)) .collect::>(); let removed_pkgs = old_pkg_map .iter() - .filter(|(key, _)| !new_pkg_map.contains_key(key)) + .filter(|(key, _)| !new_pkg_map.contains_key(*key)) .map(|(key, &node)| (key, node)) .collect::>(); let added_direct_pkgs = new_direct_pkg_map .iter() - .filter(|(key, _)| !old_direct_pkg_map.contains_key(key)) + .filter(|(key, _)| !old_direct_pkg_map.contains_key(*key)) .map(|(key, &node)| (key, node)) .collect::>(); let removed_direct_pkgs = old_direct_pkg_map .iter() - .filter(|(key, _)| !new_direct_pkg_map.contains_key(key)) + .filter(|(key, _)| !new_direct_pkg_map.contains_key(*key)) .map(|(key, &node)| (key, node)) .collect::>(); @@ -448,12 +435,12 @@ pub fn print_package_diff(prefix: &str, old_graph: DependencyGraph, new_graph: D }; println!("{}", format!("{ty_name}:").yellow().bold()); - for ((name, version), added) in set { + for (id, added) in set { println!( "{} {} {}", if added { "+".green() } else { "-".red() }, - name, - version.to_string().dimmed() + id.name(), + id.version_id().to_string().dimmed() ); } } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index e279106..32aae74 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -11,7 +11,8 @@ use pesde::{ }, names::{PackageName, PackageNames}, source::{ - specifiers::DependencySpecifiers, version_id::VersionId, + ids::{PackageId, VersionId}, + specifiers::DependencySpecifiers, workspace::specifier::VersionTypeOrReq, }, Project, @@ -116,7 +117,6 @@ pub async fn up_to_date_lockfile(project: &Project) -> anyhow::Result, E: Into, N: FromStr, F: Into anyhow::Result<(PackageNames, VersionId)> { + fn get(self, graph: &pesde::lockfile::DownloadedGraph) -> anyhow::Result { let version_id = match self.1 { Some(version) => version, None => { - let versions = graph.get(&self.0).context("package not found in graph")?; - if versions.len() == 1 { - let version = versions.keys().next().unwrap().clone(); - tracing::debug!("only one version found, using {version}"); - version - } else { - anyhow::bail!( + let versions = graph + .keys() + .filter(|id| *id.name() == self.0) + .collect::>(); + + match versions.len() { + 0 => anyhow::bail!("package not found"), + 1 => versions[0].version_id().clone(), + _ => anyhow::bail!( "multiple versions found, please specify one of: {}", versions - .keys() + .iter() .map(|v| v.to_string()) .collect::>() .join(", ") - ); + ), } } }; - Ok((self.0, version_id)) + Ok(PackageId::new(self.0, version_id)) } } diff --git a/src/download.rs b/src/download.rs index f1d1291..5166e16 100644 --- a/src/download.rs +++ b/src/download.rs @@ -1,11 +1,10 @@ use crate::{ lockfile::{DependencyGraph, DownloadedDependencyGraphNode}, manifest::DependencyType, - names::PackageNames, reporters::{DownloadProgressReporter, DownloadsReporter}, source::{ + ids::PackageId, traits::{DownloadOptions, PackageRef, PackageSource, RefreshOptions}, - version_id::VersionId, }, Project, RefreshedSources, PACKAGES_CONTAINER_NAME, }; @@ -112,10 +111,7 @@ impl Project { options: DownloadGraphOptions, ) -> Result< impl Stream< - Item = Result< - (DownloadedDependencyGraphNode, PackageNames, VersionId), - errors::DownloadGraphError, - >, + Item = Result<(DownloadedDependencyGraphNode, PackageId), errors::DownloadGraphError>, >, errors::DownloadGraphError, > @@ -139,19 +135,10 @@ impl Project { let mut tasks = graph .iter() - .flat_map(|(name, versions)| { - versions - .iter() - .map(|(version_id, node)| (name.clone(), version_id.clone(), node.clone())) - }) // we need to download pesde packages first, since scripts (for target finding for example) can depend on them - .filter(|(_, _, node)| node.pkg_ref.like_wally() == wally) - .map(|(name, version_id, node)| { - let span = tracing::info_span!( - "download", - name = name.to_string(), - version_id = version_id.to_string() - ); + .filter(|(_, node)| node.pkg_ref.like_wally() == wally) + .map(|(package_id, node)| { + let span = tracing::info_span!("download", package_id = package_id.to_string(),); let project = self.clone(); let reqwest = reqwest.clone(); @@ -159,12 +146,13 @@ impl Project { let refreshed_sources = refreshed_sources.clone(); let package_dir = project.package_dir().to_path_buf(); let semaphore = semaphore.clone(); + let package_id = package_id.clone(); + let node = node.clone(); async move { - let display_name = format!("{name}@{version_id}"); let progress_reporter = reporter .as_deref() - .map(|reporter| reporter.report_download(&display_name)); + .map(|reporter| reporter.report_download(&package_id.to_string())); let _permit = semaphore.acquire().await; @@ -184,10 +172,12 @@ impl Project { let container_folder = node.container_folder( &package_dir - .join(manifest_target_kind.packages_folder(version_id.target())) + .join( + manifest_target_kind + .packages_folder(package_id.version_id().target()), + ) .join(PACKAGES_CONTAINER_NAME), - &name, - version_id.version(), + &package_id, ); fs::create_dir_all(&container_folder).await?; @@ -234,7 +224,7 @@ impl Project { } let downloaded_node = DownloadedDependencyGraphNode { node, target }; - Ok((downloaded_node, name, version_id)) + Ok((downloaded_node, package_id)) } .instrument(span) }) diff --git a/src/download_and_link.rs b/src/download_and_link.rs index 66dba63..7226108 100644 --- a/src/download_and_link.rs +++ b/src/download_and_link.rs @@ -15,24 +15,18 @@ use std::{ use tracing::{instrument, Instrument}; /// Filters a graph to only include production dependencies, if `prod` is `true` -pub fn filter_graph(graph: &DownloadedGraph, prod: bool) -> DownloadedGraph { +pub fn filter_graph(graph: &DownloadedGraph, prod: bool) -> Arc { if !prod { - return graph.clone(); + return Arc::new(graph.clone()); } - graph - .iter() - .map(|(name, versions)| { - ( - name.clone(), - versions - .iter() - .filter(|(_, node)| node.node.resolved_ty != DependencyType::Dev) - .map(|(v_id, node)| (v_id.clone(), node.clone())) - .collect(), - ) - }) - .collect() + Arc::new( + graph + .iter() + .filter(|(_, node)| node.node.resolved_ty != DependencyType::Dev) + .map(|(id, node)| (id.clone(), node.clone())) + .collect(), + ) } /// Receiver for dependencies downloaded and linked @@ -206,11 +200,8 @@ impl Project { self.download_graph(&graph, download_graph_options.clone()) .instrument(tracing::debug_span!("download (pesde)")) .await? - .try_for_each(|(downloaded_node, name, version_id)| { - downloaded_graph - .entry(name) - .or_default() - .insert(version_id, downloaded_node); + .try_for_each(|(downloaded_node, id)| { + downloaded_graph.insert(id, downloaded_node); future::ready(Ok(())) }) @@ -218,7 +209,7 @@ impl Project { // step 2. link pesde dependencies. do so without types if write { - self.link_dependencies(&filter_graph(&downloaded_graph, prod), false) + self.link_dependencies(filter_graph(&downloaded_graph, prod), false) .instrument(tracing::debug_span!("link (pesde)")) .await?; } @@ -239,11 +230,8 @@ impl Project { self.download_graph(&graph, download_graph_options.clone().wally(true)) .instrument(tracing::debug_span!("download (wally)")) .await? - .try_for_each(|(downloaded_node, name, version_id)| { - downloaded_graph - .entry(name) - .or_default() - .insert(version_id, downloaded_node); + .try_for_each(|(downloaded_node, id)| { + downloaded_graph.insert(id, downloaded_node); future::ready(Ok(())) }) @@ -251,7 +239,7 @@ impl Project { // step 4. link ALL dependencies. do so with types if write { - self.link_dependencies(&filter_graph(&downloaded_graph, prod), true) + self.link_dependencies(filter_graph(&downloaded_graph, prod), true) .instrument(tracing::debug_span!("link (all)")) .await?; } diff --git a/src/lib.rs b/src/lib.rs index 08bb423..72d5c48 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -211,7 +211,18 @@ impl Project { #[instrument(skip(self), ret(level = "trace"), level = "debug")] pub async fn deser_lockfile(&self) -> Result { let string = fs::read_to_string(self.package_dir().join(LOCKFILE_FILE_NAME)).await?; - Ok(toml::from_str(&string)?) + Ok(match toml::from_str(&string) { + Ok(lockfile) => lockfile, + Err(e) => { + #[allow(deprecated)] + let Ok(old_lockfile) = toml::from_str::(&string) else { + return Err(errors::LockfileReadError::Serde(e)); + }; + + #[allow(deprecated)] + old_lockfile.to_new() + } + }) } /// Write the lockfile diff --git a/src/linking/mod.rs b/src/linking/mod.rs index a9f9d72..78185fc 100644 --- a/src/linking/mod.rs +++ b/src/linking/mod.rs @@ -2,24 +2,22 @@ use crate::{ linking::generator::get_file_types, lockfile::{DownloadedDependencyGraphNode, DownloadedGraph}, manifest::Manifest, - names::PackageNames, scripts::{execute_script, ExecuteScriptHooks, ScriptName}, source::{ fs::{cas_path, store_in_cas}, + ids::PackageId, traits::PackageRef, - version_id::VersionId, }, Project, LINK_LIB_NO_FILE_FOUND, PACKAGES_CONTAINER_NAME, SCRIPTS_LINK_FOLDER, }; use fs_err::tokio as fs; -use futures::future::try_join_all; use std::{ collections::HashMap, ffi::OsStr, path::{Path, PathBuf}, sync::Arc, }; -use tokio::task::spawn_blocking; +use tokio::task::{spawn_blocking, JoinSet}; use tracing::{instrument, Instrument}; /// Generates linking modules for a project @@ -59,7 +57,7 @@ impl Project { #[instrument(skip(self, graph), level = "debug")] pub async fn link_dependencies( &self, - graph: &DownloadedGraph, + graph: Arc, with_types: bool, ) -> Result<(), errors::LinkingError> { let manifest = self.deser_manifest().await?; @@ -68,7 +66,7 @@ impl Project { // step 1. link all non-wally packages (and their dependencies) temporarily without types // we do this separately to allow the required tools for the scripts to be installed - self.link(graph, &manifest, &Arc::new(Default::default()), false) + self.link(&graph, &manifest, &Arc::new(Default::default()), false) .await?; if !with_types { @@ -76,83 +74,86 @@ impl Project { } // step 2. extract the types from libraries, prepare Roblox packages for syncing - let package_types = try_join_all(graph.iter().map(|(name, versions)| async move { - Ok::<_, errors::LinkingError>(( - name, - try_join_all(versions.iter().map(|(version_id, node)| { - async move { - let Some(lib_file) = node.target.lib_path() else { - return Ok((version_id, vec![])); - }; + let mut tasks = graph + .iter() + .map(|(package_id, node)| { + let span = + tracing::info_span!("extract types", package_id = package_id.to_string(),); - let container_folder = node.node.container_folder( - &self - .package_dir() - .join(manifest_target_kind.packages_folder(version_id.target())) - .join(PACKAGES_CONTAINER_NAME), - name, - version_id.version(), - ); + let package_id = package_id.clone(); + let node = node.clone(); + let project = self.clone(); - let types = if lib_file.as_str() != LINK_LIB_NO_FILE_FOUND { - let lib_file = lib_file.to_path(&container_folder); + async move { + let Some(lib_file) = node.target.lib_path() else { + return Ok((package_id, vec![])); + }; - let contents = match fs::read_to_string(&lib_file).await { - Ok(contents) => contents, - Err(e) if e.kind() == std::io::ErrorKind::NotFound => { - return Err(errors::LinkingError::LibFileNotFound( - lib_file.display().to_string(), - )); - } - Err(e) => return Err(e.into()), - }; - - let types = spawn_blocking(move || get_file_types(&contents)) - .await - .unwrap(); - - tracing::debug!("contains {} exported types", types.len()); - - types - } else { - vec![] - }; - - if let Some(build_files) = Some(&node.target) - .filter(|_| !node.node.pkg_ref.like_wally()) - .and_then(|t| t.build_files()) - { - execute_script( - ScriptName::RobloxSyncConfigGenerator, - self, - LinkingExecuteScriptHooks, - std::iter::once(container_folder.as_os_str()) - .chain(build_files.iter().map(OsStr::new)), - false, + let container_folder = node.node.container_folder( + &project + .package_dir() + .join( + manifest_target_kind + .packages_folder(package_id.version_id().target()), ) - .await - .map_err(errors::LinkingError::ExecuteScript)?; - } + .join(PACKAGES_CONTAINER_NAME), + &package_id, + ); - Ok((version_id, types)) + let types = if lib_file.as_str() != LINK_LIB_NO_FILE_FOUND { + let lib_file = lib_file.to_path(&container_folder); + + let contents = match fs::read_to_string(&lib_file).await { + Ok(contents) => contents, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + return Err(errors::LinkingError::LibFileNotFound( + lib_file.display().to_string(), + )); + } + Err(e) => return Err(e.into()), + }; + + let types = spawn_blocking(move || get_file_types(&contents)) + .await + .unwrap(); + + tracing::debug!("contains {} exported types", types.len()); + + types + } else { + vec![] + }; + + if let Some(build_files) = Some(&node.target) + .filter(|_| !node.node.pkg_ref.like_wally()) + .and_then(|t| t.build_files()) + { + execute_script( + ScriptName::RobloxSyncConfigGenerator, + &project, + LinkingExecuteScriptHooks, + std::iter::once(container_folder.as_os_str()) + .chain(build_files.iter().map(OsStr::new)), + false, + ) + .await + .map_err(errors::LinkingError::ExecuteScript)?; } - .instrument(tracing::info_span!( - "extract types", - name = name.to_string(), - version_id = version_id.to_string() - )) - })) - .await? - .into_iter() - .collect::>(), - )) - })) - .await? - .into_iter() - .collect::>(); + + Ok((package_id, types)) + } + .instrument(span) + }) + .collect::>(); + + let mut package_types = HashMap::new(); + while let Some(task) = tasks.join_next().await { + let (version_id, types) = task.unwrap()?; + package_types.insert(version_id, types); + } // step 3. link all packages (and their dependencies), this time with types - self.link(graph, &manifest, &Arc::new(package_types), true) + self.link(&graph, &manifest, &Arc::new(package_types), true) .await } @@ -164,10 +165,9 @@ impl Project { root_container_folder: &Path, relative_container_folder: &Path, node: &DownloadedDependencyGraphNode, - name: &PackageNames, - version_id: &VersionId, + package_id: &PackageId, alias: &str, - package_types: &HashMap<&PackageNames, HashMap<&VersionId, Vec>>, + package_types: &HashMap>, manifest: &Manifest, ) -> Result<(), errors::LinkingError> { static NO_TYPES: Vec = Vec::new(); @@ -184,10 +184,7 @@ impl Project { relative_container_folder, manifest, )?, - package_types - .get(name) - .and_then(|v| v.get(version_id)) - .unwrap_or(&NO_TYPES), + package_types.get(package_id).unwrap_or(&NO_TYPES), ); write_cas( @@ -239,68 +236,65 @@ impl Project { async fn link( &self, - graph: &DownloadedGraph, + graph: &Arc, manifest: &Arc, - package_types: &Arc>>>, + package_types: &Arc>>, is_complete: bool, ) -> Result<(), errors::LinkingError> { - try_join_all(graph.iter().flat_map(|(name, versions)| { - versions.iter().map(|(version_id, node)| { - let name = name.clone(); + let mut tasks = graph + .iter() + .map(|(package_id, node)| { + let graph = graph.clone(); let manifest = manifest.clone(); let package_types = package_types.clone(); - let span = tracing::info_span!( - "link", - name = name.to_string(), - version_id = version_id.to_string() - ); + let span = tracing::info_span!("link", package_id = package_id.to_string()); + let package_id = package_id.clone(); + let node = node.clone(); + let project = self.clone(); async move { let (node_container_folder, node_packages_folder) = { let base_folder = create_and_canonicalize( - self.package_dir() - .join(manifest.target.kind().packages_folder(version_id.target())), + project.package_dir().join( + manifest + .target + .kind() + .packages_folder(package_id.version_id().target()), + ), ) .await?; let packages_container_folder = base_folder.join(PACKAGES_CONTAINER_NAME); - let container_folder = node.node.container_folder( - &packages_container_folder, - &name, - version_id.version(), - ); + let container_folder = node + .node + .container_folder(&packages_container_folder, &package_id); if let Some((alias, _, _)) = &node.node.direct { - self.link_files( - &base_folder, - &container_folder, - &base_folder, - container_folder.strip_prefix(&base_folder).unwrap(), - node, - &name, - version_id, - alias, - &package_types, - &manifest, - ) - .await?; + project + .link_files( + &base_folder, + &container_folder, + &base_folder, + container_folder.strip_prefix(&base_folder).unwrap(), + &node, + &package_id, + alias, + &package_types, + &manifest, + ) + .await?; } (container_folder, base_folder) }; - for (dependency_name, (dependency_version_id, dependency_alias)) in - &node.node.dependencies - { - let Some(dependency_node) = graph - .get(dependency_name) - .and_then(|v| v.get(dependency_version_id)) - else { + for (dependency_id, dependency_alias) in &node.node.dependencies { + let Some(dependency_node) = graph.get(dependency_id) else { if is_complete { return Err(errors::LinkingError::DependencyNotFound( - format!("{dependency_name}@{dependency_version_id}"), - format!("{name}@{version_id}"), + dependency_id.to_string(), + package_id.to_string(), )); } @@ -308,51 +302,54 @@ impl Project { }; let base_folder = create_and_canonicalize( - self.package_dir().join( - version_id + project.package_dir().join( + package_id + .version_id() .target() - .packages_folder(dependency_version_id.target()), + .packages_folder(dependency_id.version_id().target()), ), ) .await?; let packages_container_folder = base_folder.join(PACKAGES_CONTAINER_NAME); - let container_folder = dependency_node.node.container_folder( - &packages_container_folder, - dependency_name, - dependency_version_id.version(), - ); + let container_folder = dependency_node + .node + .container_folder(&packages_container_folder, dependency_id); - let linker_folder = create_and_canonicalize( - node_container_folder.join( - node.node - .base_folder(version_id, dependency_node.target.kind()), + let linker_folder = create_and_canonicalize(node_container_folder.join( + node.node.base_folder( + package_id.version_id(), + dependency_node.target.kind(), ), - ) + )) .await?; - self.link_files( - &linker_folder, - &container_folder, - &node_packages_folder, - container_folder.strip_prefix(&base_folder).unwrap(), - dependency_node, - dependency_name, - dependency_version_id, - dependency_alias, - &package_types, - &manifest, - ) - .await?; + project + .link_files( + &linker_folder, + &container_folder, + &node_packages_folder, + container_folder.strip_prefix(&base_folder).unwrap(), + dependency_node, + dependency_id, + dependency_alias, + &package_types, + &manifest, + ) + .await?; } Ok(()) } .instrument(span) }) - })) - .await - .map(|_| ()) + .collect::>(); + + while let Some(task) = tasks.join_next().await { + task.unwrap()?; + } + + Ok(()) } } diff --git a/src/lockfile.rs b/src/lockfile.rs index ca1af8c..9e431e4 100644 --- a/src/lockfile.rs +++ b/src/lockfile.rs @@ -1,13 +1,16 @@ +#![allow(deprecated)] use crate::{ manifest::{ overrides::OverrideKey, target::{Target, TargetKind}, DependencyType, }, - names::{PackageName, PackageNames}, + names::PackageName, source::{ - refs::PackageRefs, specifiers::DependencySpecifiers, traits::PackageRef, - version_id::VersionId, + ids::{PackageId, VersionId}, + refs::PackageRefs, + specifiers::DependencySpecifiers, + traits::PackageRef, }, }; use relative_path::RelativePathBuf; @@ -19,7 +22,7 @@ use std::{ }; /// A graph of dependencies -pub type Graph = BTreeMap>; +pub type Graph = BTreeMap; /// A dependency graph node #[derive(Serialize, Deserialize, Debug, Clone)] @@ -29,7 +32,7 @@ pub struct DependencyGraphNode { pub direct: Option<(String, DependencySpecifiers, DependencyType)>, /// The dependencies of the package #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] - pub dependencies: BTreeMap, + pub dependencies: BTreeMap, /// The resolved (transformed, for example Peer -> Standard) type of the dependency pub resolved_ty: DependencyType, /// Whether the resolved type should be Peer if this isn't depended on @@ -49,18 +52,15 @@ impl DependencyGraphNode { } /// Returns the folder to store the contents of the package in - pub fn container_folder>( - &self, - path: &P, - name: &PackageNames, - version: &Version, - ) -> PathBuf { + pub fn container_folder>(&self, path: &P, package_id: &PackageId) -> PathBuf { + let (name, version) = package_id.parts(); + if self.pkg_ref.like_wally() { return path .as_ref() .join(format!( "{}_{}@{}", - name.as_str().0, + package_id.name().as_str().0, name.as_str().1, version )) @@ -111,3 +111,118 @@ pub struct Lockfile { #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub graph: DownloadedGraph, } + +/// Old lockfile stuff. Will be removed in a future version. +#[deprecated( + note = "Intended to be used to migrate old lockfiles to the new format. Will be removed in a future version." +)] +pub mod old { + use crate::{ + manifest::{ + overrides::OverrideKey, + target::{Target, TargetKind}, + DependencyType, + }, + names::{PackageName, PackageNames}, + source::{ + ids::{PackageId, VersionId}, + refs::PackageRefs, + specifiers::DependencySpecifiers, + }, + }; + use relative_path::RelativePathBuf; + use semver::Version; + use serde::{Deserialize, Serialize}; + use std::collections::BTreeMap; + + /// An old dependency graph node + #[derive(Serialize, Deserialize, Debug, Clone)] + pub struct DependencyGraphNodeOld { + /// The alias, specifier, and original (as in the manifest) type for the dependency, if it is a direct dependency (i.e. used by the current project) + #[serde(default, skip_serializing_if = "Option::is_none")] + pub direct: Option<(String, DependencySpecifiers, DependencyType)>, + /// The dependencies of the package + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub dependencies: BTreeMap, + /// The resolved (transformed, for example Peer -> Standard) type of the dependency + pub resolved_ty: DependencyType, + /// Whether the resolved type should be Peer if this isn't depended on + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub is_peer: bool, + /// The package reference + pub pkg_ref: PackageRefs, + } + + /// A downloaded dependency graph node, i.e. a `DependencyGraphNode` with a `Target` + #[derive(Serialize, Deserialize, Debug, Clone)] + pub struct DownloadedDependencyGraphNodeOld { + /// The target of the package + pub target: Target, + /// The node + #[serde(flatten)] + pub node: DependencyGraphNodeOld, + } + + /// An old version of a lockfile + #[derive(Serialize, Deserialize, Debug, Clone)] + pub struct LockfileOld { + /// The name of the package + pub name: PackageName, + /// The version of the package + pub version: Version, + /// The target of the package + pub target: TargetKind, + /// The overrides of the package + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub overrides: BTreeMap, + + /// The workspace members + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub workspace: BTreeMap>, + + /// The graph of dependencies + #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] + pub graph: BTreeMap>, + } + + impl LockfileOld { + /// Converts this lockfile to a new lockfile + pub fn to_new(self) -> super::Lockfile { + super::Lockfile { + name: self.name, + version: self.version, + target: self.target, + overrides: self.overrides, + workspace: self.workspace, + graph: self + .graph + .into_iter() + .flat_map(|(name, versions)| { + versions.into_iter().map(move |(version, node)| { + ( + PackageId(name.clone(), version), + super::DownloadedDependencyGraphNode { + target: node.target, + node: super::DependencyGraphNode { + direct: node.node.direct, + dependencies: node + .node + .dependencies + .into_iter() + .map(|(name, (version, alias))| { + (PackageId(name, version), alias) + }) + .collect(), + resolved_ty: node.node.resolved_ty, + is_peer: node.node.is_peer, + pkg_ref: node.node.pkg_ref, + }, + }, + ) + }) + }) + .collect(), + } + } + } +} diff --git a/src/manifest/mod.rs b/src/manifest/mod.rs index a2373b1..9efe6ad 100644 --- a/src/manifest/mod.rs +++ b/src/manifest/mod.rs @@ -78,12 +78,12 @@ pub struct Manifest { #[cfg_attr( feature = "schema", schemars( - with = "BTreeMap>" + with = "BTreeMap>" ) )] pub patches: BTreeMap< crate::names::PackageNames, - BTreeMap, + BTreeMap, >, #[serde(default, skip_serializing)] /// Which version of the pesde CLI this package uses diff --git a/src/patches.rs b/src/patches.rs index 4a7a091..995deba 100644 --- a/src/patches.rs +++ b/src/patches.rs @@ -1,6 +1,7 @@ use crate::{ lockfile::DownloadedGraph, reporters::{PatchProgressReporter, PatchesReporter}, + source::ids::PackageId, Project, MANIFEST_FILE_NAME, PACKAGES_CONTAINER_NAME, }; use fs_err::tokio as fs; @@ -93,12 +94,10 @@ impl Project { for (version_id, patch_path) in versions { let patch_path = patch_path.to_path(self.package_dir()); - let Some(node) = graph - .get(&name) - .and_then(|versions| versions.get(&version_id)) - else { + let package_id = PackageId::new(name.clone(), version_id); + let Some(node) = graph.get(&package_id) else { tracing::warn!( - "patch for {name}@{version_id} not applied because it is not in the graph" + "patch for {package_id} not applied because it is not in the graph" ); continue; }; @@ -106,25 +105,24 @@ impl Project { let container_folder = node.node.container_folder( &self .package_dir() - .join(manifest.target.kind().packages_folder(version_id.target())) + .join( + manifest + .target + .kind() + .packages_folder(package_id.version_id().target()), + ) .join(PACKAGES_CONTAINER_NAME), - &name, - version_id.version(), + &package_id, ); let reporter = reporter.clone(); - let span = tracing::info_span!( - "apply patch", - name = name.to_string(), - version_id = version_id.to_string() - ); - let display_name = format!("{name}@{version_id}"); + let span = tracing::info_span!("apply patch", package_id = package_id.to_string(),); tasks.spawn( async move { tracing::debug!("applying patch"); - let progress_reporter = reporter.report_patch(&display_name); + let progress_reporter = reporter.report_patch(&package_id.to_string()); let patch = fs::read(&patch_path) .await diff --git a/src/resolver.rs b/src/resolver.rs index 45631ad..4f4bbcd 100644 --- a/src/resolver.rs +++ b/src/resolver.rs @@ -1,12 +1,11 @@ use crate::{ lockfile::{DependencyGraph, DependencyGraphNode}, manifest::{overrides::OverrideSpecifier, DependencyType}, - names::PackageNames, source::{ + ids::PackageId, pesde::PesdePackageSource, specifiers::DependencySpecifiers, traits::{PackageRef, PackageSource, RefreshOptions, ResolveOptions}, - version_id::VersionId, PackageSources, }, Project, RefreshedSources, DEFAULT_INDEX_NAME, @@ -16,22 +15,17 @@ use tracing::{instrument, Instrument}; fn insert_node( graph: &mut DependencyGraph, - name: &PackageNames, - version: &VersionId, + package_id: &PackageId, mut node: DependencyGraphNode, is_top_level: bool, ) { if !is_top_level && node.direct.take().is_some() { tracing::debug!( - "tried to insert {name}@{version} as direct dependency from a non top-level context", + "tried to insert {package_id} as direct dependency from a non top-level context", ); } - match graph - .entry(name.clone()) - .or_default() - .entry(version.clone()) - { + match graph.entry(package_id.clone()) { Entry::Vacant(entry) => { entry.insert(node); } @@ -40,7 +34,7 @@ fn insert_node( match (¤t_node.direct, &node.direct) { (Some(_), Some(_)) => { - tracing::warn!("duplicate direct dependency for {name}@{version}"); + tracing::warn!("duplicate direct dependency for {package_id}"); } (None, Some(_)) => { @@ -85,83 +79,67 @@ impl Project { let mut graph = DependencyGraph::default(); if let Some(previous_graph) = previous_graph { - for (name, versions) in previous_graph { - for (version, node) in versions { - let Some((old_alias, specifier, source_ty)) = &node.direct else { - // this is not a direct dependency, will be added if it's still being used later - continue; - }; + for (package_id, node) in previous_graph { + let Some((old_alias, specifier, source_ty)) = &node.direct else { + // this is not a direct dependency, will be added if it's still being used later + continue; + }; - if matches!(specifier, DependencySpecifiers::Workspace(_)) { - // workspace dependencies must always be resolved brand new - continue; - } + if matches!(specifier, DependencySpecifiers::Workspace(_)) { + // workspace dependencies must always be resolved brand new + continue; + } - let Some(alias) = all_specifiers.remove(&(specifier.clone(), *source_ty)) - else { - tracing::debug!( - "dependency {name}@{version} (old alias {old_alias}) from old dependency graph is no longer in the manifest", + let Some(alias) = all_specifiers.remove(&(specifier.clone(), *source_ty)) else { + tracing::debug!( + "dependency {package_id} (old alias {old_alias}) from old dependency graph is no longer in the manifest", ); - continue; - }; + continue; + }; - let span = tracing::info_span!("resolve from old graph", alias); - let _guard = span.enter(); + let span = tracing::info_span!("resolve from old graph", alias); + let _guard = span.enter(); - tracing::debug!("resolved {}@{} from old dependency graph", name, version); - insert_node( - &mut graph, - name, - version, - DependencyGraphNode { - direct: Some((alias.clone(), specifier.clone(), *source_ty)), - ..node.clone() - }, - true, - ); + tracing::debug!("resolved {package_id} from old dependency graph"); + insert_node( + &mut graph, + package_id, + DependencyGraphNode { + direct: Some((alias.clone(), specifier.clone(), *source_ty)), + ..node.clone() + }, + true, + ); - let mut queue = node - .dependencies - .iter() - .map(|(name, (version, dep_alias))| { - ( - name, - version, - vec![alias.to_string(), dep_alias.to_string()], - ) - }) - .collect::>(); + let mut queue = node + .dependencies + .iter() + .map(|(id, dep_alias)| (id, vec![alias.to_string(), dep_alias.to_string()])) + .collect::>(); - while let Some((dep_name, dep_version, path)) = queue.pop_front() { - let inner_span = - tracing::info_span!("resolve dependency", path = path.join(">")); - let _inner_guard = inner_span.enter(); - if let Some(dep_node) = previous_graph - .get(dep_name) - .and_then(|v| v.get(dep_version)) - { - tracing::debug!("resolved sub-dependency {dep_name}@{dep_version}"); - insert_node(&mut graph, dep_name, dep_version, dep_node.clone(), false); + while let Some((dep_id, path)) = queue.pop_front() { + let inner_span = + tracing::info_span!("resolve dependency", path = path.join(">")); + let _inner_guard = inner_span.enter(); + if let Some(dep_node) = previous_graph.get(dep_id) { + tracing::debug!("resolved sub-dependency {dep_id}"); + insert_node(&mut graph, dep_id, dep_node.clone(), false); - dep_node - .dependencies - .iter() - .map(|(name, (version, alias))| { - ( - name, - version, - path.iter() - .cloned() - .chain(std::iter::once(alias.to_string())) - .collect(), - ) - }) - .for_each(|dep| queue.push_back(dep)); - } else { - tracing::warn!( - "dependency {dep_name}@{dep_version} not found in previous graph" - ); - } + dep_node + .dependencies + .iter() + .map(|(id, alias)| { + ( + id, + path.iter() + .cloned() + .chain(std::iter::once(alias.to_string())) + .collect(), + ) + }) + .for_each(|dep| queue.push_back(dep)); + } else { + tracing::warn!("dependency {dep_id} not found in previous graph"); } } } @@ -173,7 +151,7 @@ impl Project { ( spec, ty, - None::<(PackageNames, VersionId)>, + None::, vec![alias], false, manifest.target.kind(), @@ -260,17 +238,12 @@ impl Project { .await .map_err(|e| Box::new(e.into()))?; - let Some(target_version_id) = graph - .get(&name) - .and_then(|versions| { - versions - .keys() - // only consider versions that are compatible with the specifier - .filter(|ver| resolved.contains_key(ver)) - .max() - }) - .or_else(|| resolved.last_key_value().map(|(ver, _)| ver)) + let Some(package_id) = graph + .keys() + .filter(|id| *id.name() == name && resolved.contains_key(id.version_id())) + .max() .cloned() + .or_else(|| resolved.last_key_value().map(|(ver, _)| PackageId::new(name, ver.clone()))) else { return Err(Box::new(errors::DependencyGraphError::NoMatchingVersion( format!("{specifier} ({target})"), @@ -284,33 +257,22 @@ impl Project { ty }; - if let Some((dependant_name, dependant_version_id)) = dependant { + if let Some(dependant_id) = dependant { graph - .get_mut(&dependant_name) - .and_then(|versions| versions.get_mut(&dependant_version_id)) - .and_then(|node| { - node.dependencies - .insert(name.clone(), (target_version_id.clone(), alias.clone())) - }); + .get_mut(&dependant_id) + .expect("dependant package not found in graph") + .dependencies + .insert(package_id.clone(), alias.clone()); } - let pkg_ref = &resolved[&target_version_id]; + let pkg_ref = &resolved[package_id.version_id()]; - if let Some(already_resolved) = graph - .get_mut(&name) - .and_then(|versions| versions.get_mut(&target_version_id)) - { - tracing::debug!( - "{}@{} already resolved", - name, - target_version_id - ); + if let Some(already_resolved) = graph.get_mut(&package_id) { + tracing::debug!("{package_id} already resolved"); - if std::mem::discriminant(&already_resolved.pkg_ref) - != std::mem::discriminant(pkg_ref) - { + if std::mem::discriminant(&already_resolved.pkg_ref) != std::mem::discriminant(pkg_ref) { tracing::warn!( - "resolved package {name}@{target_version_id} has a different source than previously resolved one, this may cause issues", + "resolved package {package_id} has a different source than previously resolved one, this may cause issues", ); } @@ -346,17 +308,12 @@ impl Project { }; insert_node( &mut graph, - &name, - &target_version_id, + &package_id, node, depth == 0, ); - tracing::debug!( - "resolved {}@{} from new dependency graph", - name, - target_version_id - ); + tracing::debug!("resolved {package_id} from new dependency graph"); for (dependency_alias, (dependency_spec, dependency_ty)) in pkg_ref.dependencies().clone() @@ -399,13 +356,13 @@ impl Project { None => dependency_spec, }, dependency_ty, - Some((name.clone(), target_version_id.clone())), + Some(package_id.clone()), path.iter() .cloned() .chain(std::iter::once(dependency_alias)) .collect(), overridden.is_some(), - *target_version_id.target(), + *package_id.version_id().target(), )); } @@ -415,15 +372,13 @@ impl Project { .await?; } - for (name, versions) in &mut graph { - for (version_id, node) in versions { - if node.is_peer && node.direct.is_none() { - node.resolved_ty = DependencyType::Peer; - } + for (id, node) in &mut graph { + if node.is_peer && node.direct.is_none() { + node.resolved_ty = DependencyType::Peer; + } - if node.resolved_ty == DependencyType::Peer { - tracing::warn!("peer dependency {name}@{version_id} was not resolved"); - } + if node.resolved_ty == DependencyType::Peer { + tracing::warn!("peer dependency {id} was not resolved"); } } diff --git a/src/source/version_id.rs b/src/source/ids.rs similarity index 60% rename from src/source/version_id.rs rename to src/source/ids.rs index ca758d1..8e59f71 100644 --- a/src/source/version_id.rs +++ b/src/source/ids.rs @@ -1,4 +1,4 @@ -use crate::manifest::target::TargetKind; +use crate::{manifest::target::TargetKind, names::PackageNames}; use semver::Version; use serde_with::{DeserializeFromStr, SerializeDisplay}; use std::{fmt::Display, str::FromStr}; @@ -34,6 +34,11 @@ impl VersionId { pub fn from_escaped(s: &str) -> Result { VersionId::from_str(s.replacen('+', " ", 1).as_str()) } + + /// Access the parts of the version ID + pub fn parts(&self) -> (&Version, &TargetKind) { + (&self.0, &self.1) + } } impl Display for VersionId { @@ -86,6 +91,55 @@ impl schemars::JsonSchema for VersionId { } } +/// A package ID, which is a combination of a name and a version ID +#[derive( + Debug, SerializeDisplay, DeserializeFromStr, Clone, PartialEq, Eq, Hash, PartialOrd, Ord, +)] +pub struct PackageId(pub(crate) PackageNames, pub(crate) VersionId); + +impl PackageId { + /// Creates a new package ID + pub fn new(names: PackageNames, version_id: VersionId) -> Self { + PackageId(names, version_id) + } + + /// Access the name + pub fn name(&self) -> &PackageNames { + &self.0 + } + + /// Access the version ID + pub fn version_id(&self) -> &VersionId { + &self.1 + } + + /// Access the parts of the package ID + pub fn parts(&self) -> (&PackageNames, &VersionId) { + (&self.0, &self.1) + } +} + +impl Display for PackageId { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}@{}", self.0, self.1) + } +} + +impl FromStr for PackageId { + type Err = errors::PackageIdParseError; + + fn from_str(s: &str) -> Result { + let Some((names, version_id)) = s.split_once('@') else { + return Err(errors::PackageIdParseError::Malformed(s.to_string())); + }; + + let names = names.parse()?; + let version_id = version_id.parse()?; + + Ok(PackageId(names, version_id)) + } +} + /// Errors that can occur when using a version ID pub mod errors { use thiserror::Error; @@ -106,4 +160,21 @@ pub mod errors { #[error("malformed target")] Target(#[from] crate::manifest::target::errors::TargetKindFromStr), } + + /// Errors that can occur when parsing a package ID + #[derive(Debug, Error)] + #[non_exhaustive] + pub enum PackageIdParseError { + /// The package ID is malformed (not in the form `name@version`) + #[error("malformed package id {0}")] + Malformed(String), + + /// The name is malformed + #[error("malformed name")] + Names(#[from] crate::names::errors::PackageNamesError), + + /// The version ID is malformed + #[error("malformed version id")] + VersionId(#[from] VersionIdParseError), + } } diff --git a/src/source/mod.rs b/src/source/mod.rs index 3004807..9fee5aa 100644 --- a/src/source/mod.rs +++ b/src/source/mod.rs @@ -3,8 +3,8 @@ use crate::{ names::PackageNames, reporters::DownloadProgressReporter, source::{ - fs::PackageFS, refs::PackageRefs, specifiers::DependencySpecifiers, traits::*, - version_id::VersionId, + fs::PackageFS, ids::VersionId, refs::PackageRefs, specifiers::DependencySpecifiers, + traits::*, }, }; use std::{collections::BTreeMap, fmt::Debug}; @@ -15,6 +15,8 @@ pub mod fs; pub mod git; /// Git index-based package source utilities pub mod git_index; +/// Package identifiers for different contexts +pub mod ids; /// The path package source pub mod path; /// The pesde package source @@ -25,8 +27,6 @@ pub mod refs; pub mod specifiers; /// Traits for sources and packages pub mod traits; -/// Version IDs -pub mod version_id; /// The Wally package source #[cfg(feature = "wally-compat")] pub mod wally; diff --git a/src/source/path/mod.rs b/src/source/path/mod.rs index d238131..b05e673 100644 --- a/src/source/path/mod.rs +++ b/src/source/path/mod.rs @@ -5,10 +5,10 @@ use crate::{ reporters::DownloadProgressReporter, source::{ fs::PackageFS, + ids::VersionId, path::pkg_ref::PathPackageRef, specifiers::DependencySpecifiers, traits::{DownloadOptions, PackageSource, ResolveOptions}, - version_id::VersionId, ResolveResult, }, DEFAULT_INDEX_NAME, diff --git a/src/source/wally/mod.rs b/src/source/wally/mod.rs index f9e7e49..1a0f9cb 100644 --- a/src/source/wally/mod.rs +++ b/src/source/wally/mod.rs @@ -5,8 +5,8 @@ use crate::{ source::{ fs::{store_in_cas, FSEntry, PackageFS}, git_index::{read_file, root_tree, GitBasedSource}, + ids::VersionId, traits::{DownloadOptions, PackageSource, RefreshOptions, ResolveOptions}, - version_id::VersionId, wally::{ compat_util::get_target, manifest::{Realm, WallyManifest}, diff --git a/src/source/workspace/mod.rs b/src/source/workspace/mod.rs index 718d520..01ec6ef 100644 --- a/src/source/workspace/mod.rs +++ b/src/source/workspace/mod.rs @@ -5,9 +5,9 @@ use crate::{ reporters::DownloadProgressReporter, source::{ fs::PackageFS, + ids::VersionId, specifiers::DependencySpecifiers, traits::{DownloadOptions, PackageSource, ResolveOptions}, - version_id::VersionId, workspace::pkg_ref::WorkspacePackageRef, ResolveResult, },