unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Ioannis Kappas <ioannis.kappas@gmail.com>
To: 48137@debbugs.gnu.org
Subject: bug#48137: 27.2; `package-install-file' fails when loading a package file with DOS line endings
Date: Sat, 1 May 2021 12:48:06 +0100	[thread overview]
Message-ID: <CAMRHuGDMxW4QmpaO=1VWPKxH_J_KKqHAfUezeY=_WSPi5cm_Rw@mail.gmail.com> (raw)
In-Reply-To: <CAMRHuGBkracML9sF-Arsq-R204E6s215EOdU2p2zGowzRp_8Ew@mail.gmail.com>

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

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

[-- Attachment #2: issue-package-install-file-test-up1.el --]
[-- Type: application/octet-stream, Size: 2561 bytes --]

;;; -*- lexical-binding: t; -*-
(require 'ert)

(defconst test/package ";;; package-install-file-test-win.el --- test issue 

;; Version: 0.1

(defun package-install-file-test-win/5 () 5)

(provide 'package-install-file-test-win)

;;; package-install-file-test-win.el ends here
"
  "Minimal test package file")

(ert-deftest package-install-file ()
  "Test that package install file works on a minimal package file
written to disk by Emacs."
  (message ":system %s :version %s" system-configuration emacs-version)

  (let* ((temp-dir (make-temp-file "package-install-file-test-win" t))
         (package-path (expand-file-name
                        "package-install-file-test-win.el"
                        temp-dir)))
    ;; write file in the default encoding
    (with-temp-buffer
      (insert test/package)
      (write-region (point-min) (point-max) package-path))
    (message ":package-file-created-at %s" package-path)

    ;; report on file encoding
    (with-temp-buffer
      (insert-file-contents package-path)
      (let* ((contents (buffer-string)))
        (message ":package-file-coding-system %s" buffer-file-coding-system)))

    ;; install package
    (package-install-file package-path)
    (delete-directory temp-dir t)

    
    ;; call dummy function
    (require 'package-install-file-test-win)
    (should (equal 5 (package-install-file-test-win/5)))

    ;; remove package
    (package-delete (cadr (assq 'package-install-file-test-win package-alist)))))

(require 'lisp-mnt)
(ert-deftest header ()
  "Test that lm-header can return version information on a dos
coded packaged file loaded literally (mimicking 'package)"
  (message ":system %s :version %s" system-configuration emacs-version)

  (let* ((temp-dir (make-temp-file "package-install-file-test-win" t))
         (package-path (expand-file-name
                        "package-install-file-test-win.el"
                        temp-dir)))
    ;; write with dos line endings
    (with-temp-buffer
      (insert test/package)
      (let ((coding-system-for-write 'prefer-utf-8-dos))
        (write-region (point-min) (point-max) package-path))
      (message ":package-file-coding-system %s" buffer-file-coding-system))
    
    (message ":package-file-created-at %s" package-path)
    ;; load literally
    (with-temp-buffer
      (insert-file-contents-literally package-path)
      (goto-char (point-min))
      (should (string= "0.1" (lm-header "version"))))

    (delete-directory temp-dir t)))

  reply	other threads:[~2021-05-01 11:48 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-01 11:38 bug#48137: 27.2; `package-install-file' fails when loading a package file with DOS line endings Ioannis Kappas
2021-05-01 11:48 ` Ioannis Kappas [this message]
2021-05-01 12:15   ` Eli Zaretskii
2021-05-01 13:51     ` Stefan Monnier
2021-05-03 17:47       ` Ioannis Kappas
2021-05-03 18:23         ` Stefan Monnier
2021-05-03 18:33           ` Eli Zaretskii
2021-05-03 18:49             ` Ioannis Kappas
2021-05-03 18:52               ` Eli Zaretskii
2021-05-03 20:12                 ` Stefan Monnier
2021-05-04 11:39                   ` Eli Zaretskii
2021-05-03 19:41             ` Stefan Monnier
2021-05-04 11:34               ` Eli Zaretskii
2021-05-04 15:57                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-05-04 16:14                   ` Eli Zaretskii
2021-05-04 16:27                     ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-05-04 16:51                       ` Eli Zaretskii
2021-05-05  7:03                         ` Ioannis Kappas
2021-05-05 12:01                           ` Eli Zaretskii
     [not found]                             ` <CAMRHuGAi9+q-MKRGPxLqxdP_7SSF4Nqj+JuSsZigviAQs_d7Rw@mail.gmail.com>
2021-05-06  6:55                               ` Ioannis Kappas
2021-05-06  8:12                                 ` Eli Zaretskii
2021-05-06 13:27                                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-05-06 15:26                                   ` Eli Zaretskii
2021-05-11  6:52                                     ` Ioannis Kappas
2021-05-11 12:55                                       ` Eli Zaretskii
2021-05-15 13:52                                         ` Ioannis Kappas
2021-05-16  9:09                                           ` Ioannis Kappas
2021-05-29  8:20                                             ` Eli Zaretskii
2021-05-29 13:59                                               ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-05-29 14:09                                                 ` Eli Zaretskii
2021-06-06  9:11                                                   ` Ioannis Kappas
2021-07-20 13:54                                                     ` Lars Ingebrigtsen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAMRHuGDMxW4QmpaO=1VWPKxH_J_KKqHAfUezeY=_WSPi5cm_Rw@mail.gmail.com' \
    --to=ioannis.kappas@gmail.com \
    --cc=48137@debbugs.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).