Package: emacs;
Reported by: Joseph Turner <joseph <at> breatheoutbreathe.in>
Date: Thu, 31 Aug 2023 06:30:02 UTC
Severity: normal
Tags: patch
Done: Philip Kaludercic <philipk <at> posteo.net>
Bug is archived. No further changes may be made.
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))))
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.