GNU bug report logs - #65649
[PATCH] package-vc: Continue installing package when documentation build fails

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 65649 in the body.
You can then email your comments to 65649 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: bug-gnu-emacs <at> gnu.org
Cc: Philip Kaludercic <philipk <at> posteo.net>
Subject: [PATCH] package-vc: Continue installing package when documentation
 build fails
Date: Wed, 30 Aug 2023 23:26:35 -0700

[Message part 1 (text/plain, inline)]

In light of the recent fix regarding building documentation from
org-mode files, I think it makes sense to ensure that installation
continues even without documentation.

Joseph

[0001-Continue-installing-package-when-documentation-build.patch (text/x-diff, attachment)]

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: bug-gnu-emacs <at> gnu.org, Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: [PATCH] package-vc: Continue installing package when
 documentation build fails
Date: Wed, 30 Aug 2023 23:32:42 -0700

[Message part 1 (text/plain, inline)]

Oops, the format string should be %S, not %s.

[0001-Continue-installing-package-when-documentation-build.patch (text/x-diff, inline)]

From 83c62d80b5199cee911e30c1f3bb03c5a585b689 Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph <at> breatheoutbreathe.in>
Date: Wed, 30 Aug 2023 23:24:16 -0700
Subject: [PATCH] Continue installing package when documentation building fails

* lisp/emacs-lisp/package-vc.el (package-vc--build-documentation):
Demote errors to messages.
---
 lisp/emacs-lisp/package-vc.el | 61 ++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 30 deletions(-)

diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index 747fe696204..bc1200c583f 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -409,36 +409,37 @@ prepared."
   "Build documentation for package PKG-DESC from documentation source in FILE.
 FILE can be an Org file, indicated by its \".org\" extension,
 otherwise it's assumed to be an Info file."
-  (let* ((pkg-name (package-desc-name pkg-desc))
-         (default-directory (package-desc-dir pkg-desc))
-         (docs-directory (file-name-directory (expand-file-name file)))
-         (output (expand-file-name (format "%s.info" pkg-name)))
-         clean-up)
-    (when (string-match-p "\\.org\\'" file)
-      (require 'ox)
-      (require 'ox-texinfo)
-      (with-temp-buffer
-        (insert-file-contents file)
-        (setq file (make-temp-file "ox-texinfo-"))
-        (let ((default-directory docs-directory))
-          (org-export-to-file 'texinfo file))
-        (setq clean-up t)))
-    (with-current-buffer (get-buffer-create " *package-vc doc*")
-      (erase-buffer)
-      (cond
-       ((/= 0 (call-process "makeinfo" nil t nil
-                            "-I" docs-directory
-                            "--no-split" file
-                            "-o" output))
-        (message "Failed to build manual %s, see buffer %S"
-                 file (buffer-name)))
-       ((/= 0 (call-process "install-info" nil t nil
-                            output (expand-file-name "dir")))
-        (message "Failed to install manual %s, see buffer %S"
-                 output (buffer-name)))
-       ((kill-buffer))))
-    (when clean-up
-      (delete-file file))))
+  (with-demoted-errors "package-vc: Could not build documentation: %S"
+    (let* ((pkg-name (package-desc-name pkg-desc))
+           (default-directory (package-desc-dir pkg-desc))
+           (docs-directory (file-name-directory (expand-file-name file)))
+           (output (expand-file-name (format "%s.info" pkg-name)))
+           clean-up)
+      (when (string-match-p "\\.org\\'" file)
+        (require 'ox)
+        (require 'ox-texinfo)
+        (with-temp-buffer
+          (insert-file-contents file)
+          (setq file (make-temp-file "ox-texinfo-"))
+          (let ((default-directory docs-directory))
+            (org-export-to-file 'texinfo file))
+          (setq clean-up t)))
+      (with-current-buffer (get-buffer-create " *package-vc doc*")
+        (erase-buffer)
+        (cond
+         ((/= 0 (call-process "makeinfo" nil t nil
+                              "-I" docs-directory
+                              "--no-split" file
+                              "-o" output))
+          (message "Failed to build manual %s, see buffer %S"
+                   file (buffer-name)))
+         ((/= 0 (call-process "install-info" nil t nil
+                              output (expand-file-name "dir")))
+          (message "Failed to install manual %s, see buffer %S"
+                   output (buffer-name)))
+         ((kill-buffer))))
+      (when clean-up
+        (delete-file file)))))
 
 (defun package-vc-install-dependencies (deps)
   "Install missing dependencies according to DEPS.
-- 
2.41.0

Message #11 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: bug-gnu-emacs <at> gnu.org, Philip Kaludercic <philipk <at> posteo.net>
Subject: Re: [PATCH] package-vc: Continue installing package when
 documentation build fails
Date: Wed, 30 Aug 2023 23:34:29 -0700

It would be nice to report more information about the package with failed
documentation, but AFAICT with-demoted-errors does not allow its FORMAT
string to be a symbol whose value is a string.

Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Oops, the format string should be %S, not %s.
>
> [2. text/x-diff; 0001-Continue-installing-package-when-documentation-build.patch]
> From 83c62d80b5199cee911e30c1f3bb03c5a585b689 Mon Sep 17 00:00:00 2001
> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Date: Wed, 30 Aug 2023 23:24:16 -0700
> Subject: [PATCH] Continue installing package when documentation building fails
>
> * lisp/emacs-lisp/package-vc.el (package-vc--build-documentation):
> Demote errors to messages.
> ---
>  lisp/emacs-lisp/package-vc.el | 61 ++++++++++++++++++-----------------
>  1 file changed, 31 insertions(+), 30 deletions(-)
>
> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
> index 747fe696204..bc1200c583f 100644
> --- a/lisp/emacs-lisp/package-vc.el
> +++ b/lisp/emacs-lisp/package-vc.el
> @@ -409,36 +409,37 @@ prepared."
>    "Build documentation for package PKG-DESC from documentation source in FILE.
>  FILE can be an Org file, indicated by its \".org\" extension,
>  otherwise it's assumed to be an Info file."
> -  (let* ((pkg-name (package-desc-name pkg-desc))
> -         (default-directory (package-desc-dir pkg-desc))
> -         (docs-directory (file-name-directory (expand-file-name file)))
> -         (output (expand-file-name (format "%s.info" pkg-name)))
> -         clean-up)
> -    (when (string-match-p "\\.org\\'" file)
> -      (require 'ox)
> -      (require 'ox-texinfo)
> -      (with-temp-buffer
> -        (insert-file-contents file)
> -        (setq file (make-temp-file "ox-texinfo-"))
> -        (let ((default-directory docs-directory))
> -          (org-export-to-file 'texinfo file))
> -        (setq clean-up t)))
> -    (with-current-buffer (get-buffer-create " *package-vc doc*")
> -      (erase-buffer)
> -      (cond
> -       ((/= 0 (call-process "makeinfo" nil t nil
> -                            "-I" docs-directory
> -                            "--no-split" file
> -                            "-o" output))
> -        (message "Failed to build manual %s, see buffer %S"
> -                 file (buffer-name)))
> -       ((/= 0 (call-process "install-info" nil t nil
> -                            output (expand-file-name "dir")))
> -        (message "Failed to install manual %s, see buffer %S"
> -                 output (buffer-name)))
> -       ((kill-buffer))))
> -    (when clean-up
> -      (delete-file file))))
> +  (with-demoted-errors "package-vc: Could not build documentation: %S"
> +    (let* ((pkg-name (package-desc-name pkg-desc))
> +           (default-directory (package-desc-dir pkg-desc))
> +           (docs-directory (file-name-directory (expand-file-name file)))
> +           (output (expand-file-name (format "%s.info" pkg-name)))
> +           clean-up)
> +      (when (string-match-p "\\.org\\'" file)
> +        (require 'ox)
> +        (require 'ox-texinfo)
> +        (with-temp-buffer
> +          (insert-file-contents file)
> +          (setq file (make-temp-file "ox-texinfo-"))
> +          (let ((default-directory docs-directory))
> +            (org-export-to-file 'texinfo file))
> +          (setq clean-up t)))
> +      (with-current-buffer (get-buffer-create " *package-vc doc*")
> +        (erase-buffer)
> +        (cond
> +         ((/= 0 (call-process "makeinfo" nil t nil
> +                              "-I" docs-directory
> +                              "--no-split" file
> +                              "-o" output))
> +          (message "Failed to build manual %s, see buffer %S"
> +                   file (buffer-name)))
> +         ((/= 0 (call-process "install-info" nil t nil
> +                              output (expand-file-name "dir")))
> +          (message "Failed to install manual %s, see buffer %S"
> +                   output (buffer-name)))
> +         ((kill-buffer))))
> +      (when clean-up
> +        (delete-file file)))))
>
>  (defun package-vc-install-dependencies (deps)
>    "Install missing dependencies according to DEPS.

Message #14 received at 65649 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 65649 <at> debbugs.gnu.org, philipk <at> posteo.net
Subject: Re: bug#65649: [PATCH] package-vc: Continue installing package when
 documentation build fails
Date: Thu, 31 Aug 2023 09:50:31 +0300

> Cc: Philip Kaludercic <philipk <at> posteo.net>
> Date: Wed, 30 Aug 2023 23:26:35 -0700
> From:  Joseph Turner via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> In light of the recent fix regarding building documentation from
> org-mode files, I think it makes sense to ensure that installation
> continues even without documentation.

I don't think we should do this silently.  The user should be asked
to confirm, at the very least.

Message #17 received at 65649 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 65649 <at> debbugs.gnu.org
Subject: Re: bug#65649: [PATCH] package-vc: Continue installing package when
 documentation build fails
Date: Thu, 31 Aug 2023 07:18:01 +0000

Btw, watch out when reporting a bug, you shouldn't CC people directly,
but add a X-Debbugs-CC header
(https://debbugs.gnu.org/Reporting.html#xcc), as otherwise I'd be
creating a new bug report when responding to your message.

Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> It would be nice to report more information about the package with failed
> documentation, but AFAICT with-demoted-errors does not allow its FORMAT
> string to be a symbol whose value is a string.

What are the main errors we are concerned with?  I am a bit concerned
that `with-demoted-errors' is a too coarse approach and might be applied
to widely in your current patch.  It might be better to use a classical
condition-case and handle the right errors in the right place, or if
practicable try to detect if an error would occur before doing anything.

Also, the buffer *package-vc doc* should stay persistent and would
include error messages, that could also be used here.

> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>
>> Oops, the format string should be %S, not %s.
>>
>> [2. text/x-diff; 0001-Continue-installing-package-when-documentation-build.patch]
>> From 83c62d80b5199cee911e30c1f3bb03c5a585b689 Mon Sep 17 00:00:00 2001
>> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
>> Date: Wed, 30 Aug 2023 23:24:16 -0700
>> Subject: [PATCH] Continue installing package when documentation building fails
>>
>> * lisp/emacs-lisp/package-vc.el (package-vc--build-documentation):
>> Demote errors to messages.
>> ---
>>  lisp/emacs-lisp/package-vc.el | 61 ++++++++++++++++++-----------------
>>  1 file changed, 31 insertions(+), 30 deletions(-)
>>
>> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
>> index 747fe696204..bc1200c583f 100644
>> --- a/lisp/emacs-lisp/package-vc.el
>> +++ b/lisp/emacs-lisp/package-vc.el
>> @@ -409,36 +409,37 @@ prepared."
>>    "Build documentation for package PKG-DESC from documentation source in FILE.
>>  FILE can be an Org file, indicated by its \".org\" extension,
>>  otherwise it's assumed to be an Info file."
>> -  (let* ((pkg-name (package-desc-name pkg-desc))
>> -         (default-directory (package-desc-dir pkg-desc))
>> -         (docs-directory (file-name-directory (expand-file-name file)))
>> -         (output (expand-file-name (format "%s.info" pkg-name)))
>> -         clean-up)
>> -    (when (string-match-p "\\.org\\'" file)
>> -      (require 'ox)
>> -      (require 'ox-texinfo)
>> -      (with-temp-buffer
>> -        (insert-file-contents file)
>> -        (setq file (make-temp-file "ox-texinfo-"))
>> -        (let ((default-directory docs-directory))
>> -          (org-export-to-file 'texinfo file))
>> -        (setq clean-up t)))
>> -    (with-current-buffer (get-buffer-create " *package-vc doc*")
>> -      (erase-buffer)
>> -      (cond
>> -       ((/= 0 (call-process "makeinfo" nil t nil
>> -                            "-I" docs-directory
>> -                            "--no-split" file
>> -                            "-o" output))
>> -        (message "Failed to build manual %s, see buffer %S"
>> -                 file (buffer-name)))
>> -       ((/= 0 (call-process "install-info" nil t nil
>> -                            output (expand-file-name "dir")))
>> -        (message "Failed to install manual %s, see buffer %S"
>> -                 output (buffer-name)))
>> -       ((kill-buffer))))
>> -    (when clean-up
>> -      (delete-file file))))
>> +  (with-demoted-errors "package-vc: Could not build documentation: %S"
>> +    (let* ((pkg-name (package-desc-name pkg-desc))
>> +           (default-directory (package-desc-dir pkg-desc))
>> +           (docs-directory (file-name-directory (expand-file-name file)))
>> +           (output (expand-file-name (format "%s.info" pkg-name)))
>> +           clean-up)
>> +      (when (string-match-p "\\.org\\'" file)
>> +        (require 'ox)
>> +        (require 'ox-texinfo)
>> +        (with-temp-buffer
>> +          (insert-file-contents file)
>> +          (setq file (make-temp-file "ox-texinfo-"))
>> +          (let ((default-directory docs-directory))
>> +            (org-export-to-file 'texinfo file))
>> +          (setq clean-up t)))
>> +      (with-current-buffer (get-buffer-create " *package-vc doc*")
>> +        (erase-buffer)
>> +        (cond
>> +         ((/= 0 (call-process "makeinfo" nil t nil
>> +                              "-I" docs-directory
>> +                              "--no-split" file
>> +                              "-o" output))
>> +          (message "Failed to build manual %s, see buffer %S"
>> +                   file (buffer-name)))
>> +         ((/= 0 (call-process "install-info" nil t nil
>> +                              output (expand-file-name "dir")))
>> +          (message "Failed to install manual %s, see buffer %S"
>> +                   output (buffer-name)))
>> +         ((kill-buffer))))
>> +      (when clean-up
>> +        (delete-file file)))))
>>
>>  (defun package-vc-install-dependencies (deps)
>>    "Install missing dependencies according to DEPS.

Message #20 received at 65649 <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65649 <at> debbugs.gnu.org
Subject: Re: bug#65649: [PATCH] package-vc: Continue installing package when
 documentation build fails
Date: Thu, 31 Aug 2023 14:19:11 -0700

Philip Kaludercic <philipk <at> posteo.net> writes:

> Btw, watch out when reporting a bug, you shouldn't CC people directly,
> but add a X-Debbugs-CC header
> (https://debbugs.gnu.org/Reporting.html#xcc), as otherwise I'd be
> creating a new bug report when responding to your message.

Good to know! Thank you! Is it okay to CC people when responding to a
bug report email (e.g. if I CC'd Eli in this email)?

> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>
>> It would be nice to report more information about the package with failed
>> documentation, but AFAICT with-demoted-errors does not allow its FORMAT
>> string to be a symbol whose value is a string.
>
> What are the main errors we are concerned with?  I am a bit concerned
> that `with-demoted-errors' is a too coarse approach and might be applied
> to widely in your current patch.  It might be better to use a classical
> condition-case and handle the right errors in the right place, or if
> practicable try to detect if an error would occur before doing anything.

I don't know what kinds of errors to expect, which is why I went with
the coarse approach.  Under what circumstances would we want to stop
package installation entirely when documentation can't be built?

> Also, the buffer *package-vc doc* should stay persistent and would
> include error messages, that could also be used here.

Like this?

(with-demoted-errors "package-vc: Could not build documentation. See *package-vc doc* for details" ... )

Message #23 received at 65649 <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>, 65649 <at> debbugs.gnu.org
Subject: Re: bug#65649: [PATCH] package-vc: Continue installing package when
 documentation build fails
Date: Thu, 31 Aug 2023 14:39:22 -0700

Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Also, the buffer *package-vc doc* should stay persistent and would
>> include error messages, that could also be used here.
>
> Like this?
>
> (with-demoted-errors "package-vc: Could not build documentation. See *package-vc doc* for details" ... )

Actually, that would not have handled the error related to relative org
#+include statements, since that error happened above this line:

(with-current-buffer (get-buffer-create " *package-vc doc*")

Message #26 received at 65649 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 65649 <at> debbugs.gnu.org
Subject: Re: bug#65649: [PATCH] package-vc: Continue installing package when
 documentation build fails
Date: Fri, 01 Sep 2023 13:14:45 +0000

Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Btw, watch out when reporting a bug, you shouldn't CC people directly,
>> but add a X-Debbugs-CC header
>> (https://debbugs.gnu.org/Reporting.html#xcc), as otherwise I'd be
>> creating a new bug report when responding to your message.
>
> Good to know! Thank you! Is it okay to CC people when responding to a
> bug report email (e.g. if I CC'd Eli in this email)?

Yes, as soon as one is sending a message to [bugnumber]@debbugs.gnu.org,
everything is fine.  The issue if you Cc me directly, is that if I don't
watch out, I'll send my response to bug-gnu-emacs <at> gnu.org, and thus
create a new bug.

>> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>>
>>> It would be nice to report more information about the package with failed
>>> documentation, but AFAICT with-demoted-errors does not allow its FORMAT
>>> string to be a symbol whose value is a string.
>>
>> What are the main errors we are concerned with?  I am a bit concerned
>> that `with-demoted-errors' is a too coarse approach and might be applied
>> to widely in your current patch.  It might be better to use a classical
>> condition-case and handle the right errors in the right place, or if
>> practicable try to detect if an error would occur before doing anything.
>
> I don't know what kinds of errors to expect, which is why I went with
> the coarse approach.  Under what circumstances would we want to stop
> package installation entirely when documentation can't be built?

I am not sure we want that behaviour at all actually.  Just because
there is a typo in the documentation, doesn't mean the package is
unusable.  The user should be able to install the package, be notified
about the error -- if the have the time, they can fix it and send the
maintainer a patch resolving the issue for everyone.  Likewise, if the
user updates a package, it wouldn't make sense to ignore everything or
worse still revert the update due to a small mistake in the
documentation file.

>> Also, the buffer *package-vc doc* should stay persistent and would
>> include error messages, that could also be used here.
>
> Like this?
>
> (with-demoted-errors "package-vc: Could not build documentation. See *package-vc doc* for details" ... )

Yes, that is what I had in mind.

Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Joseph Turner <joseph <at> breatheoutbreathe.in> writes:
>
>> Philip Kaludercic <philipk <at> posteo.net> writes:
>>
>>> Also, the buffer *package-vc doc* should stay persistent and would
>>> include error messages, that could also be used here.
>>
>> Like this?
>>
>> (with-demoted-errors "package-vc: Could not build documentation. See *package-vc doc* for details" ... )
>
> Actually, that would not have handled the error related to relative org
> #+include statements, since that error happened above this line:
>
> (with-current-buffer (get-buffer-create " *package-vc doc*")

There is no reason why we cannot already create and use the buffer
earlier, to log org-related bugs.  One has to be careful when emptying
the buffer, but it might make sense to have a separate buffer for each
package, especially when updating multiple packages at once...

Message #29 received at 65649 <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65649 <at> debbugs.gnu.org
Subject: Re: bug#65649: [PATCH] package-vc: Continue installing package when
 documentation build fails
Date: Fri, 01 Sep 2023 16:28:59 -0700

[Message part 1 (text/plain, inline)]

Philip Kaludercic <philipk <at> posteo.net> writes:

> Yes, as soon as one is sending a message to [bugnumber]@debbugs.gnu.org,
> everything is fine.  The issue if you Cc me directly, is that if I don't
> watch out, I'll send my response to bug-gnu-emacs <at> gnu.org, and thus
> create a new bug.

Thanks, that's clear now.

> I am not sure we want that behaviour at all actually.  Just because
> there is a typo in the documentation, doesn't mean the package is
> unusable.  The user should be able to install the package, be notified
> about the error -- if the have the time, they can fix it and send the
> maintainer a patch resolving the issue for everyone.  Likewise, if the
> user updates a package, it wouldn't make sense to ignore everything or
> worse still revert the update due to a small mistake in the
> documentation file.
>
> ...
>
> There is no reason why we cannot already create and use the buffer
> earlier, to log org-related bugs.  One has to be careful when emptying
> the buffer, but it might make sense to have a separate buffer for each
> package, especially when updating multiple packages at once...

Please see attached patches.

[0001-Include-package-name-in-package-vc-documentation-log.patch (text/x-diff, attachment)]

[0002-Log-org-export-errors-to-package-vc-doc-buffer.patch (text/x-diff, attachment)]

Message #32 received at 65649 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 65649 <at> debbugs.gnu.org
Subject: Re: bug#65649: [PATCH] package-vc: Continue installing package when
 documentation build fails
Date: Sat, 02 Sep 2023 12:03:57 +0000

Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> Yes, as soon as one is sending a message to [bugnumber]@debbugs.gnu.org,
>> everything is fine.  The issue if you Cc me directly, is that if I don't
>> watch out, I'll send my response to bug-gnu-emacs <at> gnu.org, and thus
>> create a new bug.
>
> Thanks, that's clear now.
>
>> I am not sure we want that behaviour at all actually.  Just because
>> there is a typo in the documentation, doesn't mean the package is
>> unusable.  The user should be able to install the package, be notified
>> about the error -- if the have the time, they can fix it and send the
>> maintainer a patch resolving the issue for everyone.  Likewise, if the
>> user updates a package, it wouldn't make sense to ignore everything or
>> worse still revert the update due to a small mistake in the
>> documentation file.
>>
>> ...
>>
>> There is no reason why we cannot already create and use the buffer
>> earlier, to log org-related bugs.  One has to be careful when emptying
>> the buffer, but it might make sense to have a separate buffer for each
>> package, especially when updating multiple packages at once...
>
> Please see attached patches.
>
>>From aa356f561ab7861f463d3024f574fc71d45cb00b Mon Sep 17 00:00:00 2001
> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Date: Wed, 30 Aug 2023 23:24:16 -0700
> Subject: [PATCH 1/2] Include package name in package-vc documentation log
>  buffer name
>
> * lisp/emacs-lisp/package-vc.el (package-vc--build-documentation):
> ---
>  lisp/emacs-lisp/package-vc.el | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
> index 747fe696204..ea8d9ecf488 100644
> --- a/lisp/emacs-lisp/package-vc.el
> +++ b/lisp/emacs-lisp/package-vc.el
> @@ -423,7 +423,7 @@ otherwise it's assumed to be an Info file."
>          (let ((default-directory docs-directory))
>            (org-export-to-file 'texinfo file))
>          (setq clean-up t)))
> -    (with-current-buffer (get-buffer-create " *package-vc doc*")
> +    (with-current-buffer (get-buffer-create (format " *package-vc doc: %s*" pkg-name))
>        (erase-buffer)
>        (cond
>         ((/= 0 (call-process "makeinfo" nil t nil
> -- 
> 2.41.0

This looks good, thanks!

>
>>From 010dabfbba8ebeb7f7193482ae2ffc7ec5b694e3 Mon Sep 17 00:00:00 2001
> From: Joseph Turner <joseph <at> breatheoutbreathe.in>
> Date: Fri, 1 Sep 2023 16:22:45 -0700
> Subject: [PATCH 2/2] Log org export errors to package-vc doc buffer
>
> * lisp/emacs-lisp/package-vc.el (package-vc--build-documentation):
> Wrap the org-export logic in condition-case, allowing package
> installation to continue while preserving error messages.
> ---
>  lisp/emacs-lisp/package-vc.el | 52 +++++++++++++++++++----------------
>  1 file changed, 29 insertions(+), 23 deletions(-)
>
> diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
> index ea8d9ecf488..a8393cb7e75 100644
> --- a/lisp/emacs-lisp/package-vc.el
> +++ b/lisp/emacs-lisp/package-vc.el
> @@ -413,30 +413,36 @@ otherwise it's assumed to be an Info file."
>           (default-directory (package-desc-dir pkg-desc))
>           (docs-directory (file-name-directory (expand-file-name file)))
>           (output (expand-file-name (format "%s.info" pkg-name)))
> +         (log-buffer (get-buffer-create (format " *package-vc doc: %s*" pkg-name)))
>           clean-up)
> -    (when (string-match-p "\\.org\\'" file)
> -      (require 'ox)
> -      (require 'ox-texinfo)
> -      (with-temp-buffer
> -        (insert-file-contents file)
> -        (setq file (make-temp-file "ox-texinfo-"))
> -        (let ((default-directory docs-directory))
> -          (org-export-to-file 'texinfo file))
> -        (setq clean-up t)))
> -    (with-current-buffer (get-buffer-create (format " *package-vc doc: %s*" pkg-name))
> -      (erase-buffer)
> -      (cond
> -       ((/= 0 (call-process "makeinfo" nil t nil
> -                            "-I" docs-directory
> -                            "--no-split" file
> -                            "-o" output))
> -        (message "Failed to build manual %s, see buffer %S"
> -                 file (buffer-name)))
> -       ((/= 0 (call-process "install-info" nil t nil
> -                            output (expand-file-name "dir")))
> -        (message "Failed to install manual %s, see buffer %S"
> -                 output (buffer-name)))
> -       ((kill-buffer))))
> +    (with-current-buffer log-buffer
> +      (erase-buffer))
> +    (condition-case err
> +        (progn
> +          (when (string-match-p "\\.org\\'" file)
> +            (require 'ox)
> +            (require 'ox-texinfo)
> +            (with-temp-buffer
> +              (insert-file-contents file)
> +              (setq file (make-temp-file "ox-texinfo-"))
> +              (let ((default-directory docs-directory))
> +                (org-export-to-file 'texinfo file))
> +              (setq clean-up t)))
> +          (cond
> +           ((/= 0 (call-process "makeinfo" nil log-buffer nil
> +                                "-I" docs-directory
> +                                "--no-split" file
> +                                "-o" output))
> +            (message "Failed to build manual %s, see buffer %S"
> +                     file (buffer-name)))
> +           ((/= 0 (call-process "install-info" nil log-buffer nil
> +                                output (expand-file-name "dir")))
> +            (message "Failed to install manual %s, see buffer %S"
> +                     output (buffer-name)))
> +           ((kill-buffer log-buffer))))
> +      (error (with-current-buffer log-buffer
> +               (insert (error-message-string err)))
> +             (message "Failed to export org manual for %s, see buffer %S" pkg-name log-buffer)))

I think it would be better to wrap only the org code in the
`condition-case' body, ideally with a more specific error type (if that
doesn't exist, that is something we could mention to the Org
maintainers).

>      (when clean-up
>        (delete-file file))))

Message #35 received at 65649 <at> debbugs.gnu.org (full text, mbox):

From: Joseph Turner <joseph <at> breatheoutbreathe.in>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 65649 <at> debbugs.gnu.org
Subject: Re: bug#65649: [PATCH] package-vc: Continue installing package when
 documentation build fails
Date: Sat, 02 Sep 2023 09:49:24 -0700

Philip Kaludercic <philipk <at> posteo.net> writes:

> I think it would be better to wrap only the org code in the
> `condition-case' body, ideally with a more specific error type (if that
> doesn't exist, that is something we could mention to the Org
> maintainers).

The reason I wrapped both the org-export and makeinfo logic in
condition-case is so that the makeinfo logic does not run when the
org-export logic fails. Is there a better way?

We could let-bind a flag like failed-org-export-p, then set it to t in
the condition case handler, then only run makeinfo when
failed-org-export-p is nil. Seems a bit ugly.

The only define-error in org-mode that's relevant to org-export is not
general enough:

(define-error 'org-link-broken "Unable to resolve link; aborting")

Message #40 received at 65649-done <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Joseph Turner <joseph <at> breatheoutbreathe.in>
Cc: 65649-done <at> debbugs.gnu.org
Subject: Re: bug#65649: [PATCH] package-vc: Continue installing package when
 documentation build fails
Date: Sun, 03 Sep 2023 07:00:22 +0000

Joseph Turner <joseph <at> breatheoutbreathe.in> writes:

> Philip Kaludercic <philipk <at> posteo.net> writes:
>
>> I think it would be better to wrap only the org code in the
>> `condition-case' body, ideally with a more specific error type (if that
>> doesn't exist, that is something we could mention to the Org
>> maintainers).
>
> The reason I wrapped both the org-export and makeinfo logic in
> condition-case is so that the makeinfo logic does not run when the
> org-export logic fails. Is there a better way?
>
> We could let-bind a flag like failed-org-export-p, then set it to t in
> the condition case handler, then only run makeinfo when
> failed-org-export-p is nil. Seems a bit ugly.

Oh right, never-mind then, I guess this is the simplest solution.

> The only define-error in org-mode that's relevant to org-export is not
> general enough:
>
> (define-error 'org-link-broken "Unable to resolve link; aborting")

Then this should be fine.  I'll apply the changes the master.  Thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.