Magit-wip mode not working over TRAMP

[devonzh]

What happened?

Using magit-wip-mode over TRAMP never results in after-save commits.

What did you expect to happen?

When magit-wip-mode is enabled, the globalized minor mode magit-wip-after-save-mode should enable magit-wip-after-save-local-mode in all buffers where the turn-on function magit-wip-after-save-local-mode-turn-on returns true.

Steps to reproduce

1. Open a git-tracked file over TRAMP.
2. Enable magit-wip-mode in the buffer.
3. Evaluate the buffer local variable after-save-hook. I posit that it will not contain magit-wip-commit-buffer-file
4. Evaluate (magit-file-tracked-p buffer-file-name). I posit that it will return nil.

System information

GNU Emacs v29.0.92 Doom core v3.0.0-pre Doom modules v23.03.0-pre

(Yes, I know I’m on an unsupported Emacs version, but the lisp code in question has been in place for years so that’s probably not relevant.)

Analysis

The turn-on function magit-wip-after-save-local-mode-turn-on only enables magit-wip-after-save-local-mode if the following is true: (magit-file-tracked-p buffer-file-name). But it will never be true with that argument in a TRAMP buffer that is visiting a file over ssh/scp.

Elsewhere in the magit repo, the TRAMP issue has been addressed, for example in this commit, where the call has been changed to (magit-file-tracked-p (file-relative-name file)). This one does return t when visiting a git-tracked file via TRAMP.

It looks like the offending code was changed on 1 April 2023. While the commit message doesn’t mention the TRAMP issue and the commit is part of a performance-related refactoring, it looks like some (edited: all->some) references to the buffer file go through file-relative-name now.

I intend to bump the magit version pin and give it a shot, but before I open that can of worms, I wanted to see if anyone else has tried to use magit-wip over TRAMP in the last year or so. If it works for you, please report the values that result from “Steps to reproduce” above because in that case I’m missing some piece of the puzzle.

Update

Unpinning magit did not resolve the issue, which is apparent after looking more closely at the revised magit-wip-after-save-local-mode-turn-on code: it still calls (magit-file-tracked-p buffer-file-name).

It looks like upstream tried to resolve this issue for other parts of magit, but magit-wip was missed.

For now I tried to replace all occurences in magit-wip.el of (magit-file-tracked-p buffer-file-name) with (magit-file-tracked-p (file-relative-name buffer-file-name). It seems to be working, but it makes me nervous to make this change not knowing the codebase well. I’m usually pretty cavalier about such things, but a misbehaving magit-wip could very easily cause a major headache.

I’d still like to hear from someone else about this, if only to confirm the issue. But it looks like I’ll have to file a bug report upstream. I’m just surprised that I’ve found no mentions of this anywhere. Can it be that I’m the only person who noticed that magic-wip-mode doesn’t work over TRAMP?

[devonzh]

Update: I filed a bug report upstream at magit-wip-mode doesn't work over Tramp · Issue #4966 · magit/magit · GitHub

[devonzh]

Upstream resolved the issue. This commit changes the function magit-file-tracked-p to remove Tramp-related connection suffixes via (magit-convert-filename-for-git file)).

It looks like version pinning for magit, forge, code-review etc. has gotten a little complicated due to some upstream issues that were resolved at different times in different packages. It would be great to bump magit to include this fix, but I don’t have the context to do so safely without regressions in related packages.

In the meantime, cherry picking https://github.com/magit/magit/commit/2c2b34d7ac8a595be68702e53ff6f77755bfcd52 should be safe. For anyone doing this, remember to point both magit and git-commit at the same version of magit, since they both pull from the same repo. (And git-commit is processed before magit.)