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

* 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(-)

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

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

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.

>> 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 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

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