[bug#52388] [PATCH] build-system: emacs: Add generation of -pkg.el files.

[Andrew Tropin <andrew@trop.in>]

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

On 2022-01-14 21:56, Liliana Marie Prikler wrote:

> Hi Andrew,
>
> Am Donnerstag, dem 09.12.2021 um 12:01 +0300 schrieb Andrew Tropin:
>> 
>> * guix/build/emacs-build-system.scm (%default-exclude): Add
>> generation of
>> -pkg.el files for packages, which do not provide them.
>> ---
>> Implemented phase, which generates -pkg.el from comments in library
>> file.  The solution for finding main el file of the package is a
>> little hacky, because package name isn't available build time.
>> 
>> I took a part of the elisp implementation from melpa source code.
>> https://github.com/melpa/melpa/blob/master/package-build/package-build.el#L553
> As promised, I took a deeper look at your patch.  As already noted,
> there were some clean-up actions I had to perform, such as keeping to
> our line limit (it was not easy, I tell you) among other things.  Also,
> confusingly, your condition-case code did not handle errors and wrong
> handling of the version field blew up everything for me.  Did you test
> this code?

I thought it was the version I used for past a few weeks, however I
could send a little outdated version, not the latest one by accident.
Tested it with guix build emacs-paredit.

See your fix which wraps both forms in one let, thank you, yep, it's
very likely I sent an intermediate version of the patch accidentially.

>
> In any case, attached is my revised patch.  I so far only checked it
> with emacs-olivetti -- a package whose description is missing in
> current Guix Emacs.  I'll give everyone some time to test things before
> pushing this however; I don't want to break a bunch of Emacs packages
> scattered around various files.
>
> Cheers
> From 9fa6a09a131cfe436ca053c960ed9625263bc650 Mon Sep 17 00:00:00 2001
> From: Andrew Tropin <andrew@trop.in>
> Date: Thu, 9 Dec 2021 12:01:46 +0300
> Subject: [PATCH] build-system: emacs: Ensure that package descriptions are
>  generated.
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> This patch addresses the second part of <https://bugs.gnu.org/48331>.
> While existing -pkg.el files were previously installed, no such files
> were generated for packages lacking them, resulting in packages not
> being listed as installed and not being available towards
> “describe-package”.
>
> * guix/build/emacs-build-system.scm (find-root-library-file)
> (ensure-package-description): New variables.
> (%standard-phases): Add ‘ensure-package-description’.
> ---
>  guix/build/emacs-build-system.scm | 77 ++++++++++++++++++++++++++++++-
>  1 file changed, 76 insertions(+), 1 deletion(-)
>
> diff --git a/guix/build/emacs-build-system.scm b/guix/build/emacs-build-system.scm
> index ab77e57f33..58d2a9b9f4 100644
> --- a/guix/build/emacs-build-system.scm
> +++ b/guix/build/emacs-build-system.scm
> @@ -140,6 +140,79 @@ (define (substitute-program-names)
>            (substitute-program-names))))
>      #t))
>  
> +(define (find-root-library-file name)
> +  (let loop ((parts (string-split
> +                     (package-name-version->elpa-name-version name) #\-))
> +             (candidate ""))
> +    (cond
> +     ;; at least one version part is given, so we don't terminate "early"
> +     ((null? parts) #f)
> +     ((string-null? candidate) (loop (cdr parts) (car parts)))
> +     ((file-exists? (string-append candidate ".el")) candidate)
> +     (else
> +      (loop (cdr parts) (string-append candidate "-" (car parts)))))))
> +
> +(define* (ensure-package-description #:key outputs #:allow-other-keys)
> +  (define (write-pkg-file name)
> +    (define summary-regexp
> +      "^;;; [^ ]*\\.el ---[ \t]*\\(.*?\\)[ \t]*\\(-\\*-.*-\\*-[ \t]*\\)?$")
> +    (define %write-pkg-file-form
> +      `(progn
> +        (require 'lisp-mnt)
> +        (require 'package)
> +
> +        (defun build-package-desc-from-library (name)
> +          (package-desc-from-define
> +           name
> +           ;; Workaround for malformed version string (for example "24 (beta)"
> +           ;; in paredit.el), try to parse version obtained by lm-version,
> +           ;; before trying to create package-desc.  Otherwis the whole process

s/Otherwis/Otherwise,

> +           ;; of generation -pkg.el will fail.
> +           (condition-case
> +            nil
> +            (let ((version (lm-version)))
> +              ;; raises an error if version is invalid
> +              (and (version-to-list version) version))
> +            (error "0.0.0"))
> +           (or (save-excursion
> +                (goto-char (point-min))
> +                (and (re-search-forward ,summary-regexp nil t)
> +                     (match-string-no-properties 1)))
> +               package--default-summary)
> +           (let ((require-lines (lm-header-multiline "package-requires")))
> +             (and require-lines
> +                  (package--prepare-dependencies
> +                   (package-read-from-string
> +                    (mapconcat 'identity require-lines " ")))))
> +           :kind       'single
> +           :url        (lm-homepage)
> +           :keywords   (lm-keywords-list)
> +           :maintainer (lm-maintainer)
> +           :authors    (lm-authors)))
> +
> +        (defun generate-package-description-file (name)
> +          (package-generate-description-file
> +           (build-package-desc-from-library name)
> +           (concat name "-pkg.el")))
> +
> +        (condition-case
> +         err
> +         (let ((name (file-name-base (buffer-file-name))))
> +           (generate-package-description-file name)
> +           (message (concat name "-pkg.el file generated.")))
> +         (error
> +          (message "There are some errors during generation of -pkg.el file:")
> +          (message "%s" (error-message-string err))))))
> +
> +    (unless (file-exists? (string-append name "-pkg.el"))
> +      (emacs-batch-edit-file (string-append name ".el")
> +        %write-pkg-file-form)))
> +
> +  (let* ((out (assoc-ref outputs "out"))
> +         (elpa-name-ver (store-directory->elpa-name-version out)))
> +    (with-directory-excursion (elpa-directory out)
> +      (and=> (find-root-library-file elpa-name-ver) write-pkg-file))))
> +
>  (define* (check #:key tests? (test-command '("make" "check"))
>                  (parallel-tests? #t) #:allow-other-keys)
>    "Run the tests by invoking TEST-COMMAND.
> @@ -279,8 +352,10 @@ (define %standard-phases
>      (add-after 'make-autoloads 'enable-autoloads-compilation
>        enable-autoloads-compilation)
>      (add-after 'enable-autoloads-compilation 'patch-el-files patch-el-files)
> +    (add-after 'patch-el-files 'ensure-package-description
> +      ensure-package-description)
>      ;; The .el files are byte compiled directly in the store.
> -    (add-after 'patch-el-files 'build build)
> +    (add-after 'ensure-package-description 'build build)
>      (add-after 'build 'validate-compiled-autoloads validate-compiled-autoloads)
>      (add-after 'validate-compiled-autoloads 'move-doc move-doc)))

Looks cleaner than original implementation, thank you!)

Applied on my local checkout of guix channel, list-packages now shows
117 external packages =)  Will keep it for a while and let you know if
any problems will reveal themselves.

-- 
Best regards,
Andrew Tropin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]