Magit: Make magit more robust in the face of transient index.lock collisions

[jsyjr]

As I have become more proficient at using magit's support for interactive rebasing (and its moral equivalents, _e.g._ cF) I encounter increasingly frequent instances where magit abandons a rebase claiming that it could not acquire an index.lock. The diagnostic in the magit-process buffer says:

fatal: Unable to create '/path/to/workspace/.git/index.lock': File exists.

If no other git process is currently running, this probably means a
git process crashed in this repository earlier. Make sure no other git
process is running and remove the file manually to continue.

Historically - as instructed - I have simply rm-ed the file and resumed the rebase. Lately though I have started checking to see if the file actually exists and have yet to find and instance when it does. This leads me to believe that magit is aggressively issuing a subsequent git command before a (possibly asynchronous?) portion of the previous command has fully cleaned up.

If my hypothesis is correct then magit could be made more robust by adding logic to special case index.lock failures. I suggest spinning for a limited amount of time waiting for the lock to disappear before reporting a failure. If the lock does disappear on its own then magit can simply retry the original git command.

[npostavs]

Do you have auto revert enabled and vc-git enabled? This could be a manifestation of Emacs bug #21559

[jsyjr]

Apparently I do have auto-revert-mode indirectly enabled via (now obsolete) magit-revert-buffers. My guess is vc-git is getting involved since my mode-line announces when a file is tracked by git. Thus it would appear that this is indeed Emacs bug #21559 (a very interesting thread).

Are there any magit-recommended best practices? Any planned counter-measures?

I have just switched to magit-auto-revert-mode but have not yet attempted a rebase. I assume that I will see no improvement.

[npostavs]

Are there any magit-recommended best practices? Any planned counter-measures?

Maybe there is some way to disable one of vc-git or auto revert while rebasing? I don't have auto revert enabled, so I never bothered investigating.

I have just switched to magit-auto-revert-mode but have not yet attempted a rebase. I assume that I will see no improvement.

magit-revert-buffers is obsolete, but its effect is identical to magit-auto-revert-mode, so indeed no change should be expected.

[tarsius]

@jsyjr Can you still reproduce this issue?

I had the same issue in late spring and I believe it was due to changes in Git. I then applied some countermeasures in f6601b3c04f823454457ce7bb6c5ceda930f2301. That was before you opened this issue though. Maybe you didn't update before opening this issue? But that commit only affects fixup/squash rebase variants, so maybe this is a different (though related) issue. Personally I have not seen any race conditions during rebases since I have applied that commit (if I remember correctly, I have just returned from a six week break during which I did not work on, or use, Magit).

[tarsius]

I think this issue is no more, so I am closing. Please reopen if the issue persists.

[jsyjr]

Sorry about being slow to reply. Combination of vacation and being more focused on design work these days.

When I was having my problems I was performing many magit "Instant Fixup" commits, sometimes digging 10, 20 or 30 commits into local history. I agree that this issue now seems to have disappeared. In the interim I have updated both git and magit. No sure which deserves credit.

Bottom line: I agree that the issue can be closed.

[rdgreen]

I received the index.lock error while doing a rebase from magit, with magit 20160902 from melpa.

[rdgreen]

git version 2.7.4

[tarsius]

while doing a rebase

Which particular flavor of rebase? For me this never happens anymore, so it would help to know which flavor to use when trying to reproduce this.

[rdgreen]

I pressed r for rebase, e for elsewhere and then entered master as the branch to rebase my branch onto.

[tarsius]

Magit does not start any other git process while the rebase process is running. Are you getting this error a lot or did you only get it once or twice? If you get this a lot, then try pausing briefly _before_ initiating the rebase, maybe something is _still_ running at that point (push or such).

[jsyjr]

My use of magit has changed. Back when I was getting the problem I was
grooming a stack of ~40 commits. I was regularly doing instant fixup deep
into that stack. It seemed to be the process of rebasing tens of commits
that lead to the issue.

I am no longer seeing the issue. Currently I am doing active development.
Because I am not groom work for review and submission I am doing minimal
rebasing.

HTH,

/john

On Tue, Sep 20, 2016 at 6:26 AM, Jonas Bernoulli [email protected]
wrote:

Magit does not start any other git process while the rebase process is
running. Are you getting this error a lot or did you only get it once or
twice? If you get this a lot, then try pausing briefly _before_
initiating the rebase, maybe something is _still_ running at that point
(push or such).

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/magit/magit/issues/2708#issuecomment-248262843, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABPhmyH0p3HA7o8X2DGK880YFrE1JvMgks5qr7TmgaJpZM4I9x_o
.

[tarsius]

Thanks @jsyjr, but the question was intended for @rdgreen ;-)

[tarsius]

Please let me know when you have more information. I just cannot reproduce this and still suspect that something already running _before_ the rebase starts is the issue. You might also want to look into any git hooks that might be triggered during the rebase.

[rdgreen]

I am still experiencing this regularly with the latest magit. I know that it is auto-revert-mode's fault because I have a manual workaround that I have plumbed in with advice-add, and when I activate the manual workaround, this problem never happens.

[tarsius]

What's the advice doing (maybe just post it)?

[rdgreen]

I have a personal script called pullreq which creates a temporary file called .pullreq in the repository root to let emacs know it is running. In my .emacs I have this advice to handle that:

(defun auto-revert-handler--unless-pullreq ()
  "If pullreq is running, then do nothing."
  (let ((root (vc-root-dir)))
    (and root (file-exists-p (concat root ".pullreq")))))

; Try to prevent us from interfering with the pullreq script
(advice-add 'auto-revert-handler :before-until
            'auto-revert-handler--unless-pullreq)

So as a manual workaround for this issue, I can just do touch .pullreq in a shell, then do a rebase in magit, and then delete the .pullreq file when I'm done.

[Alexander-Shukaev]

Lately, I'm having these issues aggravated with new Magit and/or Git. It seems to happen even without invoking Git manually but only due to Magit itself. I still can't understand how it can happen and how to reproduce it. Though, any time I want to stage something through the Magit status buffer after long editing cycle, I hit the

Unable to create '*/.git/index.lock': File exists.

problem. Is it a known issue with the new Git and/or Magit? Is it a combination of my settings? Let me know which part of configuration would be relevant to report here.

[tarsius]

Yeah, I've been getting that too again. Reopening.

[Alexander-Shukaev]

Any news on this one? Literally struggling every day with those locks.

[tarsius]

This is a top priority now. I'll start working on it very soon. (Actually meant to come here to say that on my own, but since it is "very soon" not "right now", I decided against it.)

[Alexander-Shukaev]

I see, thank you for the quick turn. Let me know if some additional information would be needed. Keep up the good work! :+1:

[mishoo]

I get this too every now and then, seems random. Today I switched to command line, I made sure that .git/index.lock does not exist, and I ran git rebase, and I still got the same error. This suggests the issue might be in Git, not in Magit.

I aborted my rebase and retried from the command line. I kept getting the error 2-3 times, but then it magically worked.

[darkfeline]

@mishoo The problem isn't in Git (arguably) and not in Magit either (arguably), but in vc-git that ships with Emacs. (Emacs bug #21559)

Briefly, vc-git adds a hook to auto-revert for files under Git that checks whether the current file has a merge conflict and enables smerge-mode. This check grabs the index lock. When a rebase is done, Git releases the index lock in between each cherry-pick. This gives auto-revert a chance to steal the lock, which causes the next operation in the rebase to fail.

The solution posted in the bug that rewrites the conflict check to not grab the index lock has worked for me so far.

[Alexander-Shukaev]

I have disabled VC long ago. Does not solve the problem though.

[tarsius]

@darkfeline Thanks, that's very useful (but a lot to take in). Though one could still argue that git rebase should hold the lock until done, i.e. that it is a bug in git, and magit/vc just fail to work around it.

@Alexander-Shukaev Which might mean that magit has a similar issue than VC somewhere, but which kicks in less often (since it appears not to for @darkfeline). I actually had VC enabled, because disabling it would result in smerge not being automatically turned on (visiting from the status buffer still does though). Ironically that appears to be the feature that triggers the issue on VC's side.

[tarsius]

Even if magit itself happened to have a similar issue, the vc issue still has to be fixed in magit because many magit users do not turn off vc and also won't immediately update to the yet to be released Emacs with the fix.

So I will likely redefine vc-git-conflicting-files in magit, applying a patch like this (taken from debbugs):

 (defun vc-git-conflicted-files (directory)
   "Return the list of files with conflicts in DIRECTORY."
   (let* ((status
-          (vc-git--run-command-string directory "status" "--porcelain" "--"))
+          (vc-git--run-command-string directory "diff-files" "--name-status"))
          (lines (when status (split-string status "\n" 'omit-nulls)))
          files)
     ;; TODO: Look into reimplementing `vc-git-state', as well as
     ;; `vc-git-dir-status-files', based on this output, thus making the
     ;; extra process call in `vc-git-find-file-hook' unnecessary.
     (dolist (line lines files)
-      (when (string-match "\\([ MADRCU?!][ MADRCU?!]\\) \\(.+\\)\\(?: -> \\(.+\\)\\)?"
-                          line)
+      (when (string-match "\\([ MADRCU?!]\\)[ \t]+\\(.+\\)" line)
         (let ((state (match-string 1 line))
               (file (match-string 2 line)))
           ;; See git-status(1).
-          (when (member state '("AU" "UD" "UA" ;; "DD"
-                                "DU" "AA" "UU"))
+          (when (equal state "U")
             (push (expand-file-name file directory) files)))))))

[Jiri-Macha]

My observation:

I had the same symptoms - was unable to make interactive rebase ~35
patches. The rebase always stopped (was reproducible) somewhere in
the middle of the list of patches (was a bit stochastic). Many files
from the list of ~35 patches ware opened in Emacs.

After hint of somebody above I set "Magit Auto Revert Mode" from default
value ON to the new value OFF and the problem with .git/index.lock
file disappeared. Magit version: 2.10.3

[tarsius]

I've done that now.

[tarsius]

I'll leave this open for a while until we know whether we have to look for a similar bug in Magit itself.

[Alexander-Shukaev]

I can confirm that this didn't change anything for me as expected. As I mentioned before, I have vc disabled completely long ago, so I guess the issue is somewhere else. I could of course try to bisect my Emacs configuration or downgrade magit to see if that helps identifying the issue. Which action would you recommend to take first?

[lddubeau]

I ran into this problem for the first time a few minutes ago. I was getting 100% failure on a fairly long rebase stack similar to one described above. I took the fix introduced by cafd209ce7f4d9da4b483f29da2ea1e9ebd8b44c, plopped it into my .emacs, ran eval on the code, and the issue was immediately gone.

[Alexander-Shukaev]

Ever since syohex/emacs-dired-k#45, I decided to roll without dired-k for some time to give it a try and the problem is gone for me. I only got this one once or twice throughout that long period of time and those were probably caused by some other external interactions. It looks like this can be deemed as fixed for now unless somebody else has complaints remaining.

[michael-heerdegen]

I'm regularly seeing the problem. I'm using the current Magit version from Melpa and an up-to-date master Emacs. Could be that some of my own config is involved. Is there a way to find out what other thing caused git to create the lock?

Anyway, the problem with this is that for me, rebasing can loose commits: when I continue the rebase after getting the message about the lock, it can apparently happen that the result has left out a commit, and there is no sign that this happened. This is very dangerous because it can make you loose some of your work without any indication.

[greenrd]

I have also experienced the rebase-loses-commits issue with magit from melpa (on Windows).

[deerpig]

I'm getting this problem now five or ten times a day. I have a bash script that I run to look for *.lock files and delete them. I just did a clean install of emacs on another machine with a clean install of Debian and everything installed new from use-package and I'm still getting the lock problem. For me at least this does not seem to be related to rebase, which I seldom use.

But now it actually hangs emacs two or three times a day. This could be another issue, but happens after I've made a commit and before I can do a git push and then emacs freezes solid and has to be killed and restarted.

[dkm]

2.11.0-110-g3adc1dc4, emacs 26.0.50, still have the issue when doing rebase. My solution is to do rebase on cli and suspend emacs with ^Z during the process.

[michael-heerdegen]

I wonder if diff-hl could be responsible for the locks in my case. AFAIK it calls git after every buffer revert.
I'll try to find that out.

Still, would be good to get Magit to handle this case better. In particular, the commit (and work) losing behavior is bad.

[tarsius]

I have started looking into this again. So far I have only read up on what has been said here before and on debbug #21559. There has been new activity on the bug and it actually was closed two weeks ago.

TL;DR Update to Git 2.15.0 and rebuild Emacs from the master branch. That should at least make this issue less likely to occur.

I am quoting a few key passages from that issue.

---

In 21559#44 an important question is raised:

I think the real question is: why is vc-find-file-hook (or
vc-refresh-state) trying to mutate the index? Shouldn't refreshing a
buffer be a read-only operation (at least with respect to the VCS)?

You probably want to skip comments before that

---

In 21559#71 it is established that underlying all this is an issue in Git itself.

This problem is really a git bug. For any VCS, I should be able to, in
one terminal, run

while true; do $VCS status; done

and in another terminal, run

$VCS any-damn-operation

and not cause repository corruption or mysterious operation failures.
Maybe it's a good idea for Emacs to work around this particular bug in
git, but there is nothing semantically wrong with what vc-git is doing here.

This is true and means that no matter what changes are made in Emacs and or Magit we can never be sure that nothing else ^1 holds the lock while a rebase is in progress. All we can do is make sure that VC, auto-revert and Magit don't do it themselves.

The only way this could be prevented is to rewrite git rebase in C so that it can hold the lock until it is done. (Or implement some "nobody may grab the index log except for child process of this process" secondary lock.)

^1 Including but not limited to other Git-related Emacs packages, other front-ends, git-hooks, Git-aware shell prompts...

---

As far as auto-revert and vc-git vs. git rebase (anywhere) was concerned, I though this issue was already fixed with the change proposed in 21559#74. That has been merged into Emacs and for older Emacsen Magit redefines that function to use this improved version.

However it turns out only one instance of vc-git needlessly holding the lock was fixed.

---

After one and a half years the conversation resumes with 21559#77.

---

The patch proposed in 21559#131 gets merged and the issues is closed a day later.

---

I haven't decided yet whether I am going to backport that fix too. I guess it depends on whether this makes a difference to anyone here.

[dkm]

Following your answer, I've rebuilt my emacs and git to latest version, and as far as I've been able to test, it just works. I've been doing some rebases over 10s of commits without any problem.

As far as I'm concerned, there is no need to backport anything as it's working as expected. Thanks for the time spent on the issue (and for magit in general!)

[mishoo]

This issue didn't occur to me either in a long time, I guess it's safe to say it's somehow fixed.

(Edit: I'm using Emacs and Git from the standard Arch Linux repos; M-x magit-version reports "Magit 20171009.1252, Git 2.16.2, Emacs 25.3.1, gnu/linux")

[jsyjr]

Been doing "one man" development for quite some time now so minimal
opportunities for rebasing.
My git is 2.14.1 so I will upgrade to 2.15.0 or later just for good measure.

Rather than spend any time on backporting my vote is to push forward with
your grand vision.

/john

On Tue, Mar 6, 2018 at 9:43 AM, Jonas Bernoulli [e-mail]
wrote:

I have started looking into this again. So far I have only read up on what
has been said here before and on debbug #21559
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21559. There has been new
activity on the bug and it actually was closed two weeks ago.

TL;DR Update to Git 2.15.0 and rebuild Emacs from the master branch. That
should at least make this issue less likely to occur.

I am quoting a few key passages from that issue.

In 21559#44 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21559#44 an
important question is raised:

I think the real question is: why is vc-find-file-hook (or
vc-refresh-state) trying to mutate the index? Shouldn't refreshing a
buffer be a read-only operation (at least with respect to the VCS)?

You probably want to skip comments before that

In 21559#71 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21559#71 it
is established that underlying all this is an issue in Git itself.

This problem is really a git bug. For any VCS, I should be able to, in
one terminal, run

while true; do $VCS status; done

and in another terminal, run

$VCS any-damn-operation

and not cause repository corruption or mysterious operation failures.
Maybe it's a good idea for Emacs to work around this particular bug in
git, but there is nothing semantically wrong with what vc-git is doing
here.

This is true and means that no matter what changes are made in Emacs and
or Magit we can never be sure that nothing else holds the lock while a
rebase is in progress. All we can do is make sure that VC, auto-revert
and Magit don't do it themselves.

The only way this could be prevented is to rewrite git rebase in C so
that it can hold the lock until it is done. (Or implement some "nobody may
grab the index log except for child process of this process" secondary

lock.)

As far as auto-revert and vc-git vs. git rebase (anywhere) was concerned,
I though this issue was already fixed with the change proposed in
In 21559#74 https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21559#74.
That has been merged into Emacs and for older Emacsen Magit redefines that
function to use this improved version.

However it turns out only one instance of vc-git needlessly holding the

lock was fixed.

After one and a half years the conversation resumes with 21559#77

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21559#77.

The patch proposed in 21559#131
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21559#131 gets merged and

the issues is closed a day later.

I haven't decided yet whether I am going to backport that fix too.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/magit/magit/issues/2708#issuecomment-370803329, or mute
the thread
https://github.com/notifications/unsubscribe-auth/ABPhmwanduoZbNE_EAFYA6AMH-SePWB9ks5tbqCJgaJpZM4I9x_o
.

--
John Yates
505 Tremont St, #803
Boston, MA 02116

[dkm]

After some more usage, I can still reproduce the problem... :/ Clearly it happens less often as it works most of the time... But still :/

[michael-heerdegen]

Yes, still happens to me, too.

[phil-s]

This is very vague, but I used to encounter this error quite regularly, and I feel that for a while it stopped being a problem (not 100% about that), but it's definitely been happening regularly again in more recent times. I can't be more precise about times, I'm afraid. If the error occurs I can always just type r r to continue (repeatedly, if necessary) until the rebase completes.

I'm running Git 2.6.6 though. I shall update to 2.15 as per https://github.com/magit/magit/issues/2708#issuecomment-370803329

I can see that the fix to Emacs is necessary to utilise that new feature in Git. I'm using Emacs 26 at present, and wasn't planning to switch to an unstable 27 build; however the patch at https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21559#131 looks applicable to 26.1 as well, so I'll apply that and see how I get on.

[phil-s]

Interesting -- I've done both of those things, and I just hit the issue again during a rebase. Magit seems to be seeing the new Git version ok:

  0 git --version
git version 2.16.3

The patched auto-revert-handler and vc-git-command are loaded (I can see revert-buffer-in-progress-p in their byte-compiled symbol-function output).

Perhaps patching 26.1 is not sufficient?

[phil-s]

A quick proof-of-concept in postponing calls to auto-revert-buffers when Magit is running git asynchronously. Thus far this has completely resolved the problem for me, although it's possible there are issues I've not yet encountered. I'm posting it now partly in case other users find it useful, and partly in case it prompts better ideas.

diff --git a/lisp/magit-process.el b/lisp/magit-process.el
index 1356c2b2..6b3f340c 100644
--- a/lisp/magit-process.el
+++ b/lisp/magit-process.el
@@ -501,6 +501,14 @@ See `magit-start-process' for more information."
     (apply #'magit-start-process magit-git-executable input
            (magit-process-git-arguments args))))

+(defvar postpone-auto-revert-buffers nil)
+
+(defadvice auto-revert-buffers (around maybe-postpone-auto-revert-buffers)
+  (if postpone-auto-revert-buffers
+      (run-with-timer 1 nil 'auto-revert-buffers)
+    ad-do-it))
+(ad-activate 'auto-revert-buffers)
+
 (defun magit-start-process (program &optional input &rest args)
   "Start PROGRAM, prepare for refresh, and return the process object.

@@ -519,6 +527,7 @@ After the process returns, `magit-process-sentinel' refreshes the
 buffer that was current when `magit-start-process' was called (if
 it is a Magit buffer and still alive), as well as the respective
 Magit status buffer."
+  (setq postpone-auto-revert-buffers t)
   (-let* (((process-buf . section)
            (magit-process-setup program args))
           (process
@@ -966,6 +975,7 @@ Limited by `magit-process-error-tooltip-max-lines'."

 (defun magit-process-finish (arg &optional process-buf command-buf
                                  default-dir section)
+  (setq postpone-auto-revert-buffers nil)
   (unless (integerp arg)
     (setq process-buf (process-buffer arg))
     (setq command-buf (process-get arg 'command-buf))

A couple of other thoughts (which may or may not be applicable to any final code).

• We could register whether a call to auto-revert-buffers actually happened while the lock was in place, and arrange to call it immediately rather than waiting on the timer; but with such a short interval (1s) between tests, the simpler approach seemed adequate (and 1s is probably much longer than it needs to be).

• We could push and pop postpone-auto-revert-buffers values to account for multiple concurrent async processes (i.e. only release the lock when all processes have returned), but that's possibly not actually a good idea, as it's possible for a pending process to be timing out, and the task in question may since have been achieved by a subsequent process which has completed. For example, sometimes I forget to open a VPN connection before pushing over that, then I realise what I've done, open the connection, and push again; but the original process is going to continue to hang around for quite a long time -- which I currently don't care about enough to bother killing it, but I wouldn't want it to continue to interfere with other functionality after I've achieved the thing I was trying to do; so my feeling is that the simpler boolean flag is the better option of those two approaches in practice.

  Of course maybe all attempts are timing out for some reason not under the user's control, in which case we are stuck waiting for the timeout, which isn't great. I can't think of a solution to that, other than to have a configurable timeout for the lock itself, and accept that race conditions can occur in those scenarios. Which, of course, isn't any worse than the original situation, so it's probably fine.

  If using a timeout on our lock, then maybe the process filter could be useful for identifying that Git activity is in fact taking place. The timeout could then default to a not-too-excessive value (say 20s), and the process filter could reset it every time it handled some activity. That way we time out in a reasonable duration if absolutely nothing is happening, but in the case of a git process which simply has a great deal of work to do, the lock would hopefully remain set for as long as needed. That relies on the process filter actually receiving input on a regular basis, though, and in practice for a large operation there might be long periods of time where no input is received?

  I figure there's a lot of "how long is a piece of string" guess-work in deciding what a reasonable timeout value is, and chances are that any given default would prove inadequate for someone, depending on repository sizes, network speeds, etc, etc. Regardless, I feel that some kind of configurable workaround would be significantly better than nothing. And even if Emacs 27 handles this out of the box, a stable 27.1 release is a long way off, so most users are not going to benefit from that any time soon.

[phil-s]

I'm wondering whether triggering my lock in magit-process-filter would be an improvement on doing so in magit-process-start? My theory is that this would avoid my concern about locking for processes which are timing out on an initial network connection, but still provide locking in the cases when files are getting modified. This would depend on there always being some process output before files start getting modified, though, which seems a little bit brittle. I'm not sure whether we expect such an assumption to hold or, even if it does, whether the process filter would reliably be called before auto-revert kicked in.

(I've started using this variant now, so I'll see whether I run into any problems.)

The other thought I had on avoiding the connection timeout problem was to test for the process-status of the process, on the basis that connect is a documented status; but I thought the process sentinel would be triggered for all status changes, and I never saw a connect status in my initial testing; so either it happened too fast (perhaps before the sentinel was even attached), or the sentinel doesn't actually trigger on that state change. (edit: I'm probably mistaken in thinking that a connect status would be relevant, come to think of it -- the relevant connection is being handled by Git, not Emacs.)

Regardless, post-connection timeouts are still possible of course, so the initial connection isn't the sole concern here; but it's certainly a notable one.

[phil-s]

I've finally run into an issue with this hack of mine -- I initiated a commit, and then got distracted, and ended up going to lunch without committing. I don't think I was really aware that having the commit message buffer open for editing meant that an async git process was running (although it's more obvious in hindsight). When I got back from lunch, Emacs was almost (but not quite) completely unresponsive. With a bit of patience I identified that there were 800 timers running for auto-revert-buffers :) Whoops!

A quick (or rather, extremely slow) C-c C-c in the commit message buffer meant that the async process exited, and things returned things to normal.

I don't want to just call auto-revert-set-timer instead, as that might cause a lengthy delay, but I can create a separate timer object for this purpose, and ensure that it remains singular. Or maybe I can play with updating-and-subsequently-restoring the auto-revert-interval value.

Currently trying the latter idea, like so:

diff --git a/lisp/magit-process.el b/lisp/magit-process.el
index 1356c2b2..bc25dfa5 100644
--- a/lisp/magit-process.el
+++ b/lisp/magit-process.el
@@ -501,6 +501,30 @@ See `magit-start-process' for more information."
     (apply #'magit-start-process magit-git-executable input
            (magit-process-git-arguments args))))

+(defvar postpone-auto-revert-buffers nil)
+
+(defvar postpone-auto-revert-interval nil)
+
+(defadvice auto-revert-buffers (around maybe-postpone-auto-revert-buffers)
+  "Delay `auto-revert-buffers' if `postpone-auto-revert-buffers' is non-nil."
+  (if postpone-auto-revert-buffers
+      ;; Do not run `auto-revert-buffers', but make its timer run more
+      ;; frequently in the meantime, so that it will run promptly once
+      ;; it's safe.  Remember the original `auto-revert-interval'.
+      (unless postpone-auto-revert-interval
+        (setq postpone-auto-revert-interval auto-revert-interval)
+        (setq auto-revert-interval 0.5)
+        (auto-revert-set-timer))
+    ;; We are no longer postponed, so restore the original
+    ;; `auto-revert-interval', and run `auto-revert-buffers'.
+    (when postpone-auto-revert-interval
+      (setq auto-revert-interval postpone-auto-revert-interval)
+      (setq postpone-auto-revert-interval nil)
+      (auto-revert-set-timer))
+    ad-do-it)) ;; Run `auto-revert-buffers'.
+
+(ad-activate 'auto-revert-buffers)
+
 (defun magit-start-process (program &optional input &rest args)
   "Start PROGRAM, prepare for refresh, and return the process object.

@@ -662,6 +686,7 @@ Magit status buffer."

 (defun magit-process-filter (proc string)
   "Default filter used by `magit-start-process'."
+  (setq postpone-auto-revert-buffers t)
   (with-current-buffer (process-buffer proc)
     (let ((inhibit-read-only t))
       (magit-process-yes-or-no-prompt proc string)
@@ -966,6 +991,7 @@ Limited by `magit-process-error-tooltip-max-lines'."

 (defun magit-process-finish (arg &optional process-buf command-buf
                                  default-dir section)
+  (setq postpone-auto-revert-buffers nil)
   (unless (integerp arg)
     (setq process-buf (process-buffer arg))
     (setq command-buf (process-get arg 'command-buf))

[dandavison]

I previously had this problem, then for a while it went away (emacs 25, magit was on a commit from March I believe), and now with Emacs 26 it's back for me and causing a lot of trouble.

Magit 2.2.2-2485-g2196dc60, Git 2.18.0, Emacs 26.1, darwin

It would be fantastic to have a robust solution/workaround -- the magit rebase-based commands are really excellent, and I use them many times a day.

[phil-s]

it's back for me and causing a lot of trouble.

I recommend using my patch (above). I think it will eliminate all your rebasing problems. (It certainly has done for me.)

[dandavison]

@phil-s thanks, I tried applying your patch. However, magit-process-filter is not called with the workflow that I am having problems with. What I'm doing is:

1. Issue EDITOR=emacsclient git rebase --interactive from the shell
2. In the git-rebase-todo buffer, edit the commit list and issue C-c C-c

[phil-s]

I think you're essentially not using Magit in that workflow at all?

Or at any rate, if the git process wasn't created by Emacs, there's not much that Emacs it can do about it, as it doesn't know when the process has completed.

Is there a reason you don't trigger your rebases from Magit? It's a very nice streamlined experience. If it's purely a matter of it not being a habit, I highly recommend making it one. In your case you would move point to a commit and type r i

[dandavison]

I think you're essentially not using Magit in that workflow at all?

When I issue EDITOR=emacsclient git rebase --interactive from the shell, it brings up a buffer with major mode git-rebase-mode -- this major mode is defined by Magit.

Or at any rate, if the git process wasn't created by Emacs, there's not much that Emacs it can do about it, as it doesn't know when the process has completed.

It seems that, as discussed elsewhere in this thread, the error is due to Emacs' and/or Magit's auto-revert modes. The following workaround seems to allow me to do rebases without error: disable auto-revert mode while the rebase is in progress. Currently I'm doing that with the shell script below but perhaps I could advise something in the git-rebase flow.

Is there a reason you don't trigger your rebases from Magit? It's a very nice streamlined experience. If it's purely a matter of it not being a habit, I highly recommend making it one. In your case you would move point to a commit and type r i

Unfortunately I work routinely in a repo with several hundreds of thousands of lines of code and many commits. Although I use Magit for all staging, committing etc activity, some actions (in particular viewing the log) are too slow for Magit to replace all shell-based git interactions. I have worked on optimizing magit-status speed, but viewing the log (C-c g l l) is still slow.

#!/bin/bash

emacsclient --no-wait \
    --eval "(magit-auto-revert-mode $1)" \
    --eval "(global-auto-revert-mode $1)" \
    > /dev/null

[phil-s]

perhaps I could advise something in the git-rebase flow

If you've not done so already, you can easily combine those things into a single shell script which disables auto-revert, initiates the rebase and, given a successful exit status, re-enables auto-revert?

Speculation: Call that script git-rebase and have it invoke the real git-rebase command (passing "$@"), and maybe that Just Works, including handling the continue and abort scenarios??

[phil-s]

A quick test suggests that I'm wrong about overriding git rebase with a custom script, so you probably would need to call the custom script directly instead of using git rebase; however the rest of the idea seems sound:

#!/bin/sh
EDITOR=emacsclient
export EDITOR
emacs-auto-revert -1
git rebase "$@" && emacs-auto-revert 1

Where emacs-auto-revert is your other script for switching those modes on and off.

[aeruder]

Alright, did a bunch of digging and have discovered a problem in the upstream fix for Bug 21559.

I am hoping that the following commit should take care of this!

https://github.com/aeruder/emacs/commit/930aaa9808ed174d64feeb1c70f6f6a83a9e8e01
You'll still need https://github.com/aeruder/emacs/commit/cb55ccae8be946f1562d74718086a4c8c8308ee5 if you are starting from 26.1.

I still need to figure out the contribution guide and get this submitted to emacs upstream.

@phil-s @dandavison

[phil-s]

Oh, excellent. I'll apply that and revert my hack and see what happens.

I can see that you've re-opened that bug, so pretty much just send email to the address for the bug ID, and I would suggest that you CC the people who were previously involved in that discussion.

If you've not contributed code to Emacs before then this patch is small enough that it won't require copyright assignment; but I'd recommend doing that regardless. Just write to assign at gnu.org and say that you'd like to assign copyright for patches to GNU Emacs, and they'll sort it out with you.

Your commit message looks pretty good, but if you've not seen it then https://www.gnu.org/prep/standards/html_node/Change-Logs.html is worth reading (or keeping for reference) regarding commit message formatting.

[phil-s]

@aeruder I am convinced that your patch fixes the issue. Thank you!!

I repeated rebased a moderately large branch with and without your patch. Without your patch I was encountering the "Unable to create '.git/index.lock': File exists." error between 3 and 5 times during each rebase. With your patch I did not encounter the problem at all in any of the rebases.

[aeruder]

Perfect! Thanks for the very helpful instructions on submitting to upstream. I believe I've gotten everything sent in now and eventually they'll show up on https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21559 .

[tarsius]

Does 1a3671c68238916210c647e69676fba04f9b24ed look like a reasonable backport?

I am not using define-advice inside the cl-eval-when because that expands to what I have placed there except that the advice functions are sharp-quoted like so:

(advice-add 'vc-git-command :around #'vc-git-command@bug21559)

which leads to

In end of data:
magit-utils.el:1064:1:Warning: the following functions are not known to be
    defined: vc-git-command@bug21559, auto-revert-handler@bug21559

I am assuming that this is because the check for undefined functions happens at read-time and so suppressing evaluation of the body at compile-time does not prevent this. (If I add compile to cl-eval-when, then there also is no warning).

[phil-s]

I just tried to test master vs the bug21559 branch in Emacs 25.3 and failed to trigger the bug even with master. The upstream bug was logged against Emacs 25.0.50 (and a cursory look showed that both of the current affected functions did exist in 25.3), so I guess there must be other factors at work here.

I'll make a point of doing some more testing sometime later in the 26.1 instance where I'm easily reproducing the issue, with your backport applied and the upstream patches reverted.

[dandavison]

@aeruder I'm running with your commit cherry-picked on top of master and I have not experienced the lock file problems in any rebases so far. Thanks very much!

[aeruder]

Alright, patch is applied in upstream emacs, too.

[phil-s]

@tarsius Confirming that your advice appears to be working -- with both the upstream patches reverted I am again easily reproducing the error in my 26.1 session; and with your advice then added, the issue is once again resolved.

With the new patch now committed to upstream master, you probably want to make all of the advice conditional on (< emacs-major-version 27) ?

[phil-s]

I think the backport looks perfect for vc-git-command and vc-git--call, and that it's fine for auto-revert-handler provided that binding revert-buffer-in-progress-p for everything that happens prior to (vc-refresh-state) is ok. I imagine it is, but I haven't attempted to verify it.

[tarsius]

Merged.

[phil-s]

I suggest the release notes say that users of Emacs 27 development builds must be using http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=fd369be7ebf08414f4c517c7c2e854ccb9d520bc or newer.

[michael-heerdegen]

I still see the issue with my setup. Very frequently. It's a horror to rebase, I have to artificially devide each rebase into several steps to avoid losing work. Using current Emacs master and current Magit from Melpa.

[phil-s]

And you are running Git version 2.15.0 or later?

Also, I never lost any work from this. I just had to repeatedly 'continue' the rebase an annoying number of times. Are you certain that's a real problem?

[tarsius]

This is probably a different, though similar in nature, issue from what we hope to have fixed above. And it is probably another manifestation of #3646.

Does (setq magit-wip-merge-branch nil) make it go away?

[michael-heerdegen]

phil-s [e-mail] writes:

And you are running Git version 2.15.0 or later?

micha: git --version
git version 2.19.1

Michael.

[dfrib]

Hi,

I don't know if this is related, but it might be useful to someone, so I'm posting it here.

I just updated to magit-20181104.1651, and was running into the index.lock issues when trying to rebase, not only from magit but even from the terminal, given that I had my emacs client open (and had opened magit in it, at some point):

$ git pull -r
First, rewinding head to replay your work on top of it...
fatal: Unable to create '/my_project_path/.git/index.lock': File exists.

Once I closed my emacs client, I could successfully rebase from the terminal.

I was using quite an old version of git, however:

$ git --version
git version 2.7.4

$ emacs --version
GNU Emacs 26.1

After updating to a newer git version, it seems (so far) as if the problem has been resolved:

git --version
git version 2.19.1

I will follow up if I for some reason still run into the problem with the newer git version.

[michael-heerdegen]

I saw the Unable to create index.lock: File exists" error again. This time it interrupted F u. I had to interrupt the rebase manually (I had changes not yet pushed and configured Magit to rebase in this case when fetching).
The bad thing was that just somehow, after aborting rebase, I had to manually delete two files I had never touched - aborting the rebase obviously didn't give me exactly the worktree state I had begun with.

[aeruder]

I just checked the git sources, this requires git v2.15.0 or newer! @michael-heerdegen could you M-x eval-expression and then (magit-git-version) I'm just curious if there's a chance that magit is picking up a different git executable than the rest of your system.

[tarsius]

I just checked the git sources, this requires git v2.15.0 or newer!

Please provide a link to where you found out about that.

[michael-heerdegen]

Andrew Ruder [e-mail] writes:

I just checked the git sources, this requires git v2.15.0 or newer!

(magit-git-version) ==> "2.19.1"

Michael.

[aeruder]

I just checked the git sources, this requires git v2.15.0 or newer!

Please provide a link to where you found out about that.

I just dug through git.git:

% pwd
/Users/aeruder/repo/git
% git log -SGIT_OPTIONAL_LOCKS -1 | cat
commit 27344d6a6c8056664966e11acf674e5da6dd7ee3
Author: Jeff King <[email protected]>
Date:   Wed Sep 27 02:54:30 2017 -0400

    git: add --no-optional-locks option

[...]
% git tag --contains 27344d6a6c8056664966e11acf674e5da6dd7ee3
v2.15.0
[...]
%

[aeruder]

@michael-heerdegen

Sorry, I don't know the right way to do this, can you M-x describe-function then enter vc-git--call, then jump to the vc-git.el file in the top-right and just see if you have the

        ,@(when revert-buffer-in-progress-p
        '("GIT_OPTIONAL_LOCKS=0"))

lines in there? Should be once in the vc-git-command function and once in the vc-git--call function.

[michael-heerdegen]

Andrew Ruder [e-mail] writes:

  ,@(when revert-buffer-in-progress-p
  '("GIT_OPTIONAL_LOCKS=0"))

Yes, I have these lines, together with this comment:

;; Avoid repository locking during background operations
;; (bug#21559).

I also didn't advice `vc-git--call' or something like that. I'm quite
sure these lines are used.

Note I'm not sure that I saw the original issue when F u failed
yesterday - I have never had seen this before. It also isn't
necessarily Magits fault that this happened.

OTOH, since this new wip issue had been fixed, normal interactive
rebasing not failed for me since then, though I tried it 10 times or so.
Seems there is really a significant improvement.

Michael.

[aeruder]

It also isn't necessarily Magits fault that this happened.

The fun thing is that none of this has been magit's fault :laughing: - magit is just running a git rebase (or git pull --rebase in your example), and something else is running git status while the rebase is running. The only way git status is safe during a git rebase is if GIT_OPTIONAL_LOCKS=0 in the environment.

In the original case, this was code down in the auto-revert-mode (which admittedly, I think magit enabled), but it is possible you have something else running (like a git GUI of some sort?) that is running git status periodically and messing with the rebase?

[michael-heerdegen]

Andrew Ruder [e-mail] writes:

In the original case, this was code down in the auto-revert-mode
(which admittedly, I think magit enabled), but it is possible you have
something else running (like a git GUI of some sort?) that is running
git status periodically and messing with the rebase?

No, I only use Emacs ;-)

But other things could indeed be related: diff-hl (though I thought I
have already turned all things it does automatically off), and
helm-backup. I'm also using magit-wip.el: I use the modes, but also
have some explicit calls to 'magit-wip-commit' in my own code.

Is there some function I could advice that would automatically give me
hints about which git command is currently running when a lock collision
happened?

[dandavison]

FWIW it may be worth remembering that some users will have shell code executing in their $PROMPT_COMMAND that does git status (or at least git diff) type calls when generating their shell prompt.

[michael-heerdegen]

Dan Davison [e-mail] writes:

FWIW it may be worth remembering that some users will have shell code
executing in their $PROMPT_COMMAND that does git status (or at least
git diff) type calls when generating their shell prompt.

Good hint. I'm not one of those, though.

Michael.

[aeruder]

When I was originally investigating this, I put a script called git early in my path that did the checks and then called the real git.

An example

#!/bin/bash

for a in "$@"; do
  if [[ $a != -* ]]; then
    if [[ $a == status ]] && [ "$GIT_OPTIONAL_LOCKS" != "0" ]; then
      echo "`date` BROKEN STATUS" >> /tmp/broken-git-status.txt
    fi
    break
  fi
done

exec /usr/local/bin/git "$@"

and then in a terminal you can watch with tail -f /tmp/broken-git-status.txt and add any other additional logging that might help track it down. Not pretty, but that's how I figured it out originally.

[michael-heerdegen]

Thanks, I'll give it a try.