History

fix: obtain correct worktree directories for submodule status when behind symlinks.

Related to https://github.com/gitui-org/gitui/issues/2820

Sebastian Thiel committed 10 months ago

Commit 2ffee221 parent ab673015 files · +83 −5Browse files

Unified · Split

5 files
  1. gix/src/submodule/errors.rs +2 −0
  2. gix/src/submodule/mod.rs +22 −6
  3. gix/tests/fixtures/generated-archives/make_submodules.tar +0 −0
  4. gix/tests/fixtures/make_submodules.sh +16 −0
  5. gix/tests/submodule_open.rs +44 −0
gix/src/submodule/errors.rs+2−0
@@ -76,6 +76,8 @@ pub mod open { · 75 unmodified lines
76 OpenRepository(#[from] crate::open::Error),76 OpenRepository(#[from] crate::open::Error),
77 #[error(transparent)]77 #[error(transparent)]
78 PathConfiguration(#[from] gix_submodule::config::path::Error),78 PathConfiguration(#[from] gix_submodule::config::path::Error),
79 #[error(transparent)]
80 WorktreeDirInaccessible(#[from] std::io::Error),
79 }81 }
80}82}
81 83
gix/src/submodule/mod.rs+22−6
@@ -224,12 +224,12 @@ impl Submodule<'_> { · 223 unmodified lines
224 ///224 ///
225 /// Also note that the returned path may not actually exist.225 /// Also note that the returned path may not actually exist.
226 pub fn git_dir_try_old_form(&self) -> Result<PathBuf, config::path::Error> {226 pub fn git_dir_try_old_form(&self) -> Result<PathBuf, config::path::Error> {
227 let worktree_git = self.work_dir()?.join(gix_discover::DOT_GIT_DIR);227 let worktree_gitdir_or_modules_gitdir = if self.worktree_gitdir()?.is_dir() {
228 Ok(if worktree_git.is_dir() {228 self.worktree_gitdir()?
229 worktree_git
230 } else {229 } else {
231 self.git_dir()230 self.git_dir()
232 })231 };
232 Ok(worktree_gitdir_or_modules_gitdir)
233 }233 }
234 234
235 /// Query various parts of the submodule and assemble it into state information.235 /// Query various parts of the submodule and assemble it into state information.
@@ -237,7 +237,7 @@ impl Submodule<'_> { · 1 unmodified line
237 pub fn state(&self) -> Result<State, config::path::Error> {237 pub fn state(&self) -> Result<State, config::path::Error> {
238 let maybe_old_path = self.git_dir_try_old_form()?;238 let maybe_old_path = self.git_dir_try_old_form()?;
239 let git_dir = self.git_dir();239 let git_dir = self.git_dir();
240 let worktree_git = self.work_dir()?.join(gix_discover::DOT_GIT_DIR);240 let worktree_git = self.worktree_gitdir()?;
241 let superproject_configuration = self241 let superproject_configuration = self
242 .state242 .state
243 .repo243 .repo
@@ -268,11 +268,27 @@ impl Submodule<'_> { · 24 unmodified lines
268 /// which may differ compared to the superproject's index or `HEAD` commit.268 /// which may differ compared to the superproject's index or `HEAD` commit.
269 pub fn open(&self) -> Result<Option<Repository>, open::Error> {269 pub fn open(&self) -> Result<Option<Repository>, open::Error> {
270 match crate::open_opts(self.git_dir_try_old_form()?, self.state.repo.options.clone()) {270 match crate::open_opts(self.git_dir_try_old_form()?, self.state.repo.options.clone()) {
271 Ok(repo) => Ok(Some(repo)),271 Ok(mut repo) => {
272 if repo.workdir().is_none() {
273 let wd = self.work_dir()?;
274 // We should always have a workdir, as bare submodules don't exist.
275 // However, it's possible for no workdir to be accessible if there is a symlink in the way.
276 // Just setting it by hand fixes this issue effectively, even though the question remains
277 // if this should work automatically.
278 // For now, let's *not* use the `self.worktree_git()` directory which has its own edge-cases,
279 // while the current solution yields the cleanest paths (i.e. it keeps relative ones).
280 repo.set_workdir(Some(wd))?;
281 }
282 Ok(Some(repo))
283 }
272 Err(crate::open::Error::NotARepository { .. }) => Ok(None),284 Err(crate::open::Error::NotARepository { .. }) => Ok(None),
273 Err(err) => Err(err.into()),285 Err(err) => Err(err.into()),
274 }286 }
275 }287 }
288
289 fn worktree_gitdir(&self) -> Result<PathBuf, config::path::Error> {
290 Ok(self.work_dir()?.join(gix_discover::DOT_GIT_DIR))
291 }
276}292}
277 293
278///294///
gix/tests/fixtures/generated-archives/make_submodules.tar+0−0

Binary file not shown.

gix/tests/fixtures/make_submodules.sh+16−0
@@ -63,6 +63,22 @@ git init submodule-head-changed-and-modified · 62 unmodified lines
63 )63 )
64)64)
65 65
66git init submodule-head-changed-and-modified-with-untracked-in-nested-dir
67(cd submodule-head-changed-and-modified-with-untracked-in-nested-dir
68 git submodule add ../module1 m1
69 git commit -m "add submodule"
70
71 mkdir -p a/b
72 touch a/b/untracked
73
74 (cd m1
75 git checkout @~1
76 echo change >> this
77 )
78)
79
80ln -s submodule-head-changed-and-modified-with-untracked-in-nested-dir/a/b link-to-dir-in-changed-parent-repo
81
66git init modified-untracked-and-submodule-head-changed-and-modified82git init modified-untracked-and-submodule-head-changed-and-modified
67(cd modified-untracked-and-submodule-head-changed-and-modified83(cd modified-untracked-and-submodule-head-changed-and-modified
68 git submodule add ../module1 m184 git submodule add ../module1 m1
gix/tests/submodule_open.rs+44−0
@@ -0,0 +1,44 @@
1#[test]
2#[cfg(feature = "status")]
3#[cfg_attr(
4 windows,
5 ignore = "for now, windows gets Os { code: 267, kind: NotADirectory, message: 'The directory name is invalid.' } (maybe because of relative symlink)?"
6)]
7fn on_nested_symlink() -> gix_testtools::Result {
8 let symlink_root =
9 gix_testtools::scripted_fixture_read_only("make_submodules.sh")?.join("link-to-dir-in-changed-parent-repo");
10 // Note: even though this refers to a symlink, the CWD that is actually set will be the resolved directory.
11 std::env::set_current_dir(&symlink_root)?;
12
13 assert!(
14 gix::open(&symlink_root).is_err(),
15 "this is not a repository directly, it's a directory within one"
16 );
17 // TODO(symlink): This should work though
18 // let repo = gix::discover(&repo_root)?;
19 let repo = gix::discover(".")?;
20 let sm = repo.submodules()?.into_iter().flatten().next().expect("one submodule");
21 assert_eq!(
22 sm.work_dir()?,
23 p("../../m1"),
24 "the workdir remains relative and is available"
25 );
26 let sm_repo = sm.open()?.expect("repo is present and accessible");
27 assert_eq!(sm_repo.git_dir(), "../../.git/modules/m1");
28 assert_eq!(
29 sm_repo.workdir().expect("worktree present as we have one"),
30 p("../../m1")
31 );
32
33 for item in repo.status(gix::progress::Discard)?.into_iter(None)? {
34 assert!(
35 item.is_ok(),
36 "{item:?}: if there was no worktree, the changed submodule changed would fail the status computation"
37 );
38 }
39 Ok(())
40}
41
42fn p(path: &str) -> &std::path::Path {
43 std::path::Path::new(path)
44}

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t