[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
?
Your comment

This issue is archived.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 42736
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch