[bug#42736] [PATCH] gnu: emacs-doom-themes: Update to 2.1.6-5.

[bug#42736] [PATCH] gnu: emacs-doom-themes: Update to 2.1.6-5.

[Jack Hill]

* gnu/packages/emacs-xyz.scm (emacs-doom-themes): Update to 2.1.6-5. This
commit revision add support for Emacs 27.
---
 gnu/packages/emacs-xyz.scm | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index f857b594eb..7dc8e42f81 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -22053,8 +22053,8 @@ contrast and few colors.")
       (license license:gpl3+))))
 
 (define-public emacs-doom-themes
-  (let ((commit "54039c5171e3f8c9cef1f82122549b66cd8c8f7b")
-        (revision "4")
+  (let ((commit "e803fc4ac8cf7118e2d1544d8241b848b5e79e9f")
+        (revision "5")
         (version "2.1.6"))
     (package
       (name "emacs-doom-themes")
@@ -22066,7 +22066,7 @@ contrast and few colors.")
                       (commit commit)))
                 (file-name (git-file-name name version))
                 (sha256
-                 (base32 "1iwdjq4q2gkhi6jwas3ywgmdz5dg14sfb3fzhqd7wih6j3i2l3cr"))))
+                 (base32 "128hdmf0jkzr12fv2r6z349qiwba6q97hsb6b1n2qlhi0v5v3mfh"))))
       (build-system emacs-build-system)
       (native-inputs
        `(("emacs-ert-runner" ,emacs-ert-runner)))
-- 
2.28.0

[bug#42736] [PATCH] gnu: emacs-doom-themes: Update to 2.1.6-5.

[Brett Gilio]

Jack Hill <jackhill@jackhill.us> writes:

> * gnu/packages/emacs-xyz.scm (emacs-doom-themes): Update to 2.1.6-5. This
> commit revision add support for Emacs 27.
> ---
>  gnu/packages/emacs-xyz.scm | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
> index f857b594eb..7dc8e42f81 100644
> --- a/gnu/packages/emacs-xyz.scm
> +++ b/gnu/packages/emacs-xyz.scm
> @@ -22053,8 +22053,8 @@ contrast and few colors.")
>        (license license:gpl3+))))
>  
>  (define-public emacs-doom-themes
> -  (let ((commit "54039c5171e3f8c9cef1f82122549b66cd8c8f7b")
> -        (revision "4")
> +  (let ((commit "e803fc4ac8cf7118e2d1544d8241b848b5e79e9f")
> +        (revision "5")
>          (version "2.1.6"))
>      (package
>        (name "emacs-doom-themes")
> @@ -22066,7 +22066,7 @@ contrast and few colors.")
>                        (commit commit)))
>                  (file-name (git-file-name name version))
>                  (sha256
> -                 (base32 "1iwdjq4q2gkhi6jwas3ywgmdz5dg14sfb3fzhqd7wih6j3i2l3cr"))))
> +                 (base32 "128hdmf0jkzr12fv2r6z349qiwba6q97hsb6b1n2qlhi0v5v3mfh"))))
>        (build-system emacs-build-system)
>        (native-inputs
>         `(("emacs-ert-runner" ,emacs-ert-runner)))

Hey Jack,

Thanks for taking time to revise this package. When I originally wrote
it I made notice to the fact that some elisp bytecompilations were
failing or not behaving appropriately. Since then I am pretty sure
hlissner has disabled the bytecompilation completely? Could you review
this for me, and if true please revise the appropriate arguments. If you
aren't sure what I am talking about, please let me know.

Thanks,
Brett Gilio

[bug#42736] [PATCH] gnu: emacs-doom-themes: Update to 2.1.6-5.

[Jack Hill]

On Thu, 6 Aug 2020, Brett Gilio wrote:

>
> Hey Jack,
>
> Thanks for taking time to revise this package. When I originally wrote
> it I made notice to the fact that some elisp bytecompilations were
> failing or not behaving appropriately. Since then I am pretty sure
> hlissner has disabled the bytecompilation completely? Could you review
> this for me, and if true please revise the appropriate arguments. If you
> aren't sure what I am talking about, please let me know.

Brett,

I saw your lovely comment, but in my excitement that the update solved the 
problem I was having with Emacs 27 compatibility, I didn't think too hard 
about it.

I believe that I understand what you're asking. I'll take a look tomorrow 
to see if we should change the package definition in light of upstream 
changes. If I get stuck, I'll be sure to let you know.

Best,
Jack

[bug#42736] [PATCH] gnu: emacs-doom-themes: Update to 2.1.6-5.

[Brett Gilio]

Jack Hill <jackhill@jackhill.us> writes:

> On Thu, 6 Aug 2020, Brett Gilio wrote:
>
>>
>> Hey Jack,
>>
>> Thanks for taking time to revise this package. When I originally wrote
>> it I made notice to the fact that some elisp bytecompilations were
>> failing or not behaving appropriately. Since then I am pretty sure
>> hlissner has disabled the bytecompilation completely? Could you review
>> this for me, and if true please revise the appropriate arguments. If you
>> aren't sure what I am talking about, please let me know.
>
> Brett,
>
> I saw your lovely comment, but in my excitement that the update solved
> the problem I was having with Emacs 27 compatibility, I didn't think
> too hard about it.
>
> I believe that I understand what you're asking. I'll take a look
> tomorrow to see if we should change the package definition in light of
> upstream changes. If I get stuck, I'll be sure to let you know.
>
> Best,
> Jack

Sounds great! Thanks.

[bug#42736] [PATCH] gnu: emacs-doom-themes: Update to 2.1.6-5.

[Jack Hill]

>> On Thu, 6 Aug 2020, Brett Gilio wrote:
>>
>>>
>>> Hey Jack,
>>>
>>> Thanks for taking time to revise this package. When I originally wrote
>>> it I made notice to the fact that some elisp bytecompilations were
>>> failing or not behaving appropriately. Since then I am pretty sure
>>> hlissner has disabled the bytecompilation completely? Could you review
>>> this for me, and if true please revise the appropriate arguments. If you
>>> aren't sure what I am talking about, please let me know.

Brett,

I think the way forward is to follow upstream's choices and not enable or 
disable byte compilation in Guix.

After upstream introduced commit 9cd6872 [0], our trick to selectively 
leave batch compilation enabled for some files didn't work because they 
already had `-*- no-byte-compile: t; -*-` at the top of the file. In my 
testing, I added a phase to substitute this away. Indeed, this allowed our 
trick to work again. However, the material, snazzy, and tomorrow-day 
themes now have problems with byte compilation.

Therefore, I removed the disable-breaking-compilation phase entirely. With 
it removed, doom-themes-autoloads.el, doom-themes-base.el, doom-themes.el, 
doom-themes-ext-org.el, and doom-themes-ext-visual-bell.el do get byte 
compiled. From this evidence I concluded that upstream is aware of this 
issue and is only disabling byte compilation where necessary.

I'll send a version 2 of the patch with the phase removed shortly.

[0] https://github.com/hlissner/emacs-doom-themes/commit/9cd6872b1af88165834230abd45743036861f925

Best,
Jack

[bug#42736] [PATCH v2] gnu: emacs-doom-themes: Update to 2.1.6-5.

[Jack Hill]

* gnu/packages/emacs-xyz.scm (emacs-doom-themes): Update to 2.1.6-5. This
commit revision add support for Emacs 27.
[arguments]: Remove disable-breaking-compilation phase.
---
 gnu/packages/emacs-xyz.scm | 31 +++----------------------------
 1 file changed, 3 insertions(+), 28 deletions(-)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index f857b594eb..0b66e9507b 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -22053,8 +22053,8 @@ contrast and few colors.")
       (license license:gpl3+))))
 
 (define-public emacs-doom-themes
-  (let ((commit "54039c5171e3f8c9cef1f82122549b66cd8c8f7b")
-        (revision "4")
+  (let ((commit "e803fc4ac8cf7118e2d1544d8241b848b5e79e9f")
+        (revision "5")
         (version "2.1.6"))
     (package
       (name "emacs-doom-themes")
@@ -22066,7 +22066,7 @@ contrast and few colors.")
                       (commit commit)))
                 (file-name (git-file-name name version))
                 (sha256
-                 (base32 "1iwdjq4q2gkhi6jwas3ywgmdz5dg14sfb3fzhqd7wih6j3i2l3cr"))))
+                 (base32 "128hdmf0jkzr12fv2r6z349qiwba6q97hsb6b1n2qlhi0v5v3mfh"))))
       (build-system emacs-build-system)
       (native-inputs
        `(("emacs-ert-runner" ,emacs-ert-runner)))
@@ -22086,31 +22086,6 @@ contrast and few colors.")
                (for-each (lambda (f)
                            (rename-file f (basename f)))
                          (find-files "./themes" ".*\\.el$"))
-               #t))
-           ;; 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 this issue is resolved.
-           ;; <https://github.com/hlissner/emacs-doom-themes/issues/314>
-           ;;
-           ;; NOTE: Byte-comp has been disabled in/after commit 9cd6872.
-           ;; However our method of selective disabling is preferential to
-           ;; just widely disabling byte-compilation.
-           (add-after 'move-themes 'disable-breaking-compilation
-             (lambda _
-               (for-each (lambda (file)
-                           (chmod file #o600) ; needed to write changes.
-                           (emacs-batch-disable-compilation file))
-                         (cons "doom-themes-ext-neotree.el"
-                               ;; NOTE: When updating this package, determine
-                               ;; whether changed theme files can byte-compile.
-                               ;; If they can successfully byte-compile, add them
-                               ;; to this list of exceptions.
-                               (lset-difference string-contains
-                                                (find-files "." ".*-theme.el")
-                                                '("material"
-                                                  "snazzy"
-                                                  "tomorrow-day"))))
                #t)))))
       (synopsis "Wide collection of color themes for Emacs")
       (description "Emacs-doom-themes contains numerous popular color themes for
-- 
2.28.0

[bug#42736] [PATCH] gnu: emacs-doom-themes: Update to 2.1.6-5.

[Brett Gilio]

Jack Hill <jackhill@jackhill.us> writes:

>>> On Thu, 6 Aug 2020, Brett Gilio wrote:
>>>
>>>>
>>>> Hey Jack,
>>>>
>>>> Thanks for taking time to revise this package. When I originally wrote
>>>> it I made notice to the fact that some elisp bytecompilations were
>>>> failing or not behaving appropriately. Since then I am pretty sure
>>>> hlissner has disabled the bytecompilation completely? Could you review
>>>> this for me, and if true please revise the appropriate arguments. If you
>>>> aren't sure what I am talking about, please let me know.
>
> Brett,
>
> I think the way forward is to follow upstream's choices and not enable
> or disable byte compilation in Guix.
>
> After upstream introduced commit 9cd6872 [0], our trick to selectively
> leave batch compilation enabled for some files didn't work because
> they already had `-*- no-byte-compile: t; -*-` at the top of the
> file. In my testing, I added a phase to substitute this away. Indeed,
> this allowed our trick to work again. However, the material, snazzy,
> and tomorrow-day themes now have problems with byte compilation.
>
> Therefore, I removed the disable-breaking-compilation phase
> entirely. With it removed, doom-themes-autoloads.el,
> doom-themes-base.el, doom-themes.el, doom-themes-ext-org.el, and
> doom-themes-ext-visual-bell.el do get byte compiled. From this
> evidence I concluded that upstream is aware of this issue and is only
> disabling byte compilation where necessary.
>
> I'll send a version 2 of the patch with the phase removed shortly.
>
> [0] https://github.com/hlissner/emacs-doom-themes/commit/9cd6872b1af88165834230abd45743036861f925
>
> Best,
> Jack

Jack,

Thank you for investigating this issue. I agree with your solution, and
I will apply your patch removing the phase.

Brett

bug#42736: [PATCH v2] gnu: emacs-doom-themes: Update to 2.1.6-5.

[Brett Gilio]

Applied with c9eb5ad6b0443fafe1cd1a18aae507c1ee5fea1f. Closing.

Thanks!

[bug#42736] [PATCH] gnu: emacs-doom-themes: Update to 2.1.6-5.

[Jack Hill]

On Fri, 7 Aug 2020, Brett Gilio wrote:

> Jack,
>
> Thank you for investigating this issue. I agree with your solution, and
> I will apply your patch removing the phase.
>
> Brett

Thanks for the review!

Best,
Jack