Skip to content

Execute git commands in -C{path} when git cwd and toplevel are different#236

Closed
Kortantic wants to merge 2 commits into
dlyongemallo:mainfrom
Kortantic:cpath
Closed

Execute git commands in -C{path} when git cwd and toplevel are different#236
Kortantic wants to merge 2 commits into
dlyongemallo:mainfrom
Kortantic:cpath

Conversation

@Kortantic

@Kortantic Kortantic commented Jun 13, 2026

Copy link
Copy Markdown

Summary

I ran into a case where git -C <path> diff <rev> returned a proper diff, but :DiffviewOpen -C=<path> <rev> couldn't.

This PR makes DiffviewOpen -C=<path> behave the way git would in snapshot/worktree-like layouts where --show-toplevel and --git-dir do not resolve to the same directory.

Problem

Diffview was treating two concepts as the same path:

  • the directory git commands should execute from
  • the repo toplevel used for relative path resolution

-C usage works for normal repos, but breaks where:

  • git commands must run from the -C directory
  • git rev-parse --show-toplevel points somewhere else
  • git still emits repo-relative paths against the reported toplevel

My use case was against an OpenCode snapshot directory, but the underlying problem would occur in snapshot/worktree-like setups where git's cwd and its reported toplevel diverge.

While working through this, I hit another issue affecting single-rev commit-vs-local diffs. The RHS LOCAL file could collapse to diffview://null when it was mistakenly classified as a binary.

Fix

This PR separates the execution root from the path-resolution root in the case that toplevel and git-dir are not the same.

  • add ctx.exec_root for git command execution
  • keep ctx.toplevel for repo-relative path resolution
  • make parse_revs() resolve HEAD lazily so a COMMIT to LOCAL diff doesn't require a HEAD
  • test if a tracked LOCAL file is text before falling back to the existing untracked binary check

Result

This restores the expected behavior for snapshot/worktree-style -C diffs, including:

  • :DiffviewOpen -C=<snapshot> <rev>
  • :DiffviewOpen -C=<snapshot> <revA>..<revB>

In particular, the RHS LOCAL side now opens the real working-tree file instead of collapsing to a null buffer.

Note

This is not a general -C fix for ordinary repos; those already worked. The narrower issue here is that git's execution root and reported repo toplevel can differ, and Diffview was assuming they were always the same.

The main semantic change is:

  • toplevel stays the base for repo-relative paths
  • exec_root becomes the cwd for git operations

@dlyongemallo

Copy link
Copy Markdown
Owner

I use git worktrees and don't observe the problem as you described it. The divergence seems to happen only when core.worktree is set, but this setup isn't reproduced in your added test. Also, I think the is_binary bug is actually introduced by your exec_root change, which causes the untracked detector to misfire to begin with.

Are you sure the problem is with diffview+ and isn't due to some OpenCode setting or some other plugin? Can you share a minimum nvim config and repro case?

@Kortantic

Kortantic commented Jun 13, 2026

Copy link
Copy Markdown
Author

I think it stems from how OpenCode is building its snapshots. I assume it's using direct git-worktree calls. I do know that it ends up generating a dangling worktree that can be found with git fsck --lost-found, but doesn't link it to a commit. The thing is, git -C <snapshot-dir> <snapshot hash> is still able to render a diff vs the local file system and git -C <snapshot-dir> <snapshot-A>..<snapshot-B> works too, so I figured that :DiffviewOpen -C=<snapshot-dir> <snapshot>[..<snapshot>] should be able to as well.

I dug through DiffView's Log output and noticed it was showing that the CWD was getting reset to the git toplevel directory retrieved in get_toplevel(). So I ran git rev-parse --show-toplevel (from a terminal in the snapshot directory) and it returned the actual code repo, while git rev-parse --git-dir would report the snapshot directory. Diffview was using the result of find_toplevel() (which calls get_toplevel()) to set the git CWD, so I figured that's what was causing the split from git's functionality.

I'll take a look at the OpenCode repo to see if I can figure out the exact calls it's using to put its snapshot git directory into the state it's in to come up with a test that can reproduce it better. I'll also get you a minimum config and git repo to try it against. I'll have some more time this evening after the chores are done and kids are in bed, and probably tomorrow as well (since this may not be as cut and dry as I assumed).

@dlyongemallo

Copy link
Copy Markdown
Owner

So I ran git rev-parse --show-toplevel (from a terminal in the snapshot directory) and it returned the actual code repo, while git rev-parse --git-dir would report the snapshot directory.

Based on this, your PR doesn't fix the issue. Your exec_sync override every call that passed cwd = toplevel to exec_root, including those that legitimately need toplevel. This is what causes the is_binary bug to begin with, which is then patched to return the behaviour to the previous correct state.

I don't understand why you're overriding exec_sync, which breaks other things you're then forced to patch, when it sounds like all you really need to do is to pin an explicit -C directory which resolves to a different git dir than toplevel (e.g., using --git-dir and --work-tree in get_command).

@dlyongemallo

Copy link
Copy Markdown
Owner

I've fixed the issue with the divergence between the git dir and the worktree dir as you described it, in #238. Please let me know if this fixes the problem wtih OpenCode for you.

@Kortantic

Copy link
Copy Markdown
Author

Your commit fixes it. I agree entirely with the commit you made, it's cleaner and doesn't have the side effect of botching the binary check. I swung too far in the other direction with the idea that "cpath" means we should always execute git commands there. Thank you so much for solving it for me.

@Kortantic Kortantic closed this Jun 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants