GNU bug report logs -
#21559
25.0.50; auto-revert-mode breaks git rebase
Previous Next
Reported by: Ben Gamari <ben <at> smart-cactus.org>
Date: Fri, 25 Sep 2015 14:31:02 UTC
Severity: normal
Found in version 25.0.50
Fixed in version 27.1
Done: Michael Albinus <michael.albinus <at> gmx.de>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 21559 in the body.
You can then email your comments to 21559 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Fri, 25 Sep 2015 14:31:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Ben Gamari <ben <at> smart-cactus.org>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Fri, 25 Sep 2015 14:31:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
emacs running with `auto-revert-mode` enabled breaks `git rebase` in
repositories where files are open. The problem appears to be that
`auto-revert-mode` attempts to refresh version control information with
`vc-find-file-hook` on revert events. `vc-find-file-hook` calls out to
`git`, taking the repository's index lock.
This interferes badly with `git rebase`, which performs many git
commands in quick succession. When `auto-revert-mode` is enabled there
is a very high chance that the following race will occur,
git emacs
---------------------- -----------------------------
1. git rebase checks out
a commit, releases
`index.lock`
2. `auto-revert-mode` notices
change, firing off a `git`
process and taking `index.lock`.
3. git rebase applies patch
and attempts to commit.
Notices that `index.lock`
is taken and fails.
4. emacs' `git` process
finishes, releasing lock
In the end the user is left with a badly broken rebase process and an
error message complaining that `index.lock` exists, which he then goes
to confirm and finds no such file as emacs has already released the lock.
Arguably `git rebase` should be holding the `index.lock` for the entire
duration of the process (or be more resilient to the lock being taken)
but sadly this isn't the case. Emacs should behave appropriately to
accomodate this behavior.
One imperfect workaround would be to instead schedule a worker to call
`vc-fine-file-hook` at some point in the future when the repository is
more likely to be idle (for instance, when there have been no change
events for a second or so).
git version 2.5.1
In GNU Emacs 25.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.16.6)
of 2015-08-20 on ben-laptop
Windowing system distributor `The X.Org Foundation', version 11.0.11702000
System Description: Debian GNU/Linux testing (stretch)
Configured using:
`configure --build x86_64-linux-gnu --prefix=/usr
--sharedstatedir=/var/lib --libexecdir=/usr/lib
--localstatedir=/var/lib --infodir=/usr/share/info
--mandir=/usr/share/man --with-pop=yes
--enable-locallisppath=/etc/emacs25:/etc/emacs:/usr/local/share/emacs/25.0/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.0/site-lisp:/usr/share/emacs/site-lisp
--build x86_64-linux-gnu --prefix=/usr --sharedstatedir=/var/lib
--libexecdir=/usr/lib --localstatedir=/var/lib
--infodir=/usr/share/info --mandir=/usr/share/man --with-pop=yes
--enable-locallisppath=/etc/emacs25:/etc/emacs:/usr/local/share/emacs/25.0/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/25.0/site-lisp:/usr/share/emacs/site-lisp
--with-x=yes --with-x-toolkit=gtk3 --with-toolkit-scroll-bars
'CFLAGS=-g -O2 -fstack-protector-strong -Wformat
-Werror=format-security -Wall' CPPFLAGS=-D_FORTIFY_SOURCE=2
LDFLAGS=-Wl,-z,relro'
Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL LIBSELINUX GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11
Important settings:
value of $LANG: en_US.UTF-8
locale-coding-system: utf-8-unix
Major mode: Emacs-Lisp
Minor modes in effect:
elisp-slime-nav-mode: t
goto-address-prog-mode: t
auto-highlight-symbol-mode: t
clean-aindent-mode: t
highlight-numbers-mode: t
highlight-parentheses-mode: t
rainbow-delimiters-mode: t
yas-global-mode: t
yas-minor-mode: t
helm-descbinds-mode: t
helm-mode: t
shell-dirtrack-mode: t
projectile-global-mode: t
projectile-mode: t
recentf-mode: t
winner-mode: t
window-numbering-mode: t
volatile-highlights-mode: t
global-vi-tilde-fringe-mode: t
global-undo-tree-mode: t
undo-tree-mode: t
savehist-mode: t
popwin-mode: t
global-page-break-lines-mode: t
page-break-lines-mode: t
Info-breadcrumbs-in-mode-line-mode: t
ido-vertical-mode: t
flx-ido-mode: t
global-evil-surround-mode: t
evil-surround-mode: t
global-evil-search-highlight-persist: t
evil-search-highlight-persist: t
show-smartparens-global-mode: t
show-smartparens-mode: t
smartparens-mode: t
evil-jumper-mode: t
evil-escape-mode: t
global-anzu-mode: t
anzu-mode: t
eval-sexp-fu-flash-mode: t
company-mode: t
global-hl-line-mode: t
xterm-mouse-mode: t
global-auto-revert-mode: t
evil-leader-mode: t
evil-mode: t
evil-local-mode: t
which-key-mode: t
override-global-mode: t
spacemacs-additional-leader-mode: t
global-eldoc-mode: t
eldoc-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
buffer-read-only: t
column-number-mode: t
line-number-mode: t
transient-mark-mode: t
hs-minor-mode: t
Recent messages:
Resetting customization items...done
Creating customization setup...done
Creating customization items...
Creating group...
Creating group entries...done
Creating customization items ...done
Resetting customization items...done
Creating customization setup...done
Info-mouse-follow-link: Args out of range: 2394
Text is read-only [2 times]
Load-path shadows:
/home/ben/.emacs.d/elpa/helm-20150923.2134/helm-multi-match hides /home/ben/.emacs.d/elpa/helm-core-20150923.959/helm-multi-match
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-show hides /usr/local/share/emacs/site-lisp/notmuch-show
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-print hides /usr/local/share/emacs/site-lisp/notmuch-print
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-wash hides /usr/local/share/emacs/site-lisp/notmuch-wash
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-jump hides /usr/local/share/emacs/site-lisp/notmuch-jump
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-crypto hides /usr/local/share/emacs/site-lisp/notmuch-crypto
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-hello hides /usr/local/share/emacs/site-lisp/notmuch-hello
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-tree hides /usr/local/share/emacs/site-lisp/notmuch-tree
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch hides /usr/local/share/emacs/site-lisp/notmuch
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-address hides /usr/local/share/emacs/site-lisp/notmuch-address
/home/ben/.emacs.d/elpa/notmuch-20150907.527/coolj hides /usr/local/share/emacs/site-lisp/coolj
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-parser hides /usr/local/share/emacs/site-lisp/notmuch-parser
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-lib hides /usr/local/share/emacs/site-lisp/notmuch-lib
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-query hides /usr/local/share/emacs/site-lisp/notmuch-query
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-mua hides /usr/local/share/emacs/site-lisp/notmuch-mua
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-message hides /usr/local/share/emacs/site-lisp/notmuch-message
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-tag hides /usr/local/share/emacs/site-lisp/notmuch-tag
/home/ben/.emacs.d/elpa/notmuch-20150907.527/notmuch-maildir-fcc hides /usr/local/share/emacs/site-lisp/notmuch-maildir-fcc
/home/ben/.emacs.d/elpa/cmake-mode-20150817.725/cmake-mode hides /usr/share/emacs24/site-lisp/cmake-data/cmake-mode
/home/ben/.emacs.d/elpa/cmake-mode-20150817.725/cmake-mode hides /usr/share/emacs/site-lisp/cmake-mode
/usr/share/emacs/site-lisp/rst hides /usr/share/emacs/25.0.50/lisp/textmodes/rst
/home/ben/.emacs.d/elpa/seq-20150917.1508/seq hides /usr/share/emacs/25.0.50/lisp/emacs-lisp/seq
Features:
(shadow sort mail-extr warnings emacsbug message rfc822 mml mml-sec
mailabbrev gmm-utils mailheader sendmail mail-utils cus-edit cus-start
cus-load company-files company-keywords company-etags company-gtags
company-template company-dabbrev-code company-dabbrev company-capf
elisp-slime-nav goto-addr auto-highlight-symbol clean-aindent-mode
highlight-numbers parent-mode highlight-parentheses hideshow
rainbow-delimiters yasnippet jka-compr eieio-opt speedbar sb-image
ezimage dframe find-func helm-command helm-elisp helm-eval edebug
helm-descbinds helm-mode helm-files image-dired tramp tramp-compat
tramp-loaddefs trampver shell pcomplete format-spec dired-x dired-aux
ffap helm-buffers helm-elscreen helm-tags helm-bookmark helm-adaptive
helm-info bookmark helm-locate helm-grep helm-regexp helm-plugin
helm-external helm-net browse-url xml url url-proxy url-privacy
url-expand url-methods url-history url-cookie url-domsuf url-util
url-parse auth-source gnus-util password-cache url-vars mailcap
helm-utils helm-help helm-types helm helm-source eieio-compat
helm-multi-match helm-lib dired projectile grep compile ibuf-ext ibuffer
recentf tree-widget disp-table server winner window-numbering etags xref
project volatile-highlights vi-tilde-fringe undo-tree diff solarized
smooth-scrolling smartparens-config saveplace savehist py-yapf powerline
powerline-separators color powerline-themes popwin page-break-lines
info+ ido-vertical-mode flx-ido flx ido exec-path-from-shell
evil-surround evil-search-highlight-persist evil-numbers evil-lisp-state
smartparens evil-jumper evil-indent-textobject evil-exchange evil-escape
evil-args evil-anzu anzu eval-sexp-fu rx highlight diminish company-web
company web-completion-data info tex-site adaptive-wrap hybrid-mode ielm
pp comint ansi-color hl-line xt-mouse autorevert filenotify
core-evilified-state evil-leader evil evil-integration evil-maps
evil-commands evil-command-window evil-types evil-search evil-ex
evil-macros evil-repeat evil-states evil-core evil-common windmove
thingatpt rect evil-digraphs evil-vars ring which-key quelpa
package-build mm-decode mm-bodies mm-encode mail-parse rfc2231 rfc2047
rfc2045 ietf-drums mm-util help-fns mail-prsvr json lisp-mnt use-package
bind-key s ucs-normalize dash wid-edit zenburn-theme
core-configuration-layer cl-seq finder-inf core-dotspacemacs ht cl
package epg-config eieio byte-opt bytecomp byte-compile cl-extra
help-mode easymenu seq cconv eieio-core cl-macs gv core-spacemacs
derived edmacro kmacro cl-loaddefs cl-lib core-evilify-keymap
core-keybindings easy-mmode core-use-package-ext core-micro-state corelv
core-toggle core-fonts-support core-spacemacs-buffer core-funcs
core-themes-support advice core-auto-completion core-release-management
core-emacs-backports subr-x pcase devhelp time-date mule-util tooltip
eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list newcomment elisp-mode lisp-mode prog-mode register page
menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core frame cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
dbusbind inotify dynamic-setting system-font-setting font-render-setting
move-toolbar gtk x-toolkit x multi-tty make-network-process emacs)
Memory information:
((conses 16 854111 669089)
(symbols 48 49466 68)
(miscs 40 1968 2565)
(strings 32 311688 272104)
(string-bytes 1 11727672)
(vectors 16 64346)
(vector-slots 8 1157167 184119)
(floats 8 1008 1405)
(intervals 56 4139 1183)
(buffers 976 26)
(heap 1024 129296 66437))
[Message part 2 (text/plain, inline)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Sat, 26 Sep 2015 07:42:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 21559 <at> debbugs.gnu.org (full text, mbox):
> From: Ben Gamari <ben <at> smart-cactus.org>
> Date: Fri, 25 Sep 2015 14:45:56 +0200
>
> One imperfect workaround would be to instead schedule a worker to call
> `vc-fine-file-hook` at some point in the future when the repository is
> more likely to be idle (for instance, when there have been no change
> events for a second or so).
But autorevert already does exactly that: it doesn't act on file
changes immediately, only once every N seconds. N defaults to 5,
perhaps you could try customizing auto-revert-interval to a larger
value to see if that solves the problem.
In any case, the issue here is that Emacs doesn't know when "the
repository is idle". If someone can suggest how to tell that,
autorevert could be augmented not to revert VC files while the
repository is "busy". From your description I understand that just
looking at index.lock is not good enough, since Git releases it more
than once in this scenario, while the rebase operation is still
on-going.
Btw, what is your value of auto-revert-check-vc-info? If it's
non-nil, perhaps resetting it to nil will also avoid these problems.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 28 Sep 2015 14:12:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Ben Gamari <ben <at> smart-cactus.org>
>> Date: Fri, 25 Sep 2015 14:45:56 +0200
>>
>> One imperfect workaround would be to instead schedule a worker to call
>> `vc-fine-file-hook` at some point in the future when the repository is
>> more likely to be idle (for instance, when there have been no change
>> events for a second or so).
>
> But autorevert already does exactly that: it doesn't act on file
> changes immediately, only once every N seconds. N defaults to 5,
> perhaps you could try customizing auto-revert-interval to a larger
> value to see if that solves the problem.
>
This is not true is auto-revert-use-notify is enabled.
If I'm not mistaken, your suggestion amounts to increasing the poll
interval. This will never solve the problem, it will only mean that the
race has fewer opportunities to pop up. Of course, the same can be said
of my suggestion, which only narrows the race window, not remove it
altogether (which is sadly not possible without fixing git rebase).
As an aside, it came as quite a surprise that autorevert (if I
understand the code correctly) still polls even if
auto-revert-use-notify is set. It seems to me that this is itself a bug.
As a laptop user who often works on battery, I keep tabs on which
processes eat CPU time and I have noticed that emacs is consistently
behind only firefox and Xorg in CPU time demanded at idle.
> In any case, the issue here is that Emacs doesn't know when "the
> repository is idle". If someone can suggest how to tell that,
> autorevert could be augmented not to revert VC files while the
> repository is "busy". From your description I understand that just
> looking at index.lock is not good enough, since Git releases it more
> than once in this scenario, while the rebase operation is still
> on-going.
>
Right.
I threw together a patch [1] implementing the suggestion I presented
above. I have yet to rigorously test it but I have yet to experience the
issue since starting to use it.
> Btw, what is your value of auto-revert-check-vc-info? If it's
> non-nil, perhaps resetting it to nil will also avoid these problems.
It is nil but this doesn't matter as vc-find-file-hook is called if
either auto-revert-check-vc-info or the revert flag are set.
Cheers,
- Ben
[1] https://github.com/bgamari/emacs/compare/master...auto-revert-vc-idle
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 28 Sep 2015 14:36:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 21559 <at> debbugs.gnu.org (full text, mbox):
> From: Ben Gamari <ben <at> smart-cactus.org>
> Cc: 21559 <at> debbugs.gnu.org
> Date: Mon, 28 Sep 2015 16:11:16 +0200
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> From: Ben Gamari <ben <at> smart-cactus.org>
> >> Date: Fri, 25 Sep 2015 14:45:56 +0200
> >>
> >> One imperfect workaround would be to instead schedule a worker to call
> >> `vc-fine-file-hook` at some point in the future when the repository is
> >> more likely to be idle (for instance, when there have been no change
> >> events for a second or so).
> >
> > But autorevert already does exactly that: it doesn't act on file
> > changes immediately, only once every N seconds. N defaults to 5,
> > perhaps you could try customizing auto-revert-interval to a larger
> > value to see if that solves the problem.
> >
> This is not true is auto-revert-use-notify is enabled.
You are right, I forgot: things were originally like I described, but
were later changed to fix bug#18958.
> I threw together a patch [1] implementing the suggestion I presented
> above. I have yet to rigorously test it but I have yet to experience the
> issue since starting to use it.
Thanks.
However, it looks like your suggested patch reverts back to what we
had before we fixed bug 18958, doesn't it? If so, we will have to
look for some other solution, because we don't want to reintroduce
that bug. See http://debbugs.gnu.org/cgi/bugreport.cgi?bug=18958#5
for the details.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 28 Sep 2015 15:07:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Ben Gamari <ben <at> smart-cactus.org>
>> Cc: 21559 <at> debbugs.gnu.org
>> Date: Mon, 28 Sep 2015 16:11:16 +0200
>>
>> I threw together a patch [1] implementing the suggestion I presented
>> above. I have yet to rigorously test it but I have yet to experience the
>> issue since starting to use it.
>
> Thanks.
>
> However, it looks like your suggested patch reverts back to what we
> had before we fixed bug 18958, doesn't it? If so, we will have to
> look for some other solution, because we don't want to reintroduce
> that bug. See http://debbugs.gnu.org/cgi/bugreport.cgi?bug=18958#5
> for the details.
I'm not sure that this is true. Note that my patch defers *only* the
call to vc-find-file-hook, not reverting the buffer (which, correct me
if I'm wrong, appears to be what this bug is about).
Cheers,
- Ben
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Tue, 29 Sep 2015 08:48:01 GMT)
Full text and
rfc822 format available.
Message #20 received at 21559 <at> debbugs.gnu.org (full text, mbox):
Ben Gamari <ben <at> smart-cactus.org> writes:
> As an aside, it came as quite a surprise that autorevert (if I
> understand the code correctly) still polls even if
> auto-revert-use-notify is set. It seems to me that this is itself a bug.
> As a laptop user who often works on battery, I keep tabs on which
> processes eat CPU time and I have noticed that emacs is consistently
> behind only firefox and Xorg in CPU time demanded at idle.
I remember there were some reasons to keep this polling behavior. Must
check years-old conversation in order to remember the reason.
> I threw together a patch [1] implementing the suggestion I presented
> above. I have yet to rigorously test it but I have yet to experience the
> issue since starting to use it.
Wouldn't it be better to provide a buffer-stale-function for files or
directories under vc control? Mode-specific functionality shouldn't be
added to auto-revert.el, I believe.
> Cheers,
>
> - Ben
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 26 Oct 2015 18:44:01 GMT)
Full text and
rfc822 format available.
Message #23 received at 21559 <at> debbugs.gnu.org (full text, mbox):
Hi all,
This is a patch that I've been using locally for a few months now to resolve
#21599, where auto-revert and vc conspire to break git-rebase. Ultimately the
problem is a race-condition which would require a change in git to resolve
completely, but in practice this timeout-based approach eliminates the issue.
Cheers,
- Ben
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 26 Oct 2015 18:44:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 21559 <at> debbugs.gnu.org (full text, mbox):
This provides a resolution for Bug #21559.
emacs running with `auto-revert-mode` enabled breaks `git rebase` in
repositories where files are open. The problem appears to be that
`auto-revert-mode` attempts to refresh version control information with
`vc-find-file-hook` on revert events. `vc-find-file-hook` calls out to
`git`, taking the repository's index lock.
This interferes badly with `git rebase`, which performs many git
commands in quick succession. When `auto-revert-mode` is enabled there
is a very high chance that the following race will occur,
git emacs
---------------------- -----------------------------
1. git rebase checks out
a commit, releases
`index.lock`
2. `auto-revert-mode` notices
change, firing off a `git`
process and taking
`index.lock`.
3. git rebase applies patch
and attempts to commit.
Notices that `index.lock`
is taken and fails.
4. emacs' `git` process
finishes, releasing lock
In the end the user is left with a badly broken rebase process and an
error message complaining that `index.lock` exists, which he then goes
to confirm and finds no such file as emacs has already released the
lock.
We work around this by scheduling a worker to call `vc-fine-file-hook`
at some point in the future when the repository is more likely to be
idle (for instance, when there have been no change events for a second
or so).
Signed-Off-By: Ben Gamari <ben <at> smart-cactus.org>
---
lisp/autorevert.el | 35 ++++++++++++++++++++++++++++++++++-
1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/lisp/autorevert.el b/lisp/autorevert.el
index 37ee8ee..0c25ac3 100644
--- a/lisp/autorevert.el
+++ b/lisp/autorevert.el
@@ -136,6 +136,11 @@ auto-revert-tail-mode
(defvar auto-revert-timer nil
"Timer used by Auto-Revert Mode.")
+(defvar auto-revert-vc-check-timer nil
+ "Timer used by Auto-Revert Mode to schedule
+checks of version control information. See
+`auto-revert-schedule-vc-check' for details.")
+
(defcustom auto-revert-interval 5
"Time, in seconds, between Auto-Revert Mode file checks.
The value may be an integer or floating point number.
@@ -260,6 +265,13 @@ auto-revert-check-vc-info
:type 'boolean
:version "22.1")
+(defcustom auto-revert-vc-check-idle-time 2
+ "How much time to wait after noticing a changed file before calling
+`vc-find-file-hook' or nil to check immediately."
+ :group 'auto-revert
+ :type 'number
+ :version "25.1")
+
(defvar-local global-auto-revert-ignore-buffer nil
"When non-nil, Global Auto-Revert Mode will not revert this buffer.
This variable becomes buffer local when set in any fashion.")
@@ -671,7 +683,28 @@ auto-revert-handler
;; `preserve-modes' avoids changing the (minor) modes. But we do
;; want to reset the mode for VC, so we do it manually.
(when (or revert auto-revert-check-vc-info)
- (vc-find-file-hook))))
+ (auto-revert-schedule-vc-check))))
+
+(defun auto-revert-schedule-vc-check ()
+ "Schedule a call to `vc-find-file-hook'.
+
+We need to be careful when calling `vc-find-file-hook' after file changes
+as some version control utilities (e.g. git rebase) have a tendency
+to do many successive calls and will fail ungracefully if they find
+we have taken the repository lock.
+
+For this reason we wait for the repository to be idle for at least
+`auto-revert-vc-check-idle-time' seconds before calling
+`vc-find-file-hook'."
+ (if auto-revert-vc-check-idle-time
+ (progn
+ (when (timerp auto-revert-vc-check-timer)
+ (cancel-timer auto-revert-vc-check-timer))
+ (setq auto-revert-vc-check-idle-timer
+ (run-at-time auto-revert-vc-check-idle-time nil
+ 'vc-find-file-hook))
+ )
+ (vc-find-file-hook)))
(defun auto-revert-tail-handler (size)
(let ((modified (buffer-modified-p))
--
2.6.1
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Tue, 27 Oct 2015 08:27:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[removing emacs-devel, discussion in one ML is sufficient]
Ben Gamari <ben <at> smart-cactus.org> writes:
Hi Ben,
> This provides a resolution for Bug #21559.
Thanks for the patch.
But as I said already, I'm not happy to insert so much hard-wired vc
related operations into autorevert.el. And this is triggered just by
git, IIUC the other vc backends do not suffer from this problem.
Have you considered another solution, maybe by providing vc(-git)
specific buffer-stale-function and revert-buffer? This would be much
more in the spirit how autorevert is supposed to work. And this would
simplify maintenance, when git changes its behaviour, and we need to
adapt vc-git accordingly.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Wed, 28 Oct 2015 12:01:02 GMT)
Full text and
rfc822 format available.
Message #32 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi Michael,
Michael Albinus <michael.albinus <at> gmx.de> writes:
> [removing emacs-devel, discussion in one ML is sufficient]
>
Okay. Works for me.
> Ben Gamari <ben <at> smart-cactus.org> writes:
>
> Hi Ben,
>
>> This provides a resolution for Bug #21559.
>
> Thanks for the patch.
>
> But as I said already, I'm not happy to insert so much hard-wired vc
> related operations into autorevert.el. And this is triggered just by
> git, IIUC the other vc backends do not suffer from this problem.
Fair enough.
> Have you considered another solution, maybe by providing vc(-git)
> specific buffer-stale-function and revert-buffer? This would be much
> more in the spirit how autorevert is supposed to work. And this would
> simplify maintenance,
Could you perhaps describe in a bit detail what the semantics of these
functions would be? I'm not entirely sure I understand what you are
proposing.
> when git changes its behaviour, and we need to adapt vc-git
> accordingly.
You are more optimistic than me on whether git will be fixed ;)
Thanks for your feedback!
Cheers,
- Ben
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Wed, 28 Oct 2015 14:46:02 GMT)
Full text and
rfc822 format available.
Message #35 received at 21559 <at> debbugs.gnu.org (full text, mbox):
Ben Gamari <ben <at> smart-cactus.org> writes:
> Hi Michael,
Hi Ben,
>> Have you considered another solution, maybe by providing vc(-git)
>> specific buffer-stale-function and revert-buffer? This would be much
>> more in the spirit how autorevert is supposed to work. And this would
>> simplify maintenance,
>
> Could you perhaps describe in a bit detail what the semantics of these
> functions would be? I'm not entirely sure I understand what you are
> proposing.
As starting point you might read about `revert-buffer-function' in
(info "(elisp)Reverting") and about `buffer-stale-function' in
(info "(emacs)Supporting additional buffers")
See also in a dired buffer the values for both variables, and how the
functions work for dired.
>> when git changes its behaviour, and we need to adapt vc-git
>> accordingly.
>
> You are more optimistic than me on whether git will be fixed ;)
I have no special opinion about git. I just use it, without being impressed.
> Cheers,
>
> - Ben
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Wed, 28 Oct 2015 17:06:02 GMT)
Full text and
rfc822 format available.
Message #38 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi Michael,
Michael Albinus <michael.albinus <at> gmx.de> writes:
>>> Have you considered another solution, maybe by providing vc(-git)
>>> specific buffer-stale-function and revert-buffer? This would be much
>>> more in the spirit how autorevert is supposed to work. And this would
>>> simplify maintenance,
>>
>> Could you perhaps describe in a bit detail what the semantics of these
>> functions would be? I'm not entirely sure I understand what you are
>> proposing.
>
> As starting point you might read about `revert-buffer-function' in
> (info "(elisp)Reverting") and about `buffer-stale-function' in
> (info "(emacs)Supporting additional buffers")
>
> See also in a dired buffer the values for both variables, and how the
> functions work for dired.
Thanks for the references.
I now have a better idea of what you are proposing. I do, however, fear
that your solution might be too large a hammer for the problem at hand.
Ultimately the problem here is an annoyingly narrow one, namely the fact
that auto-revert's action is correlated with file modifications due to
rebasing. For this reason I think it may be best to keep the solution
confined to auto-revert.
In particular, I'm afraid that the changing the semantics of
revert-buffer may break other users of this function who expect
its effect to be apparent immediately after invocation. It seems like
this approach may easily and unwittingly trade one subtle form of
breakage for another (even harder to find) one.
Cheers,
- Ben
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Thu, 29 Oct 2015 08:21:02 GMT)
Full text and
rfc822 format available.
Message #41 received at 21559 <at> debbugs.gnu.org (full text, mbox):
Ben Gamari <ben <at> smart-cactus.org> writes:
> Hi Michael,
Hi Ben,
> Ultimately the problem here is an annoyingly narrow one, namely the fact
> that auto-revert's action is correlated with file modifications due to
> rebasing. For this reason I think it may be best to keep the solution
> confined to auto-revert.
>
> In particular, I'm afraid that the changing the semantics of
> revert-buffer may break other users of this function who expect
> its effect to be apparent immediately after invocation. It seems like
> this approach may easily and unwittingly trade one subtle form of
> breakage for another (even harder to find) one.
I agree with you not to change the behaviour of revert-buffer for other
use cases but auto-revert. One could do it by introducing a new local
variable auto-revert-buffer-in-progress-p (similar to
revert-buffer-in-progress-p), which is bound during auto-revert. Then it
would be possible to define a new function
(defun vc-git-revert-buffer-function (ignore-auto noconfirm)
"vc-git specific revert function."
(if (and auto-revert-buffer-in-progress-p
(check-for-git-rebase))
(do-something-special)
(revert-buffer--default ignore-auto noconfirm)))
One could also check, whether adding a function to before-revert-hook
could be helpful.
An alternative, w/o touching revert-buffer for vc-git files, would be
the provisiong of a special buffer-stale-function for vc-git. Something
like
(defun vc-git-buffer-stale-p (&optional noconfirm)
"vc-git specific buffer-stale function."
(check-that-git-needs-revert-and-it-is-safe-to-do-so))
This function returns t, when a buffer with a file under vc-git mode
needs a revert, and it is safe to revert it (no git rebase is
running). When autorevert either polls, or it is indicated by a file
notification event, it calls this function and reverts the buffer if the
function returns non-nil. If the function returns nil, auto-revert is
skipped for that buffer, and it is checked again by the next poll (after
5 seconds). By this, autorevert.el would stay clean (no additional vc
related code), and you would even not need to implement an own timer.
> Cheers,
>
> - Ben
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Sat, 06 Feb 2016 00:02:02 GMT)
Full text and
rfc822 format available.
Message #44 received at 21559 <at> debbugs.gnu.org (full text, mbox):
On Fri, Sep 25 2015 at 03:45:56 PM, Ben Gamari <ben <at> smart-cactus.org> wrote:
> Arguably `git rebase` should be holding the `index.lock` for the entire
> duration of the process (or be more resilient to the lock being taken)
> but sadly this isn't the case. Emacs should behave appropriately to
> accomodate this behavior.
I don't think this is quite right. As I understand it, git only takes
index.lock for operations that mutate the index [1].
It wouldn't make sense for git to hold index.lock for the entirety of
the rebase operation since you can stop in the middle of an interactive
rebase and do anything you want (mess with the index, add new commits,
anything).
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)?
[1] http://git.661346.n2.nabble.com/Git-rebase-dies-with-fatal-Unable-to-create-git-index-lock-File-exists-td7596365.html
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Sat, 06 Feb 2016 12:35:02 GMT)
Full text and
rfc822 format available.
Message #47 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Mitchel Humpherys <mitch.special <at> gmail.com> writes:
> On Fri, Sep 25 2015 at 03:45:56 PM, Ben Gamari <ben <at> smart-cactus.org> wrote:
>> Arguably `git rebase` should be holding the `index.lock` for the entire
>> duration of the process (or be more resilient to the lock being taken)
>> but sadly this isn't the case. Emacs should behave appropriately to
>> accomodate this behavior.
>
> I don't think this is quite right. As I understand it, git only takes
> index.lock for operations that mutate the index [1].
>
> It wouldn't make sense for git to hold index.lock for the entirety of
> the rebase operation since you can stop in the middle of an interactive
> rebase and do anything you want (mess with the index, add new commits,
> anything).
>
Ahh, yes. You are quite right.
> 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)?
>
As far as I can tell vc-find-file-hook is merely calling `git ls-files
-c -z -- $FILE`, which sounds to me like it should indeed be a read-only
operation.
Cheers,
- Ben
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Sun, 07 Feb 2016 01:35:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 21559 <at> debbugs.gnu.org (full text, mbox):
On Sat, Feb 06 2016 at 01:34:06 PM, Ben Gamari <ben <at> smart-cactus.org> wrote:
> Mitchel Humpherys <mitch.special <at> gmail.com> writes:
>
>> On Fri, Sep 25 2015 at 03:45:56 PM, Ben Gamari <ben <at> smart-cactus.org> wrote:
>>> Arguably `git rebase` should be holding the `index.lock` for the entire
>>> duration of the process (or be more resilient to the lock being taken)
>>> but sadly this isn't the case. Emacs should behave appropriately to
>>> accomodate this behavior.
>>
>> I don't think this is quite right. As I understand it, git only takes
>> index.lock for operations that mutate the index [1].
>>
>> It wouldn't make sense for git to hold index.lock for the entirety of
>> the rebase operation since you can stop in the middle of an interactive
>> rebase and do anything you want (mess with the index, add new commits,
>> anything).
>>
> Ahh, yes. You are quite right.
>
>> 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)?
>>
> As far as I can tell vc-find-file-hook is merely calling `git ls-files
> -c -z -- $FILE`, which sounds to me like it should indeed be a read-only
> operation.
I'm able to reliably reproduce this on the tip of the emacs-25 branch
with emacs -Q with:
$ cd /path/to/emacs
$ ./src/emacs -Q lisp/*.el
[ M-x global-auto-revert-mode ]
$ for i in $(seq 30); do for f in lisp/*.el; do echo "; $i" >> $f; done; git commit -am "test $i"; done
You might need to increase the `30' value in order to see it happen
depending on your disk speed, etc.
I took a quick look at the git source and the main source of this
particular error seems to be from the `git update-index' command. I see
that vc-git.el is calling `git update-index' in a few places but my
attempts at instrumenting the code to track down where it was coming
from were fruitless. I tried:
(setq vc-command-messages t)
as well as:
(defun vc-git-command (buffer okstatus file-or-list &rest flags)
"A wrapper around `vc-do-command' for use in vc-git.el.
The difference to vc-do-command is that this function always invokes
`vc-git-program'."
(let ((coding-system-for-read vc-git-commits-coding-system)
(coding-system-for-write vc-git-commits-coding-system))
+ (message "git: %s %s" file-or-list flags)
Any ideas on how we can trace every git command that vc-git.el is
running? I'm suspicious that we're calling `git update-index' in the
auto revert path somewhere...
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Sun, 07 Feb 2016 05:04:02 GMT)
Full text and
rfc822 format available.
Message #53 received at 21559 <at> debbugs.gnu.org (full text, mbox):
On Sat, Feb 06 2016 at 05:34:49 PM, Mitchel Humpherys <mitch.special <at> gmail.com> wrote:
> I took a quick look at the git source and the main source of this
> particular error seems to be from the `git update-index' command. I see
> that vc-git.el is calling `git update-index' in a few places but my
> attempts at instrumenting the code to track down where it was coming
> from were fruitless. I tried:
>
> (setq vc-command-messages t)
>
> as well as:
>
> (defun vc-git-command (buffer okstatus file-or-list &rest flags)
> "A wrapper around `vc-do-command' for use in vc-git.el.
> The difference to vc-do-command is that this function always invokes
> `vc-git-program'."
> (let ((coding-system-for-read vc-git-commits-coding-system)
> (coding-system-for-write vc-git-commits-coding-system))
> + (message "git: %s %s" file-or-list flags)
>
> Any ideas on how we can trace every git command that vc-git.el is
> running? I'm suspicious that we're calling `git update-index' in the
> auto revert path somewhere...
Sorry, I take it all back. `git update-index' is the source of that one
particular index.lock error message, but it's certainly not the only
thing holding index.lock... And actually it looks like `git ls-files'
does take index.lock:
$ cd /path/to/emacs/
$ git ls-files # in another shell after running the following inotifywait:
$ inotifywait -m .git | grep index.lock
Setting up watches.
Watches established.
.git/ CREATE index.lock
.git/ OPEN index.lock
.git/ CLOSE_WRITE,CLOSE index.lock
.git/ DELETE index.lock
.git/ CREATE index.lock
.git/ OPEN index.lock
.git/ CLOSE_WRITE,CLOSE index.lock
.git/ DELETE index.lock
I did a bit of git debugging and it looks like someone must be
registering an atexit handler or something that takes index.lock,
because it doesn't get taken in the ls-files code itself. It's taken
sometime after the `exit' function gets called in `handle_builtin' in
git.c.
Anyways, I'm more inclined now to agree that this a git bug. I don't
see why `git ls-files' would be taking index.lock... So we're back to
the question of "how can Emacs handle this more gracefully".
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Sun, 07 Feb 2016 05:22:01 GMT)
Full text and
rfc822 format available.
Message #56 received at 21559 <at> debbugs.gnu.org (full text, mbox):
On Sat, Feb 06 2016 at 09:03:35 PM, Mitchel Humpherys <mitch.special <at> gmail.com> wrote:
> On Sat, Feb 06 2016 at 05:34:49 PM, Mitchel Humpherys <mitch.special <at> gmail.com> wrote:
>> I took a quick look at the git source and the main source of this
>> particular error seems to be from the `git update-index' command. I see
>> that vc-git.el is calling `git update-index' in a few places but my
>> attempts at instrumenting the code to track down where it was coming
>> from were fruitless. I tried:
>>
>> (setq vc-command-messages t)
>>
>> as well as:
>>
>> (defun vc-git-command (buffer okstatus file-or-list &rest flags)
>> "A wrapper around `vc-do-command' for use in vc-git.el.
>> The difference to vc-do-command is that this function always invokes
>> `vc-git-program'."
>> (let ((coding-system-for-read vc-git-commits-coding-system)
>> (coding-system-for-write vc-git-commits-coding-system))
>> + (message "git: %s %s" file-or-list flags)
>>
>> Any ideas on how we can trace every git command that vc-git.el is
>> running? I'm suspicious that we're calling `git update-index' in the
>> auto revert path somewhere...
>
> Sorry, I take it all back. `git update-index' is the source of that one
> particular index.lock error message, but it's certainly not the only
> thing holding index.lock... And actually it looks like `git ls-files'
> does take index.lock:
>
> $ cd /path/to/emacs/
> $ git ls-files # in another shell after running the following inotifywait:
> $ inotifywait -m .git | grep index.lock
> Setting up watches.
> Watches established.
> .git/ CREATE index.lock
> .git/ OPEN index.lock
> .git/ CLOSE_WRITE,CLOSE index.lock
> .git/ DELETE index.lock
> .git/ CREATE index.lock
> .git/ OPEN index.lock
> .git/ CLOSE_WRITE,CLOSE index.lock
> .git/ DELETE index.lock
>
> I did a bit of git debugging and it looks like someone must be
> registering an atexit handler or something that takes index.lock,
> because it doesn't get taken in the ls-files code itself. It's taken
> sometime after the `exit' function gets called in `handle_builtin' in
> git.c.
>
> Anyways, I'm more inclined now to agree that this a git bug. I don't
> see why `git ls-files' would be taking index.lock... So we're back to
> the question of "how can Emacs handle this more gracefully".
SORRY! I take it all back AGAIN. It was actually my shell's PS1 that
was locking index.lock (due to a `git status'), not `git ls-files'!
That's why it looked like it was happening when the program exited.
Because it really was after the program exited :). That's
embarassing...
After re-running my experiment above in a non-fancy shell I don't see
index.lock being taken at all when I run `git ls-files'.
So we're back to instrumenting vc{,-git}.el to see exactly which git
commands are being run when auto-revert refreshes a buffer.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Sun, 07 Feb 2016 10:23:01 GMT)
Full text and
rfc822 format available.
Message #59 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Mitchel Humpherys <mitch.special <at> gmail.com> writes:
> So we're back to instrumenting vc{,-git}.el to see exactly which git
> commands are being run when auto-revert refreshes a buffer.
I recently revisited this and came up with this hack (after realizing
that my previous tracing approach was badly broken),
(defun my-tracing-function (orig &rest args)
(message "call-process %s" args))
(advice-add 'call-process :before #'my-tracing-function)
(message "installed")
This produces,
call-process (nil (t nil) nil ls-files -c -z -- COPYING)
call-process (nil (t nil) nil rev-parse HEAD)
call-process (nil (t nil) nil symbolic-ref HEAD)
call-process (nil (t nil) nil diff-index -p --raw -z HEAD -- COPYING)
call-process (nil (t nil) nil status --porcelain -- COPYING)
call-process (nil (t nil) nil ls-files -c -z -- COPYING)
call-process (nil (t nil) nil rev-parse HEAD)
call-process (nil (t nil) nil symbolic-ref HEAD)
call-process (nil (t nil) nil diff-index -p --raw -z HEAD -- COPYING)
call-process (nil (t nil) nil status --porcelain -- COPYING)
Quit
call-process (nil (t nil) nil ls-files -c -z -- COPYING)
After stracing each of these I've found that status --porcelain indeed
takes index.lock. Unfortunately I can't find a good explanation of why
this is necessary.
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Sun, 07 Feb 2016 10:56:01 GMT)
Full text and
rfc822 format available.
Message #62 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
I've spoken with some folks in #git about this issue.
* the index lock is held "because it needs to read the index and
compare it to the worktree. If it doesn't take the index lock, other
things could change either the index or the worktree underneath it,
making git status lie (or even crash)"
* It sounds as though a patch refactoring `git rebase` such that it
holds the index lock may be considered, although this is a
non-trivial refactoring as `rebase` is currently a shell script
* One option would be to check whether a rebase is in progress before
auto-reverting. In pseudo-shell,
if [ -e .git/rebase-apply ] || [ -e .git/rebase-merge ]; then
do_not_call_git_status_because_we_are_rebasing;
else
auto_revert
fi
Unfortunately it sounds like this wouldn't do the right thing when a
rebase pauses due to conflict (when you'd ideally want to
auto-revert).
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Sun, 07 Feb 2016 17:07:01 GMT)
Full text and
rfc822 format available.
Message #65 received at 21559 <at> debbugs.gnu.org (full text, mbox):
On Sun, Feb 07 2016 at 11:55:07 AM, Ben Gamari <ben <at> smart-cactus.org> wrote:
> I've spoken with some folks in #git about this issue.
>
> * the index lock is held "because it needs to read the index and
> compare it to the worktree. If it doesn't take the index lock, other
> things could change either the index or the worktree underneath it,
> making git status lie (or even crash)"
Makes sense...
> * It sounds as though a patch refactoring `git rebase` such that it
> holds the index lock may be considered, although this is a
> non-trivial refactoring as `rebase` is currently a shell script
>
> * One option would be to check whether a rebase is in progress before
> auto-reverting. In pseudo-shell,
>
> if [ -e .git/rebase-apply ] || [ -e .git/rebase-merge ]; then
> do_not_call_git_status_because_we_are_rebasing;
> else
> auto_revert
> fi
>
> Unfortunately it sounds like this wouldn't do the right thing when a
> rebase pauses due to conflict (when you'd ideally want to
> auto-revert).
Yeah that would definitely be a problem. The other problem with this is
fix is that it wouldn't fix any other use case that locks the index,
like my repro earlier:
$ cd /path/to/emacs
$ ./src/emacs -Q lisp/*.el
[ M-x global-auto-revert-mode ]
$ for i in $(seq 30); do for f in lisp/*.el; do echo "; $i" >> $f; done; git commit -am "test $i"; done
There's no rebase there, so detecting a rebase to skip auto-revert
wouldn't help...
Hopefully we can get rid of the `git status' from the auto-revert path
altogether. It looks like the only place we do a `git status' is in
`vc-git-conflicted-files'. We call that from `vc-git-find-file-hook' to
see if we should start smerge. But I don't see how we get there from
auto-revert.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Sun, 07 Feb 2016 17:24:01 GMT)
Full text and
rfc822 format available.
Message #68 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Mitchel Humpherys <mitch.special <at> gmail.com> writes:
> There's no rebase there, so detecting a rebase to skip auto-revert
> wouldn't help...
>
Indeed. This is a good point.
> Hopefully we can get rid of the `git status' from the auto-revert path
> altogether. It looks like the only place we do a `git status' is in
> `vc-git-conflicted-files'. We call that from `vc-git-find-file-hook' to
> see if we should start smerge. But I don't see how we get there from
> auto-revert.
I believe the path is,
auto-revert-handler
vc-refresh-state
vc-call-backend 'file-life-hook
vc-git-find-file-hook
I'm not sure whether it is possible to eliminate the `git status' call.
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 08 Feb 2016 21:21:02 GMT)
Full text and
rfc822 format available.
Message #71 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 02/07/2016 02:55 AM, Ben Gamari wrote:
> I've spoken with some folks in #git about this issue.
>
> * the index lock is held "because it needs to read the index and
> compare it to the worktree. If it doesn't take the index lock, other
> things could change either the index or the worktree underneath it,
> making git status lie (or even crash)"
>
> * It sounds as though a patch refactoring `git rebase` such that it
> holds the index lock may be considered, although this is a
> non-trivial refactoring as `rebase` is currently a shell script
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.
FWIW, whatever its other faults, hg at least operates correctly here.
[signature.asc (application/pgp-signature, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Fri, 09 Sep 2016 21:14:01 GMT)
Full text and
rfc822 format available.
Message #74 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
The problematic invocation of git status comes from
(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"
"--"))
I'm working around this issue by changing vc-git-conflicted-files to use
diff-files --name-status, which doesn't lock the index:
(defun vc-git-conflicted-files (directory)
"Return the list of files with conflicts in DIRECTORY."
(let* ((status
(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?!]\\)[ \t]+\\(.+\\)" line)
(let ((state (match-string 1 line))
(file (match-string 2 line)))
;; See git-status(1).
(when (equal state "U")
(push (expand-file-name file directory) files)))))))
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Wed, 14 Feb 2018 16:30:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Since some version, Git supports "--no-optional-locks" switch and
"GIT_OPTIONAL_LOCKS" environment variable for avoiding locking during "git
status".
That's how they are documented:
----------------------------------------
https://git-scm.com/docs/git#git---no-optional-locks
--no-optional-locks
Do not perform optional operations that require locks. This is equivalent
to setting the GIT_OPTIONAL_LOCKS to 0.
https://git-scm.com/docs/git-status#_background_refresh
BACKGROUND REFRESH
By default, git status will automatically refresh the index, updating the
cached stat information from the working tree and writing out the result.
Writing out the updated index is an optimization that isn’t strictly
necessary (status computes the values for itself, but writing them out is
just to save subsequent programs from repeating our computation). When
status is run in the background, the lock held during the write may
conflict with other simultaneous processes, causing them to fail. Scripts
running status in the background should consider using git
--no-optional-locks status (see git[1] for details).
----------------------------------------
Using this "--no-optional-locks" switch looks like a solution to the issue?
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Thu, 15 Feb 2018 19:10:02 GMT)
Full text and
rfc822 format available.
Message #80 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Judging from the comment of the commit implementing the
"--no-optional-locks" switch,
https://github.com/git/git/commit/27344d6a6c8056664966e11acf674e5da6dd7ee3
, the switch was implemented exactly for background refresh in "tools like
IDEs or fancy editors".
I.e. for mitigating this particular bug! Now we only have to use this
switch in our "fancy editor".
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Thu, 15 Feb 2018 22:34:01 GMT)
Full text and
rfc822 format available.
Message #83 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
I am sending patch that fixes the issue. It fixes the issue for me on
Emacs' current master branch. The patch also cleanly applies to emacs-26
branch.
I tested using Mitchel Humpherys' repro method, i.e.
$ cd /path/to/emacs
$ ./src/emacs -Q lisp/*.el
[ M-x global-auto-revert-mode ]
$ for i in $(seq 30); do for f in lisp/*.el; do echo "; $i" >> $f;
done; git commit -am "test $i"; done
Without the fix I could reproduce the issue, and with the fix I could not
reproduce even if running the loop 100 times instead of 30. Emacs was
reloading a lot of files very fast, but no locking errors happened in the
shell which ran the loop.
2018-02-15 20:08 GMT+01:00 Alexei Khlebnikov <alexei.khlebnikov <at> gmail.com>:
> Judging from the comment of the commit implementing the
> "--no-optional-locks" switch,
>
> https://github.com/git/git/commit/27344d6a6c8056664966e11acf674e5da6dd7ee3
>
> , the switch was implemented exactly for background refresh in "tools like
> IDEs or fancy editors".
> I.e. for mitigating this particular bug! Now we only have to use this
> switch in our "fancy editor".
>
>
[Message part 2 (text/html, inline)]
[0001-Fix-for-25.0.50-auto-revert-mode-breaks-git-rebase-B.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 19 Feb 2018 10:26:01 GMT)
Full text and
rfc822 format available.
Message #86 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi All,
How can I get my patch reviewed?
The patch is sent in the previous message, several days ago:
https://debbugs.gnu.org/cgi/bugreport.cgi?msg=83;bug=21559;att=2;filename=0001-Fix-for-25.0.50-auto-revert-mode-breaks-git-rebase-B.patch
The same patch on GitHub:
https://github.com/alexeikh/emacs/commit/4bba51ded42954a62c99696453c17e7fa3f2b730
Adding to the discussion:
* Dmitry Gutov, because he is the maintainer for lisp/vc/* .
* Eli Zaretskii and Michael Albinus, because they participated in the
discussion on this bug before.
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 19 Feb 2018 12:40:02 GMT)
Full text and
rfc822 format available.
Message #89 received at 21559 <at> debbugs.gnu.org (full text, mbox):
Alexei Khlebnikov <alexei.khlebnikov <at> gmail.com> writes:
> Hi All,
Hi Alexei,
> https://debbugs.gnu.org/cgi/bugreport.cgi?msg=83;bug=21559;att=2;filename=0001-Fix-for-25.0.50-auto-revert-mode-breaks-git-rebase-B.patch
I don't know git in detail, so I don't know whether this argument is
useful.
However, I don't believe this argument shall be added, depending on git
version number. A better approach would be to prepend
"GIT_OPTIONAL_LOCKS=0" in `process-environment'. This would work for any
git version, w/o further check. For git version <= 2.15.0, it would
simply do nothing.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 19 Feb 2018 15:30:02 GMT)
Full text and
rfc822 format available.
Message #92 received at 21559 <at> debbugs.gnu.org (full text, mbox):
> From: Alexei Khlebnikov <alexei.khlebnikov <at> gmail.com>
> Date: Mon, 19 Feb 2018 11:24:57 +0100
>
> How can I get my patch reviewed?
I already reviewed it. I was waiting for a few days to give others a
chance to comment.
> Adding to the discussion:
> * Dmitry Gutov, because he is the maintainer for lisp/vc/* .
> * Eli Zaretskii and Michael Albinus, because they participated in the discussion on this bug before.
Thanks, but there's no need to add me: I read all the messages sent to
the bug list.
I think Michael's suggestion makes a lot of sense, btw.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 19 Feb 2018 18:41:01 GMT)
Full text and
rfc822 format available.
Message #95 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli, Michael, thanks for the review.
> A better approach would be to prepend "GIT_OPTIONAL_LOCKS=0" in `process-environment'.
Thanks for the suggestion, I am attaching version 2 of the patch which
implements this suggestion.
> > Adding to the discussion:
> > * Dmitry Gutov, because he is the maintainer for lisp/vc/* .
> > * Eli Zaretskii and Michael Albinus, because they participated in the discussion on this bug before.
>
> Thanks, but there's no need to add me: I read all the messages sent to
> the bug list.
OK, I've removed you from the addressees. I'm not sure if I should do
the same for others, so I left Dmitry and Michael in CC.
[v2-0001-Fix-for-25.0.50-auto-revert-mode-breaks-git-rebas.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 19 Feb 2018 18:51:02 GMT)
Full text and
rfc822 format available.
Message #98 received at 21559 <at> debbugs.gnu.org (full text, mbox):
Alexei Khlebnikov <alexei.khlebnikov <at> gmail.com> writes:
> Eli, Michael, thanks for the review.
Hi Alexei,
>> A better approach would be to prepend "GIT_OPTIONAL_LOCKS=0" in
>> `process-environment'.
>
> Thanks for the suggestion, I am attaching version 2 of the patch which
> implements this suggestion.
Looks OK to me.
>> Thanks, but there's no need to add me: I read all the messages sent to
>> the bug list.
>
> OK, I've removed you from the addressees. I'm not sure if I should do
> the same for others, so I left Dmitry and Michael in CC.
I don't care. I try to react on relevant messages in the respective
mailing lists, but it could happen that I oversee something.
Unlike Eli, I don't read *every* message. The subject must tell me that
I shall do.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 19 Feb 2018 19:07:01 GMT)
Full text and
rfc822 format available.
Message #101 received at 21559 <at> debbugs.gnu.org (full text, mbox):
2018-02-19 19:50 GMT+01:00 Michael Albinus <michael.albinus <at> gmx.de>:
> Looks OK to me.
Cool, so quick answer, thanks! :)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 19 Feb 2018 23:36:02 GMT)
Full text and
rfc822 format available.
Message #104 received at 21559 <at> debbugs.gnu.org (full text, mbox):
On 2/19/18 8:50 PM, Michael Albinus wrote:
> I try to react on relevant messages in the respective
> mailing lists, but it could happen that I oversee something.
Same here, except I slightly prefer to be in Cc. That makes the thread a
bit easier to notice.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Mon, 19 Feb 2018 23:43:01 GMT)
Full text and
rfc822 format available.
Message #107 received at 21559 <at> debbugs.gnu.org (full text, mbox):
On 2/15/18 9:08 PM, Alexei Khlebnikov wrote:
> Judging from the comment of the commit implementing the
> "--no-optional-locks" switch,
>
> https://github.com/git/git/commit/27344d6a6c8056664966e11acf674e5da6dd7ee3
>
> , the switch was implemented exactly for background refresh in "tools
> like IDEs or fancy editors".
> I.e. for mitigating this particular bug! Now we only have to use this
> switch in our "fancy editor".
OK, here's my question: what is a "background refresh"? Must be consider
every VC operation to be "background"?
From what I see of this switch's description, it's going to (slightly?
imperceptibly?) slow down the VC operations. It would be cool to see
some measurement of that effect.
Failing that, why don't we try something else first? If the problem
occurs solely due to auto-revert-mode's calls to Git, let's try adding
that environment variable binding inside auto-revert-handler. The patch
is below. Does it solve the problem as well?
diff --git a/lisp/autorevert.el b/lisp/autorevert.el
index cf145e0ee3..41e9f00049 100644
--- a/lisp/autorevert.el
+++ b/lisp/autorevert.el
@@ -709,7 +709,9 @@ auto-revert-handler
;; `preserve-modes' avoids changing the (minor) modes. But we do
;; want to reset the mode for VC, so we do it manually.
(when (or revert auto-revert-check-vc-info)
- (vc-refresh-state))))
+ (let ((process-environment
+ (cons "GIT_OPTIONAL_LOCKS=0" process-environment)))
+ (vc-refresh-state)))))
(defun auto-revert-tail-handler (size)
(let ((modified (buffer-modified-p))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Tue, 20 Feb 2018 00:08:01 GMT)
Full text and
rfc822 format available.
Message #110 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
2018-02-20 0:41 GMT+01:00 Dmitry Gutov <dgutov <at> yandex.ru>:
> Failing that, why don't we try something else first? If the problem occurs
> solely due to auto-revert-mode's calls to Git, let's try adding that
> environment variable binding inside auto-revert-handler. The patch is
> below. Does it solve the problem as well?
>
I've just tested - yes, it solves the problem as well!
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Tue, 20 Feb 2018 00:19:02 GMT)
Full text and
rfc822 format available.
Message #113 received at 21559 <at> debbugs.gnu.org (full text, mbox):
On 2/20/18 2:06 AM, Alexei Khlebnikov wrote:
> 2018-02-20 0:41 GMT+01:00 Dmitry Gutov <dgutov <at> yandex.ru
> <mailto:dgutov <at> yandex.ru>>:
>
> Failing that, why don't we try something else first? If the problem
> occurs solely due to auto-revert-mode's calls to Git, let's try
> adding that environment variable binding inside auto-revert-handler.
> The patch is below. Does it solve the problem as well?
>
>
> I've just tested - yes, it solves the problem as well!
Terrific, thank you.
What do others think? Michael, Eli?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Tue, 20 Feb 2018 04:08:01 GMT)
Full text and
rfc822 format available.
Message #116 received at 21559 <at> debbugs.gnu.org (full text, mbox):
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Tue, 20 Feb 2018 02:17:51 +0200
> Cc: Michael Albinus <michael.albinus <at> gmx.de>
>
> On 2/20/18 2:06 AM, Alexei Khlebnikov wrote:
> > 2018-02-20 0:41 GMT+01:00 Dmitry Gutov <dgutov <at> yandex.ru
> > <mailto:dgutov <at> yandex.ru>>:
> >
> > Failing that, why don't we try something else first? If the problem
> > occurs solely due to auto-revert-mode's calls to Git, let's try
> > adding that environment variable binding inside auto-revert-handler.
> > The patch is below. Does it solve the problem as well?
> >
> >
> > I've just tested - yes, it solves the problem as well!
>
> Terrific, thank you.
>
> What do others think? Michael, Eli?
Fine with me, but please add a comment there explaining why we do
that, as the relation between auto-revert and Git is entirely not
obvious.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Tue, 20 Feb 2018 07:42:01 GMT)
Full text and
rfc822 format available.
Message #119 received at 21559 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
Hi Dmitry,
> From what I see of this switch's description, it's going to (slightly?
> imperceptibly?) slow down the VC operations. It would be cool to see
> some measurement of that effect.
>
> Failing that, why don't we try something else first? If the problem
> occurs solely due to auto-revert-mode's calls to Git, let's try adding
> that environment variable binding inside auto-revert-handler. The
> patch is below. Does it solve the problem as well?
I'm not so happy seeing git specific code in autorevert.el (yes, I know,
it's just an environment variable). All git specifics shall be kept in
vc-git.el. Otherwise, we would go into a dependency nightmare, soon. Do
you know, that magit would like to see this setting?
If this variable hurts for standard operation of vc-git, we could set it
depending whether (auto)revert is running, or not. There is the variable
`revert-buffer-in-progress-p', which is non-nil if reverting is in
progress. I could set it also before calling `vc-refresh-state' in
`auto-revert-handler'. And in vc-git.el, `process-environment' could be
extended or not, depending on the value of `revert-buffer-in-progress-p'.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Tue, 20 Feb 2018 11:38:02 GMT)
Full text and
rfc822 format available.
Message #122 received at 21559 <at> debbugs.gnu.org (full text, mbox):
On 2/20/18 9:40 AM, Michael Albinus wrote:
> I'm not so happy seeing git specific code in autorevert.el (yes, I know,
> it's just an environment variable). All git specifics shall be kept in
> vc-git.el. Otherwise, we would go into a dependency nightmare, soon.
But this way the patch is much smaller, isn't it? That has
maintainability benefits too.
And using a variable would make more sense if we determine that other
facilities, not just autorevert, make VC calls that we want to consider
"background".
> Do
> you know, that magit would like to see this setting?
I imagine it would, if it makes Git calls during autorevert (and if
doesn't, the var won't do anything). But we could let it make that
choice by itself, of course.
> If this variable hurts for standard operation of vc-git, we could set it
> depending whether (auto)revert is running, or not. There is the variable
> `revert-buffer-in-progress-p', which is non-nil if reverting is in
> progress. I could set it also before calling `vc-refresh-state' in
> `auto-revert-handler'. And in vc-git.el, `process-environment' could be
> extended or not, depending on the value of `revert-buffer-in-progress-p'.
In which part of vc-git.el? Changing the implementation of two commands,
like the original patch proposed, makes for a bigger change.
We could do that in vc-git-command, I suppose...
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Tue, 20 Feb 2018 11:54:02 GMT)
Full text and
rfc822 format available.
Message #125 received at 21559 <at> debbugs.gnu.org (full text, mbox):
Dmitry Gutov <dgutov <at> yandex.ru> writes:
Hi Dmitry,
>> I'm not so happy seeing git specific code in autorevert.el (yes, I know,
>> it's just an environment variable). All git specifics shall be kept in
>> vc-git.el. Otherwise, we would go into a dependency nightmare, soon.
>
> But this way the patch is much smaller, isn't it? That has
> maintainability benefits too.
A dependency nightmare counts much more, IMHO.
> And using a variable would make more sense if we determine that other
> facilities, not just autorevert, make VC calls that we want to
> consider "background".
autorevert wouldn't care who uses this variable. In a broader sense, it
could let-bind it for the whole auto-revert-handler body.
>> If this variable hurts for standard operation of vc-git, we could set it
>> depending whether (auto)revert is running, or not. There is the variable
>> `revert-buffer-in-progress-p', which is non-nil if reverting is in
>> progress. I could set it also before calling `vc-refresh-state' in
>> `auto-revert-handler'. And in vc-git.el, `process-environment' could be
>> extended or not, depending on the value of `revert-buffer-in-progress-p'.
>
> In which part of vc-git.el? Changing the implementation of two
> commands, like the original patch proposed, makes for a bigger change.
>
> We could do that in vc-git-command, I suppose...
Perhaps. autorevert shouldn't know anything about vc-git, it should just
let-bind the variable, and let other packages decide whether they use
it.
FWIW, I'm also not so enthusiastic, that aut-revert-handler calls
vc-refresh-state directly. This would be better organized by a hook.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Tue, 20 Feb 2018 22:29:02 GMT)
Full text and
rfc822 format available.
Message #128 received at 21559 <at> debbugs.gnu.org (full text, mbox):
On 2/20/18 1:53 PM, Michael Albinus wrote:
> A dependency nightmare counts much more, IMHO.
A nightmare isn't going to arrive overnight.
>> And using a variable would make more sense if we determine that other
>> facilities, not just autorevert, make VC calls that we want to
>> consider "background".
>
> autorevert wouldn't care who uses this variable. In a broader sense, it
> could let-bind it for the whole auto-revert-handler body.
I meant that if there's going to be more places that are going to *bind*
this variable. In that case, moving the relevant code into vc/* would be
unavoidable.
>> We could do that in vc-git-command, I suppose...
>
> Perhaps. autorevert shouldn't know anything about vc-git, it should just
> let-bind the variable, and let other packages decide whether they use
> it.
All right, so you just want to move the responsibility.
> FWIW, I'm also not so enthusiastic, that aut-revert-handler calls
> vc-refresh-state directly. This would be better organized by a hook.
On the other hand, since it already has this direct call, an extra let
binding isn't going to change much.
Anyway, it's your choice here. Do you want to show an alternative patch?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Wed, 21 Feb 2018 22:08:02 GMT)
Full text and
rfc822 format available.
Message #131 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Here is version 4 of the patch.
"revert-buffer-in-progress-p" flag is set in "auto-revert-handler",
according to Michael's idea and then checked in "vc-git-command", according
to Dmitry's idea.
I have tested the patch, it works.
[Message part 2 (text/html, inline)]
[v4-0001-Fix-for-25.0.50-auto-revert-mode-breaks-git-rebas.patch (text/x-patch, attachment)]
Reply sent
to
Michael Albinus <michael.albinus <at> gmx.de>:
You have taken responsibility.
(Thu, 22 Feb 2018 11:25:01 GMT)
Full text and
rfc822 format available.
Notification sent
to
Ben Gamari <ben <at> smart-cactus.org>:
bug acknowledged by developer.
(Thu, 22 Feb 2018 11:25:03 GMT)
Full text and
rfc822 format available.
Message #136 received at 21559-done <at> debbugs.gnu.org (full text, mbox):
Version: 27.1
Alexei Khlebnikov <alexei.khlebnikov <at> gmail.com> writes:
Hi Alexei,
> Here is version 4 of the patch.
>
> "revert-buffer-in-progress-p" flag is set in "auto-revert-handler",
> according to Michael's idea and then checked in "vc-git-command",
> according to Dmitry's idea.
LGTM, I've pushed this to master.
> I have tested the patch, it works.
Thanks. I'm closing the bug.
Best regards, Michael.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Thu, 22 Feb 2018 11:46:02 GMT)
Full text and
rfc822 format available.
Message #139 received at 21559-done <at> debbugs.gnu.org (full text, mbox):
On 2/22/18 1:24 PM, Michael Albinus wrote:
>> Here is version 4 of the patch.
>>
>> "revert-buffer-in-progress-p" flag is set in "auto-revert-handler",
>> according to Michael's idea and then checked in "vc-git-command",
>> according to Dmitry's idea.
>
> LGTM, I've pushed this to master.
Great, thank you both.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Fri, 23 Mar 2018 11:24:04 GMT)
Full text and
rfc822 format available.
bug unarchived.
Request was from
Andrew Ruder <andy <at> aeruder.net>
to
control <at> debbugs.gnu.org.
(Tue, 25 Sep 2018 05:52:02 GMT)
Full text and
rfc822 format available.
bug No longer marked as fixed in versions 27.1 and reopened.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Tue, 25 Sep 2018 05:52:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
michael.albinus <at> gmx.de, alexei.khlebnikov <at> gmail.com, dgutov <at> yandex.ru, bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Tue, 25 Sep 2018 15:15:01 GMT)
Full text and
rfc822 format available.
Message #148 received at 21559 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Please see attached patch. There is also some early reports of
success at https://github.com/magit/magit/issues/2708
In short, I believe the analysis for this bug was thorough (and
correct!), but it turns out that the call to `git status` doesn't
actually go through the patched function so we'd end up with a few
commands getting GIT_OPTIONAL_LOCKS=0 but not the most important one.
- Andrew Ruder
P.S. still figuring out the contribution process for emacs.
[0001-Fix-for-25.0.50-auto-revert-mode-breaks-git-rebase-B.patch (text/plain, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Wed, 26 Sep 2018 03:17:01 GMT)
Full text and
rfc822 format available.
Message #151 received at 21559 <at> debbugs.gnu.org (full text, mbox):
On 2018-09-26 00:23, Andrew Ruder wrote:
> Please see attached patch. There is also some early reports of
> success at https://github.com/magit/magit/issues/2708
Just to reiterate my comments from there, I believe this new patch
does indeed resolve this bug.
I've been using Emacs 26.1 with the earlier patch applied:
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=21559#131
And git version 2.16.3
With that, I was very regularly encountering the error "Unable to
create '.git/index.lock': File exists." when rebasing in Magit with
global-auto-revert-mode enabled.
With this new patch applied too, I am not seeing the error at all.
For testing I repeatedly rebased a moderately large branch, with and
without the new patch (3 times each). Without the new patch I was
encountering "Unable to create '.git/index.lock': File exists."
between 3 and 5 times during each rebase. With the new patch I didn't
encounter the problem at all in any of the rebases.
-Phil
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#21559; Package
emacs.
(Wed, 26 Sep 2018 09:46:01 GMT)
Full text and
rfc822 format available.
Message #154 received at 21559 <at> debbugs.gnu.org (full text, mbox):
Phil Sainty <psainty <at> orcon.net.nz> writes:
> On 2018-09-26 00:23, Andrew Ruder wrote:
>> Please see attached patch. There is also some early reports of
>> success at https://github.com/magit/magit/issues/2708
>
> Just to reiterate my comments from there, I believe this new patch
> does indeed resolve this bug.
It looks also good to me. If nobody opposes, I'll commit it next days.
> -Phil
Best regards, Michael.
Reply sent
to
Michael Albinus <michael.albinus <at> gmx.de>:
You have taken responsibility.
(Sat, 29 Sep 2018 11:17:03 GMT)
Full text and
rfc822 format available.
Notification sent
to
Ben Gamari <ben <at> smart-cactus.org>:
bug acknowledged by developer.
(Sat, 29 Sep 2018 11:17:03 GMT)
Full text and
rfc822 format available.
Message #159 received at 21559-done <at> debbugs.gnu.org (full text, mbox):
Version: 27.1
> It looks also good to me. If nobody opposes, I'll commit it next days.
Pushed to master. I'm closing the bug.
Best regards, Michael.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Sat, 27 Oct 2018 11:24:05 GMT)
Full text and
rfc822 format available.
This bug report was last modified 6 years and 293 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.