Possible bug: org-capture fails when emacs started as daemon

Possible bug: org-capture fails when emacs started as daemon

[Iñigo Serna]

Hi,

Since some months ago (sorry, can't remember when it started) I'm 
suffering a problem with org-capture: every time I run it and 
select a template it errors with message "org-capture: Capture 
template ‘i’: Buffer name ‘CAPTURE-todo.org’ is in use".
I'm posting the issue here in case it's a silly thing on my side 
before opening a bug report.

This happens on my main emacs instance: daily master, pgtk, 
native-compilation, on a x64 Fedora Linux 38 under swaywm 
(Wayland).  Emacs started as systemd user unit with --fg-daemon 
flag and connecting with emacs-client.

I replicated the errors consistently in these tests:
- with pgtk / gtk3 / lucid
- running on Wayland (swaywm) or X (GNOME Classic)
- with/without native-compilation
- starting emacs as systemd unit or from terminal *with* 
  --{fg,bg}-daemon options
- with minimum / complete configuration

All the same tests set with v29.1 on this machine fail as well.

Instead, if I run emacs from terminal without daemon option it can 
run org-capture successfully, even after executing `server-start'. 
But once I run the first emacs-client, org-capture fails from 
both, main and client.

As I see the error message is defined in either 
`make-indirect-buffer' or `rename-buffer' functions, and the first 
one is called from `org-capture-make-indirect-buffer'.
How could I investigate further the issue? Any hints?


Thanks in advance,
-- 
Iñigo Serna

Re: Possible bug: org-capture fails when emacs started as daemon

[Morgan Willcock]

Iñigo Serna <inigoserna@gmx.com> writes:

> Since some months ago (sorry, can't remember when it started) I'm 
> suffering a problem with org-capture: every time I run it and 
> select a template it errors with message "org-capture: Capture 
> template ‘i’: Buffer name ‘CAPTURE-todo.org’ is in use".
> I'm posting the issue here in case it's a silly thing on my side 
> before opening a bug report.

If it helps, I was having the same issue and narrowed it down to changes
I had made to face-remapping-alist, i.e. I fixed it by removing the
following from my configuration:

  (add-to-list 'face-remapping-alist
               (cons 'jabber-title-large 'jabber-title-medium))
  (add-to-list 'face-remapping-alist
               (cons 'jabber-title-medium 'jabber-title-small))

The jabber package was not in a great state so I presumed it was
something in there that was causing the problem, but possibly there is a
more general issue with face remapping when starting as a daemon.

Thanks,
Morgan

-- 
Morgan Willcock

Re: Possible bug: org-capture fails when emacs started as daemon

[Iñigo Serna]

Hi Morgan,

On 12 August 2023 at 19:05 +02, Morgan Willcock 
<morgan@ice9.digital> wrote:

> Iñigo Serna <inigoserna@gmx.com> writes:
>
>> Since some months ago (sorry, can't remember when it started) 
>> I'm 
>> suffering a problem with org-capture: every time I run it and 
>> select a template it errors with message "org-capture: Capture 
>> template ‘i’: Buffer name ‘CAPTURE-todo.org’ is in use".
>> I'm posting the issue here in case it's a silly thing on my 
>> side 
>> before opening a bug report.
>
> If it helps, I was having the same issue and narrowed it down to 
> changes
> I had made to face-remapping-alist [...]

Bingo! I missed a hook of `after-make-frame-functions` in my
configuration that precisely calls a function which plays with
face-remap.

Removing it makes org-capture work again.

Thanks a lot!
-- 
Iñigo Serna

Re: Possible bug: org-capture fails when emacs started as daemon

[Ihor Radchenko]

Iñigo Serna <inigoserna@gmx.com> writes:

> As I see the error message is defined in either 
> `make-indirect-buffer' or `rename-buffer' functions, and the first 
> one is called from `org-capture-make-indirect-buffer'.
> How could I investigate further the issue? Any hints?

`org-capture-get-indirect-buffer' constructs a name of a new capture
buffer as

(setq bname (concat prefix "-" base))
    (while (buffer-live-p (get-buffer bname))
      (setq bname (concat prefix "-" (number-to-string (cl-incf n)) "-" base)))

So, I can see the error happening when the buffer is not live, but yet
exists for some reason.

AFAIK, `get-buffer' check Vbuffer_alist, which should contain only live
buffers. If it is not the case, something fishy may be going on in Emacs.

I'd try to check the value of (get-buffer "CAPTURE-todo.org") and see if
it is a killed buffer or not.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: Possible bug: org-capture fails when emacs started as daemon

[Iñigo Serna]

Hi Ihor,

On 13 August 2023 at 10:37 +02, Ihor Radchenko 
<yantar92@posteo.net> wrote:

> Iñigo Serna <inigoserna@gmx.com> writes:
>
>> As I see the error message is defined in either 
>> `make-indirect-buffer' or `rename-buffer' functions, and the 
>> first 
>> one is called from `org-capture-make-indirect-buffer'.
>> How could I investigate further the issue? Any hints?
>
> `org-capture-get-indirect-buffer' constructs a name of a new 
> capture
> buffer as [...]

As Morgan suggested in other email, the problem comes from the use 
of
face-remap in a hook of `after-make-frame-functions`.

Thanks for your help,
Iñigo

Re: Possible bug: org-capture fails when emacs started as daemon

[Ihor Radchenko]

"Iñigo Serna" <inigoserna@gmail.com> writes:

>> `org-capture-get-indirect-buffer' constructs a name of a new 
>> capture
>> buffer as [...]
>
> As Morgan suggested in other email, the problem comes from the use 
> of
> face-remap in a hook of `after-make-frame-functions`.

That's very strange. I am wondering haw remapping faces has anything to
do with buffer objects. Sounds like some kind of arcane Emacs bug to me.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: Possible bug: org-capture fails when emacs started as daemon

[Morgan Willcock]

Ihor Radchenko <yantar92@posteo.net> writes:

> "Iñigo Serna" <inigoserna@gmail.com> writes:
>
>>> `org-capture-get-indirect-buffer' constructs a name of a new 
>>> capture
>>> buffer as [...]
>>
>> As Morgan suggested in other email, the problem comes from the use 
>> of
>> face-remap in a hook of `after-make-frame-functions`.
>
> That's very strange. I am wondering haw remapping faces has anything to
> do with buffer objects. Sounds like some kind of arcane Emacs bug to me.

From memory, the problem didn't exist in Emacs 28, but I encountered it
in the Emacs 29 pre-releases I was trying out.

So it seems like a bug introduced in Emacs 29.

-- 
Morgan Willcock

Re: Possible bug: org-capture fails when emacs started as daemon

[Ihor Radchenko]

Morgan Willcock <morgan@ice9.digital> writes:

> From memory, the problem didn't exist in Emacs 28, but I encountered it
> in the Emacs 29 pre-releases I was trying out.
>
> So it seems like a bug introduced in Emacs 29.

If you can create a reproducer, it would be nice to report this as
proper Emacs bug.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Re: Possible bug: org-capture fails when emacs started as daemon

[Eli Zaretskii]

> From: Ihor Radchenko <yantar92@posteo.net>
> Cc: Iñigo Serna <inigoserna@gmail.com>, emacs-devel@gnu.org
> Date: Tue, 15 Aug 2023 16:57:34 +0000
> 
> Morgan Willcock <morgan@ice9.digital> writes:
> 
> > From memory, the problem didn't exist in Emacs 28, but I encountered it
> > in the Emacs 29 pre-releases I was trying out.
> >
> > So it seems like a bug introduced in Emacs 29.
> 
> If you can create a reproducer, it would be nice to report this as
> proper Emacs bug.

And preferably without Org, please.  Since this is about
face-remapping-alist in some way, I see no reason for Org to be
involved, if it indeed is an Emacs bug.

Re: Possible bug: org-capture fails when emacs started as daemon

[Iñigo Serna]

[-- Attachment #1: Type: text/plain, Size: 1461 bytes --]

Hi,

> From: Ihor Radchenko <yantar92@posteo.net>
> 
> If you can create a reproducer, it would be nice to report this 
> as
> proper Emacs bug.

I've attached a simple case for the bug.
I can reproduce the issue with emacs v29 and master.

In the example, there is a `minibuffer-setup-hook' to modify
minibuffer face, and a simple minor-mode which changes
`mode-line-active' face background color when enabled.

Then, when `test-mode' is enabled, `clone-indirect-buffer' fails.

Steps:
1. $ emacs -q -l test.el    -> note minibuffer font size is bigger
2. M-x test-mode            -> note mode-line background color 
changes
3. M-x clone-indirect-buffer
   => Error: "face-attrs--make-indirect-safe: Wrong type argument: 
   listp, test-modeline"


A complete backtrace:

Debugger entered--Lisp error: (wrong-type-argument listp 
test-modeline)
  face-remap--copy-face((mode-line-active . test-modeline))
  face-attrs--make-indirect-safe()
  clone-indirect-buffer(nil t)
  funcall-interactively(clone-indirect-buffer nil t)
  command-execute(clone-indirect-buffer record)
  execute-extended-command(nil "clone-indirect-buffer" "clone-in")
  funcall-interactively(execute-extended-command nil 
  "clone-indirect-buffer" "clone-in")
  command-execute(execute-extended-command)


Btw, I also saw this problem with `mini-modeline' package, from 
where
I've taken some ideas to write this example.


Thanks,
Iñigo


[-- Attachment #2: test.el --]
[-- Type: text/plain, Size: 1084 bytes --]

;; 1. $ emacs -q -l test.el    -> note minibuffer font size is bigger
;; 2. M-x test-mode            -> note mode-line background color changes
;; 3. M-x clone-indirect-buffer
;; => Error: "face-attrs--make-indirect-safe: Wrong type argument: listp, test-modeline"


;; This could be in init.el
(add-hook 'minibuffer-setup-hook #'(lambda ()
                                     (face-remap-add-relative 'default :background "#8888cc" :height 2.0)))


;; Example code
(defface test-modeline
  '((t
     :background "#FF8888"))
  "Test face.")

(defvar test--orig-modeline nil)

(defun test--enable ()
  (setq-default test-orig-modeline 'mode-line-active)
  (setf (alist-get 'mode-line-active face-remapping-alist) 'test-modeline)
  (redisplay))

(defun test--disable ()
  (setq-default mode-line-active 'test-orig-modeline)
  (setf (alist-get 'mode-line-active face-remapping-alist) 'test-orig-modeline)
  (redisplay))

(define-minor-mode test-mode
  "Enable Test minor mode."
  :init-value nil
  :global t
  :lighter " Test"
  (if test-mode
      (test--enable)
    (test--disable)))

Re: Possible bug: org-capture fails when emacs started as daemon

[Eli Zaretskii]

> From: Iñigo Serna <inigoserna@gmx.com>
> Cc: Ihor Radchenko <yantar92@posteo.net>, morgan@ice9.digital,
>  emacs-devel@gnu.org
> Date: Wed, 16 Aug 2023 11:25:01 +0200
> 
> I've attached a simple case for the bug.
> I can reproduce the issue with emacs v29 and master.

Thanks.

> In the example, there is a `minibuffer-setup-hook' to modify
> minibuffer face, and a simple minor-mode which changes
> `mode-line-active' face background color when enabled.

AFAIU, the minibuffer-setup-hook part is not needed for this recipe to
demonstrate the problem.

> Then, when `test-mode' is enabled, `clone-indirect-buffer' fails.
> 
> Steps:
> 1. $ emacs -q -l test.el    -> note minibuffer font size is bigger
> 2. M-x test-mode            -> note mode-line background color 
> changes
> 3. M-x clone-indirect-buffer
>    => Error: "face-attrs--make-indirect-safe: Wrong type argument: 
>    listp, test-modeline"
> 
> 
> A complete backtrace:
> 
> Debugger entered--Lisp error: (wrong-type-argument listp 
> test-modeline)
>   face-remap--copy-face((mode-line-active . test-modeline))
>   face-attrs--make-indirect-safe()
>   clone-indirect-buffer(nil t)
>   funcall-interactively(clone-indirect-buffer nil t)
>   command-execute(clone-indirect-buffer record)
>   execute-extended-command(nil "clone-indirect-buffer" "clone-in")
>   funcall-interactively(execute-extended-command nil 
>   "clone-indirect-buffer" "clone-in")
>   command-execute(execute-extended-command)

Please see if the patch below gives good results.

Stefan, any comments?  You wrote the face-remap--copy-face stuff,
AFAICT.

diff --git a/lisp/face-remap.el b/lisp/face-remap.el
index 3ec271b..e86a292 100644
--- a/lisp/face-remap.el
+++ b/lisp/face-remap.el
@@ -70,6 +70,7 @@ internal-lisp-face-attributes
    :foreground :background :stipple :overline :strike-through :box
    :font :inherit :fontset :distant-foreground :extend :vector])
 
+(autoload 'cl-copy-list "cl-lib")
 (defun face-remap--copy-face (val)
   "Return a copy of the `face' property value VAL."
   ;; A `face' property can be either a face name (a symbol), or a face
@@ -79,7 +80,10 @@ face-remap--copy-face
   ;; `add-face-text-property'.
   (if (or (not (listp val)) (keywordp (car val)))
       val
-    (copy-sequence val)))
+    ;; Handle association (foo . bar) from face-remapping-alist.
+    (if (not (proper-list-p val))
+        (cl-copy-list val)
+      (copy-sequence val))))
 
 (defun face-attrs--make-indirect-safe ()
   "Deep-copy the buffer's `face-remapping-alist' upon cloning the buffer."

Re: Possible bug: org-capture fails when emacs started as daemon

[Iñigo Serna]

Hi Eli,

On 16 August 2023 at 16:04 +02, Eli Zaretskii <eliz@gnu.org> 
wrote:

> AFAIU, the minibuffer-setup-hook part is not needed for this 
> recipe to
> demonstrate the problem.

It's needed here to provoke the error.


> Please see if the patch below gives good results.
> [...]

This fixes the issue on all my cases: test, my complete 
configuration
and even with mini-modeline package.

Thanks a lot,
-- 
Iñigo Serna

Re: Possible bug: org-capture fails when emacs started as daemon

[Stefan Monnier]

> Stefan, any comments?  You wrote the face-remap--copy-face stuff,
> AFAICT.

Not sure how I ended up with that code, which is fundamentally wrong:
`face-remap--copy-face` should take a face value but we pass it
elements of `face-remap-alist` which are not
facevalues but are cons cells (FACENAME . FACEVALUE) instead.
If we want to preserve the "careful" approach in the current code, then
the patch below should fix it.

If we want to make the code simpler, then we should throw away
`face-remap--copy-face` and use `copy-tree` instead.


        Stefan


diff --git a/lisp/face-remap.el b/lisp/face-remap.el
index 3ec271b67a4..ae4e9853ad3 100644
--- a/lisp/face-remap.el
+++ b/lisp/face-remap.el
@@ -79,12 +79,13 @@ face-remap--copy-face
   ;; `add-face-text-property'.
   (if (or (not (listp val)) (keywordp (car val)))
       val
-    (copy-sequence val)))
+    (mapcar #'face-remap--copy-face val)))
 
 (defun face-attrs--make-indirect-safe ()
   "Deep-copy the buffer's `face-remapping-alist' upon cloning the buffer."
   (setq-local face-remapping-alist
-              (mapcar #'face-remap--copy-face face-remapping-alist)))
+              (mapcar (lambda (x) (cons (car x) (face-remap--copy-face (cdr x))))
+                      face-remapping-alist)))
 
 (add-hook 'clone-indirect-buffer-hook #'face-attrs--make-indirect-safe)
 

Re: Possible bug: org-capture fails when emacs started as daemon

[Iñigo Serna]

Hi,

On 16 August 2023 at 17:00 +02, Stefan Monnier 
<monnier@iro.umontreal.ca> wrote:

> If we want to make the code simpler, then we should throw away
> `face-remap--copy-face` and use `copy-tree` instead.


This patch also works.

Thanks Stefan!
-- 
Iñigo Serna

Re: Possible bug: org-capture fails when emacs started as daemon

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: inigoserna@gmx.com,  yantar92@posteo.net,  morgan@ice9.digital,
>   emacs-devel@gnu.org
> Date: Wed, 16 Aug 2023 11:00:21 -0400
> 
> If we want to make the code simpler, then we should throw away
> `face-remap--copy-face` and use `copy-tree` instead.

Like below?

diff --git a/lisp/face-remap.el b/lisp/face-remap.el
index 3ec271b..c5f7af3 100644
--- a/lisp/face-remap.el
+++ b/lisp/face-remap.el
@@ -70,21 +70,10 @@ internal-lisp-face-attributes
    :foreground :background :stipple :overline :strike-through :box
    :font :inherit :fontset :distant-foreground :extend :vector])
 
-(defun face-remap--copy-face (val)
-  "Return a copy of the `face' property value VAL."
-  ;; A `face' property can be either a face name (a symbol), or a face
-  ;; property list like (:foreground "red" :inherit default),
-  ;; or a list of such things.
-  ;; FIXME: This should probably be shared to some extent with
-  ;; `add-face-text-property'.
-  (if (or (not (listp val)) (keywordp (car val)))
-      val
-    (copy-sequence val)))
-
 (defun face-attrs--make-indirect-safe ()
   "Deep-copy the buffer's `face-remapping-alist' upon cloning the buffer."
   (setq-local face-remapping-alist
-              (mapcar #'face-remap--copy-face face-remapping-alist)))
+              (mapcar #'copy-tree face-remapping-alist)))
 
 (add-hook 'clone-indirect-buffer-hook #'face-attrs--make-indirect-safe)
 

Re: Possible bug: org-capture fails when emacs started as daemon

[Stefan Monnier]

>> From: Stefan Monnier <monnier@iro.umontreal.ca>
>> Cc: inigoserna@gmx.com,  yantar92@posteo.net,  morgan@ice9.digital,
>>   emacs-devel@gnu.org
>> Date: Wed, 16 Aug 2023 11:00:21 -0400
>> 
>> If we want to make the code simpler, then we should throw away
>> `face-remap--copy-face` and use `copy-tree` instead.
>
> Like below?

Yup.


        Stefan

Re: Possible bug: org-capture fails when emacs started as daemon

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: inigoserna@gmx.com,  yantar92@posteo.net,  morgan@ice9.digital,
>   emacs-devel@gnu.org
> Date: Wed, 16 Aug 2023 13:46:48 -0400
> 
> >> From: Stefan Monnier <monnier@iro.umontreal.ca>
> >> Cc: inigoserna@gmx.com,  yantar92@posteo.net,  morgan@ice9.digital,
> >>   emacs-devel@gnu.org
> >> Date: Wed, 16 Aug 2023 11:00:21 -0400
> >> 
> >> If we want to make the code simpler, then we should throw away
> >> `face-remap--copy-face` and use `copy-tree` instead.
> >
> > Like below?
> 
> Yup.

Thanks, now installed on the emacs-29 branch.