unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
@ 2017-04-14  0:44 Steve Purcell
  2017-04-14  7:31 ` Eli Zaretskii
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Steve Purcell @ 2017-04-14  0:44 UTC (permalink / raw)
  To: 26490

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




`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.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-patch, Size: 3769 bytes --]

From bfa9b23e6b4ef030e36fdcf5c15cef15fb01074a Mon Sep 17 00:00:00 2001
From: Steve Purcell <steve@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


[-- Attachment #3: Type: text/plain, Size: 586 bytes --]






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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
  2017-04-14  0:44 bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive Steve Purcell
@ 2017-04-14  7:31 ` Eli Zaretskii
  2017-04-14  9:12   ` Steve Purcell
  2017-04-14 20:29 ` Glenn Morris
  2019-08-24  5:57 ` Stefan Kangas
  2 siblings, 1 reply; 13+ messages in thread
From: Eli Zaretskii @ 2017-04-14  7:31 UTC (permalink / raw)
  To: Steve Purcell; +Cc: 26490

> From: Steve Purcell <steve@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.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
  2017-04-14  7:31 ` Eli Zaretskii
@ 2017-04-14  9:12   ` Steve Purcell
  0 siblings, 0 replies; 13+ messages in thread
From: Steve Purcell @ 2017-04-14  9:12 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 26490

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.)






^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
  2017-04-14  0:44 bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive Steve Purcell
  2017-04-14  7:31 ` Eli Zaretskii
@ 2017-04-14 20:29 ` Glenn Morris
  2017-04-14 23:02   ` Steve Purcell
  2019-08-24  5:57 ` Stefan Kangas
  2 siblings, 1 reply; 13+ messages in thread
From: Glenn Morris @ 2017-04-14 20:29 UTC (permalink / raw)
  To: Steve Purcell; +Cc: 26490

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.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
  2017-04-14 20:29 ` Glenn Morris
@ 2017-04-14 23:02   ` Steve Purcell
  0 siblings, 0 replies; 13+ messages in thread
From: Steve Purcell @ 2017-04-14 23:02 UTC (permalink / raw)
  To: Glenn Morris, 26490

> 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.




^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
  2017-04-14  0:44 bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive Steve Purcell
  2017-04-14  7:31 ` Eli Zaretskii
  2017-04-14 20:29 ` Glenn Morris
@ 2019-08-24  5:57 ` Stefan Kangas
  2019-08-24  6:35   ` Steve Purcell
  2 siblings, 1 reply; 13+ messages in thread
From: Stefan Kangas @ 2019-08-24  5:57 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 26490, Steve Purcell

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

Glenn Morris <rgm@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@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

[-- Attachment #2: 0001-Don-t-require-ending-comment-to-consider-a-package-v.patch --]
[-- Type: application/octet-stream, Size: 1154 bytes --]

From b1ba2477be985595bf27a569087fabc3bdba158d Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Sat, 24 Aug 2019 07:44:00 +0200
Subject: [PATCH] Don't require ending comment to consider a package valid

* lisp/emacs-lisp/package.el (package-buffer-info): Don't require
the ending comment ";;; foo-package.el ends here".  (Bug#26490)
---
 lisp/emacs-lisp/package.el | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index cd127e1a8e..c6a3f30452 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1045,10 +1045,6 @@ 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.
-    (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))
-- 
2.22.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
  2019-08-24  5:57 ` Stefan Kangas
@ 2019-08-24  6:35   ` Steve Purcell
  2019-09-28 10:55     ` Stefan Kangas
  0 siblings, 1 reply; 13+ messages in thread
From: Steve Purcell @ 2019-08-24  6:35 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 26490


> On 24 Aug 2019, at 17:57, Stefan Kangas <stefan@marxist.se> wrote:
> Steve Purcell <steve@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.




^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
  2019-08-24  6:35   ` Steve Purcell
@ 2019-09-28 10:55     ` Stefan Kangas
  2019-10-01 23:54       ` Basil L. Contovounesios
  2019-10-20 14:16       ` Stefan Kangas
  0 siblings, 2 replies; 13+ messages in thread
From: Stefan Kangas @ 2019-09-28 10:55 UTC (permalink / raw)
  To: Steve Purcell; +Cc: 26490

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

Steve Purcell <steve@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

[-- Attachment #2: 0001-Don-t-refuse-to-install-packages-without-a-footer-li.patch --]
[-- Type: text/x-patch, Size: 2549 bytes --]

From 9c49666dc857000c838db4beff64a3b3484aa812 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Fri, 20 Sep 2019 19:18:03 +0200
Subject: [PATCH] Don't refuse to install packages without a "footer line"

* lisp/emacs-lisp/package.el (package-buffer-info): Don't signal an
error when the "footer line" is missing.  Warn only.  (Bug#26490)
* etc/NEWS: Announce it.
---
 etc/NEWS                   | 14 ++++++++++++++
 lisp/emacs-lisp/package.el |  8 +++++---
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index f8322104d4..0cbbf6e693 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -942,6 +942,20 @@ it can't find the config file.
 
 ** Package
 
+*** Warn if "footer line" is missing, but still install package.
+package.el used to refuse to install a package without the so-called
+"footer line", which appears at the very end of the file:
+
+;;; FILENAME ends here
+
+package.el will now install packages without this line, but it will
+issue a warning.  To avoid this warning, packages should keep the
+"footer line".
+
+Note that versions of Emacs older than 27.1 will not only refuse to
+install packages without such a line -- they will be unable to parse
+package data.  It is therefore recommended to keep this line.
+
 *** Change of 'package-check-signature' for packages with multiple sigs
 In previous Emacsen, 't' checked that all signatures are valid.
 Now 't' only checks that at least one signature is valid and the new 'all'
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"))
     ;; Try to include a trailing newline.
     (forward-line)
     (narrow-to-region start (point))
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
  2019-09-28 10:55     ` Stefan Kangas
@ 2019-10-01 23:54       ` Basil L. Contovounesios
  2019-10-04 12:48         ` Stefan Kangas
  2019-10-20 14:16       ` Stefan Kangas
  1 sibling, 1 reply; 13+ messages in thread
From: Basil L. Contovounesios @ 2019-10-01 23:54 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 26490, Steve Purcell

Stefan Kangas <stefan@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





^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
  2019-10-01 23:54       ` Basil L. Contovounesios
@ 2019-10-04 12:48         ` Stefan Kangas
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Kangas @ 2019-10-04 12:48 UTC (permalink / raw)
  To: Basil L. Contovounesios; +Cc: 26490, Steve Purcell

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

Basil L. Contovounesios <contovob@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

[-- Attachment #2: 0001-Don-t-refuse-to-install-packages-without-a-footer-li.patch --]
[-- Type: text/x-patch, Size: 2600 bytes --]

From 8953215fb83b6b40a12bf7e9295c8ebf3febad75 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Fri, 20 Sep 2019 19:18:03 +0200
Subject: [PATCH] Don't refuse to install packages without a "footer line"

* lisp/emacs-lisp/package.el (package-buffer-info): Don't signal an
error when the "footer line" is missing.  Warn only.  (Bug#26490)
* etc/NEWS: Announce it.
---
 etc/NEWS                   | 14 ++++++++++++++
 lisp/emacs-lisp/package.el |  9 ++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index f8322104d4..0cbbf6e693 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -942,6 +942,20 @@ it can't find the config file.
 
 ** Package
 
+*** Warn if "footer line" is missing, but still install package.
+package.el used to refuse to install a package without the so-called
+"footer line", which appears at the very end of the file:
+
+;;; FILENAME ends here
+
+package.el will now install packages without this line, but it will
+issue a warning.  To avoid this warning, packages should keep the
+"footer line".
+
+Note that versions of Emacs older than 27.1 will not only refuse to
+install packages without such a line -- they will be unable to parse
+package data.  It is therefore recommended to keep this line.
+
 *** Change of 'package-check-signature' for packages with multiple sigs
 In previous Emacsen, 't' checked that all signatures are valid.
 Now 't' only checks that at least one signature is valid and the new 'all'
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ef0c5171de..295edc7f37 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1046,10 +1046,13 @@ 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"))
+      (lwarn '(package package-format) :warning
+             "Package lacks a terminating comment"))
     ;; Try to include a trailing newline.
     (forward-line)
     (narrow-to-region start (point))
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
  2019-09-28 10:55     ` Stefan Kangas
  2019-10-01 23:54       ` Basil L. Contovounesios
@ 2019-10-20 14:16       ` Stefan Kangas
  2019-10-21 16:05         ` Stefan Monnier
  1 sibling, 1 reply; 13+ messages in thread
From: Stefan Kangas @ 2019-10-20 14:16 UTC (permalink / raw)
  To: Steve Purcell; +Cc: 26490, Stefan Monnier

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

Stefan Kangas <stefan@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

[-- Attachment #2: 0001-Allow-installation-packages-without-a-footer-line.patch --]
[-- Type: application/octet-stream, Size: 2821 bytes --]

From cbad98f4ac8029e383255ff45395b5aa01935306 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Fri, 20 Sep 2019 19:18:03 +0200
Subject: [PATCH] Allow installation packages without a "footer line"

* lisp/emacs-lisp/package.el (package-buffer-info): Warn instead of
signaling an error when there is no "footer line".  This allows such
packages to be installed.  (Bug#26490)
* etc/NEWS: Announce it.
---
 etc/NEWS                   | 15 +++++++++++++++
 lisp/emacs-lisp/package.el | 11 ++++++++---
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 46ed40dfcb..a47c6098d4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1030,6 +1030,21 @@ it can't find the config file.
 
 ** Package
 
+*** Allow installation of packages missing a "footer line".
+Previously, the package system refused to install a package without
+the so-called "footer line", which should appear at the very end of
+the file:
+
+;;; FILENAME ends here
+
+Emacs will now only issue a warning for such packages.  To avoid this
+warning, packages should keep the "footer line".
+
+Note that versions of Emacs older than 27.1 will not only refuse to
+install packages without such a line -- they will be unable to parse
+the package data.  It is therefore strongly recommended to keep this
+line, even if your package only supports Emacs 27.1 or later.
+
 *** Change of 'package-check-signature' for packages with multiple sigs
 In previous Emacsen, 't' checked that all signatures are valid.
 Now 't' only checks that at least one signature is valid and the new 'all'
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index 20462064af..69ca658b81 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -1082,10 +1082,15 @@ 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.
+    ;; Before version 27.1, it was not possible to install packages
+    ;; without a terminating comment.  We now only issue a warning.
+    ;; This warning should probably be around until at least Emacs
+    ;; version 32.1 or so, or possibly longer.  The idea is to phase
+    ;; out the requirement for a "footer line" without unduly
+    ;; impacting users on earlier Emacs versions.  (Bug#26490)
     (unless (search-forward (concat ";;; " file-name ".el ends here"))
-      (error "Package lacks a terminating comment"))
+      (lwarn '(package package-format) :warning
+             "Package lacks a terminating comment"))
     ;; Try to include a trailing newline.
     (forward-line)
     (narrow-to-region start (point))
-- 
2.23.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
  2019-10-20 14:16       ` Stefan Kangas
@ 2019-10-21 16:05         ` Stefan Monnier
  2019-11-02  0:31           ` Stefan Kangas
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2019-10-21 16:05 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 26490, Steve Purcell

> 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






^ permalink raw reply	[flat|nested] 13+ messages in thread

* bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive
  2019-10-21 16:05         ` Stefan Monnier
@ 2019-11-02  0:31           ` Stefan Kangas
  0 siblings, 0 replies; 13+ messages in thread
From: Stefan Kangas @ 2019-11-02  0:31 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 26490-done, Steve Purcell

Stefan Monnier <monnier@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





^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-11-02  0:31 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-14  0:44 bug#26490: 25.1; package-buffer-info is incorrectly case-insensitive Steve Purcell
2017-04-14  7:31 ` Eli Zaretskii
2017-04-14  9:12   ` Steve Purcell
2017-04-14 20:29 ` Glenn Morris
2017-04-14 23:02   ` Steve Purcell
2019-08-24  5:57 ` Stefan Kangas
2019-08-24  6:35   ` Steve Purcell
2019-09-28 10:55     ` Stefan Kangas
2019-10-01 23:54       ` Basil L. Contovounesios
2019-10-04 12:48         ` Stefan Kangas
2019-10-20 14:16       ` Stefan Kangas
2019-10-21 16:05         ` Stefan Monnier
2019-11-02  0:31           ` Stefan Kangas

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).