bug#48137: 27.2; `package-install-file' fails when loading a package file with DOS line endings

[Ioannis Kappas]

The error appears to be a side effect of `package-install-file'
loading the file literally with `insert-file-contents-literally' and
thus setting the coding system to 'no-conversion (as reported by the
`buffer-file-coding-system' variable ). This means that for a DOS
encoded file the ?\r (Carriage Return) in the DOS line ending ?\r?\n
will be treated as an independent control character rather than
consumed as part of the line ending as far as Emacs is concerned.

This extra control character in our case appears in the version number
extracted with `lm-header' (.e.g. "0.1^M"), failing the
`version-to-list' conversion in
`package-buffer-info'->`package-strip-rcs-id', resulting to the error
that it lacks a version header.

It is not obvious why `package-install-file' loads package files
literally at all times, `package--with-response-buffer-1' also does
the same. If the reason turns out not to be important, then a possible
solution could be to load package files metaphorically:

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ecb2573cab..98d63f1aff 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -2147,8 +2147,11 @@ package-install-file
         (progn
           (setq default-directory file)
           (dired-mode))
-      (insert-file-contents-literally file)
-      (when (string-match "\\.tar\\'" file) (tar-mode)))
+      (if (string-match "\\.tar\\'" file)
+          (progn
+            (insert-file-contents-literally file)
+            (tar-mode))
+        (insert-file-contents file)))
     (package-install-from-buffer)))

 ;;;###autoload


and perhaps the same can be extended to
`package--with-response-buffer-1'.

Another solution could be to upgrade the 'lisp-mnt package to ignore
?\r characters. Looking at the `lm-header' fn invoked by
`package-buffer-info', it does have a list of characters to stop at
when looking for a header, we can thus add the carriage return to the
list:

diff --git a/lisp/emacs-lisp/lisp-mnt.el b/lisp/emacs-lisp/lisp-mnt.el
index 9cba232e16..3eb493d286 100644
--- a/lisp/emacs-lisp/lisp-mnt.el
+++ b/lisp/emacs-lisp/lisp-mnt.el
@@ -267,7 +267,7 @@ lm-header
                (if (save-excursion
                      (skip-chars-backward "^$" (match-beginning 0))
                      (= (point) (match-beginning 0)))
-                   "[^\n]+" "[^$\n]+")))
+                   "[^\n\r]+" "[^$\n\r]+")))
       (match-string-no-properties 0))))

 (defun lm-header-multiline (header)


The attached issue-package-install-file-test-up1.el, is an updated ert
including an additional `header' test for the above.

Or is it perhaps that packages should only be written with Unix line
endings and thus the solution is to update documentation and create
more targeted error messages?

I don't have a thorough understanding of 'package, so please feel free
to scrutinize the above/suggest alternatives.

Thanks

issue-package-install-file-test-up1.el
Description: Binary data