[PATCH emacs-team] gnu: emacs-flim-lb: Explicitly compile files to bytecode.

  • Done
  • quality assurance status badge
Details
2 participants
  • Hilton Chain
  • Liliana Marie Prikler
Owner
unassigned
Submitted by
Hilton Chain
Severity
normal
H
H
Hilton Chain wrote on 23 Aug 2023 20:00
(address . guix-patches@gnu.org)(name . Hilton Chain)(address . hako@ultrarare.space)
5629f5f427c30caa6d43f37ac28c3dd3f1501d01.1692813553.git.hako@ultrarare.space
* gnu/packages/emacs-xyz.scm (emacs-flim-lb)[#:phases]: Replace 'build phase
to byte compile files explicitly.
---
gnu/packages/emacs-xyz.scm | 21 ++++++++++++++++++---
1 file changed, 18 insertions(+), 3 deletions(-)

Toggle diff (43 lines)
diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index 412ace373e..ecdfa235c7 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -37655,6 +37655,24 @@ (define-public emacs-flim-lb
(base32
"1wsnipyl3blldcl8ynmpj1mxfvl7kjmxd8gapl83vqd3r0l9cr6q"))))
(build-system emacs-build-system)
+ (arguments
+ (list
+ #:phases
+ #~(modify-phases %standard-phases
+ ;; FIXME: Following error occurs when native compiled:
+ ;; condition-case: Eager macro-expansion failure: (void-variable
+ ;; eof-block-branches).
+ (replace 'build
+ ;; Copied from emacs-build-system's build phase, with
+ ;; `emacs-compile-directory' replaced to
+ ;; `emacs-byte-compile-directory'.
+ (lambda* (#:key outputs inputs #:allow-other-keys)
+ (for-each delete-file (find-files "." "\\.el[cn]$"))
+ (let* ((emacs (search-input-file inputs "/bin/emacs"))
+ (out (assoc-ref outputs "out")))
+ (setenv "SHELL" "sh")
+ (parameterize ((%emacs emacs))
+ (emacs-byte-compile-directory (elpa-directory out)))))))))
(propagated-inputs (list emacs-apel-lb emacs-oauth2))
(home-page "https://www.emacswiki.org/emacs/WanderLust")
(synopsis
@@ -37694,9 +37712,6 @@ (define-public emacs-semi-epg
(define-public emacs-wanderlust
;; No release since Jan 15, 2010.
- ;; FIXME: Building with emacs-next-pgtk would yield a void variable related
- ;; macro-expansion failure at runtime, so don't rewrite emacs input of this
- ;; package.
(let ((version "2.15.9")
(revision "791")
(commit "8369b2d5170a174652294835dd9a18ed21a38cb2"))

base-commit: d58c6e25ff3d1e70fd9b0e07bdad1b335de8a979
--
2.41.0
L
L
Liliana Marie Prikler wrote on 27 Aug 2023 08:24
(name . Andrew Tropin)(address . andrew@trop.in)
d651371fe126c9b35eef22343a427b287e39c308.camel@gmail.com
Am Donnerstag, dem 24.08.2023 um 02:00 +0800 schrieb Hilton Chain:
Toggle quote (3 lines)
> * gnu/packages/emacs-xyz.scm (emacs-flim-lb)[#:phases]: Replace
> 'build phase to byte compile files explicitly.
> ---
Why do we need this? If it's to disable native compilation, please add
a file-local variable to do so.

Cheers
H
H
Hilton Chain wrote on 27 Aug 2023 14:52
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
8735047il4.wl-hako@ultrarare.space
On Sun, 27 Aug 2023 14:24:01 +0800,
Liliana Marie Prikler wrote:
Toggle quote (8 lines)
>
> Am Donnerstag, dem 24.08.2023 um 02:00 +0800 schrieb Hilton Chain:
> > * gnu/packages/emacs-xyz.scm (emacs-flim-lb)[#:phases]: Replace
> > 'build phase to byte compile files explicitly.
> > ---
> Why do we need this? If it's to disable native compilation, please add
> a file-local variable to do so.

I have tested the package on Emacs 28 and 29 with combinations of elc
or eln files in the output, and byte-compiled elc files seem to be a
requirement to use it -- otherwise the error would occur. Disabling
native compilation won't help...
L
L
Liliana Marie Prikler wrote on 27 Aug 2023 17:13
(name . Hilton Chain)(address . hako@ultrarare.space)
00b74be565b90c6f35cc14e607141da3e3540fbe.camel@gmail.com
Am Sonntag, dem 27.08.2023 um 20:52 +0800 schrieb Hilton Chain:
Toggle quote (14 lines)
> On Sun, 27 Aug 2023 14:24:01 +0800,
> Liliana Marie Prikler wrote:
> >
> > Am Donnerstag, dem 24.08.2023 um 02:00 +0800 schrieb Hilton Chain:
> > > * gnu/packages/emacs-xyz.scm (emacs-flim-lb)[#:phases]: Replace
> > > 'build phase to byte compile files explicitly.
> > > ---
> > Why do we need this?  If it's to disable native compilation, please
> > add a file-local variable to do so.
>
> I have tested the package on Emacs 28 and 29 with combinations of elc
> or eln files in the output, and byte-compiled elc files seem to be a
> requirement to use it -- otherwise the error would occur.  Disabling
> native compilation won't help...
How is your phase functionally different from disabling native
compilation? IIUC, it deletes all pre-existing bytecode and native
code and then runs bytecode compilation. Where do these elc and eln
files even come from?
H
H
Hilton Chain wrote on 27 Aug 2023 17:39
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
871qfo7avb.wl-hako@ultrarare.space
On Sun, 27 Aug 2023 23:13:44 +0800,
Liliana Marie Prikler wrote:
Toggle quote (21 lines)
>
> Am Sonntag, dem 27.08.2023 um 20:52 +0800 schrieb Hilton Chain:
> > On Sun, 27 Aug 2023 14:24:01 +0800,
> > Liliana Marie Prikler wrote:
> > >
> > > Am Donnerstag, dem 24.08.2023 um 02:00 +0800 schrieb Hilton Chain:
> > > > * gnu/packages/emacs-xyz.scm (emacs-flim-lb)[#:phases]: Replace
> > > > 'build phase to byte compile files explicitly.
> > > > ---
> > > Why do we need this?  If it's to disable native compilation, please
> > > add a file-local variable to do so.
> >
> > I have tested the package on Emacs 28 and 29 with combinations of elc
> > or eln files in the output, and byte-compiled elc files seem to be a
> > requirement to use it -- otherwise the error would occur.  Disabling
> > native compilation won't help...
> How is your phase functionally different from disabling native
> compilation? IIUC, it deletes all pre-existing bytecode and native
> code and then runs bytecode compilation. Where do these elc and eln
> files even come from?

Currently on master:

$ guix build emacs-flim-lb
has elc files.

$ guix build emacs-flim-lb --with-input=emacs-minimal=emacs
has elc and eln files.

$ guix build emacs-flim-lb --with-input=emacs-minimal=emacs-next
has eln files.

And I tried to build it with 'build phase deleted as well. Had them
all tested and came to the conclusion that elc files are required.
L
L
Liliana Marie Prikler wrote on 27 Aug 2023 17:49
(name . Hilton Chain)(address . hako@ultrarare.space)
2ea4d41e810638483c6c8e9f2dbfd901ecae8c98.camel@gmail.com
Am Sonntag, dem 27.08.2023 um 23:39 +0800 schrieb Hilton Chain:
Toggle quote (13 lines)
> Currently on master:
>
> $ guix build emacs-flim-lb
> has elc files.
>
> $ guix build emacs-flim-lb --with-input=emacs-minimal=emacs
> has elc and eln files.
>
> $ guix build emacs-flim-lb --with-input=emacs-minimal=emacs-next
> has eln files.
>
> And I tried to build it with 'build phase deleted as well.  Had them
> all tested and came to the conclusion that elc files are required.
Hmm, this sounds like a failure of emacs-build-system then. The
assumption (based on Emacs 28) had been that both bytecode and native
compilation are done with one function call. Maybe that no longer
holds?
H
H
Hilton Chain wrote on 28 Aug 2023 06:21
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
87y1hv6blh.wl-hako@ultrarare.space
On Sun, 27 Aug 2023 23:49:49 +0800,
Liliana Marie Prikler wrote:
Toggle quote (20 lines)
>
> Am Sonntag, dem 27.08.2023 um 23:39 +0800 schrieb Hilton Chain:
> > Currently on master:
> >
> > $ guix build emacs-flim-lb
> > has elc files.
> >
> > $ guix build emacs-flim-lb --with-input=emacs-minimal=emacs
> > has elc and eln files.
> >
> > $ guix build emacs-flim-lb --with-input=emacs-minimal=emacs-next
> > has eln files.
> >
> > And I tried to build it with 'build phase deleted as well.  Had them
> > all tested and came to the conclusion that elc files are required.
> Hmm, this sounds like a failure of emacs-build-system then. The
> assumption (based on Emacs 28) had been that both bytecode and native
> compilation are done with one function call. Maybe that no longer
> holds?

Found a change in `batch-byte+native-compile' with
$ git diff emacs-28.2..emacs-29.1 lisp/emacs-lisp/comp.el

Toggle snippet (45 lines)
+(defun comp-write-bytecode-file (eln-file)
+ "After native compilation write the bytecode file for ELN-FILE.
+Make sure that eln file is younger than byte-compiled one and
+return the filename of this last.
+
+This function can be used only in conjuntion with
+`byte+native-compile' `byte-to-native-output-buffer-file' (see
+`batch-byte+native-compile')."
+ (pcase byte-to-native-output-buffer-file
+ (`(,temp-buffer . ,target-file)
+ (unwind-protect
+ (progn
+ (byte-write-target-file temp-buffer target-file)
+ ;; Touch the .eln in order to have it older than the
+ ;; corresponding .elc.
+ (when (stringp eln-file)
+ (set-file-times eln-file)))
+ (kill-buffer temp-buffer))
+ target-file)))

;;;###autoload
(defun batch-byte+native-compile ()
@@ -4221,17 +4345,16 @@ Generate .elc files in addition to the .eln files.
Force the produced .eln to be outputted in the eln system
directory (the last entry in `native-comp-eln-load-path') unless
`native-compile-target-directory' is non-nil. If the environment
-variable 'NATIVE_DISABLED' is set, only byte compile."
+variable \"NATIVE_DISABLED\" is set, only byte compile."
(comp-ensure-native-compiler)
(if (equal (getenv "NATIVE_DISABLED") "1")
(batch-byte-compile)
(cl-assert (length= command-line-args-left 1))
- (let ((byte+native-compile t)
- (byte-to-native-output-file nil))
- (batch-native-compile)
- (pcase byte-to-native-output-file
- (`(,tempfile . ,target-file)
- (rename-file tempfile target-file t))))))
+ (let* ((byte+native-compile t)
+ (byte-to-native-output-buffer-file nil)
+ (eln-file (car (batch-native-compile))))
+ (comp-write-bytecode-file eln-file)
+ (setq command-line-args-left (cdr command-line-args-left)))))

And the following patch should work for Emacs 29, but not 28:

Toggle snippet (32 lines)
diff --git a/guix/build/emacs-utils.scm b/guix/build/emacs-utils.scm
index 850b1f5f2a..6c27b186e3 100644
--- a/guix/build/emacs-utils.scm
+++ b/guix/build/emacs-utils.scm
@@ -138,7 +138,7 @@ (define* (emacs-compile-directory dir)
(files (directory-files-recursively ,dir "\\.el$")))
(mapc
(lambda (file)
- (let (byte-to-native-output-file
+ (let (byte-to-native-output-buffer-file
;; First entry is the eln-cache of the homeless shelter,
;; second entry is the install directory.
(eln-dir (and (native-comp-available-p)
@@ -148,12 +148,12 @@ (define* (emacs-compile-directory dir)
(comp-el-to-eln-filename file eln-dir))
(byte-compile-file file))
;; Sadly, we can't use pcase because quasiquote works different in
- ;; Emacs. See `batch-byte+native-compile' in comp.el for the
- ;; actual shape of byte-to-native-output-file.
- (unless (null byte-to-native-output-file)
- (rename-file (car byte-to-native-output-file)
- (cdr byte-to-native-output-file)
- t))))
+ ;; Emacs. See `comp-write-bytecode-file' in comp.el for the actual
+ ;; shape of byte-to-native-output-buffer-file.
+ (unless (null byte-to-native-output-buffer-file)
+ (byte-write-target-file
+ (car byte-to-native-output-buffer-file)
+ (cdr byte-to-native-output-buffer-file)))))
files))
#:dynamic? #t))
L
L
Liliana Marie Prikler wrote on 28 Aug 2023 20:28
(name . Hilton Chain)(address . hako@ultrarare.space)
7c8bcdbcb731ffc12b1769bb3fb2eca0900af32e.camel@gmail.com
Am Montag, dem 28.08.2023 um 12:21 +0800 schrieb Hilton Chain:
Toggle quote (33 lines)
> And the following patch should work for Emacs 29, but not 28:
>
> --8<---------------cut here---------------start------------->8---
> diff --git a/guix/build/emacs-utils.scm b/guix/build/emacs-utils.scm
> index 850b1f5f2a..6c27b186e3 100644
> --- a/guix/build/emacs-utils.scm
> +++ b/guix/build/emacs-utils.scm
> @@ -138,7 +138,7 @@ (define* (emacs-compile-directory dir)
>             (files (directory-files-recursively ,dir "\\.el$")))
>         (mapc
>          (lambda (file)
> -          (let (byte-to-native-output-file
> +          (let (byte-to-native-output-buffer-file
>                  ;; First entry is the eln-cache of the homeless
> shelter,
>                  ;; second entry is the install directory.
>                  (eln-dir (and (native-comp-available-p)
> @@ -148,12 +148,12 @@ (define* (emacs-compile-directory dir)
>                                  (comp-el-to-eln-filename file eln-
> dir))
>                  (byte-compile-file file))
>              ;; Sadly, we can't use pcase because quasiquote works
> different in
> -            ;; Emacs.  See `batch-byte+native-compile' in comp.el
> for the
> -            ;; actual shape of byte-to-native-output-file.
> -            (unless (null byte-to-native-output-file)
> -              (rename-file (car byte-to-native-output-file)
> -                           (cdr byte-to-native-output-file)
> -                           t))))
> +            ;; Emacs.  See `comp-write-bytecode-file' in comp.el for
> the actual
> +            ;; shape of byte-to-native-output-buffer-file.
We should probably mention that we don't have to hack the timestamp.
Or maybe we can use comp-write-bytecode-file directly?
Toggle quote (7 lines)
> +            (unless (null byte-to-native-output-buffer-file)
> +              (byte-write-target-file
> +               (car byte-to-native-output-buffer-file)
> +               (cdr byte-to-native-output-buffer-file)))))
>         files))
>      #:dynamic? #t))
> --8<---------------cut here---------------end--------------->8---
Could you submit a properly cleaned version of this as v2 so that we
can get CI to build it along with proper attribution?

Cheers
H
H
Hilton Chain wrote on 29 Aug 2023 03:33
[PATCH emacs-team v2] build: emacs-utils: Adjust `emacs-compile-directory' for Emacs 29.
(address . 65478@debbugs.gnu.org)(name . Hilton Chain)(address . hako@ultrarare.space)
ba16b540f92b7ae6ad50bc98cf5deebb43ee85bc.1693272362.git.hako@ultrarare.space
* guix/build/emacs-utils.scm (emacs-compile-directory): After native
compilation, write the bytecode file with `comp-write-bytecode-file' when
using Emacs 29.
---
guix/build/emacs-utils.scm | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

Toggle diff (36 lines)
diff --git a/guix/build/emacs-utils.scm b/guix/build/emacs-utils.scm
index ac3dac57d1..f4c18af388 100644
--- a/guix/build/emacs-utils.scm
+++ b/guix/build/emacs-utils.scm
@@ -140,6 +140,7 @@ (define* (emacs-compile-directory dir)
(mapc
(lambda (file)
(let (byte-to-native-output-file
+ byte-to-native-output-buffer-file
;; First entry is the eln-cache of the homeless shelter,
;; second entry is the install directory.
(eln-dir (and (native-comp-available-p)
@@ -148,13 +149,18 @@ (define* (emacs-compile-directory dir)
(native-compile file
(comp-el-to-eln-filename file eln-dir))
(byte-compile-file file))
+ ;; After native compilation, write the bytecode file.
+ ;; (For Emacs 28)
;; Sadly, we can't use pcase because quasiquote works different in
;; Emacs. See `batch-byte+native-compile' in comp.el for the
;; actual shape of byte-to-native-output-file.
(unless (null byte-to-native-output-file)
(rename-file (car byte-to-native-output-file)
(cdr byte-to-native-output-file)
- t))))
+ t))
+ ;; (For Emacs 29)
+ (unless (null byte-to-native-output-buffer-file)
+ (comp-write-bytecode-file nil))))
files))
#:dynamic? #t))

base-commit: 9d074e16c7a9879d67c348c7b2d70b725adfbdfa
--
2.41.0
L
L
Liliana Marie Prikler wrote on 30 Aug 2023 21:40
(name . Andrew Tropin)(address . andrew@trop.in)
8dac26eac7975e14ed61a3fbfb24e94782bc17f3.camel@gmail.com
Am Dienstag, dem 29.08.2023 um 09:33 +0800 schrieb Hilton Chain:
Toggle quote (4 lines)
> * guix/build/emacs-utils.scm (emacs-compile-directory): After native
> compilation, write the bytecode file with `comp-write-bytecode-file'
> when using Emacs 29.
> ---
Is this meant for the master branch or for emacs-team? I don't think
we need to keep backwards compatibility on emacs-team.


Cheers
H
H
Hilton Chain wrote on 1 Sep 2023 18:14
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)
8734zxlvkx.wl-hako@ultrarare.space
On Thu, 31 Aug 2023 03:40:01 +0800,
Liliana Marie Prikler wrote:
Toggle quote (9 lines)
>
> Am Dienstag, dem 29.08.2023 um 09:33 +0800 schrieb Hilton Chain:
> > * guix/build/emacs-utils.scm (emacs-compile-directory): After native
> > compilation, write the bytecode file with `comp-write-bytecode-file'
> > when using Emacs 29.
> > ---
> Is this meant for the master branch or for emacs-team? I don't think
> we need to keep backwards compatibility on emacs-team.

It's for emacs-team, I kept the part for Emacs 28 since there might
still be use cases for it and changing the Emacs package to use is
easier than adjusting the build system. But currently there's no
direct configuration to replace Emacs so it's not a necessity, and I
don't have a preference for it. I'll send v3 without that part.
H
H
Hilton Chain wrote on 1 Sep 2023 18:15
[PATCH emacs-team v3] build: emacs-utils: Adjust `emacs-compile-directory' for Emacs 29.
(address . 65478@debbugs.gnu.org)(name . Hilton Chain)(address . hako@ultrarare.space)
40fb61f0682e94c0fe402efad0193463e1f8b6bf.1693583097.git.hako@ultrarare.space
* guix/build/emacs-utils.scm (emacs-compile-directory): After native
compilation, write the bytecode file with `comp-write-bytecode-file'.
---
guix/build/emacs-utils.scm | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)

Toggle diff (34 lines)
diff --git a/guix/build/emacs-utils.scm b/guix/build/emacs-utils.scm
index ac3dac57d1..8e12b5b6d4 100644
--- a/guix/build/emacs-utils.scm
+++ b/guix/build/emacs-utils.scm
@@ -139,7 +139,7 @@ (define* (emacs-compile-directory dir)
(files (directory-files-recursively ,dir "\\.el$")))
(mapc
(lambda (file)
- (let (byte-to-native-output-file
+ (let (byte-to-native-output-buffer-file
;; First entry is the eln-cache of the homeless shelter,
;; second entry is the install directory.
(eln-dir (and (native-comp-available-p)
@@ -148,13 +148,9 @@ (define* (emacs-compile-directory dir)
(native-compile file
(comp-el-to-eln-filename file eln-dir))
(byte-compile-file file))
- ;; Sadly, we can't use pcase because quasiquote works different in
- ;; Emacs. See `batch-byte+native-compile' in comp.el for the
- ;; actual shape of byte-to-native-output-file.
- (unless (null byte-to-native-output-file)
- (rename-file (car byte-to-native-output-file)
- (cdr byte-to-native-output-file)
- t))))
+ ;; After native compilation, write the bytecode file.
+ (unless (null byte-to-native-output-buffer-file)
+ (comp-write-bytecode-file nil))))
files))
#:dynamic? #t))

base-commit: 9d074e16c7a9879d67c348c7b2d70b725adfbdfa
--
2.41.0
L
L
Liliana Marie Prikler wrote on 1 Sep 2023 21:37
(name . Andrew Tropin)(address . andrew@trop.in)
19403aa2ad67bd0cd4d35cf0b3332d1c18b1a197.camel@gmail.com
Am Samstag, dem 02.09.2023 um 00:15 +0800 schrieb Hilton Chain:
Toggle quote (39 lines)
> * guix/build/emacs-utils.scm (emacs-compile-directory): After native
> compilation, write the bytecode file with `comp-write-bytecode-file'.
> ---
>  guix/build/emacs-utils.scm | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/guix/build/emacs-utils.scm b/guix/build/emacs-utils.scm
> index ac3dac57d1..8e12b5b6d4 100644
> --- a/guix/build/emacs-utils.scm
> +++ b/guix/build/emacs-utils.scm
> @@ -139,7 +139,7 @@ (define* (emacs-compile-directory dir)
>             (files (directory-files-recursively ,dir "\\.el$")))
>         (mapc
>          (lambda (file)
> -          (let (byte-to-native-output-file
> +          (let (byte-to-native-output-buffer-file
>                  ;; First entry is the eln-cache of the homeless
> shelter,
>                  ;; second entry is the install directory.
>                  (eln-dir (and (native-comp-available-p)
> @@ -148,13 +148,9 @@ (define* (emacs-compile-directory dir)
>                  (native-compile file
>                                  (comp-el-to-eln-filename file eln-
> dir))
>                  (byte-compile-file file))
> -            ;; Sadly, we can't use pcase because quasiquote works
> different in
> -            ;; Emacs.  See `batch-byte+native-compile' in comp.el
> for the
> -            ;; actual shape of byte-to-native-output-file.
> -            (unless (null byte-to-native-output-file)
> -              (rename-file (car byte-to-native-output-file)
> -                           (cdr byte-to-native-output-file)
> -                           t))))
> +            ;; After native compilation, write the bytecode file.
> +            (unless (null byte-to-native-output-buffer-file)
> +              (comp-write-bytecode-file nil))))
>         files))
>      #:dynamic? #t))
LGTM, at least regarding emacs-flim-lb. I have it applied locally with
the fancy quotes made extra fancy; will wait for CI before pushing,
however.

Cheers
L
L
Liliana Marie Prikler wrote on 9 Sep 2023 10:23
(name . Andrew Tropin)(address . andrew@trop.in)
df4f934b7db60fd810b64fc6216c7fdd27b2edb8.camel@gmail.com
Am Freitag, dem 01.09.2023 um 21:37 +0200 schrieb Liliana Marie
Prikler:
Toggle quote (1 lines)
> will wait for CI before pushing, however.
Looks like CI doesn't know whether it ought to build stuff; pushed as
it LGTM.

Cheers
Closed
?