bug#41489: `package-dir-info' fails on a directory with a non-saved file

[Paul Pogonyshev]

> Do we really want to ignore *any* error from insert-file-contents here?

Well, maybe that should be limited to `file-missing' instead (what actually happens when it tries to read a lock file).

> Should we really run package-buffer-info if inserting the file fails?
> Won't that reach (error "Package lacks a file header") and signal an
> error anyways, just a different and more cryptic one?

`package-buffer-info' is already inside a different `ignore-errors', so it will signal an error, but that error will be ignored and the file skipped.

I'm not attached to any particular way this bug is fixed. Please adjust it yourself, the patch is only an example of how it could be done. This will be faster than if we try to negotiate the best way and recreate the patch.

BTW, the bug being reproducible only in 50% of the cases makes it even more important to be fixed from my point of view. Nothing is worse than unspecified behavior when it's not justified by reasons like huge performance gain in my opinion.

Paul

On Tue, 26 May 2020 at 04:07, Stefan Kangas <stefankangas@gmail.com> wrote:

Paul Pogonyshev <pogonyshev@gmail.com> writes:

> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index 9a6d1d7319..99ba5d7107 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -1181,7 +1181,9 @@ package-dir-info
>              info)
>          (while files
>            (with-temp-buffer
> -            (insert-file-contents (pop files))
> +            ;; Skip unreadable files, e.g. locks for unsaved `.el'
> +            ;; buffers (bug#41489).
> +            (ignore-errors (insert-file-contents (pop files)))
>              ;; When we find the file with the data,
>              (when (setq info (ignore-errors (package-buffer-info)))
>                ;; stop looping,

Do we really want to ignore *any* error from insert-file-contents here?

Should we really run package-buffer-info if inserting the file fails?
Won't that reach (error "Package lacks a file header") and signal an
error anyways, just a different and more cryptic one?

Best regards,
Stefan Kangas