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

  • Done
  • quality assurance status badge
Details
2 participants
  • Brett Gilio
  • Jack Hill
Owner
unassigned
Submitted by
Jack Hill
Severity
normal
J
J
Jack Hill wrote on 7 Aug 2020 05:17
(address . guix-patches@gnu.org)
20200807031749.27160-1-jackhill@jackhill.us
* 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(-)

Toggle diff (26 lines)
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
B
B
Brett Gilio wrote on 7 Aug 2020 05:37
(name . Jack Hill)(address . jackhill@jackhill.us)(address . 42736@debbugs.gnu.org)
878serjfdk.fsf@gnu.org
Jack Hill <jackhill@jackhill.us> writes:

Toggle quote (31 lines)
> * 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
J
J
Jack Hill wrote on 7 Aug 2020 06:28
(name . Brett Gilio)(address . brettg@gnu.org)(address . 42736@debbugs.gnu.org)
alpine.DEB.2.21.2008070019470.4809@marsh.hcoop.net
On Thu, 6 Aug 2020, Brett Gilio wrote:

Toggle quote (10 lines)
>
> 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
B
B
Brett Gilio wrote on 7 Aug 2020 20:10
(name . Jack Hill)(address . jackhill@jackhill.us)(address . 42736@debbugs.gnu.org)
87ft8y5nto.fsf@gnu.org
Jack Hill <jackhill@jackhill.us> writes:

Toggle quote (25 lines)
> 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.
J
J
Jack Hill wrote on 8 Aug 2020 04:09
(name . Brett Gilio)(address . brettg@gnu.org)(address . 42736@debbugs.gnu.org)
alpine.DEB.2.21.2008072148480.4809@marsh.hcoop.net
Toggle quote (12 lines)
>> 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.


Best,
Jack
J
J
Jack Hill wrote on 8 Aug 2020 04:11
[PATCH v2] gnu: emacs-doom-themes: Update to 2.1.6-5.
(address . 42736@debbugs.gnu.org)
20200808021130.20027-1-jackhill@jackhill.us
* 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(-)

Toggle diff (58 lines)
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
B
B
Brett Gilio wrote on 8 Aug 2020 04:27
Re: [bug#42736] [PATCH] gnu: emacs-doom-themes: Update to 2.1.6-5.
(name . Jack Hill)(address . jackhill@jackhill.us)(address . 42736@debbugs.gnu.org)
87o8nlc1n5.fsf@gnu.org
Jack Hill <jackhill@jackhill.us> writes:

Toggle quote (38 lines)
>>> 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
B
B
Brett Gilio wrote on 8 Aug 2020 04:29
Re: [bug#42736] [PATCH v2] gnu: emacs-doom-themes: Update to 2.1.6-5.
(name . Jack Hill)(address . jackhill@jackhill.us)(address . 42736-close@debbugs.gnu.org)
87k0y9c1jy.fsf@gnu.org
Applied with c9eb5ad6b0443fafe1cd1a18aae507c1ee5fea1f. Closing.

Thanks!
J
J
Jack Hill wrote on 8 Aug 2020 04:40
Re: [bug#42736] [PATCH] gnu: emacs-doom-themes: Update to 2.1.6-5.
(name . Brett Gilio)(address . brettg@gnu.org)(address . 42736@debbugs.gnu.org)
alpine.DEB.2.21.2008072239190.4809@marsh.hcoop.net
On Fri, 7 Aug 2020, Brett Gilio wrote:

Toggle quote (7 lines)
> 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
?