magit-commit editor breakage after updating emacs master
miangraham opened this issue · 12 comments
Hello and many thanks for magit.
I'm a NixOS user building emacs from master and started seeing failures from magit-commit between yesterday and today after updating emacs (magit and other packages unchanged). I can't immediately tell whether this issue should be filed against magit or emacs. Advice appreciated.
What I did: Open magit-status in a project with staged changes. Run magit-commit (c c).
What I expected: Magit (via git and emacsclient) opens a new window with commit message buffer for editing.
What happened: New window does not appear. With no debugging enabled, magit shows a GitError saying there was a problem with the editor but containing no additional error. With debug enabled, git seems to wait for the editor for a long time before (hitting a timeout?) then outputting the same.
I don't see an obvious culprit in the emacs source diff but I'm unfamiliar so maybe there's something there. I also can't spot (any way to turn on?) output or errors from the emacsclient commit message editor side of things.
Please let me know if there are other relevant flags to set or info I can grab and share. Otherwise, any ideas for how to narrow this down further? Thanks in advance.
Error output
*Messages*
Running git commit --
Only whitespace and/or comments; message not saved
Type C-c C-c to finish, C-c C-k to cancel, and M-p and M-n to recover older messages
There was a problem with the editor '/nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0/bin/emacsclient --socket-name=/run/user/1000/emacs/server'. ... [Hit $ to see buffer magit-process: .nix for details]
magit-process
1 git … commit --
hint: Waiting for your editor to close the file...
Waiting for Emacs...
*ERROR*: Don’t kill this buffer #<buffer COMMIT_EDITMSG>. Instead cancel using C-c C-k
error: There was a problem with the editor '/nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0/bin/emacsclient --socket-name=/run/user/1000/emacs/server'.
Please supply the message using either -m or -F option.
Version info
OS: NixOS
GUI: Sway (wayland)
Emacs version: emacs-mirror/emacs@b568a41 (--with-native-compilation --with-pgtk, have also reproduced the failure with these disabled)
Nix-side good-before bad-after commit: nix-community/emacs-overlay@3a52bb5
Emacs-side diff: emacs-mirror/emacs@516ff42...b568a41
516ff42 works as expected
b568a41 breaks
magit-version
Magit 20220426.949 [>= 3.3.0-git], Git 2.33.3, Emacs 29.0.50, gnu/linux
emacs -Q command used
/nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0/bin/emacs -Q --eval \(setq\ debug-on-error\ t\) -L /nix/store/2mc7nbxc9x4dr6b52xb2xcn041c6cvc9-emacs-packages-deps/share/emacs/site-lisp/elpa/compat-28.1.1.0/ -L /nix/store/2mc7nbxc9x4dr6b52xb2xcn041c6cvc9-emacs-packages-deps/share/emacs/site-lisp/elpa/dash-20220417.2250/ -L /nix/store/2mc7nbxc9x4dr6b52xb2xcn041c6cvc9-emacs-packages-deps/share/emacs/site-lisp/elpa/transient-20220425.1314/ -L /nix/store/2mc7nbxc9x4dr6b52xb2xcn041c6cvc9-emacs-packages-deps/share/emacs/site-lisp/elpa/with-editor-20220422.1628/ -L /nix/store/2mc7nbxc9x4dr6b52xb2xcn041c6cvc9-emacs-packages-deps/share/emacs/site-lisp/elpa/magit-20220426.949/ -L /nix/store/2mc7nbxc9x4dr6b52xb2xcn041c6cvc9-emacs-packages-deps/share/emacs/site-lisp/elpa/magit-section-20220425.1002/ -L /nix/store/2mc7nbxc9x4dr6b52xb2xcn041c6cvc9-emacs-packages-deps/share/emacs/site-lisp/elpa/git-commit-20220422.1903/ -l /nix/store/2mc7nbxc9x4dr6b52xb2xcn041c6cvc9-emacs-packages-deps/share/emacs/site-lisp/elpa/magit-20220426.949/magit
with-editor-debug
with-editor: /nix/store/2mc7nbxc9x4dr6b52xb2xcn041c6cvc9-emacs-packages-deps/share/emacs/site-lisp/elpa/with-editor-20220422.1628/with-editor.el
emacs: /nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0/bin/emacs (29.0.50)
system:
system-type: gnu/linux
system-configuration: x86_64-pc-linux-gnu
system-configuration-options: --prefix=/nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0 --disable-build-details --with-modules --with-x-toolkit=gtk3 --with-cairo --with-native-compilation --with-pgtk
server:
server-running-p: t
server-process: #<process server>
server-use-tcp: nil
server-name: server
server-socket-dir: /run/user/1000/emacs
server
server-auth-dir: ~/.emacs.d/server/
WARNING: not an accessible directory
with-editor-emacsclient-executable:
value: /nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0/bin/emacsclient (29.0.50)
default: /nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0/bin/emacsclient (29.0.50)
funcall: /nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0/bin/emacsclient (29.0.50)
path:
$PATH: "/run/wrappers/bin:/home/ian/.nix-profile/bin:/etc/profiles/per-user/ian/bin:/nix/var/nix/profiles/default/bin:/run/current-system/sw/bin"
exec-path: (/run/wrappers/bin /home/ian/.nix-profile/bin /etc/profiles/per-user/ian/bin /nix/var/nix/profiles/default/bin /run/current-system/sw/bin /nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0/libexec/emacs/29.0.50/x86_64-pc-linux-gnu)
with-editor-emacsclient-path:
/nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0/bin (t)
/nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0/bin/.emacsclient-wrapped (29.0.50)
/nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0/bin/emacsclient (29.0.50)
/run/wrappers/bin (/run/wrappers/wrappers.A8R66dvz6u)
/home/ian/.nix-profile/bin (nil)
/etc/profiles/per-user/ian/bin (t)
/nix/var/nix/profiles/default/bin (nil)
/run/current-system/sw/bin (t)
/run/current-system/sw/bin/emacsclient (29.0.50)
/nix/store/brfnmbq8kl08ifkk9j1bhd9f19v2jmr0-emacs-pgtk-native-comp-20220427.0/libexec/emacs/29.0.50/x86_64-pc-linux-gnu (t)
I just updated (from 516ff422c5 to 5aef2623a3) and now I cannot commit anymore either. Hurray!
I am out the door now. Maybe someone could bisect the issue for me? (And maybe even report it to the Emacs maintainers, if that seems appropriate.)
I should be able to narrow to a single emacs commit within an hour or two if no one's already poking at it. Will shout back if I'm not beaten to it.
I've started bisection but have a slow machine and need to do "real" (paid) work in parallel. We'll see who is first. ;-)
I have it:
emacs on HEAD (bc9be54) (BISECTING) took 14s
❯ git bisect good
f30625943edefbd88ebf84acbc254ed88db27beb is the first bad commit
commit f30625943edefbd88ebf84acbc254ed88db27beb
Author: Stefan Monnier <monnier@iro.umontreal.ca>
Date: Tue Apr 26 16:39:41 2022 -0400
nadvice.el: Use OClosures
* lisp/emacs-lisp/nadvice.el (advice): New OClosure type.
(advice--how-alist): Make it hold prototype OClosures rather
than bytecode strings.
(advice--bytecodes): Delete var.
(advice--where): Make it an obsolete alias of new `advice--how`.
(oclosure-interactive-form, cl-print-object) <advice>: New methods.
(advice--make-1): Delete function.
(advice--make): Use `advice-copy` and `advice-cons`.
(advice--tweak): Use `advice-cons`.
(add-function, advice-add): Rename `where` arg to `how`.
* lisp/emacs-lisp/cl-print.el (cl-print-object) <:extra "nadvice">:
Remove now-redundant ad-hoc method.
* test/lisp/emacs-lisp/nadvice-tests.el (advice-test-print): New test.
lisp/emacs-lisp/cl-print.el | 21 -------
lisp/emacs-lisp/nadvice.el | 108 +++++++++++++++++-----------------
test/lisp/emacs-lisp/nadvice-tests.el | 9 +++
3 files changed, 64 insertions(+), 74 deletions(-)
I'll write a bug report for emacs.
f30625943edefbd88ebf84acbc254ed88db27beb is the first bad commit
Confirmed here with slow clean builds.
Parent bc9be5449e1127bc1b05a6cad8471c6eba52c8e9 good
Child f30625943edefbd88ebf84acbc254ed88db27beb bad
Reported to emacs: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=55149
@miangraham Does "slow clean builds" mean "make bootstrap"? If not, could you try that to check if the bug vanishes when doing so?
I'm building/installing via nixos and changing the source revision which results in shared-nothing. Downloads a new source tarball to a new location and shares nothing from the previous build. So pretty sure there's a make bootstrap in there somewhere if it's required.
Alright, I can confirm that after make bootstrap the issue is still there.
Slightly OT: Do bug-gnu-emacs and company filter out non-subscribers? Tried a reply on the bug over there and not seeing it show up in the archive after 30m+, feels like I'm getting spam filtered.
@miangraham Your message appeared and I've already responded with an "emacs -Q" recipe. I haven't noticed earlier that the breakage only occurred when with-editor was loaded.
The fix is in on master. I've rebuilt and verified that simple test cases and magit-commit both work without issue. Looks solved!
Quick summary for posterity: Return values for adviced :after functions were broken and ignoring the original function's return value, instead overriding it with the advice function's result. This broke with-editor because it advices server-visit-files and would cause it to always return nil after the regression, killing all emacsclient usage after with-editor was loaded.
Thanks much to @tsdh for the help with reporting and testing.