fix(forc-pkg): Unify member source types, fix manifest entries, fix F…

…uelLabs#4235 (FuelLabs#4237)

## Description

Fixes an issue where member source types unintentionally omitted entries
from the manifest map during the fetch traversal after the source
refactor in FuelLabs#4168.

Tested locally with the `fuels-rs` test suite using the steps described
in FuelLabs#4235.

Closes FuelLabs#4235.

This patch will likely also apply to other large workspaces too, we
might want to consider cutting a patch release soon after landing.

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] I have added tests that prove my fix is effective or that my
feature works.
- [x] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.

forc-pkg/src/pkg.rs

@@ -679,7 +679,7 @@ impl BuildPlan {
         self.compilation_order()
             .iter()
             .cloned()
-            .filter(|&n| self.graph[n].source == source::Pinned::Member)
+            .filter(|&n| self.graph[n].source == source::Pinned::MEMBER)
     }
 
     /// Produce an iterator yielding all workspace member pinned pkgs in order of compilation.
@@ -806,7 +806,7 @@ fn validate_pkg_version(pkg_manifest: &PackageManifestFile) -> Result<()> {
 
 fn member_nodes(g: &Graph) -> impl Iterator<Item = NodeIx> + '_ {
     g.node_indices()
-        .filter(|&n| g[n].source == source::Pinned::Member)
+        .filter(|&n| g[n].source == source::Pinned::MEMBER)
 }
 
 /// Validates the state of the pinned package graph against the given ManifestFile.
@@ -1041,12 +1041,7 @@ impl Pinned {
 
     /// Retrieve the unpinned version of this source.
     pub fn unpinned(&self, path: &Path) -> Pkg {
-        let source = match &self.source {
-            source::Pinned::Member => Source::Member(path.to_owned()),
-            source::Pinned::Git(git) => Source::Git(git.source.clone()),
-            source::Pinned::Path(_) => Source::Path(path.to_owned()),
-            source::Pinned::Registry(reg) => Source::Registry(reg.source.clone()),
-        };
+        let source = self.source.unpinned(path);
         let name = self.name.clone();
         Pkg { name, source }
     }
@@ -1206,8 +1201,8 @@ fn validate_path_root(graph: &Graph, path_dep: NodeIx, path_root: PinnedId) -> R
 fn find_path_root(graph: &Graph, mut node: NodeIx) -> Result<NodeIx> {
     loop {
         let pkg = &graph[node];
-        match &pkg.source {
-            source::Pinned::Path(src) => {
+        match pkg.source {
+            source::Pinned::Path(ref src) => {
                 let parent = graph
                     .edges_directed(node, Direction::Incoming)
                     .next()
@@ -1220,7 +1215,7 @@ fn find_path_root(graph: &Graph, mut node: NodeIx) -> Result<NodeIx> {
                     })?;
                 node = parent;
             }
-            source::Pinned::Git(_) | source::Pinned::Registry(_) | source::Pinned::Member => {
+            source::Pinned::Git(_) | source::Pinned::Registry(_) | source::Pinned::Member(_) => {
                 return Ok(node);
             }
         }
@@ -1279,7 +1274,7 @@ fn fetch_pkg_graph(
         Ok(proj_node) => proj_node,
         Err(_) => {
             let name = proj_manifest.project.name.clone();
-            let source = source::Pinned::Member;
+            let source = source::Pinned::MEMBER;
             let pkg = Pinned { name, source };
             let pkg_id = pkg.id();
             manifest_map.insert(pkg_id, proj_manifest.clone());
@@ -1398,7 +1393,7 @@ fn fetch_deps(
         })?;
 
         let path_root = match dep_pinned.source {
-            source::Pinned::Member | source::Pinned::Git(_) | source::Pinned::Registry(_) => {
+            source::Pinned::Member(_) | source::Pinned::Git(_) | source::Pinned::Registry(_) => {
                 dep_pkg_id
             }
             source::Pinned::Path(_) => path_root,

forc-pkg/src/source/member.rs

@@ -0,0 +1,47 @@
+use crate::{manifest::PackageManifestFile, source};
+use serde::{Deserialize, Serialize};
+use std::{
+    fmt,
+    path::{Path, PathBuf},
+};
+
+/// Member source representation as a canonical path.
+#[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Deserialize, Serialize)]
+pub struct Source(pub(super) PathBuf);
+
+/// A pinned instance of a member source requires no information as it's a part
+/// of the workspace.
+#[derive(Clone, Debug, Eq, Hash, PartialEq, Deserialize, Serialize)]
+pub struct Pinned;
+
+impl fmt::Display for Pinned {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        write!(f, "member")
+    }
+}
+
+impl source::Pin for Source {
+    type Pinned = Pinned;
+    fn pin(&self, _ctx: source::PinCtx) -> anyhow::Result<(Self::Pinned, PathBuf)> {
+        Ok((Pinned, self.0.clone()))
+    }
+}
+
+impl source::Fetch for Pinned {
+    fn fetch(&self, _ctx: source::PinCtx, local: &Path) -> anyhow::Result<PackageManifestFile> {
+        let manifest = PackageManifestFile::from_dir(local)?;
+        Ok(manifest)
+    }
+}
+
+impl source::DepPath for Pinned {
+    fn dep_path(&self, _name: &str) -> anyhow::Result<source::DependencyPath> {
+        Ok(source::DependencyPath::Member)
+    }
+}
+
+impl From<Pinned> for source::Pinned {
+    fn from(p: Pinned) -> Self {
+        Self::Member(p)
+    }
+}

forc-pkg/src/source/mod.rs

@@ -8,6 +8,7 @@
 //! 4. Add variant support to the `from_manifest_dep` and `FromStr` implementations.
 
 pub mod git;
+mod member;
 pub mod path;
 mod reg;
 
@@ -54,7 +55,7 @@ type FetchId = u64;
 #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd, Deserialize, Serialize)]
 pub enum Source {
     /// Used to refer to a workspace member project.
-    Member(PathBuf),
+    Member(member::Source),
     /// A git repo with a `Forc.toml` manifest at its root.
     Git(git::Source),
     /// A path to a directory with a `Forc.toml` manifest at its root.
@@ -69,7 +70,7 @@ pub enum Source {
 /// pinned version or commit is updated upon creation of the lock file and on `forc update`.
 #[derive(Clone, Debug, Eq, Hash, PartialEq, Deserialize, Serialize)]
 pub enum Pinned {
-    Member,
+    Member(member::Pinned),
     Git(git::Pinned),
     Path(path::Pinned),
     Registry(reg::Pinned),
@@ -135,7 +136,7 @@ impl Source {
                         .values()
                         .any(|pkg_manifest| pkg_manifest.dir() == canonical_path)
                     {
-                        Source::Member(canonical_path)
+                        Source::Member(member::Source(canonical_path))
                     } else {
                         Source::Path(canonical_path)
                     }
@@ -211,7 +212,7 @@ impl Source {
 
     /// Attempt to determine the pinned version or commit for the source.
     ///
-    /// Also updates the `path_map` with a path to the local copy of the source.
+    /// Also updates the manifest map with a path to the local copy of the pkg.
     ///
     /// The `path_root` is required for `Path` dependencies and must specify the package that is the
     /// root of the current subgraph of path dependencies.
@@ -223,15 +224,14 @@ impl Source {
             Pinned: From<T::Pinned>,
         {
             let (pinned, fetch_path) = source.pin(ctx.clone())?;
-            // TODO: Could potentially omit `name`? Breaks all existing locks though...
             let id = PinnedId::new(ctx.name(), &Pinned::from(pinned.clone()));
             if let hash_map::Entry::Vacant(entry) = manifests.entry(id) {
                 entry.insert(pinned.fetch(ctx, &fetch_path)?);
             }
             Ok(pinned)
         }
         match self {
-            Source::Member(_path) => Ok(Pinned::Member),
+            Source::Member(source) => Ok(Pinned::Member(f(source, ctx, manifests)?)),
             Source::Path(source) => Ok(Pinned::Path(f(source, ctx, manifests)?)),
             Source::Git(source) => Ok(Pinned::Git(f(source, ctx, manifests)?)),
             Source::Registry(source) => Ok(Pinned::Registry(f(source, ctx, manifests)?)),
@@ -240,10 +240,12 @@ impl Source {
 }
 
 impl Pinned {
+    pub(crate) const MEMBER: Self = Self::Member(member::Pinned);
+
     /// Return how the pinned source for a dependency can be found on the local file system.
     pub(crate) fn dep_path(&self, name: &str) -> Result<DependencyPath> {
         match self {
-            Self::Member => Ok(DependencyPath::Member),
+            Self::Member(pinned) => pinned.dep_path(name),
             Self::Path(pinned) => pinned.dep_path(name),
             Self::Git(pinned) => pinned.dep_path(name),
             Self::Registry(pinned) => pinned.dep_path(name),
@@ -272,6 +274,16 @@ impl Pinned {
             manifest_dir,
         }
     }
+
+    /// Retrieve the unpinned instance of this source.
+    pub fn unpinned(&self, path: &Path) -> Source {
+        match self {
+            Self::Member(_) => Source::Member(member::Source(path.to_owned())),
+            Self::Git(git) => Source::Git(git.source.clone()),
+            Self::Path(_) => Source::Path(path.to_owned()),
+            Self::Registry(reg) => Source::Registry(reg.source.clone()),
+        }
+    }
 }
 
 impl<'a> PinCtx<'a> {
@@ -292,7 +304,7 @@ impl<'a> PinCtx<'a> {
 impl fmt::Display for Pinned {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
-            Self::Member => write!(f, "member"),
+            Self::Member(src) => src.fmt(f),
             Self::Path(src) => src.fmt(f),
             Self::Git(src) => src.fmt(f),
             Self::Registry(_reg) => todo!("pkg registries not yet implemented"),
@@ -303,7 +315,7 @@ impl fmt::Display for Pinned {
 impl<'a> fmt::Display for DisplayCompiling<'a, Pinned> {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self.source {
-            Pinned::Member => self.manifest_dir.display().fmt(f),
+            Pinned::Member(_) => self.manifest_dir.display().fmt(f),
             Pinned::Path(_src) => self.manifest_dir.display().fmt(f),
             Pinned::Git(src) => src.fmt(f),
             Pinned::Registry(_src) => todo!("registry dependencies not yet implemented"),
@@ -317,7 +329,7 @@ impl FromStr for Pinned {
         // Also check `"root"` to support reading the legacy `Forc.lock` format and to
         // avoid breaking old projects.
         let source = if s == "root" || s == "member" {
-            Self::Member
+            Self::Member(member::Pinned)
         } else if let Ok(src) = path::Pinned::from_str(s) {
             Self::Path(src)
         } else if let Ok(src) = git::Pinned::from_str(s) {