dgutov/diff-hl

Issue staging non-root project files

Closed this issue · 18 comments

Hi!
Thank you for the package, it's one of the must-haves in my emacs setup!

I am facing an issue that I don't how to resolve. Basically, when I try staging any hunk that is not at a project root-level I get an error:

vc-do-command: Failed (status 1): git --no-pager apply --cached ../../../../var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patch381lll

I can see that the issue is that the path to this patchfile file is incorrect - it is relative to the project root instead of the file I am currently in.
For instance, I just created a new dir:

mkdir new-project
cd new-proj
git init
touch new_file.py
mkdir subdir
touch subdir/subdir_file.py

Then I stage all the files and commit.
After that I go to emacs and I change the subdir_file.py and when I try to stage a hunk I get the mentioned error message.
I'm not proficient in elisp, but the stacktrace I get from evaluating (diff-hl-stage-current-hunk) seem to have the absolute path to the patchfile:

Debugger entered--Lisp error: (error "Failed (status 1): git --no-pager apply --cached ../../../../var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patchCA1ppj")
  error("Failed (%s): %s" "status 1" "git --no-pager apply --cached ../../../../var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patchCA1ppj")
  vc-do-command(#<buffer  *string-output*> 0 "git" "/var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patchCA1ppj" "--no-pager" "apply" "--cached")
  vc-git-command(#<buffer  *string-output*> 0 "/var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patchCA1ppj" "apply" "--cached")
  diff-hl-stage-diff(#<buffer something_else.md>)
  diff-hl-stage-current-hunk()
  eval((diff-hl-stage-current-hunk) t)
  #f(compiled-function () #<bytecode 0x13ab57324ef4cc12>)()
  #f(compiled-function () #<bytecode -0x5dab35955cb81d1>)()
  eval-expression((diff-hl-stage-current-hunk) nil nil 127)
  funcall-interactively(eval-expression (diff-hl-stage-current-hunk) nil nil 127)
  command-execute(eval-expression)

I am on mac.
Any suggestions how to resolve this?
Thanks.

Update:

Actually the first time I try to stage changes in subdir_file.py it works. However if I make more changes and then try staging - it doesn't.

I also tried to reproduce this without my configuration:

emacs -q --eval "(progn (require 'package) (add-to-list 'package-archives '(\"melpa\" . \"https://melpa.org/packages/\") t) (package-initialize) (require 'use-package) (use-package diff-hl :ensure t) (diff-hl-mode))"

I also tried the the solution mentioned here #197 - I added the expand-file-name to the patchfile and also shell-quote-argument - but it doesn't work

Hi!

You said that it works the first time, right? When you try to stage another change in the same file, does it overlap with the one that's already staged? If the new addition is somewhere else in the file, does it succeed then?

The first thing to try is customizing diff-hl-show-staged-changes to nil. Then the hunks would be computed against the state of the index (staging area).

The "stage hunk" command actually shows a message recommending to do the above after successful staging, if the variable is at its default value still.

Hello @dgutov!
Thanks for your response.

I have already set the diff-hl-show-staged-changes to nil. Here's my config for diff-hl:

  (use-package diff-hl
    :straight
    (:host github :repo "dgutov/diff-hl" :branch "master")
    :hook (prog-mode . diff-hl-mode)
    :general
    (leader-def :states 'normal
      "g" '(:ignore t)
      "gj" 'diff-hl-next-hunk
      "gk" 'diff-hl-previous-hunk
      "gs" 'diff-hl-stage-current-hunk
      "gr" 'diff-hl-revert-hunk
      "gv" 'diff-hl-show-hunk)
    :config
    (add-hook 'magit-pre-refresh-hook  #'diff-hl-magit-pre-refresh)
    (add-hook 'magit-post-refresh-hook #'diff-hl-magit-post-refresh)
    (setq diff-hl-draw-borders nil
          diff-hl-flydiff-delay 0.1
          diff-hl-show-staged-changes nil)
    (diff-hl-flydiff-mode)
    (diff-hl-show-hunk-mouse-mode)
    (diff-hl-margin-mode))

The hooks for magit I added recently just to see if they help.

The settings look reasonable. You could also check the current value of diff-hl-show-staged-changes using M-:.

I don't think diff-hl-flydiff-mode is the culprit, but you could also try turning it off.

And you didn't answer the first questions from my previous message:

When you try to stage another change in the same file, does it overlap with the one that's already staged? If the new addition is somewhere else in the file, does it succeed then?

If staging works at least once, then the relative file names should not be a problem (the path to patch file should be the same both times).

Also, when you describe the problem as having something to do with non-root files -- have you tried the same scenario with the same contents with a file directly inside the project root? Does staging behave differently?

The thing is, I've tried following your scenario step by step but haven't managed to reproduce the problem so far.

I'm on Linux not Mac, but it shouldn't be that different.

Oh - sorry, the question somehow slipped past me. To answer it - it doesn't matter which location I'm staging, I tried both - a completely different part of file and also if the changes overlap.
diff-hl-show-staged-changes is nil.
I also turned off the flydiff mode, just in case, but that didn't help either (saving the buffer before trying to stage any hunks).
Lastly, to answer your last question - yeah, I tried staging project-root level file hunk and it works as expected irrespective of the contents.
I had both files side by side - one is project root-level and the other is in a directory in a git project. Staging any changes in the project root-level file works flawlessly, while trying to stage any changes in a dir file always result in the same error.

Yeah it is strange that you're not encountering it. I don't see a reason why mac should be any different in this case. I couldn't find anyone else facing this, so my guess was that there's something wrong with my config. That's why I tried the minimal config:

emacs -q --eval "(progn (require 'package) (add-to-list 'package-archives '(\"melpa\" . \"https://melpa.org/packages/\") t) (package-initialize) (require 'use-package) (use-package diff-hl :ensure t) (diff-hl-mode))"
but had the same error.
So yeah, I'm kinda lost 😄

One more thing - staging hunks with magit works.

One more thing - staging hunks with magit works.

Yeah, that's good to know, but I didn't expect otherwise - we don't reuse any of its code here.

Staging any changes in the project root-level file works flawlessly, while trying to stage any changes in a dir file always result in the same error.

All right. What Git are you using? I have 2.40.1 here.

The patch file we generate are a little unusual because we put only base file name in it. But git is called from the directory that creates the file, so it has all the necessary info. But it might depend on the version of Git, I suppose.

You can try examining the file's contents and repeating the command in the terminal.

To make sure the file stays around after the error, you can comment out the delete-file call and re-evaluate:

diff --git a/diff-hl.el b/diff-hl.el
index 2a8bce5..23bb7c3 100644
--- a/diff-hl.el
+++ b/diff-hl.el
@@ -720,7 +720,8 @@ its end position."
                             patchfile
                             "apply" "--cached" )
             (setq success t)))
-      (delete-file patchfile))
+;      (delete-file patchfile)
+      )
     success))
 
 (defun diff-hl-stage-current-hunk ()

And to see which commands are launched, you can evaluate (setq vc-command-messages t).

In my case, when I try your scenario, the patch file contains this:

diff a/subdir_file.py b/subdir_file.py
--- a/subdir_file.py
+++ b/subdir_file.py
@@ -0,0 +1,11 @@
+
+asd
+asd
+as aaa
+d     
+asd
+a
+sd
+a
+sd
+asd

Yeah, I was using git 2.39, updated it, now using 2.44.0. Unfortunately, that didn't help.

Now, with (setq vc-command-messages t) this is the message buffer:

Loading /Users/my-user/.emacs.d/recentf...done
Cleaning up the recentf list...done (0 removed)
Copilot agent started.
Emacs loaded in 0.85 seconds with 1 garbage collections.
Quit
Running in foreground: git --no-pager diff-files -p -U0 new_file.txt
Done (status=0): git --no-pager diff-files -p -U0 new_file.txt
Quit
Running in foreground: git --no-pager diff-files -p -U0 subdir/subdir_file.txt
Done (status=0): git --no-pager diff-files -p -U0 subdir/subdir_file.txt
Running in foreground: git --no-pager ls-files -s subdir/subdir_file.txt
Done (status=0): git --no-pager ls-files -s subdir/subdir_file.txt
Running in foreground: git --no-pager apply --cached ../../../../var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patchQj5UPG
vc-do-command: Failed (status 1): git --no-pager apply --cached ../../../../var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patchQj5UPG

The patchfile looks like this:

diff a/subdir_file.txt b/subdir_file.txt
--- a/subdir_file.txt
+++ b/subdir_file.txt
@@ -1,4 +1,4 @@
 asdasd
-asdasd
+asasd

Running the command git --no-pager apply --cached ../../../../var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patchQj5UPG from project root I get the following error:
error: subdir_file.txt: does not exist in index

Running the same command from the subdir the error is:
error: can't open patch 'subdir/../../../../var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patchQj5UPG': No such file or directory

Lastly, running the same command with absolute path to patchfile:
git --no-pager apply --cached /var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patchQj5UPG
I get the same error about not file not existing:
error: subdir_file.txt: does not exist in index

That's quite interesting, the fille does exist.
I staged the changes with magit, commited them, the file is definitely there.

Running the same command from the subdir the error is

You're supposed to run it from the subdir indeed, with the relative path leading to the file.

If "no such file or directory" is the error that happens, that could be the clue. Is the relative path is built incorrectly somehow?

Yeah. From the subdir there's one level missing. The path should be
../../../../../var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patchEmRA2U

Running the git apply.. command from shell with this path works as expected.

But the path instead is
../../../../var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patchEmRA2U

Notice one ../ missing
.
That's what I meant in my first comment:

I can see that the issue is that the path to this patchfile file is incorrect - it is relative to the project root instead of the file I am currently in.

At least it seems that the git apply... command is executed on a wrong path and somehow the vc-do-command tries the root-level relative path.

And yet, the first time you stage a hunk in a file, it works fine?...

Here's how that file path is formed.

  1. In diff-hl-stage-diff, we take the previously constructed diff (that uses the base file name), generate the name for the patch file (using make-temp-file), writes it, and then call vc-git-command with that name in the original file buffer, assuming that default-directory is set to the normal value (the directory containing the file). That call happens in diff-hl-stage-diff. The patch file name is expected to be an absolute one -- I'm not sure when it would be relative.
  2. vc-git-command passes the file name unchanged to vc-do-command.
  3. vc-do-command makes the file name relative to the value of default-directory, which should still be the same here. It does that by evaluating (file-relative-name (expand-file-name f)).

So... my crystal ball here says that maybe make-temp-file returns a relative name in your configuration somehow? The path var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/ from your example is not familiar to me, but look like some sandboxing is involved. What does temporary-file-directory evaluates to in your Emacs?

If the relative temp file name is the problem anyway, perhaps this change would help:

diff --git a/diff-hl.el b/diff-hl.el
index 29dfe31..7f5564d 100644
--- a/diff-hl.el
+++ b/diff-hl.el
@@ -743,7 +743,7 @@ its end position."
         (with-current-buffer orig-buffer
           (with-output-to-string
             (vc-git-command standard-output 0
-                            patchfile
+                            (expand-file-name patchfile)
                             "apply" "--cached" )
             (setq success t)))
       (delete-file patchfile))

About the first time - I tried creating an additional file subdir/subdir_file_2.txt and after that staging the whole file with magit and comitting. Then I changed a hunk and tried to stage with diff-hl which didn't work (same error).
It seems to only work with a new git repository the first time.

temporary-file-directory is /var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/
default-directory is /Users/my-user/Projects/new-project/

So, I tried calling (make-temp-file "tst"), in the echo area the printed path is /var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/txtDfepHn which is absolute.

Lastly - with the suggested (expand-file-name patchfile) - no changes :/
I added printing of the patchfile just before vc-git-command:


 (with-output-to-string
             (message "PATCHFILE: ")
             (message patchfile)
             (vc-git-command standard-output 0
                            patchfile
                            (expand-file-name patchfile)
                             "apply" "--cached" )
             (setq success t)))

And the printed path is actually absolute even without expand-file-name:

PATCHFILE: 
/var/folders/fx/5yvx58_14kq7mqlfr4lgm2fm0000gn/T/diff-hl-stage-patchV8SKWp

It looks like an issue with the vc-git-command?

Oh my god. I found the culprit.
I actually had a hook in my config and I don't remember the reason I put it there:

  (add-hook 'find-file-hook #'(lambda () (setq default-directory (expand-file-name (project-root (project-current))))))

So I assume that after I use a find-file command the default-directory was set to the project-root and in turn either vc-git-command or vc-do-command commands didn't understand where the patchfile was located.
Removing the hook fixed the problem. What is bugging me still is the fact that running emacs without my configuration and only with diff-hl still produced the error.

Anyway, thank you for your patience and assistance. And sorry for bothering you

  1. vc-do-command makes the file name relative to the value of default-directory, which should still be the same here. It does that by evaluating (file-relative-name (expand-file-name f)).

This point actually led me to finding this out.

EDIT: I remembered why I put the hook - I wanted find-file to always start from a project's root 🤦🏻
Thank you again!

I actually had a hook in my config and I don't remember the reason I put it there

Oh, right. That's not a good idea.

And a reminder why it's recommended to ask for bug reports that start with emacs -Q. 🤪

Anyway, good that you found it.

EDIT: I remembered why I put the hook - I wanted find-file to always start from a project's root 🤦🏻

A much better solution would be to change the corresponding binding to a different command. Either project-find-file (it has a different interface, though), or your own simple wrapper for find-file.

(EDIT: Or just roll with how Emacs does things, even if it might feel odd after Vim.)