Disabling bytecompilation on a list of files.

DoneSubmitted by Brett Gilio.
Details
4 participants
  • Brett Gilio
  • Brett Gilio
  • Leo Prikler
  • Maxim Cournoyer
Owner
unassigned
Severity
normal
B
B
Brett Gilio wrote on 14 Dec 2019 20:45
(address . bug-guix@gnu.org)
87tv62itdl.fsf@posteo.net
Hey all.
Just forwarding along an idea discussed between Leo Prikler, Tobias, andI on IRC.
Recently I was trying to update our emacs-doom-themes package to afresher commit since it offers numerous new functionalities but there isnot a marked stable release of the package. In the process I found anissue where some files have a bytecode compilation overflow issue duringthe build phase. My sloppy work around as shown ine9d8dee6c3d6e2ddff74841a3ab3a2ba2816bf27 was as such:
Toggle snippet (8 lines);; 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)
Obviously just outright deleting the phase responsible forbytecompilation is not the _best_ solution. So what Leo and I proposedwas adding a #:no-bytecomp which takes a list of REGEXP or files thatwill be excluded from the in-place byte-compilation.
I wanted to float this idea by those of us who use theemacs-build-system regularly.
Thanks!
-- Brett M. GilioHomepage -- https://scm.pw/GNU Guix -- https://guix.gnu.org/
L
L
Leo Prikler wrote on 15 Dec 2019 01:35
(address . brettg@posteo.net)(address . 38613@debbugs.gnu.org)
c848351300cc7c7f54e1714fdf31a6271ad3425c.camel@student.tugraz.at
Hey Brett.
Am Dienstag, den 14.12.2019, 13:45 -0600 schrieb Brett Gilio:
Toggle quote (2 lines)> Just forwarding along an idea discussed between Leo Prikler, Tobias,> and I on IRC.
Thanks for the mention ;)
Toggle quote (8 lines)> 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 hintedat in IRC. 0001 implements a function to disable byte compilation fora single file, 0002 applies this to the package in question. I'm notquite sure why the files are not writable and wonder, whether the chmodshould be added into 0001, but keeping it out of it should hopefullyprevent abuse.
It's rather late and this is just a proof of concept. I haven't fullyevaluated the impact this will have on Guix (specifically in the amountof rebuilds it will cause). Also beware of my somewhat ill-formedcommit messages. After painfully checking each of the themes for thisbug however (on my machine, YMMV), I did update the comment for whatit's worth.
Regards,Leo
From 365f5c02876b51cf566224f60cd6d4c6b7023d66 Mon Sep 17 00:00:00 2001From: Leo Prikler <leo.prikler@student.tugraz.at>Date: Sun, 15 Dec 2019 00:45:08 +0100Subject: [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(+)
Toggle diff (27 lines)diff --git a/guix/build/emacs-utils.scm b/guix/build/emacs-utils.scmindex 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 2001From: Leo Prikler <leo.prikler@student.tugraz.at>Date: Sun, 15 Dec 2019 00:49:26 +0100Subject: [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(-)
Toggle diff (44 lines)diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scmindex 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
B
B
Brett Gilio wrote on 15 Dec 2019 23:26
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)(address . 38613@debbugs.gnu.org)
8736dl5ipq.fsf@posteo.net
Leo Prikler <leo.prikler@student.tugraz.at> writes:
Toggle quote (7 lines)> 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! :)
Toggle quote (16 lines)>>> 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. Ona git checkout it looks to me like they are the same as the others. Iwonder if it might have something to do with the rename-file methodmoving the themes? Idk.
Toggle quote (108 lines)>> 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 anybodyhave any objections or feel an urge for a need for addition of somethingelse? I think this feature is going to be useful, especially forallowing workarounds in cases like this which will certainly pop upagain in the future.
-- Brett M. GilioHomepage -- https://scm.pw/GNU Guix -- https://guix.gnu.org/
L
L
Leo Prikler wrote on 16 Dec 2019 00:01
(name . Brett Gilio)(address . brettg@posteo.net)
0b47c65b310b0a0c9e5821070d5e4a888a8c4a58.camel@student.tugraz.at
Am Sonntag, den 15.12.2019, 16:26 -0600 schrieb Brett Gilio:
Toggle quote (6 lines)> [...]> 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 beforemoving them elsewhere, given that files often are not moved in such amanner.
Toggle quote (7 lines)> 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 thisclosely before or after #38619, since that probably forces all Elisplibraries to be rebuilt anyways (also CC'd Maxim to have a look atthis). Hopefully at least the doom-theme autoloads compile correctly. WDYT?
Leo
B
B
Brett Gilio wrote on 16 Dec 2019 00:18
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)
87pngp41q4.fsf@posteo.net
Leo Prikler <leo.prikler@student.tugraz.at> writes:
Toggle quote (9 lines)> 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. GilioHomepage -- https://scm.pw/GNU Guix -- https://guix.gnu.org/
M
M
Maxim Cournoyer wrote on 17 Dec 2019 06:03
(name . Leo Prikler)(address . leo.prikler@student.tugraz.at)
875zifmtlq.fsf@gmail.com
Hello,
Leo Prikler <leo.prikler@student.tugraz.at>
[...]
Toggle quote (8 lines)> 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 isOK for master (Elisp packages are very cheap to build, so I don't countthem).
I've just verified this with the following script:
Toggle snippet (44 lines)(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 Emacslibraries: (faust@2.5.23 cedille@1.1.1 cflow@1.6translate-shell@0.9.6.11 kicad@5.1.4 lepton-eda@1.9.5-20180820quadrapassel@3.31.3 eog-plugins@3.26.4 workrave@1.10.34 emacsy@0.4.1guile-gi@0.2.0 nomad@0.1.1-alpha emacsy-minimal@v0.4.1-19.f3bf0dbevisum@0.2.6 enlightenment-wayland@0.23.1 terminology@1.5.0 ephoto@1.5enlightenment@0.23.1 econnman@1.1 python-efl@1.23.0 lekha@0.2.1python2-efl@1.23.0 edi@0.6.0 rage@0.3.1 dbus-c++@0.9.0[...]
HTH,
Maxim
B
B
Brett Gilio wrote on 24 Dec 2019 01:31
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87sglak1iu.fsf@gnu.org
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
Toggle quote (83 lines)> 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 tobe pushed, then? I think it looks quite fine but I want to double checkbefore potentially doing something dumb.
That said, we should probably also document this.
-- Brett M. GilioGNU Guix, Contributor | GNU Project, Webmaster[DFC0 C7F7 9EE6 0CA7 AE55 5E19 6722 43C4 A03F 0EEE]<brettg@gnu.org> <brettg@posteo.net>
M
M
Maxim Cournoyer wrote on 27 Dec 2019 06:06
(name . Brett Gilio)(address . brettg@gnu.org)
87v9q2e4rv.fsf@gmail.com
Brett Gilio <brettg@gnu.org> writes:

[...]
Toggle quote (6 lines)> 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
B
B
Brett Gilio wrote on 28 Dec 2019 00:07
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87r20ps6zq.fsf@gnu.org
Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
Toggle quote (18 lines)> 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. GilioGNU Guix, Contributor | GNU Project, Webmaster[DFC0 C7F7 9EE6 0CA7 AE55 5E19 6722 43C4 A03F 0EEE]<brettg@gnu.org> <brettg@posteo.net>
Closed
?
Your comment

This issue is archived.

To comment on this conversation send email to 38613@debbugs.gnu.org