GNU bug report logs -
#67916
30.0.50; No lexical-binding directive warning in -pkg.el files
Previous Next
Full log
View this message in rfc822 format
Philip Kaludercic <philipk <at> posteo.net> writes:
> "Basil L. Contovounesios" <contovob <at> tcd.ie> writes:
>
>> Philip Kaludercic [2023-12-20 15:14 +0000] wrote:
>>
>>> "Basil L. Contovounesios" <contovob <at> 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? ;)
>
> The latter.
>
>>>> 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).
>
> My guess is that it was just not removed, but I'll just let Stefan
> explain that.
>
>>>> (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.
>
> My understanding was that this information is supposed to indicate when
> the group was added, while each member of a group that has since been
> changed would have a newer :version tag.
>
>>>> (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?
>
> Yes.
>
>> [ To be honest, I'm slightly inclined to add this to macroexp.el
>> instead, since it's a somewhat common operation. ]
>
> My only concern is that this would slightly complicate the initiative of
> adding package.el to GNU ELPA, if that is to proceed at all.
>
>>> 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).
>
> Nevermind then.
>
>>>> +;; 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?
>
> IIRC there were some bugs related to the generation of -pkg.el files,
> but I can't find them right now. I'll ping you if I find anything.
>
>> Thanks,
So what's the conclusion here? Are these patches good to go, or is more
work needed?
This bug report was last modified 95 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.