From 2700fe9e070917f427aa7ce36ab21aeca7353d4e Mon Sep 17 00:00:00 2001 From: daimond113 <72147841+daimond113@users.noreply.github.com> Date: Mon, 30 Dec 2024 19:06:53 +0100 Subject: [PATCH] feat: remove data redundancy for workspace pkg refs --- CHANGELOG.md | 2 + src/cli/commands/run.rs | 5 +- src/cli/mod.rs | 4 +- src/lib.rs | 112 ++++++++++++++++++++++++-------- src/main.rs | 67 ++----------------- src/source/workspace/mod.rs | 23 ++++--- src/source/workspace/pkg_ref.rs | 4 +- 7 files changed, 110 insertions(+), 107 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfbfd2f..dda43a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Improve installation experience by @lukadev-0 - Support using aliases of own dependencies for overrides by @daimond113 - Support ignoring parse errors in Luau files by @daimond113 +- Add path dependencies by @daimond113 ### Removed - Remove old includes format compatibility by @daimond113 +- Remove data redundacy for workspace package references by @daimond113 ### Performance - Use `Arc` for more efficient cloning of multiple structs by @daimond113 diff --git a/src/cli/commands/run.rs b/src/cli/commands/run.rs index eb97c59..47191d3 100644 --- a/src/cli/commands/run.rs +++ b/src/cli/commands/run.rs @@ -3,6 +3,7 @@ use anyhow::Context; use clap::Args; use futures::{StreamExt, TryStreamExt}; use pesde::{ + errors::{ManifestReadError, WorkspaceMembersError}, linking::generator::generate_bin_linking_module, names::{PackageName, PackageNames}, Project, MANIFEST_FILE_NAME, PACKAGES_CONTAINER_NAME, @@ -124,9 +125,9 @@ impl RunCommand { .workspace_dir() .unwrap_or_else(|| project.package_dir()); - let members = match project.workspace_members(workspace_dir, false).await { + let members = match project.workspace_members(false).await { Ok(members) => members.boxed(), - Err(pesde::errors::WorkspaceMembersError::ManifestMissing(e)) + Err(WorkspaceMembersError::ManifestParse(ManifestReadError::Io(e))) if e.kind() == std::io::ErrorKind::NotFound => { futures::stream::empty().boxed() diff --git a/src/cli/mod.rs b/src/cli/mod.rs index eb1de0f..e279106 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -258,9 +258,7 @@ pub async fn run_on_workspace_members>>( return Ok(Default::default()); } - let members_future = project - .workspace_members(project.package_dir(), true) - .await?; + let members_future = project.workspace_members(true).await?; pin!(members_future); let mut results = BTreeMap::>::new(); diff --git a/src/lib.rs b/src/lib.rs index fbdb6d5..31a4746 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -215,21 +215,15 @@ impl Project { /// Get the workspace members #[instrument(skip(self), level = "debug")] - pub async fn workspace_members + Debug>( + pub async fn workspace_members( &self, - dir: P, can_ref_self: bool, ) -> Result< impl Stream>, errors::WorkspaceMembersError, > { - let dir = dir.as_ref().to_path_buf(); - let manifest = fs::read_to_string(dir.join(MANIFEST_FILE_NAME)) - .await - .map_err(errors::WorkspaceMembersError::ManifestMissing)?; - let manifest = toml::from_str::(&manifest).map_err(|e| { - errors::WorkspaceMembersError::ManifestDeser(dir.to_path_buf(), Box::new(e)) - })?; + let dir = self.workspace_dir().unwrap_or(self.package_dir()); + let manifest = deser_manifest(dir).await?; let members = matching_globs( dir, @@ -241,13 +235,7 @@ impl Project { Ok(try_stream! { for path in members { - let manifest = fs::read_to_string(path.join(MANIFEST_FILE_NAME)) - .await - .map_err(errors::WorkspaceMembersError::ManifestMissing)?; - let manifest = toml::from_str::(&manifest).map_err(|e| { - errors::WorkspaceMembersError::ManifestDeser(path.clone(), Box::new(e)) - })?; - + let manifest = deser_manifest(&path).await?; yield (path, manifest); } }) @@ -346,7 +334,70 @@ impl RefreshedSources { async fn deser_manifest(path: &Path) -> Result { let string = fs::read_to_string(path.join(MANIFEST_FILE_NAME)).await?; - Ok(toml::from_str(&string)?) + toml::from_str(&string).map_err(|e| errors::ManifestReadError::Serde(path.to_path_buf(), e)) +} + +/// Find the project & workspace directory roots +pub async fn find_roots( + cwd: PathBuf, +) -> Result<(PathBuf, Option), errors::FindRootsError> { + let mut current_path = Some(cwd.clone()); + let mut project_root = None::; + let mut workspace_dir = None::; + + async fn get_workspace_members( + path: &Path, + ) -> Result, errors::FindRootsError> { + let manifest = deser_manifest(path).await?; + + if manifest.workspace_members.is_empty() { + return Ok(HashSet::new()); + } + + matching_globs( + path, + manifest.workspace_members.iter().map(|s| s.as_str()), + false, + false, + ) + .await + .map_err(errors::FindRootsError::Globbing) + } + + while let Some(path) = current_path { + current_path = path.parent().map(|p| p.to_path_buf()); + + if !path.join(MANIFEST_FILE_NAME).exists() { + continue; + } + + match (project_root.as_ref(), workspace_dir.as_ref()) { + (Some(project_root), Some(workspace_dir)) => { + return Ok((project_root.clone(), Some(workspace_dir.clone()))); + } + + (Some(project_root), None) => { + if get_workspace_members(&path).await?.contains(project_root) { + workspace_dir = Some(path); + } + } + + (None, None) => { + if get_workspace_members(&path).await?.contains(&cwd) { + // initializing a new member of a workspace + return Ok((cwd, Some(path))); + } else { + project_root = Some(path); + } + } + + (None, Some(_)) => unreachable!(), + } + } + + // we mustn't expect the project root to be found, as that would + // disable the ability to run pesde in a non-project directory (for example to init it) + Ok((project_root.unwrap_or(cwd), workspace_dir)) } /// Errors that can occur when using the pesde library @@ -363,8 +414,8 @@ pub mod errors { Io(#[from] std::io::Error), /// An error occurred while deserializing the manifest file - #[error("error deserializing manifest file")] - Serde(#[from] toml::de::Error), + #[error("error deserializing manifest file at {0}")] + Serde(PathBuf, #[source] toml::de::Error), } /// Errors that can occur when reading the lockfile @@ -397,13 +448,9 @@ pub mod errors { #[derive(Debug, Error)] #[non_exhaustive] pub enum WorkspaceMembersError { - /// The manifest file could not be found - #[error("missing manifest file")] - ManifestMissing(#[source] std::io::Error), - - /// An error occurred deserializing the manifest file - #[error("error deserializing manifest file at {0}")] - ManifestDeser(PathBuf, #[source] Box), + /// An error occurred parsing the manifest file + #[error("error parsing manifest file")] + ManifestParse(#[from] ManifestReadError), /// An error occurred interacting with the filesystem #[error("error interacting with the filesystem")] @@ -426,4 +473,17 @@ pub mod errors { #[error("error building glob")] BuildGlob(#[from] wax::BuildError), } + + /// Errors that can occur when finding project roots + #[derive(Debug, Error)] + #[non_exhaustive] + pub enum FindRootsError { + /// Reading the manifest failed + #[error("error reading manifest")] + ManifestRead(#[from] ManifestReadError), + + /// Globbing failed + #[error("error globbing")] + Globbing(#[from] MatchingGlobsError), + } } diff --git a/src/main.rs b/src/main.rs index e2f23be..4d2a311 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,9 +5,8 @@ use anyhow::Context; use clap::{builder::styling::AnsiColor, Parser}; use fs_err::tokio as fs; use indicatif::MultiProgress; -use pesde::{matching_globs, AuthConfig, Project, MANIFEST_FILE_NAME}; +use pesde::{find_roots, AuthConfig, Project}; use std::{ - collections::HashSet, io, path::{Path, PathBuf}, sync::Mutex, @@ -208,67 +207,9 @@ async fn run() -> anyhow::Result<()> { .with(fmt_layer) .init(); - let (project_root_dir, project_workspace_dir) = 'finder: { - let mut current_path = Some(cwd.clone()); - let mut project_root = None::; - let mut workspace_dir = None::; - - async fn get_workspace_members(path: &Path) -> anyhow::Result> { - let manifest = fs::read_to_string(path.join(MANIFEST_FILE_NAME)) - .await - .context("failed to read manifest")?; - let manifest: pesde::manifest::Manifest = - toml::from_str(&manifest).context("failed to parse manifest")?; - - if manifest.workspace_members.is_empty() { - return Ok(HashSet::new()); - } - - matching_globs( - path, - manifest.workspace_members.iter().map(|s| s.as_str()), - false, - false, - ) - .await - .context("failed to get workspace members") - } - - while let Some(path) = current_path { - current_path = path.parent().map(|p| p.to_path_buf()); - - if !path.join(MANIFEST_FILE_NAME).exists() { - continue; - } - - match (project_root.as_ref(), workspace_dir.as_ref()) { - (Some(project_root), Some(workspace_dir)) => { - break 'finder (project_root.clone(), Some(workspace_dir.clone())); - } - - (Some(project_root), None) => { - if get_workspace_members(&path).await?.contains(project_root) { - workspace_dir = Some(path); - } - } - - (None, None) => { - if get_workspace_members(&path).await?.contains(&cwd) { - // initializing a new member of a workspace - break 'finder (cwd, Some(path)); - } else { - project_root = Some(path); - } - } - - (None, Some(_)) => unreachable!(), - } - } - - // we mustn't expect the project root to be found, as that would - // disable the ability to run pesde in a non-project directory (for example to init it) - (project_root.unwrap_or_else(|| cwd.clone()), workspace_dir) - }; + let (project_root_dir, project_workspace_dir) = find_roots(cwd.clone()) + .await + .context("failed to find project root")?; tracing::trace!( "project root: {}\nworkspace root: {}", diff --git a/src/source/workspace/mod.rs b/src/source/workspace/mod.rs index 97b4e1c..718d520 100644 --- a/src/source/workspace/mod.rs +++ b/src/source/workspace/mod.rs @@ -1,4 +1,5 @@ use crate::{ + deser_manifest, manifest::target::Target, names::PackageNames, reporters::DownloadProgressReporter, @@ -47,10 +48,9 @@ impl PackageSource for WorkspacePackageSource { } = options; let (path, manifest) = 'finder: { - let workspace_dir = project.workspace_dir().unwrap_or(project.package_dir()); let target = specifier.target.unwrap_or(*project_target); - let members = project.workspace_members(workspace_dir, true).await?; + let members = project.workspace_members(true).await?; pin!(members); while let Some((path, manifest)) = members.next().await.transpose()? { @@ -71,7 +71,8 @@ impl PackageSource for WorkspacePackageSource { // strip_prefix is guaranteed to be Some by same method // from_path is guaranteed to be Ok because we just stripped the absolute path path: RelativePathBuf::from_path( - path.strip_prefix(project.workspace_dir().unwrap()).unwrap(), + path.strip_prefix(project.workspace_dir().unwrap_or(project.package_dir())) + .unwrap(), ) .unwrap(), dependencies: manifest @@ -116,7 +117,6 @@ impl PackageSource for WorkspacePackageSource { Ok((alias, (spec, ty))) }) .collect::>()?, - target: manifest.target, }; Ok(( @@ -136,11 +136,14 @@ impl PackageSource for WorkspacePackageSource { ) -> Result<(PackageFS, Target), Self::DownloadError> { let DownloadOptions { project, .. } = options; - let path = pkg_ref.path.to_path(project.workspace_dir().unwrap()); + let path = pkg_ref + .path + .to_path(project.workspace_dir().unwrap_or(project.package_dir())); + let manifest = deser_manifest(&path).await?; Ok(( - PackageFS::Copy(path, pkg_ref.target.kind()), - pkg_ref.target.clone(), + PackageFS::Copy(path, manifest.target.kind()), + manifest.target, )) } } @@ -180,8 +183,8 @@ pub mod errors { #[derive(Debug, Error)] #[non_exhaustive] pub enum DownloadError { - /// An error occurred reading the workspace members - #[error("failed to read workspace members")] - ReadWorkspaceMembers(#[from] std::io::Error), + /// Reading the manifest failed + #[error("error reading manifest")] + ManifestRead(#[from] crate::errors::ManifestReadError), } } diff --git a/src/source/workspace/pkg_ref.rs b/src/source/workspace/pkg_ref.rs index fd38ca5..1d1c725 100644 --- a/src/source/workspace/pkg_ref.rs +++ b/src/source/workspace/pkg_ref.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; use std::collections::BTreeMap; use crate::{ - manifest::{target::Target, DependencyType}, + manifest::DependencyType, source::{workspace::WorkspacePackageSource, DependencySpecifiers, PackageRef, PackageSources}, }; @@ -15,8 +15,6 @@ pub struct WorkspacePackageRef { /// The dependencies of the package #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] pub dependencies: BTreeMap, - /// The target of the package - pub target: Target, } impl PackageRef for WorkspacePackageRef { fn dependencies(&self) -> &BTreeMap {