bug#48331: [PATCH draft] build-system: emacs: Generate -pkg.el file in case it is missing

2021-05-25 Thread Leo Prikler
Am Dienstag, den 25.05.2021, 16:40 +0300 schrieb Andrew Tropin:
> ---
> Thank you for the patches, tested, works for me!  The solution is
> much more precise than mutating package-directory-list variable, good
> job.  I don't see any major problems in the implementation (but I'm
> not very fluent elisp dev and maybe missing something).
Don't worry, I can wait for a proper review by some of our experts,
since we shouldn't mess with emacs-build-system all too often anyway :P

> I drafted a simple build phase, which generates -pkg.el in case it is
> missing.
> 
> There are at least a few problems with this implementation:
> 
> 1. There is no information about package record available during
> build, which makes it hard to get package name and package
> version.  I can't use any regexp to obtain this information from name
> or elpa-name-ver, because  package name and version can have
> arbitrary form: comment-dwim-2-1.0, cyberpunk-2019-theme-20191008-
> alpha or something like that.
> 2. It's also not so easy to extract description of the package from
> somewhere, the first option is to pass package record to build phases
> somehow, another is to parse PACKAGE-NAME.el file comments section.
> 3. This one I consider as a minor flaw: there is no generic solution
> for packages built with build systems other than emacs-build-system.
3. is easy -- just have those packages call the phase through emacs-
build-system.  There already exist packages which do that, and it
should be pretty straightforward to do.

For 1. and 2. I think you're thinking a little too complicated.  We can
call Emacs at build time.  In particular, we can use all of the lisp-
mnt stuff, that I used in emacs-dpd at build time, we just need to
write the (define-package ...) form to disk.
In order to pick the right file, we can either use package-name (see
discussion below) or the name of the pkg-file without -pkg.  Come to
think about it, we might also provide #:pkg-file-name as a keyword and
only generate the package file if it's set to a value other than #f,
i.e. let the packager choose whether they want to generate a -pkg.el
file.  Something along this line is done with the build.xml of ant-
build-system.

>  So, this patch is very dirty and I publish it only for future
> reference.
> 
> The intuition says that we should split name and version in build
> phase arguments, also it seems that it will be useful to provide
> other information about package during build time for cases like this
> one.  I'll learn this area a bit more and probably will make another
> thread someday.
IIRC, there's a demand for having package-name and package-version
available during build (which might get through next core-updates or
might not -- we'll see), but I don't think we can argue for
description, especially when the Guix description might not be the
thing package.el wants here.

>  guix/build/emacs-build-system.scm | 60
> ++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/guix/build/emacs-build-system.scm
> b/guix/build/emacs-build-system.scm
> index f13162d6c4..2bb102b4be 100644
> --- a/guix/build/emacs-build-system.scm
> +++ b/guix/build/emacs-build-system.scm
> @@ -116,6 +116,63 @@ environment variable\n" source-directory))
>  (parameterize ((%emacs emacs))
>(emacs-byte-compile-directory (elpa-directory out)
> 
> +
> +(define* (add-pkg-file-if-missing #:key name outputs #:allow-other-
> keys
> +  #:rest args)
> +  "Generate simple -pkg.el in case package doesn't have it in source
> code."
> +  (define (file-contains-nul-char? file)
> +(call-with-input-file file
> +  (lambda (in)
> +(let loop ((line (read-line in 'concat)))
> +  (cond
> +   ((eof-object? line) #f)
> +   ((string-index line #\nul) #t)
> +   (else (loop (read-line in 'concat))
> +  #:binary #t))
> +
> +  (let* ((out (assoc-ref outputs "out"))
> + (el-dir (elpa-directory out))
> + (elpa-name-ver (store-directory->elpa-name-version out))
> + (el-files (remove file-contains-nul-char?
> +   (find-files (getcwd) "\\.el$")))
> + (el-names (map (lambda (x) (basename x ".el")) el-files))
> +
> + (possible-names
> +  (fold (lambda (x acc)
> +  (cons
> +   (string-append
> +(if (not (null? acc)) (string-append (first acc)
> "-") "")
> +x)
> +   acc))
> +'()
> +(string-split elpa-name-ver #\-)))
> +
> + (package-names (append-map
> + (lambda (name)
> +   (let ((m (member name el-names)))
> + (if m (list (car m)) '(
> + possible-names))
> +
> + (package-name (if (null? package-names) "" (car package-
> names)))
> + (package-version 

bug#48331: [PATCH draft] build-system: emacs: Generate -pkg.el file in case it is missing

2021-05-25 Thread Andrew Tropin
---
 Thank you for the patches, tested, works for me!  The solution is much more
 precise than mutating package-directory-list variable, good job.  I don't see
 any major problems in the implementation (but I'm not very fluent elisp dev
 and maybe missing something).


 I drafted a simple build phase, which generates -pkg.el in case it is missing.

 There are at least a few problems with this implementation:

 1. There is no information about package record available during build, which
 makes it hard to get package name and package version.  I can't use any
 regexp to obtain this information from name or elpa-name-ver, because
 package name and version can have arbitrary form: comment-dwim-2-1.0,
 cyberpunk-2019-theme-20191008-alpha or something like that.
 2. It's also not so easy to extract description of the package from
 somewhere, the first option is to pass package record to build phases somehow,
 another is to parse PACKAGE-NAME.el file comments section.
 3. This one I consider as a minor flaw: there is no generic solution for
 packages built with build systems other than emacs-build-system.

 So, this patch is very dirty and I publish it only for future reference.

 The intuition says that we should split name and version in build phase
 arguments, also it seems that it will be useful to provide other information
 about package during build time for cases like this one.  I'll learn this
 area a bit more and probably will make another thread someday.

 guix/build/emacs-build-system.scm | 60 ++-
 1 file changed, 59 insertions(+), 1 deletion(-)

diff --git a/guix/build/emacs-build-system.scm
b/guix/build/emacs-build-system.scm
index f13162d6c4..2bb102b4be 100644
--- a/guix/build/emacs-build-system.scm
+++ b/guix/build/emacs-build-system.scm
@@ -116,6 +116,63 @@ environment variable\n" source-directory))
 (parameterize ((%emacs emacs))
   (emacs-byte-compile-directory (elpa-directory out)

+
+(define* (add-pkg-file-if-missing #:key name outputs #:allow-other-keys
+  #:rest args)
+  "Generate simple -pkg.el in case package doesn't have it in source code."
+  (define (file-contains-nul-char? file)
+(call-with-input-file file
+  (lambda (in)
+(let loop ((line (read-line in 'concat)))
+  (cond
+   ((eof-object? line) #f)
+   ((string-index line #\nul) #t)
+   (else (loop (read-line in 'concat))
+  #:binary #t))
+
+  (let* ((out (assoc-ref outputs "out"))
+ (el-dir (elpa-directory out))
+ (elpa-name-ver (store-directory->elpa-name-version out))
+ (el-files (remove file-contains-nul-char?
+   (find-files (getcwd) "\\.el$")))
+ (el-names (map (lambda (x) (basename x ".el")) el-files))
+
+ (possible-names
+  (fold (lambda (x acc)
+  (cons
+   (string-append
+(if (not (null? acc)) (string-append (first acc) "-") "")
+x)
+   acc))
+'()
+(string-split elpa-name-ver #\-)))
+
+ (package-names (append-map
+ (lambda (name)
+   (let ((m (member name el-names)))
+ (if m (list (car m)) '(
+ possible-names))
+
+ (package-name (if (null? package-names) "" (car package-names)))
+ (package-version (string-drop elpa-name-ver
+   (1+ (string-length package-name
+ (package-description "description should be here")
+ (pkg-file (string-append el-dir "/" package-name "-pkg.el")))
+
+(when (not (file-exists? pkg-file))
+  (with-output-to-file pkg-file
+(lambda ()
+  (format
+   #t
+   "\
+(define-package
+  ~s
+  ~s
+  ~s
+  nil)"
+   package-name package-version package-description
+#t))
+
 (define* (patch-el-files #:key outputs #:allow-other-keys)
   "Substitute the absolute \"/bin/\" directory with the right location in the
 store in '.el' files."
@@ -293,8 +350,9 @@ for libraries following the ELPA convention."
 (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 'add-pkg-file-if-missing
add-pkg-file-if-missing)
 ;; The .el files are byte compiled directly in the store.
-(add-after 'patch-el-files 'build build)
+(add-after 'add-pkg-file-if-missing 'build build)
 (add-after 'build 'validate-compiled-autoloads validate-compiled-autoloads)
 (add-after 'validate-compiled-autoloads 'move-doc move-doc)))

-- 
2.31.1