bug#38613: Disabling bytecompilation on a list of files.

bug#38613: Disabling bytecompilation on a list of files.

[Brett Gilio]

Hey all.

Just forwarding along an idea discussed between Leo Prikler, Tobias, and
I on IRC.

Recently I was trying to update our emacs-doom-themes package to a
fresher commit since it offers numerous new functionalities but there is
not a marked stable release of the package. In the process I found an
issue where some files have a bytecode compilation overflow issue during
the build phase. My sloppy work around as shown in
e9d8dee6c3d6e2ddff74841a3ab3a2ba2816bf27 was as such:

--8<---------------cut here---------------start------------->8---
;; XXX: There is a byte-code overflow issue in the latest
;; checkout which affects byte-compilation for several theme
;; files. The easiest way to work around this is to disable
;; byte-compilation until the issue is resolved.
;; <https://github.com/hlissner/emacs-doom-themes/issues/314>
(delete 'build)
--8<---------------cut here---------------end--------------->8---

Obviously just outright deleting the phase responsible for
bytecompilation is not the _best_ solution. So what Leo and I proposed
was adding a #:no-bytecomp which takes a list of REGEXP or files that
will be excluded from the in-place byte-compilation.

I wanted to float this idea by those of us who use the
emacs-build-system regularly.

Thanks!

-- 
Brett M. Gilio
Homepage -- https://scm.pw/
GNU Guix -- https://guix.gnu.org/

bug#38613: Disabling bytecompilation on a list of files.

[Leo Prikler]

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

Hey Brett.

Am Dienstag, den 14.12.2019, 13:45 -0600 schrieb Brett Gilio:
> Just forwarding along an idea discussed between Leo Prikler, Tobias,
> and I on IRC.
Thanks for the mention ;)

> Obviously just outright deleting the phase responsible for
> bytecompilation is not the _best_ solution. So what Leo and I
> proposed
> was adding a #:no-bytecomp which takes a list of REGEXP or files that
> will be excluded from the in-place byte-compilation.
> 
> I wanted to float this idea by those of us who use the
> emacs-build-system regularly.
I actually came up with an alternative solution, that I already hinted
at in IRC.  0001 implements a function to disable byte compilation for
a single file, 0002 applies this to the package in question.  I'm not
quite sure why the files are not writable and wonder, whether the chmod
should be added into 0001, but keeping it out of it should hopefully
prevent abuse.

It's rather late and this is just a proof of concept.  I haven't fully
evaluated the impact this will have on Guix (specifically in the amount
of rebuilds it will cause).  Also beware of my somewhat ill-formed
commit messages.  After painfully checking each of the themes for this
bug however (on my machine, YMMV), I did update the comment for what
it's worth.

Regards,
Leo

[-- Attachment #2: 0001-guix-emacs-utils-Add-emacs-batch-disable-compilation.patch --]
[-- Type: text/x-patch, Size: 1310 bytes --]

From 365f5c02876b51cf566224f60cd6d4c6b7023d66 Mon Sep 17 00:00:00 2001
From: Leo Prikler <leo.prikler@student.tugraz.at>
Date: Sun, 15 Dec 2019 00:45:08 +0100
Subject: [PATCH 1/3] guix: emacs-utils: Add emacs-batch-disable-compilation.

* guix/build/emacs-utils.scm (emacs-batch-disable-compilation):
New procedure.
---
 guix/build/emacs-utils.scm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/guix/build/emacs-utils.scm b/guix/build/emacs-utils.scm
index fdacd30dd6..2aa63c3363 100644
--- a/guix/build/emacs-utils.scm
+++ b/guix/build/emacs-utils.scm
@@ -23,6 +23,7 @@
   #:export (%emacs
             emacs-batch-eval
             emacs-batch-edit-file
+            emacs-batch-disable-compilation
             emacs-generate-autoloads
             emacs-byte-compile-directory
             emacs-substitute-sexps
@@ -50,6 +51,12 @@
           (string-append "--visit=" file)
           (format #f "--eval=~S" expr)))
 
+(define (emacs-batch-disable-compilation file)
+  (emacs-batch-edit-file file
+    '(progn
+      (add-file-local-variable 'no-byte-compile t)
+      (basic-save-buffer))))
+
 (define (emacs-generate-autoloads name directory)
   "Generate autoloads for Emacs package NAME placed in DIRECTORY."
   (let* ((file (string-append directory "/" name "-autoloads.el"))
-- 
2.24.0


[-- Attachment #3: 0002-gnu-emacs-doom-themes-Only-disable-breaking-compilat.patch --]
[-- Type: text/x-patch, Size: 2678 bytes --]

From 3f376828d8970c0751b86aef0b49e256ee09287e Mon Sep 17 00:00:00 2001
From: Leo Prikler <leo.prikler@student.tugraz.at>
Date: Sun, 15 Dec 2019 00:49:26 +0100
Subject: [PATCH 2/3] gnu: emacs-doom-themes: Only disable breaking
 compilations.

* gnu/packages/emacs-xyz.scm (emacs-doom-themes) [phases]:
<build>: Undelete it.
<disable-breaking-compilation>: New phase.
---
 gnu/packages/emacs-xyz.scm | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index 505594aa0d..1c06a9122d 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -19803,6 +19803,10 @@ contrast and few colors.")
       (arguments
        `(#:tests? #t
          #:test-command '("ert-runner")
+         #:modules ((guix build emacs-build-system)
+                    (guix build utils)
+                    (guix build emacs-utils)
+                    (srfi srfi-1))
          #:phases
          (modify-phases %standard-phases
            (add-after 'unpack 'move-themes
@@ -19813,12 +19817,21 @@ contrast and few colors.")
                            (rename-file f (basename f)))
                          (find-files "./themes" ".*\\.el$"))
                #t))
-           ;; XXX: There is a byte-code overflow issue in the latest
-           ;; checkout which affects byte-compilation for several theme
-           ;; files. The easiest way to work around this is to disable
-           ;; byte-compilation until the issue is resolved.
+           ;; There is a byte-code overflow issue in the latest checkout
+           ;; which affects byte-compilation for several (read `most') theme
+           ;; files.  In order to cope with this issue, we disable
+           ;; byte-compilation until it is resolved.
            ;; <https://github.com/hlissner/emacs-doom-themes/issues/314>
-           (delete 'build))))
+           (add-after 'move-themes 'disable-breaking-compilation
+             (lambda _
+               (for-each (lambda (file)
+                           (chmod file #o600)
+                           (emacs-batch-disable-compilation file))
+                         (cons "doom-themes-ext-neotree.el"
+                               (lset-difference string-contains
+                                                (find-files "." ".*-theme.el")
+                                                '("snazzy" "tomorrow-day"))))
+               #t)))))
       (synopsis "Wide collection of color themes for Emacs")
       (description "Emacs-doom-themes contains numerous popular color themes for
 Emacs that integrate with major modes like Org-mode.")
-- 
2.24.0

bug#38613: Disabling bytecompilation on a list of files.

[Brett Gilio]

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> Hey Brett.
>
> Am Dienstag, den 14.12.2019, 13:45 -0600 schrieb Brett Gilio:
>> Just forwarding along an idea discussed between Leo Prikler, Tobias,
>> and I on IRC.
> Thanks for the mention ;)

Any time! :)

>
>> Obviously just outright deleting the phase responsible for
>> bytecompilation is not the _best_ solution. So what Leo and I
>> proposed
>> was adding a #:no-bytecomp which takes a list of REGEXP or files that
>> will be excluded from the in-place byte-compilation.
>> 
>> I wanted to float this idea by those of us who use the
>> emacs-build-system regularly.
> I actually came up with an alternative solution, that I already hinted
> at in IRC.  0001 implements a function to disable byte compilation for
> a single file, 0002 applies this to the package in question.  I'm not
> quite sure why the files are not writable and wonder, whether the chmod
> should be added into 0001, but keeping it out of it should hopefully
> prevent abuse.

I am not sure why the file permissions are needing to be set either. On
a git checkout it looks to me like they are the same as the others. I
wonder if it might have something to do with the rename-file method
moving the themes? Idk.

>
> It's rather late and this is just a proof of concept.  I haven't fully
> evaluated the impact this will have on Guix (specifically in the amount
> of rebuilds it will cause).  Also beware of my somewhat ill-formed
> commit messages.  After painfully checking each of the themes for this
> bug however (on my machine, YMMV), I did update the comment for what
> it's worth.

>
> Regards,
> Leo
>
> From 365f5c02876b51cf566224f60cd6d4c6b7023d66 Mon Sep 17 00:00:00 2001
> From: Leo Prikler <leo.prikler@student.tugraz.at>
> Date: Sun, 15 Dec 2019 00:45:08 +0100
> Subject: [PATCH 1/3] guix: emacs-utils: Add emacs-batch-disable-compilation.
>
> * guix/build/emacs-utils.scm (emacs-batch-disable-compilation):
> New procedure.
> ---
>  guix/build/emacs-utils.scm | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/guix/build/emacs-utils.scm b/guix/build/emacs-utils.scm
> index fdacd30dd6..2aa63c3363 100644
> --- a/guix/build/emacs-utils.scm
> +++ b/guix/build/emacs-utils.scm
> @@ -23,6 +23,7 @@
>    #:export (%emacs
>              emacs-batch-eval
>              emacs-batch-edit-file
> +            emacs-batch-disable-compilation
>              emacs-generate-autoloads
>              emacs-byte-compile-directory
>              emacs-substitute-sexps
> @@ -50,6 +51,12 @@
>            (string-append "--visit=" file)
>            (format #f "--eval=~S" expr)))
>  
> +(define (emacs-batch-disable-compilation file)
> +  (emacs-batch-edit-file file
> +    '(progn
> +      (add-file-local-variable 'no-byte-compile t)
> +      (basic-save-buffer))))
> +
>  (define (emacs-generate-autoloads name directory)
>    "Generate autoloads for Emacs package NAME placed in DIRECTORY."
>    (let* ((file (string-append directory "/" name "-autoloads.el"))
> -- 
> 2.24.0
>
>
> From 3f376828d8970c0751b86aef0b49e256ee09287e Mon Sep 17 00:00:00 2001
> From: Leo Prikler <leo.prikler@student.tugraz.at>
> Date: Sun, 15 Dec 2019 00:49:26 +0100
> Subject: [PATCH 2/3] gnu: emacs-doom-themes: Only disable breaking
>  compilations.
>
> * gnu/packages/emacs-xyz.scm (emacs-doom-themes) [phases]:
> <build>: Undelete it.
> <disable-breaking-compilation>: New phase.
> ---
>  gnu/packages/emacs-xyz.scm | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
> index 505594aa0d..1c06a9122d 100644
> --- a/gnu/packages/emacs-xyz.scm
> +++ b/gnu/packages/emacs-xyz.scm
> @@ -19803,6 +19803,10 @@ contrast and few colors.")
>        (arguments
>         `(#:tests? #t
>           #:test-command '("ert-runner")
> +         #:modules ((guix build emacs-build-system)
> +                    (guix build utils)
> +                    (guix build emacs-utils)
> +                    (srfi srfi-1))
>           #:phases
>           (modify-phases %standard-phases
>             (add-after 'unpack 'move-themes
> @@ -19813,12 +19817,21 @@ contrast and few colors.")
>                             (rename-file f (basename f)))
>                           (find-files "./themes" ".*\\.el$"))
>                 #t))
> -           ;; XXX: There is a byte-code overflow issue in the latest
> -           ;; checkout which affects byte-compilation for several theme
> -           ;; files. The easiest way to work around this is to disable
> -           ;; byte-compilation until the issue is resolved.
> +           ;; There is a byte-code overflow issue in the latest checkout
> +           ;; which affects byte-compilation for several (read `most') theme
> +           ;; files.  In order to cope with this issue, we disable
> +           ;; byte-compilation until it is resolved.
>             ;; <https://github.com/hlissner/emacs-doom-themes/issues/314>
> -           (delete 'build))))
> +           (add-after 'move-themes 'disable-breaking-compilation
> +             (lambda _
> +               (for-each (lambda (file)
> +                           (chmod file #o600)
> +                           (emacs-batch-disable-compilation file))
> +                         (cons "doom-themes-ext-neotree.el"
> +                               (lset-difference string-contains
> +                                                (find-files "." ".*-theme.el")
> +                                                '("snazzy" "tomorrow-day"))))
> +               #t)))))
>        (synopsis "Wide collection of color themes for Emacs")
>        (description "Emacs-doom-themes contains numerous popular color themes for
>  Emacs that integrate with major modes like Org-mode.")

Honestly, it looks fine enough to me. At least for a draft. Does anybody
have any objections or feel an urge for a need for addition of something
else? I think this feature is going to be useful, especially for
allowing workarounds in cases like this which will certainly pop up
again in the future.

-- 
Brett M. Gilio
Homepage -- https://scm.pw/
GNU Guix -- https://guix.gnu.org/

bug#38613: Disabling bytecompilation on a list of files.

[Leo Prikler]

Am Sonntag, den 15.12.2019, 16:26 -0600 schrieb Brett Gilio:
> [...]
> I am not sure why the file permissions are needing to be set either.
> On
> a git checkout it looks to me like they are the same as the others. I
> wonder if it might have something to do with the rename-file method
> moving the themes? Idk.
Interesting.  I wonder what would happen if we patched the files before
moving them elsewhere, given that files often are not moved in such a
manner.

> Honestly, it looks fine enough to me. At least for a draft. Does
> anybody
> have any objections or feel an urge for a need for addition of
> something
> else? I think this feature is going to be useful, especially for
> allowing workarounds in cases like this which will certainly pop up
> again in the future.
Regarding the impact, I think it should be fine if we commit this
closely before or after #38619, since that probably forces all Elisp
libraries to be rebuilt anyways (also CC'd Maxim to have a look at
this).  Hopefully at least the doom-theme autoloads compile correctly. 
WDYT?

Leo

bug#38613: Disabling bytecompilation on a list of files.

[Brett Gilio]

Leo Prikler <leo.prikler@student.tugraz.at> writes:

> Regarding the impact, I think it should be fine if we commit this
> closely before or after #38619, since that probably forces all Elisp
> libraries to be rebuilt anyways (also CC'd Maxim to have a look at
> this).  Hopefully at least the doom-theme autoloads compile correctly. 
> WDYT?
>
> Leo
>

I am in agreement. Maxim, the ball is in your court. :)

-- 
Brett M. Gilio
Homepage -- https://scm.pw/
GNU Guix -- https://guix.gnu.org/

bug#38613: Disabling bytecompilation on a list of files.

[Maxim Cournoyer]

Hello,

Leo Prikler <leo.prikler@student.tugraz.at>

[...]

> Regarding the impact, I think it should be fine if we commit this
> closely before or after #38619, since that probably forces all Elisp
> libraries to be rebuilt anyways (also CC'd Maxim to have a look at
> this).  Hopefully at least the doom-theme autoloads compile correctly.
> WDYT?
>
> Leo

IIRC, emacs{-minimal} have about ~300 non-Elisp dependents, which IMO is
OK for master (Elisp packages are very cheap to build, so I don't count
them).

I've just verified this with the following script:

--8<---------------cut here---------------start------------->8---
(use-modules (gnu packages)
	     (guix graph)
	     (guix monads)
	     (guix packages)
	     (guix store)
	     (guix scripts graph)
	     (guix scripts refresh)
	     (gnu packages emacs)
	     (srfi srfi-1)
	     (srfi srfi-26))

(define (all-packages)
  "Return the list of all the distro's packages."
  (fold-packages (lambda (package result)
                   ;; Ignore deprecated packages.
                   (if (package-superseded package)
                       result
                       (cons package result)))
                 '()
                 #:select? (const #t)))

(define (packages->dependents packages)

  (define (full-name package)
    (string-append (package-name package) "@"
		   (package-version package)))

  (with-store store
    (run-with-store store
	(mlet %store-monad ((edges (node-back-edges
				    %bag-node-type
				    (package-closure (all-packages)))))
	  (let ((dependents (node-transitive-edges packages edges)))
	    (return (map full-name dependents)))))))

(define (compute-results)
  (let* ((dependents (packages->dependents (list emacs emacs-minimal)))
	 (non-emacs (remove (cut string-prefix? "emacs-" <>) dependents)))
    (format #t "Emacs{-minimal} have ~d dependents, ~d of which are not pure \
Emacs libraries: ~a~%" (length dependents) (length non-emacs) non-emacs)))

(compute-results)
--8<---------------cut here---------------end--------------->8---

Which outputs:

Emacs{-minimal} have 973 dependents, 274 of which are not pure Emacs
libraries: (faust@2.5.23 cedille@1.1.1 cflow@1.6
translate-shell@0.9.6.11 kicad@5.1.4 lepton-eda@1.9.5-20180820
quadrapassel@3.31.3 eog-plugins@3.26.4 workrave@1.10.34 emacsy@0.4.1
guile-gi@0.2.0 nomad@0.1.1-alpha emacsy-minimal@v0.4.1-19.f3bf0db
evisum@0.2.6 enlightenment-wayland@0.23.1 terminology@1.5.0 ephoto@1.5
enlightenment@0.23.1 econnman@1.1 python-efl@1.23.0 lekha@0.2.1
python2-efl@1.23.0 edi@0.6.0 rage@0.3.1 dbus-c++@0.9.0
[...]

HTH,

Maxim

bug#38613: Disabling bytecompilation on a list of files.

[Brett Gilio]

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Hello,
>
> Leo Prikler <leo.prikler@student.tugraz.at>
>
> [...]
>
>> Regarding the impact, I think it should be fine if we commit this
>> closely before or after #38619, since that probably forces all Elisp
>> libraries to be rebuilt anyways (also CC'd Maxim to have a look at
>> this).  Hopefully at least the doom-theme autoloads compile correctly.
>> WDYT?
>>
>> Leo
>
> IIRC, emacs{-minimal} have about ~300 non-Elisp dependents, which IMO is
> OK for master (Elisp packages are very cheap to build, so I don't count
> them).
>
> I've just verified this with the following script:
>
> (use-modules (gnu packages)
> 	     (guix graph)
> 	     (guix monads)
> 	     (guix packages)
> 	     (guix store)
> 	     (guix scripts graph)
> 	     (guix scripts refresh)
> 	     (gnu packages emacs)
> 	     (srfi srfi-1)
> 	     (srfi srfi-26))
>
> (define (all-packages)
>   "Return the list of all the distro's packages."
>   (fold-packages (lambda (package result)
>                    ;; Ignore deprecated packages.
>                    (if (package-superseded package)
>                        result
>                        (cons package result)))
>                  '()
>                  #:select? (const #t)))
>
> (define (packages->dependents packages)
>
>   (define (full-name package)
>     (string-append (package-name package) "@"
> 		   (package-version package)))
>
>   (with-store store
>     (run-with-store store
> 	(mlet %store-monad ((edges (node-back-edges
> 				    %bag-node-type
> 				    (package-closure (all-packages)))))
> 	  (let ((dependents (node-transitive-edges packages edges)))
> 	    (return (map full-name dependents)))))))
>
> (define (compute-results)
>   (let* ((dependents (packages->dependents (list emacs emacs-minimal)))
> 	 (non-emacs (remove (cut string-prefix? "emacs-" <>) dependents)))
>     (format #t "Emacs{-minimal} have ~d dependents, ~d of which are not pure \
> Emacs libraries: ~a~%" (length dependents) (length non-emacs) non-emacs)))
>
> (compute-results)
>
> Which outputs:
>
> Emacs{-minimal} have 973 dependents, 274 of which are not pure Emacs
> libraries: (faust@2.5.23 cedille@1.1.1 cflow@1.6
> translate-shell@0.9.6.11 kicad@5.1.4 lepton-eda@1.9.5-20180820
> quadrapassel@3.31.3 eog-plugins@3.26.4 workrave@1.10.34 emacsy@0.4.1
> guile-gi@0.2.0 nomad@0.1.1-alpha emacsy-minimal@v0.4.1-19.f3bf0db
> evisum@0.2.6 enlightenment-wayland@0.23.1 terminology@1.5.0 ephoto@1.5
> enlightenment@0.23.1 econnman@1.1 python-efl@1.23.0 lekha@0.2.1
> python2-efl@1.23.0 edi@0.6.0 rage@0.3.1 dbus-c++@0.9.0
> [...]
>
> HTH,
>
> Maxim
>
>
>
>

Leo and Maxim,

Do you think this feature addition to the emacs-build-system is good to
be pushed, then? I think it looks quite fine but I want to double check
before potentially doing something dumb.

That said, we should probably also document this.

-- 
Brett M. Gilio
GNU Guix, Contributor | GNU Project, Webmaster
[DFC0 C7F7 9EE6 0CA7 AE55 5E19 6722 43C4 A03F 0EEE]
<brettg@gnu.org> <brettg@posteo.net>

bug#38613: Disabling bytecompilation on a list of files.

[Maxim Cournoyer]

Brett Gilio <brettg@gnu.org> writes:


[...]

> Leo and Maxim,
>
> Do you think this feature addition to the emacs-build-system is good to
> be pushed, then? I think it looks quite fine but I want to double check
> before potentially doing something dumb.

LGTM!

Thank you!

Maxim

bug#38613: Disabling bytecompilation on a list of files.

[Brett Gilio]

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

> Brett Gilio <brettg@gnu.org> writes:
>
>
> [...]
>
>> Leo and Maxim,
>>
>> Do you think this feature addition to the emacs-build-system is good to
>> be pushed, then? I think it looks quite fine but I want to double check
>> before potentially doing something dumb.
>
> LGTM!
>
> Thank you!
>
> Maxim
>

Done! Thank you Leo and Maxim.

-- 
Brett M. Gilio
GNU Guix, Contributor | GNU Project, Webmaster
[DFC0 C7F7 9EE6 0CA7 AE55 5E19 6722 43C4 A03F 0EEE]
<brettg@gnu.org> <brettg@posteo.net>