Worktree parser: Get correct path for bare trees

src/git/worktree/mod.rs

@@ -11,7 +11,6 @@ use rustc_hash::FxHashMap as HashMap;
 use tap::Tap;
 use tracing::instrument;
 use utf8_command::Utf8Output;
-use winnow::Parser;
 
 use super::Git;
 use super::LocalBranchRef;
@@ -58,18 +57,7 @@ impl<'a> GitWorktree<'a> {
     #[instrument(level = "trace")]
     pub fn container(&self) -> miette::Result<Utf8PathBuf> {
         // TODO: Write `.git-prole` to indicate worktree container root?
-        let main = self.main()?;
-        let mut path = if main.head == WorktreeHead::Bare {
-            // Git has a bug(?) where `git worktree list` will show the _parent_ of a
-            // bare worktree in a directory named `.git`. Work around it by getting the
-            // `.git` directory manually.
-            //
-            // See: https://lore.kernel.org/git/[email protected]/T/
-            self.0.with_directory(main.path).path().git_common_dir()?
-        } else {
-            main.path
-        };
-
+        let mut path = self.main()?.path;
         if !path.pop() {
             Err(miette!("Main worktree path has no parent: {path}"))
         } else {
@@ -88,7 +76,7 @@ impl<'a> GitWorktree<'a> {
                     Err(context.error())
                 } else {
                     let output = &context.output().stdout;
-                    match Worktrees::parser.parse(output) {
+                    match Worktrees::parse(self.0, output) {
                         Ok(worktrees) => Ok(worktrees),
                         Err(err) => {
                             let err = miette!("{err}");

src/git/worktree/parse.rs

@@ -1,6 +1,5 @@
 use std::fmt::Display;
 use std::ops::Deref;
-use std::str::FromStr;
 
 use camino::Utf8Path;
 use camino::Utf8PathBuf;
@@ -23,6 +22,7 @@ use winnow::Parser;
 
 use crate::parse::till_null;
 use crate::CommitHash;
+use crate::Git;
 use crate::LocalBranchRef;
 use crate::NormalPath;
 use crate::Ref;
@@ -63,7 +63,7 @@ impl Worktrees {
             .find(|worktree| worktree.head.branch() == Some(branch))
     }
 
-    pub fn parser(input: &mut &str) -> PResult<Self> {
+    fn parser(input: &mut &str) -> PResult<Self> {
         let mut main = Worktree::parser.parse_next(input)?;
         main.is_main = true;
         let main_path = main.path.clone();
@@ -78,18 +78,46 @@ impl Worktrees {
 
         inner.insert(main_path.clone(), main);
 
-        Ok(Self {
+        let worktrees = Self {
             main: main_path,
             inner,
-        })
+        };
+
+        tracing::debug!(
+            worktrees=%worktrees,
+            "Parsed worktrees",
+        );
+
+        Ok(worktrees)
     }
-}
 
-impl FromStr for Worktrees {
-    type Err = miette::Report;
+    pub fn parse(git: &Git, input: &str) -> miette::Result<Self> {
+        let mut ret = Self::parser.parse(input).map_err(|err| miette!("{err}"))?;
+
+        if ret.main().head.is_bare() {
+            // Git has a bug(?) where `git worktree list` will show the _parent_ of a
+            // bare worktree in a directory named `.git`. Work around it by getting the
+            // `.git` directory manually and patching up the returned value to replace the path of
+            // the main worktree (which is, unfortunately, stored in three separate places).
+            //
+            // It's _possible_ to do this in the parser, but then the parser is no longer a pure
+            // function and the error handling is very annoying.
+            //
+            // See: https://lore.kernel.org/git/[email protected]/T/
+
+            let git_dir = git.path().git_common_dir()?;
+            if git_dir != ret.main {
+                let old_main_path = std::mem::replace(&mut ret.main, git_dir);
+                let mut main = ret
+                    .inner
+                    .remove(&old_main_path)
+                    .expect("There is always a main worktree");
+                main.path = ret.main.clone();
+                ret.inner.insert(ret.main.clone(), main);
+            }
+        }
 
-    fn from_str(input: &str) -> Result<Self, Self::Err> {
-        Self::parser.parse(input).map_err(|err| miette!("{err}"))
+        Ok(ret)
     }
 }
 
@@ -264,7 +292,7 @@ impl Display for Worktree {
 }
 
 impl Worktree {
-    pub fn parser(input: &mut &str) -> PResult<Self> {
+    fn parser(input: &mut &str) -> PResult<Self> {
         let _ = "worktree ".parse_next(input)?;
         let path = Utf8PathBuf::from(till_null.parse_next(input)?);
         let head = WorktreeHead::parser.parse_next(input)?;
@@ -376,40 +404,41 @@ mod tests {
 
     #[test]
     fn test_parse_worktrees_list() {
-        let worktrees = Worktrees::from_str(
-            &indoc!(
-                "
-                worktree /path/to/bare-source
-                bare
-
-                worktree /Users/wiggles/cabal/accept
-                HEAD 0685cb3fec8b7144f865638cfd16768e15125fc2
-                branch refs/heads/rebeccat/fix-accept-flag
-
-                worktree /Users/wiggles/lix
-                HEAD 0d484aa498b3c839991d11afb31bc5fcf368493d
-                detached
-
-                worktree /path/to/linked-worktree-locked-no-reason
-                HEAD 5678abc5678abc5678abc5678abc5678abc5678c
-                branch refs/heads/locked-no-reason
-                locked
-
-                worktree /path/to/linked-worktree-locked-with-reason
-                HEAD 3456def3456def3456def3456def3456def3456b
-                branch refs/heads/locked-with-reason
-                locked reason why is locked
-
-                worktree /path/to/linked-worktree-prunable
-                HEAD 1233def1234def1234def1234def1234def1234b
-                detached
-                prunable gitdir file points to non-existent location
-
-                "
+        let worktrees = Worktrees::parser
+            .parse(
+                &indoc!(
+                    "
+                    worktree /path/to/bare-source
+                    bare
+
+                    worktree /Users/wiggles/cabal/accept
+                    HEAD 0685cb3fec8b7144f865638cfd16768e15125fc2
+                    branch refs/heads/rebeccat/fix-accept-flag
+
+                    worktree /Users/wiggles/lix
+                    HEAD 0d484aa498b3c839991d11afb31bc5fcf368493d
+                    detached
+
+                    worktree /path/to/linked-worktree-locked-no-reason
+                    HEAD 5678abc5678abc5678abc5678abc5678abc5678c
+                    branch refs/heads/locked-no-reason
+                    locked
+
+                    worktree /path/to/linked-worktree-locked-with-reason
+                    HEAD 3456def3456def3456def3456def3456def3456b
+                    branch refs/heads/locked-with-reason
+                    locked reason why is locked
+
+                    worktree /path/to/linked-worktree-prunable
+                    HEAD 1233def1234def1234def1234def1234def1234b
+                    detached
+                    prunable gitdir file points to non-existent location
+
+                    "
+                )
+                .replace('\n', "\0"),
             )
-            .replace('\n', "\0"),
-        )
-        .unwrap();
+            .unwrap();
 
         assert_eq!(worktrees.main_path(), "/path/to/bare-source");
 

test-harness/src/repo_state.rs

@@ -155,7 +155,7 @@ impl WorktreeState {
     ///
     /// This is used if a bare worktree named `.git` is present at the repository root.
     pub fn new_bare() -> Self {
-        Self::new("").bare()
+        Self::new(".git").bare()
     }
 
     /// This worktree is bare.