workingcopy: work better with dotgit submodule repos

Summary: Replace `join.*dot_dir` with identity's `resolve_full_dot_dir` to proper handle dotgit submodule repos. Some spare related logic requires further refactoring to fix and those are not yet done in this diff. This fixes the submodule commit and checkout in dotgit mode. Reviewed By: sggutier Differential Revision: D66911303 fbshipit-source-id: 7bb5d5ca9a0e7575d3e909bea1c346f0a7166afc
facebook · Dec 9, 2024 · c7594cd · c7594cd
1 parent 7015f0f
commit c7594cd
Show file tree

Hide file tree

Showing 6 changed files with 13 additions and 9 deletions.
diff --git a/eden/scm/lib/workingcopy/src/filesystem/edenfs.rs b/eden/scm/lib/workingcopy/src/filesystem/edenfs.rs
@@ -341,6 +341,7 @@ impl FileSystem for EdenFileSystem {
             &self.vfs,
             manifests,
             self.store.clone(),
+            // XXX: This does not work for dotgit submodule.
             &self.vfs.root().join(dot_dir),
         )
     }

diff --git a/eden/scm/lib/workingcopy/src/filesystem/physicalfs.rs b/eden/scm/lib/workingcopy/src/filesystem/physicalfs.rs
@@ -184,6 +184,7 @@ impl FileSystem for PhysicalFileSystem {
             &self.vfs,
             manifests,
             self.store.clone(),
+            // XXX: This does not work for dotgit submodule.
             &self.vfs.root().join(dot_dir),
         )
     }

diff --git a/eden/scm/lib/workingcopy/src/sparse.rs b/eden/scm/lib/workingcopy/src/sparse.rs
@@ -199,6 +199,7 @@ pub fn sparse_matcher(
     for manifest in manifests.iter() {
         if let Some((matcher, _hash)) = repo_matcher(
             vfs,
+            // XXX: This does not work for dotgit submodule.
             &vfs.root().join(dot_dir),
             manifest.as_ref(),
             store.clone(),

diff --git a/eden/scm/lib/workingcopy/src/walker.rs b/eden/scm/lib/workingcopy/src/walker.rs
@@ -157,7 +157,13 @@ where
                 root,
                 matcher,
                 include_directories,
-                dot_dir,
+                // dot_dir is only used to avoid walking into nested repos.
+                // If dot_dir is ".git/sl". Then turn it into ".git".
+                dot_dir: if dot_dir.contains('/') {
+                    dot_dir.split('/').next().unwrap_or("").to_string()
+                } else {
+                    dot_dir
+                },
                 skip_dirs: skip_dirs
                     .into_iter()
                     .map(|p| Ok(p.try_into()?))

diff --git a/eden/scm/lib/workingcopy/src/workingcopy.rs b/eden/scm/lib/workingcopy/src/workingcopy.rs
@@ -47,7 +47,6 @@ use treestate::treestate::TreeState;
 use types::hgid::NULL_ID;
 use types::repo::StorageFormat;
 use types::HgId;
-use types::RepoPath;
 use types::RepoPathBuf;
 use util::file::atomic_write;
 use util::file::read_to_string_if_exists;
@@ -183,7 +182,7 @@ impl WorkingCopy {
                 return Err(errors::RepoNotFound(root.to_string_lossy().to_string()).into());
             }
         };
-        let dot_hg_path = vfs.join(RepoPath::from_str(ident.dot_dir())?);
+        let dot_hg_path = ident.resolve_full_dot_dir(vfs.root());
         let journal = Journal::open(dot_hg_path.clone())?;
 
         Ok(WorkingCopy {

diff --git a/eden/scm/tests/test-dotgit-submodule.t b/eden/scm/tests/test-dotgit-submodule.t
@@ -52,16 +52,12 @@ Status from submodule:
   ? c
 
 Committing from submodule:
-FIXME: does not work yet
 
   $ sl add c
   $ sl commit -m c
-  abort: error reading file: '$TESTTMP/parent/sub/.git/sl/graftstate': $ENOTDIR$ (os error 20)
-  [255]
 
 Checking out from submodule:
-FIXME: does not work yet
 
   $ sl prev
-  abort: error reading file: '$TESTTMP/parent/sub/.git/sl/graftstate': $ENOTDIR$ (os error 20)
-  [255]
+  update complete
+  [7e03c5] Add a