feat: remove data redundancy for workspace pkg refs

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

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()

src/cli/mod.rs

@@ -258,9 +258,7 @@ pub async fn run_on_workspace_members<F: Future<Output = anyhow::Result<()>>>(
         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::<PackageName, BTreeMap<TargetKind, RelativePathBuf>>::new();

src/lib.rs

@@ -215,21 +215,15 @@ impl Project {
 
     /// Get the workspace members
     #[instrument(skip(self), level = "debug")]
-    pub async fn workspace_members<P: AsRef<Path> + Debug>(
+    pub async fn workspace_members(
         &self,
-        dir: P,
         can_ref_self: bool,
     ) -> Result<
         impl Stream<Item = Result<(PathBuf, Manifest), errors::WorkspaceMembersError>>,
         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>(&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>(&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<Manifest, errors::ManifestReadError> {
     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<PathBuf>), errors::FindRootsError> {
+    let mut current_path = Some(cwd.clone());
+    let mut project_root = None::<PathBuf>;
+    let mut workspace_dir = None::<PathBuf>;
+
+    async fn get_workspace_members(
+        path: &Path,
+    ) -> Result<HashSet<PathBuf>, 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<toml::de::Error>),
+        /// 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),
+    }
 }

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::<PathBuf>;
-        let mut workspace_dir = None::<PathBuf>;
-
-        async fn get_workspace_members(path: &Path) -> anyhow::Result<HashSet<PathBuf>> {
-            let manifest = fs::read_to_string(path.join(MANIFEST_FILE_NAME))
+    let (project_root_dir, project_workspace_dir) = find_roots(cwd.clone())
         .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)
-    };
+        .context("failed to find project root")?;
 
     tracing::trace!(
         "project root: {}\nworkspace root: {}",

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::<Result<_, errors::ResolveError>>()?,
-            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),
     }
 }

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<String, (DependencySpecifiers, DependencyType)>,
-    /// The target of the package
-    pub target: Target,
 }
 impl PackageRef for WorkspacePackageRef {
     fn dependencies(&self) -> &BTreeMap<String, (DependencySpecifiers, DependencyType)> {