bug#67916: 30.0.50; No lexical-binding directive warning in -pkg.el files

["Basil L. Contovounesios" via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

Philip Kaludercic [2023-12-20 15:14 +0000] wrote:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> package-vc--generate-description-file currently passes:
>>   :kind 'vc
>> unquoted to define-package, which results in the -pkg.el contents:
>>   (define-package ... :kind vc ... :keywords '(...) ...)
>> Note the difference in quoting between :kind and :keywords.  Is this
>> intentional?  Or can/should :kind pass through macroexp-quote as well?
>
> This is not intentional, if anything a lucky oversight.

Lucky in the sense that it's preferable this way, or just an accident? ;)

>> Questions for Stefan:
>>
>> - Which version of Emacs can/does elpa-admin.el assume?
>
> All I can say is that the ELPA build server, the main user of
> elpa-admin.el, has Emacs 28.2 installed.

I'm wondering because elpa-admin.el seems to contain some compatibility
code for Emacs 26 (elpaa--select-revision, elpaa--write-pkg-file) and
Emacs versions <28 (elpaa--get-section, elpaa--html-build-doc).

>>  (defgroup package nil
>>    "Manager for Emacs Lisp packages."
>>    :group 'applications
>> -  :version "24.1")
>> +  :group 'tools
>> +  :version "30.1")
>
> I am not sure if bumping :version is necessary (here and above).

I think it's unnecessary in the sense that I don't know of any place
where this information is displayed, but otherwise I thought it was good
form to do this since any change in :groups is user-visible.

>>  (define-inline package-vc-p (pkg-desc)
>>    "Return non-nil if PKG-DESC is a VC package."
>> -  (inline-letevals (pkg-desc)
>> -    (inline-quote (eq (package-desc-kind ,pkg-desc) 'vc))))
>> +  (inline-quote (eq (package-desc-kind ,pkg-desc) 'vc)))
>> +
>> +(define-inline package--unquote (arg)
>> +  "Return ARG without its surrounding `quote', if any."
>> +  (inline-letevals (arg)
>> +    (inline-quote (if (eq (car-safe ,arg) 'quote) (cadr ,arg) ,arg))))
>
> Honestly, the usage of define-inline was probably a premature
> optimisation on my part, I don't think we really need it here either.

You mean, you prefer package--unquote being a plain function?

[ To be honest, I'm slightly inclined to add this to macroexp.el
  instead, since it's a somewhat common operation. ]

> You removed (require 'inline) anyway, or is it now preloaded?

define-inline is autoloaded, and there are no other in-tree occurrences
of (require 'inline).

>> +;; Potentially also used in elpa.git.
>> +(defun package--write-description-file ( file name version doc reqs extras
>> +                                         &optional extra-props verbose)
>> +  "Write a `define-package' declaration to FILE.
>> +Absolute FILE names the -pkg.el description file.
>> +NAME, VERSION, and DOC are the leading, and EXTRA-PROPS the
>> +trailing, arguments of `define-package'.
>> +REQS and EXTRAS are the eponymous `package-desc' slots.
>> +Non-nil VERBOSE means display \"Wrote file\" message."
>> +  (let* ((src (replace-regexp-in-string (rx "-pkg.el" eos) ".el"
>> +                                        (file-name-nondirectory file) t t))
>> +         (def `(define-package ,name ,version ,doc
>> +                 ,(macroexp-quote
>> +                   ;; Turn requirement version lists into string form.
>> +                   (mapcar (lambda (elt)
>> +                             (list (car elt)
>> +                                   (package-version-join (cadr elt))))
>> +                           reqs))
>> +                 ,@extra-props
>> +                 ,@(package--alist-to-plist-args extras)))
>> +         (print-cfg '((length . nil)
>> +                      (level . nil)
>> +                      (quoted . t)))
>> +         (str (concat ";;; Generated package description from " src
>> +                      "  -*- no-byte-compile: t; lexical-binding: t -*-\n"
>> +                      (prin1-to-string def nil print-cfg)
>> +                      "\n")))
>> +    (write-region str nil file nil (unless verbose 'silent))))
>
> I like this, but we should really make sure that there are no hidden
> edge-cases that might cause problems.

How do we find them if they're hidden? ;)
Did you have something specific in mind?

Thanks,
-- 
Basil