GNU bug report logs - #26490
25.1; package-buffer-info is incorrectly case-insensitive

Previous Next

Package: emacs;

Reported by: Steve Purcell <steve <at> sanityinc.com>

Date: Fri, 14 Apr 2017 00:46:02 UTC

Severity: minor

Found in version 25.1

Done: Stefan Kangas <stefan <at> marxist.se>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#26490; Package emacs. (Fri, 14 Apr 2017 00:46:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Steve Purcell <steve <at> sanityinc.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Fri, 14 Apr 2017 00:46:02 GMT) Full text and rfc822 format available.

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

From: Steve Purcell <steve <at> sanityinc.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.1; package-buffer-info is incorrectly case-insensitive
Date: Fri, 14 Apr 2017 12:44:24 +1200
[Message part 1 (text/plain, inline)]


`package-buffer-info' looks for an "filename.el ends here" comment line,
where "filename.el" matches that provided on the first line of the
file. However, it does not set `case-fold-search' to nil explicitly, and
so will happily allow "FiLeNaMe.EL" in the trailing line.

I rely on this function in package-lint to detect certain issues with
packaging, and noticed this failure there. I can work around it by
unsetting `case-fold-search', but this seems the wrong fix.

A simple patch is attached.


[0001-package.el-make-explicit-the-case-sensitivity-of-pac.patch (text/x-patch, inline)]
From bfa9b23e6b4ef030e36fdcf5c15cef15fb01074a Mon Sep 17 00:00:00 2001
From: Steve Purcell <steve <at> sanityinc.com>
Date: Fri, 14 Apr 2017 12:38:17 +1200
Subject: [PATCH] package.el: make explicit the case sensitivity of
 package-buffer-info

---
 lisp/emacs-lisp/package.el | 63 +++++++++++++++++++++++-----------------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 6728f1b80b1..d6ca14b135c 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -995,37 +995,38 @@ package-buffer-info
 error.  If there is a package, narrow the buffer to the file's
 boundaries."
   (goto-char (point-min))
-  (unless (re-search-forward "^;;; \\([^ ]*\\)\\.el ---[ \t]*\\(.*?\\)[ \t]*\\(-\\*-.*-\\*-[ \t]*\\)?$" nil t)
-    (error "Package lacks a file header"))
-  (let ((file-name (match-string-no-properties 1))
-        (desc      (match-string-no-properties 2))
-        (start     (line-beginning-position)))
-    (unless (search-forward (concat ";;; " file-name ".el ends here"))
-      (error "Package lacks a terminating comment"))
-    ;; Try to include a trailing newline.
-    (forward-line)
-    (narrow-to-region start (point))
-    (require 'lisp-mnt)
-    ;; Use some headers we've invented to drive the process.
-    (let* ((requires-str (lm-header "package-requires"))
-           ;; Prefer Package-Version; if defined, the package author
-           ;; probably wants us to use it.  Otherwise try Version.
-           (pkg-version
-            (or (package-strip-rcs-id (lm-header "package-version"))
-                (package-strip-rcs-id (lm-header "version"))))
-           (homepage (lm-homepage)))
-      (unless pkg-version
-        (error
-            "Package lacks a \"Version\" or \"Package-Version\" header"))
-      (package-desc-from-define
-       file-name pkg-version desc
-       (if requires-str
-           (package--prepare-dependencies
-            (package-read-from-string requires-str)))
-       :kind 'single
-       :url homepage
-       :maintainer (lm-maintainer)
-       :authors (lm-authors)))))
+  (let ((case-fold-search nil))
+    (unless (re-search-forward "^;;; \\([^ ]*\\)\\.el ---[ \t]*\\(.*?\\)[ \t]*\\(-\\*-.*-\\*-[ \t]*\\)?$" nil t)
+      (error "Package lacks a file header"))
+    (let ((file-name (match-string-no-properties 1))
+          (desc      (match-string-no-properties 2))
+          (start     (line-beginning-position)))
+      (unless (search-forward (concat ";;; " file-name ".el ends here"))
+        (error "Package lacks a terminating comment"))
+      ;; Try to include a trailing newline.
+      (forward-line)
+      (narrow-to-region start (point))
+      (require 'lisp-mnt)
+      ;; Use some headers we've invented to drive the process.
+      (let* ((requires-str (lm-header "package-requires"))
+             ;; Prefer Package-Version; if defined, the package author
+             ;; probably wants us to use it.  Otherwise try Version.
+             (pkg-version
+              (or (package-strip-rcs-id (lm-header "package-version"))
+                  (package-strip-rcs-id (lm-header "version"))))
+             (homepage (lm-homepage)))
+        (unless pkg-version
+          (error
+           "Package lacks a \"Version\" or \"Package-Version\" header"))
+        (package-desc-from-define
+         file-name pkg-version desc
+         (if requires-str
+             (package--prepare-dependencies
+              (package-read-from-string requires-str)))
+         :kind 'single
+         :url homepage
+         :maintainer (lm-maintainer)
+         :authors (lm-authors))))))
 
 (defun package--read-pkg-desc (kind)
   "Read a `define-package' form in current buffer.
-- 
2.12.2

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




In GNU Emacs 25.1.1 (x86_64-apple-darwin13.4.0, NS appkit-1265.21 Version 10.9.5 (Build 13F1911))
 of 2016-09-21 built on builder10-9.porkrind.org
Windowing system distributor 'Apple', version 10.3.1504
Configured using:
 'configure --with-ns '--enable-locallisppath=/Library/Application
 Support/Emacs/${version}/site-lisp:/Library/Application
 Support/Emacs/site-lisp' --with-modules'

Configured features:
NOTIFY ACL GNUTLS LIBXML2 ZLIB TOOLKIT_SCROLL_BARS NS MODULES

Important settings:
  value of $LC_CTYPE: en_US.UTF-8
  value of $LANG: en_US
  locale-coding-system: utf-8


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26490; Package emacs. (Fri, 14 Apr 2017 07:31:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Steve Purcell <steve <at> sanityinc.com>
Cc: 26490 <at> debbugs.gnu.org
Subject: Re: bug#26490: 25.1;
 package-buffer-info is incorrectly case-insensitive
Date: Fri, 14 Apr 2017 10:31:05 +0300
> From: Steve Purcell <steve <at> sanityinc.com>
> Date: Fri, 14 Apr 2017 12:44:24 +1200
> 
> `package-buffer-info' looks for an "filename.el ends here" comment line,
> where "filename.el" matches that provided on the first line of the
> file. However, it does not set `case-fold-search' to nil explicitly, and
> so will happily allow "FiLeNaMe.EL" in the trailing line.
> 
> I rely on this function in package-lint to detect certain issues with
> packaging, and noticed this failure there. I can work around it by
> unsetting `case-fold-search', but this seems the wrong fix.

Why do you think it would be wrong for you in your special use case to
bind case-fold-search to nil?  I think it's exactly the right
solution.

> A simple patch is attached.

What about Emacs running on case-insensitive filesystems?  What will
your patch do in that case?

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26490; Package emacs. (Fri, 14 Apr 2017 09:14:01 GMT) Full text and rfc822 format available.

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

From: Steve Purcell <steve <at> sanityinc.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 26490 <at> debbugs.gnu.org
Subject: Re: bug#26490: 25.1; package-buffer-info is incorrectly
 case-insensitive
Date: Fri, 14 Apr 2017 21:12:47 +1200
I’m going to bind case-fold-search to nil in my case anyway, but I don’t think you want a situation in which the case of the first and last lines differ, simply for the sake of consistency: that’s all that this patch ensures, and it’s orthogonal to filesystem case-sensitivity.

(Further, in package-lint, the intention is to check that the filenames on those lines exactly match the elisp buffer's filename, if any, and the name of the provided feature. Those comparisons should always be case-sensitive, I would think, because the elisp file could be taken from a local case-insensitive FS to a case-sensitive FS, and mismatches would presumably cause issues.)





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26490; Package emacs. (Fri, 14 Apr 2017 20:31:02 GMT) Full text and rfc822 format available.

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

From: Glenn Morris <rgm <at> gnu.org>
To: Steve Purcell <steve <at> sanityinc.com>
Cc: 26490 <at> debbugs.gnu.org
Subject: Re: bug#26490: 25.1;
 package-buffer-info is incorrectly case-insensitive
Date: Fri, 14 Apr 2017 16:29:47 -0400
Steve Purcell wrote:

> `package-buffer-info' looks for an "filename.el ends here" comment line,
> where "filename.el" matches that provided on the first line of the
> file.

Why does it care at all?
I thought the "filename ends here" was an ancient way of identifying
files that might have been truncated in transit. It doesn't seem
relevant in this day and age.
And if that is what it's for, why should the case of the filename matter?
So long as some "ends here" line is there, the file is ok.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26490; Package emacs. (Mon, 17 Apr 2017 17:15:01 GMT) Full text and rfc822 format available.

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

From: Steve Purcell <steve <at> sanityinc.com>
To: Glenn Morris <rgm <at> gnu.org>, 26490 <at> debbugs.gnu.org
Subject: Re: bug#26490: 25.1; package-buffer-info is incorrectly
 case-insensitive
Date: Sat, 15 Apr 2017 11:02:48 +1200
> Why does it care at all?
> I thought the "filename ends here" was an ancient way of identifying
> files that might have been truncated in transit. It doesn't seem
> relevant in this day and age.
> And if that is what it's for, why should the case of the filename matter?
> So long as some "ends here" line is there, the file is ok.


Nonetheless, it has been part of the format expected by package.el for years.

Making package.el more permissive over time can lead to problems with packages in older Emacsen, a prime example being the recently-added backwards-incompatible support for version-less dependencies in the `Package-Requires` header: authors check their packages in a recent Emacs and then find that an older otherwise-compatible Emacs can’t even parse their package metadata.



Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26490; Package emacs. (Sat, 24 Aug 2019 05:58:01 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 26490 <at> debbugs.gnu.org, Steve Purcell <steve <at> sanityinc.com>
Subject: Re: bug#26490: 25.1;
 package-buffer-info is incorrectly case-insensitive
Date: Sat, 24 Aug 2019 07:57:06 +0200
[Message part 1 (text/plain, inline)]
Glenn Morris <rgm <at> gnu.org> writes:

> Why does it care at all?
> I thought the "filename ends here" was an ancient way of identifying
> files that might have been truncated in transit. It doesn't seem
> relevant in this day and age.

I agree; this is an ancient ritual from times long past.  I suggest
that we get rid of this requirement to consider a package valid.

I think this requirement, while not the most important thing in the
world, just looks Very Old (TM) to new developers looking to get
started in Emacs Lisp.  And since it indeed hardly plays an important
role anymore, we have little to lose by getting rid of it, AFAIU.

Steve Purcell <steve <at> sanityinc.com> writes:

> Nonetheless, it has been part of the format expected by package.el for years.
>
> Making package.el more permissive over time can lead to problems with packages
> in older Emacsen, a prime example being the recently-added
> backwards-incompatible support for version-less dependencies in the
> `Package-Requires` header: authors check their packages in a recent Emacs and
> then find that an older otherwise-compatible Emacs can’t even parse their
> package metadata.

Sure, that can be a problem.  I think that means that we should not
(yet) encourage package developers to not use them in their packages.
But if we don't take a first step, we can never get rid of it.

At the end of the day, it's the job of package developers to maintain
backwards compatibility.  I don't see why this change would be any
different in that respect from the many other changes that we make
between releases.

I have attached a tentative patch to remove this requirement from
package.el.  Comments are more than welcome.

Thanks,
Stefan Kangas
[0001-Don-t-require-ending-comment-to-consider-a-package-v.patch (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26490; Package emacs. (Sat, 24 Aug 2019 06:36:01 GMT) Full text and rfc822 format available.

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

From: Steve Purcell <steve <at> sanityinc.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 26490 <at> debbugs.gnu.org
Subject: Re: bug#26490: 25.1; package-buffer-info is incorrectly
 case-insensitive
Date: Sat, 24 Aug 2019 18:35:31 +1200
> On 24 Aug 2019, at 17:57, Stefan Kangas <stefan <at> marxist.se> wrote:
> Steve Purcell <steve <at> sanityinc.com> writes:
> 
>> Nonetheless, it has been part of the format expected by package.el for years.
>> 
>> Making package.el more permissive over time can lead to problems with packages
>> in older Emacsen, a prime example being the recently-added
>> backwards-incompatible support for version-less dependencies in the
>> `Package-Requires` header: authors check their packages in a recent Emacs and
>> then find that an older otherwise-compatible Emacs can’t even parse their
>> package metadata.
> 
> Sure, that can be a problem.  I think that means that we should not
> (yet) encourage package developers to not use them in their packages.
> But if we don't take a first step, we can never get rid of it.

> At the end of the day, it's the job of package developers to maintain
> backwards compatibility.  I don't see why this change would be any
> different in that respect from the many other changes that we make
> between releases.


My point is that if a package file can’t even be parsed by an older Emacs version’s “package.el”, the user of that Emacs version will automatically get an obscure error when they try to install it, even if the the package author was helpful enough to add `(emacs “27”)` as a dependency to indicate incompatibility. That’s not something that the package author could reasonably foresee, and it feels avoidable by keeping the basic structure of required metadata stable and backwards compatible.



Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26490; Package emacs. (Sat, 28 Sep 2019 10:56:01 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Steve Purcell <steve <at> sanityinc.com>
Cc: 26490 <at> debbugs.gnu.org
Subject: Re: bug#26490: 25.1;
 package-buffer-info is incorrectly case-insensitive
Date: Sat, 28 Sep 2019 12:55:09 +0200
[Message part 1 (text/plain, inline)]
Steve Purcell <steve <at> sanityinc.com> writes:

> >> Nonetheless, it has been part of the format expected by package.el for years.
> >>
> >> Making package.el more permissive over time can lead to problems with packages
> >> in older Emacsen, a prime example being the recently-added
> >> backwards-incompatible support for version-less dependencies in the
> >> `Package-Requires` header: authors check their packages in a recent Emacs and
> >> then find that an older otherwise-compatible Emacs can’t even parse their
> >> package metadata.
> >
> > Sure, that can be a problem.  I think that means that we should not
> > (yet) encourage package developers to not use them in their packages.
> > But if we don't take a first step, we can never get rid of it.
>
> > At the end of the day, it's the job of package developers to maintain
> > backwards compatibility.  I don't see why this change would be any
> > different in that respect from the many other changes that we make
> > between releases.
>
> My point is that if a package file can’t even be parsed by an older Emacs
> version’s “package.el”, the user of that Emacs version will automatically get an
> obscure error when they try to install it, even if the the package author was
> helpful enough to add `(emacs “27”)` as a dependency to indicate
> incompatibility. That’s not something that the package author could reasonably
> foresee, and it feels avoidable by keeping the basic structure of required
> metadata stable and backwards compatible.

I see your point.  How about issuing a warning instead?  That should
be sufficiently discouraging for package authors, while also allowing
us to drop this requirement at some point in the future.

I've attached a patch which I believe would do this in a reasonable way.

Best regards,
Stefan Kangas
[0001-Don-t-refuse-to-install-packages-without-a-footer-li.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26490; Package emacs. (Tue, 01 Oct 2019 23:55:01 GMT) Full text and rfc822 format available.

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

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 26490 <at> debbugs.gnu.org, Steve Purcell <steve <at> sanityinc.com>
Subject: Re: bug#26490: 25.1; package-buffer-info is incorrectly
 case-insensitive
Date: Wed, 02 Oct 2019 00:54:38 +0100
Stefan Kangas <stefan <at> marxist.se> writes:

> I've attached a patch which I believe would do this in a reasonable way.

Sorry, I haven't followed the discussion, but I have a minor nit:

> diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
> index ef0c5171de..5601e4d630 100644
> --- a/lisp/emacs-lisp/package.el
> +++ b/lisp/emacs-lisp/package.el
> @@ -1046,10 +1046,12 @@ package-buffer-info
>    (let ((file-name (match-string-no-properties 1))
>          (desc      (match-string-no-properties 2))
>          (start     (line-beginning-position)))
> -    ;; The terminating comment format could be extended to accept a
> -    ;; generic string that is not in English.
> +    ;; This warning was added in Emacs 27.1, and should be removed at
> +    ;; the earliest in version 31.1.  The idea is to phase out the
> +    ;; requirement for a "footer line" without unduly impacting users
> +    ;; on earlier Emacs versions.  See Bug#26490 for more details.
>      (unless (search-forward (concat ";;; " file-name ".el ends here"))
> -      (error "Package lacks a terminating comment"))
> +      (warn "Package lacks a terminating comment"))

Shouldn't this be (lwarn 'package ...) or similar?
(See, for example, the call to lwarn in package-initialize.)

Thanks,

-- 
Basil




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26490; Package emacs. (Fri, 04 Oct 2019 12:50:02 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 26490 <at> debbugs.gnu.org, Steve Purcell <steve <at> sanityinc.com>
Subject: Re: bug#26490: 25.1;
 package-buffer-info is incorrectly case-insensitive
Date: Fri, 4 Oct 2019 14:48:51 +0200
[Message part 1 (text/plain, inline)]
Basil L. Contovounesios <contovob <at> tcd.ie> writes:

> Shouldn't this be (lwarn 'package ...) or similar?
> (See, for example, the call to lwarn in package-initialize.)

Indeed, thanks.  Fixed in the attached patch.

Best regards,
Stefan Kangas
[0001-Don-t-refuse-to-install-packages-without-a-footer-li.patch (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26490; Package emacs. (Sun, 20 Oct 2019 14:18:01 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Steve Purcell <steve <at> sanityinc.com>
Cc: 26490 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#26490: 25.1;
 package-buffer-info is incorrectly case-insensitive
Date: Sun, 20 Oct 2019 16:16:58 +0200
[Message part 1 (text/plain, inline)]
Stefan Kangas <stefan <at> marxist.se> writes:

> I see your point.  How about issuing a warning instead?  That should
> be sufficiently discouraging for package authors, while also allowing
> us to drop this requirement at some point in the future.
>
> I've attached a patch which I believe would do this in a reasonable way.

Ping!  Does anyone have any objections to this change, or any
comments?  I've attached the latest version of the patch.

In summary, it makes it possible to install packages even if they are
missing the terminating ";; foo.el ends here" line.  Instead, we raise
a warning for such packages.

Best regards,
Stefan Kangas
[0001-Allow-installation-packages-without-a-footer-line.patch (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#26490; Package emacs. (Mon, 21 Oct 2019 16:07:02 GMT) Full text and rfc822 format available.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 26490 <at> debbugs.gnu.org, Steve Purcell <steve <at> sanityinc.com>
Subject: Re: bug#26490: 25.1; package-buffer-info is incorrectly
 case-insensitive
Date: Mon, 21 Oct 2019 12:05:53 -0400
> Ping!  Does anyone have any objections to this change, or any
> comments?  I've attached the latest version of the patch.

No objection on my side,


        Stefan





Reply sent to Stefan Kangas <stefan <at> marxist.se>:
You have taken responsibility. (Sat, 02 Nov 2019 00:32:02 GMT) Full text and rfc822 format available.

Notification sent to Steve Purcell <steve <at> sanityinc.com>:
bug acknowledged by developer. (Sat, 02 Nov 2019 00:32:02 GMT) Full text and rfc822 format available.

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

From: Stefan Kangas <stefan <at> marxist.se>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 26490-done <at> debbugs.gnu.org, Steve Purcell <steve <at> sanityinc.com>
Subject: Re: bug#26490: 25.1;
 package-buffer-info is incorrectly case-insensitive
Date: Sat, 2 Nov 2019 01:31:09 +0100
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>
> > Ping!  Does anyone have any objections to this change, or any
> > comments?  I've attached the latest version of the patch.
>
> No objection on my side,

No other comments within 13 days, so I've now pushed the patch as
commit 6297eb0fca.  I'm consequently closing this bug.

Best regards,
Stefan Kangas




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 30 Nov 2019 12:24:08 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 120 days ago.

Previous Next


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