unofficial mirror of bug-guix@gnu.org 
 help / color / mirror / code / Atom feed
* bug#72333: Magit/Transient error message
@ 2024-07-28 12:41 daniel szmulewicz
  2024-07-30 22:20 ` Alvin Hsu
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: daniel szmulewicz @ 2024-07-28 12:41 UTC (permalink / raw)
  To: 72333

[-- Attachment #1: Type: text/plain, Size: 308 bytes --]

Running M-x magit-branch generates the following error:

transient-setup: Symbol’s function definition is void:
transient-prefix-object

In order to reproduce:

guix shell emacs emacs-magit -- emacs -Q

M-x magit-version
Magit 3.3.0-8.538cb2f, Transient 0.7.3, Git 2.45.2, Emacs 29.4, gnu/linux

[-- Attachment #2: Type: text/html, Size: 770 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: Magit/Transient error message
  2024-07-28 12:41 bug#72333: Magit/Transient error message daniel szmulewicz
@ 2024-07-30 22:20 ` Alvin Hsu
  2024-07-30 22:50 ` aurtzy
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Alvin Hsu @ 2024-07-30 22:20 UTC (permalink / raw)
  To: daniel.szmulewicz; +Cc: 72333

Hi Daniel,

 > Running M-x magit-branch generates the following error:
 > transient-setup: Symbol’s function definition is void:
 > transient-prefix-object
 >
 > In order to reproduce:
 >
 > guix shell emacs emacs-magit -- emacs -Q
 >
 > M-x magit-version
 > Magit 3.3.0-8.538cb2f, Transient 0.7.3, Git 2.45.2, Emacs 29.4, gnu/linux

It looks like this is because emacs is actually using the built-in (older)
version of transient, while magit relies on a function from a newer version.

Evaluating the following definition from the newer transient source code 
fixes
this particular issue for me (I've put it in my init.el for the time being):

#+begin_src lisp
   (defun transient-prefix-object ()
     (or transient--prefix transient-current-prefix))
#+end_src

This sounds similar to another issue with embark and org [1], however in 
that report it
was because emacs-org was not included in the environment; in this case, the
newer version of transient is already an input of magit in the guix 
package, so
I'm not sure what's happening here.

[1] https://github.com/oantolin/embark/issues/723

Cheers,

aurtzy




^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: Magit/Transient error message
  2024-07-28 12:41 bug#72333: Magit/Transient error message daniel szmulewicz
  2024-07-30 22:20 ` Alvin Hsu
@ 2024-07-30 22:50 ` aurtzy
  2024-07-31 16:52   ` Richard Sent
  2024-07-31 12:04 ` bug#72333: [PATCH] gnu: emacs-transient: Prioritise loading over built-in gemmaro
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: aurtzy @ 2024-07-30 22:50 UTC (permalink / raw)
  To: daniel.szmulewicz; +Cc: 72333

Hi Daniel,

 > Running M-x magit-branch generates the following error:
 >
 > transient-setup: Symbol’s function definition is void:
 > transient-prefix-object
 >
 > In order to reproduce:
 >
 > guix shell emacs emacs-magit -- emacs -Q
 >
 > M-x magit-version
 > Magit 3.3.0-8.538cb2f, Transient 0.7.3, Git 2.45.2, Emacs 29.4, gnu/linux

It looks like this is because emacs is actually using the built-in (older)
version of transient, while magit relies on a function from a newer version.

Evaluating the following definition from the newer transient source code 
fixes
this particular issue for me (I've put it in my init.el for the time being):

#+begin_src lisp
   (defun transient-prefix-object ()
     (or transient--prefix transient-current-prefix))
#+end_src

This sounds similar to another issue with embark and org [1], however in 
that
report it was because emacs-org was not included in the environment; in this
case, the newer version of transient is already an input of magit in the 
guix
package, so I'm not sure what's happening here.

[1] https://github.com/oantolin/embark/issues/723

Cheers,

aurtzy





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: [PATCH] gnu: emacs-transient: Prioritise loading over built-in.
  2024-07-28 12:41 bug#72333: Magit/Transient error message daniel szmulewicz
  2024-07-30 22:20 ` Alvin Hsu
  2024-07-30 22:50 ` aurtzy
@ 2024-07-31 12:04 ` gemmaro
  2024-07-31 17:10   ` Liliana Marie Prikler
       [not found] ` <87ikvias2d.fsf@gmail.com>
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: gemmaro @ 2024-07-31 12:04 UTC (permalink / raw)
  To: 72333; +Cc: gemmaro, Andrew Tropin, Katherine Cox-Buday,
	Liliana Marie Prikler

* gnu/packages/emacs-xyz.scm (emacs-transient)[arguments]<#:emacs>:
Use the full emacs package.

Change-Id: Id6277f365ae0a780469658818872b7277de20135
---
Hello,

I hope this fixes the problem.

I found out the followings:
* Native compiled files (.eln) might be loaded first.
* The built-in Transient has an eln version and lacks transient-prefix-object.
* Using the full emacs generates the eln and now it is prioritised.

Best,
gemamro.

 gnu/packages/emacs-xyz.scm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index d4e60d0d5e..17d58996ce 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -32056,6 +32056,9 @@ (define-public emacs-transient
     (build-system emacs-build-system)
     (arguments
      `(#:tests? #f                      ;no test suite
+       ;; Native compile if available to make it preferable to the build-in
+       ;; Transient.  See <https://issues.guix.gnu.org/72333>.
+       #:emacs ,emacs
        #:phases (modify-phases %standard-phases
                   (add-after 'unpack 'build-info-manual
                     (lambda _

base-commit: 01d4363168ed10ea223047f7a7b83201f161ec0b
-- 
2.45.2





^ permalink raw reply related	[flat|nested] 18+ messages in thread

* bug#72333: Magit/Transient error message
  2024-07-30 22:50 ` aurtzy
@ 2024-07-31 16:52   ` Richard Sent
  0 siblings, 0 replies; 18+ messages in thread
From: Richard Sent @ 2024-07-31 16:52 UTC (permalink / raw)
  To: 72333, aurtzy, daniel.szmulewicz

On July 30, 2024 6:50:12 PM EDT, aurtzy <aurtzy@gmail.com> wrote:
>Hi Daniel,
>
>> Running M-x magit-branch generates the following error:
>>
>> transient-setup: Symbol’s function definition is void:
>> transient-prefix-object
>>
>> In order to reproduce:
>>
>> guix shell emacs emacs-magit -- emacs -Q
>>
>> M-x magit-version
>> Magit 3.3.0-8.538cb2f, Transient 0.7.3, Git 2.45.2, Emacs 29.4, gnu/linux
>
>It looks like this is because emacs is actually using the built-in (older)
>version of transient, while magit relies on a function from a newer version.
>
>Evaluating the following definition from the newer transient source code fixes
>this particular issue for me (I've put it in my init.el for the time being):
>
>#+begin_src lisp
>  (defun transient-prefix-object ()
>    (or transient--prefix transient-current-prefix))
>#+end_src
>
>This sounds similar to another issue with embark and org [1], however in that
>report it was because emacs-org was not included in the environment; in this
>case, the newer version of transient is already an input of magit in the guix
>package, so I'm not sure what's happening here.
>
>[1] https://github.com/oantolin/embark/issues/723
>
>Cheers,
>
>aurtzy
>
>
>
>

I encountered a similar issue to this (void-function transient--suffix-only) when I enabled the following block of code:

#+begin_src lisp
  ;; FIXME: This caused (void-function transient--suffix-only) errors
  ;; in Guix c9cd16c630ccba655b93ff32fd9a99570b4f5373 with magit.
  ;;
  ;; (setq read-extended-command-predicate
  ;;       #'command-completion-default-include-p)
#+end_src

I'm also not sure of the underlying cause, but something seems funky with Magit.




^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: [PATCH] gnu: emacs-transient: Prioritise loading over built-in.
  2024-07-31 12:04 ` bug#72333: [PATCH] gnu: emacs-transient: Prioritise loading over built-in gemmaro
@ 2024-07-31 17:10   ` Liliana Marie Prikler
  2024-08-17  9:26     ` Tomas Volf
  0 siblings, 1 reply; 18+ messages in thread
From: Liliana Marie Prikler @ 2024-07-31 17:10 UTC (permalink / raw)
  To: gemmaro, 72333; +Cc: Katherine Cox-Buday, Andrew Tropin

Am Mittwoch, dem 31.07.2024 um 21:04 +0900 schrieb gemmaro:
> * gnu/packages/emacs-xyz.scm (emacs-transient)[arguments]<#:emacs>:
> Use the full emacs package.
> 
> Change-Id: Id6277f365ae0a780469658818872b7277de20135
> ---
> Hello,
> 
> I hope this fixes the problem.
> 
> I found out the followings:
> * Native compiled files (.eln) might be loaded first.
> * The built-in Transient has an eln version and lacks transient-
> prefix-object.
> * Using the full emacs generates the eln and now it is prioritised.
This won't work if you use any other Emacs (e.g. emacs-pgtk).

Try for example: 
  guix shell emacs-magit emacs-pgtk \
    --with-input=emacs-minimal=emacs --pure -- emacs

One workaround would be to check the symbol in magit and other packages
that rely on this symbol, but I hazard a guess that there's a reason
why they require the newer magit.

The other (recommended at the moment) would be to use the proper
transformation to natively compile your emacs packages.

We could also, for the time being, offer an Emacs without native
compilation – YMMV on how well liked that'd be.

The proper fix would be to build Emacs packages for the various
variants directly with emacs-build-system.  I have an as-of-yet
incomplete patch set to do exactly that.

Cheers




^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: [PATCH] gnu: emacs-transient: Prioritise loading over built-in.
  2024-07-31 17:10   ` Liliana Marie Prikler
@ 2024-08-17  9:26     ` Tomas Volf
  2024-08-17 11:21       ` Liliana Marie Prikler
  0 siblings, 1 reply; 18+ messages in thread
From: Tomas Volf @ 2024-08-17  9:26 UTC (permalink / raw)
  To: Liliana Marie Prikler; +Cc: Katherine Cox-Buday, gemmaro, Andrew Tropin, 72333

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

On 2024-07-31 19:10:03 +0200, Liliana Marie Prikler wrote:
> The other (recommended at the moment) would be to use the proper
> transformation to natively compile your emacs packages.

So just to make sure I understand correctly what is the correct work around
here.  Let us assume I install emacs and emacs-magit into my home environment.
So until now I have (leaving everything else out) this:

    (home-environment
     (packages (list emacs emacs-magit <more-here>)))

Assuming I want to follow the recommendation above, I should turn it into
something like this:

    (home-environment
     (packages (map (package-input-rewriting `((,emacs-minimal . ,emacs)))
                    (list emacs emacs-magit <more-here>))))

Did I get it right?  Are there any downsides to doing this (except compute)?

Have a nice day,
Tomas

Side note: I am surprised how long emacs-yaml takes to build on my ~5.5 GHz
build machine.  I compile whole of firefox faster.

--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: [PATCH] gnu: emacs-transient: Prioritise loading over built-in.
  2024-08-17  9:26     ` Tomas Volf
@ 2024-08-17 11:21       ` Liliana Marie Prikler
  0 siblings, 0 replies; 18+ messages in thread
From: Liliana Marie Prikler @ 2024-08-17 11:21 UTC (permalink / raw)
  To: Tomas Volf; +Cc: Katherine Cox-Buday, gemmaro, Andrew Tropin, 72333

Am Samstag, dem 17.08.2024 um 11:26 +0200 schrieb Tomas Volf:
> On 2024-07-31 19:10:03 +0200, Liliana Marie Prikler wrote:
> > The other (recommended at the moment) would be to use the proper
> > transformation to natively compile your emacs packages.
> 
> So just to make sure I understand correctly what is the correct work
> around here.  Let us assume I install emacs and emacs-magit into my
> home environment. So until now I have (leaving everything else out)
> this:
> 
>     (home-environment
>      (packages (list emacs emacs-magit <more-here>)))
> 
> Assuming I want to follow the recommendation above, I should turn it
> into something like this:
> 
>     (home-environment
>      (packages (map (package-input-rewriting `((,emacs-minimal .
> ,emacs)))
>                     (list emacs emacs-magit <more-here>))))
> 
> Did I get it right?  Are there any downsides to doing this (except
> compute)?
Yep, that's the low-level way of achieving this (more or less – emacs-
minimal isn't always the only package to replace).  On a higher level,
you can bind (package-input-rewriting …) to a name and use that.  I
personally prefer the (options->transformation …)¹ variant, though,
which is nearly identical and compiles to this.

Cheers

> Side note: I am surprised how long emacs-yaml takes to build on my
> ~5.5 GHz build machine.  I compile whole of firefox faster.
That sounds concerning.  We might want to look into individual bugs
such as this.

¹ the argument is '((with-input . "emacs-minimal=<your-emacs>")),
  which imho, reads nicer.  <your-emacs> could be emacs, emacs-pgtk,
  etc.




^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: builtin native-compiled is loaded instead of emacs-packages
       [not found] ` <87ikvias2d.fsf@gmail.com>
@ 2024-09-02 18:55   ` Simon Tournier
  2024-09-02 19:00     ` Simon Tournier
  2024-09-02 19:22     ` Liliana Marie Prikler
  0 siblings, 2 replies; 18+ messages in thread
From: Simon Tournier @ 2024-09-02 18:55 UTC (permalink / raw)
  To: daniel szmulewicz, 72333, Liliana Marie Prikler,
	Katherine Cox-Buday, Andrew Tropin
  Cc: Alvin Hsu, aurtzy, Richard Sent, Nicolas Goaziou, gemmaro,
	Tomas Volf

Hi,

Alvin Hsu

Once bisecting, the first occurrence appears with:

        7a2fc32ba2cec79b087932f30d77751f9133d740
        CommitDate: Sun Jul 21 13:32:27 2024 +0200
        Parent:     bb1aea46e6 gnu: emacs-gnosis: Update to 0.3.2.

        gnu: emacs-magit: Update to 3.3.0-8.538cb2f

        * gnu/packages/emacs-xyz.scm (emacs-magit): Update to 3.3.0-8.538cb2f.

        A.o. it contains the fix for https://github.com/magit/magit/issues/4940 which is a showstopper.

        Change-Id: Id83a237d38c03a97f9f1043db4b8ba594521a2f7


This update of emacs-magit depends on transient-prefix-object, i.e., it
depends on a new version of transient.

--8<---------------cut here---------------start------------->8---
$ ag transient-prefix-object \
     $(guix time-machine -q --commit=7a2fc32ba2cec79b087932f30d77751f9133d740 -- build emacs-magit -S)
/gnu/store/fgcz4a4adhnrcxa541kdkn30f0hgn6c5-emacs-magit-3.3.0-8.538cb2f-checkout/lisp/magit-branch.el
213:              (oref (transient-prefix-object) scope)))
217:             (propertize (oref (transient-prefix-object) scope)
866:             (propertize (oref (transient-prefix-object) scope)
910:  (when-let* ((branch (oref (transient-prefix-object) scope))
918:    (magit-read-upstream-branch (oref (transient-prefix-object) scope)
922:  (magit-set-upstream-branch (oref (transient-prefix-object) scope) refname)
924:        (and-let* ((branch (oref (transient-prefix-object) scope))
931:  (let ((branch (oref (transient-prefix-object) scope)))

/gnu/store/fgcz4a4adhnrcxa541kdkn30f0hgn6c5-emacs-magit-3.3.0-8.538cb2f-checkout/lisp/magit-remote.el
73:              (oref (transient-prefix-object) scope)))
318:             (propertize (oref (transient-prefix-object) scope)
--8<---------------cut here---------------end--------------->8---

When the commit parent does not:

    $ ag transient-prefix-object \
        $(guix time-machine -q --commit=bb1aea46e6 -- build emacs-magit -S)


However, it should be an issue since emacs-transient is correctly
updated and correctly contains the requirements.

--8<---------------cut here---------------start------------->8---
$ ag transient-prefix-object \
     $(guix time-machine -q --commit=bb1aea46e6 -- build emacs-transient)
/gnu/store/8zwwwprv93shxjjfcc4a406gs9qgk58i-emacs-transient-0.7.2/share/emacs/site-lisp/transient-0.7.2/transient.el
1605:(defun transient-prefix-object ()
3044:               (format "levels > %s" (oref (transient-prefix-object) level))
3058:  (transient-set-value (transient-prefix-object)))
3066:  (transient-save-value (transient-prefix-object)))
3074:  (transient-reset-value (transient-prefix-object)))
3607:  (oref (transient-prefix-object) scope))
--8<---------------cut here---------------end--------------->8---

Therefore, the issue…

On Fri, 30 Aug 2024 at 18:33, Simon Tournier <zimon.toutoune@gmail.com> wrote:

> Why is emacs-transient not loaded?

…is something about loading Emacs Lisp code.

--8<---------------cut here---------------start------------->8---
$ guix time-machine -q --commit=bb1aea46e6 \
       -- shell emacs emacs-transient \
       -- emacs --batch --eval "(progn (require 'transient) (transient-prefix-object) (print \"done\"))"
 
Error: void-function (transient-prefix-object)
  mapbacktrace(#f(compiled-function (evald func args flags) #<bytecode 0x5d8e1c931aef1bf>))
  debug-early-backtrace()
  debug-early(error (void-function transient-prefix-object))
  (transient-prefix-object)
  (progn (require 'transient) (transient-prefix-object) (print "done"))
  command-line-1(("--eval" "(progn (require 'transient) (transient-prefix-object) (print \"done\"))"))
  command-line()
  normal-top-level()
Symbol’s function definition is void: transient-prefix-object
--8<---------------cut here---------------end--------------->8---

Compared to:

--8<---------------cut here---------------start------------->8---
$ guix time-machine -q --commit=bb1aea46e6 \
       -- shell emacs emacs-transient \
       -- emacs --batch --eval "(progn (load (locate-library \"transient\")) (transient-prefix-object) (print \"done\"))"

Loading /gnu/store/8zwwwprv93shxjjfcc4a406gs9qgk58i-emacs-transient-0.7.2/share/emacs/site-lisp/transient-0.7.2/transient.elc...

"done"
--8<---------------cut here---------------end--------------->8---

Now, if native compilation is not able to find .eln files then all work
as expected.

--8<---------------cut here---------------start------------->8---
$ guix time-machine -q --commit=bb1aea46e6 \
       -- shell emacs emacs-transient \
       -- emacs --batch --eval "(progn (setq native-comp-eln-load-path nil) (require 'transient) (transient-prefix-object) (print \"done\"))"
 
"done"
--8<---------------cut here---------------end--------------->8---


Other said, the bug is not about Magit or Transient but the bug is about
all native-compiled Emacs Lisp code that are part of builtin Emacs:
these are loaded and not the ones from packages if provided.

About the example of Org-mode, I guess ’org-version’ is not marked for
the compilation and that’s why it works in appearances.


Well, maybe that’s a bug known upstream (I have not checked) and maybe
this behaviour has already been reported for Guix.  In both cases,
that’s annoying because it means that emacs-packages are useless as
dependencies since builtin is always used instead.

Somehow, one solution would to not provide native-compilation of source
code that is developed outside the Emacs tree (transient, Magit, etc.)
and let user locally native-compile them.

Or another solution would to split ’native-comp-eln-load-path’.  Say one
folder for builtin code that we know is developed outside Emacs tree,
e.g., transient, Org, etc. And append them in EMACSNATIVELOADPATH by
default.  When a known package is provided by the user, the builtin path
is removed from EMACSNATIVELOADPATH (and the package path could be added
if emacs-build-system native-compile them).


Cheers,
simon




^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: builtin native-compiled is loaded instead of emacs-packages
  2024-09-02 18:55   ` bug#72333: builtin native-compiled is loaded instead of emacs-packages Simon Tournier
@ 2024-09-02 19:00     ` Simon Tournier
  2024-09-02 19:22     ` Liliana Marie Prikler
  1 sibling, 0 replies; 18+ messages in thread
From: Simon Tournier @ 2024-09-02 19:00 UTC (permalink / raw)
  To: daniel szmulewicz, 72333, Liliana Marie Prikler,
	Katherine Cox-Buday, Andrew Tropin
  Cc: Alvin Hsu, aurtzy, Richard Sent, Nicolas Goaziou, gemmaro,
	Tomas Volf

Oups, missing some edits, as usual. :-)

> However, it should be an issue since emacs-transient is correctly
> updated and correctly contains the requirements.

it should *NOT* be an issue




^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: builtin native-compiled is loaded instead of emacs-packages
  2024-09-02 18:55   ` bug#72333: builtin native-compiled is loaded instead of emacs-packages Simon Tournier
  2024-09-02 19:00     ` Simon Tournier
@ 2024-09-02 19:22     ` Liliana Marie Prikler
  2024-09-03 15:05       ` Simon Tournier
  1 sibling, 1 reply; 18+ messages in thread
From: Liliana Marie Prikler @ 2024-09-02 19:22 UTC (permalink / raw)
  To: Simon Tournier, daniel szmulewicz, 72333, Katherine Cox-Buday,
	Andrew Tropin
  Cc: Alvin Hsu, aurtzy, Richard Sent, Nicolas Goaziou, gemmaro,
	Tomas Volf

Hi Simon

Am Montag, dem 02.09.2024 um 20:55 +0200 schrieb Simon Tournier:
> […]
> Well, maybe that’s a bug known upstream (I have not checked) and
> maybe this behaviour has already been reported for Guix.  In both
> cases, that’s annoying because it means that emacs-packages are
> useless as dependencies since builtin is always used instead.
It is a bug unique to our handling in Guix – upstream uses hashes to
guard against it, but those break grafts.  In any case, builtin
packages are only used instead if the dependency isn't natively
compiled with a compatible Emacs (sadly the current default!) – I've
started a WIP series at [1], but we're still looking for solutions that
scale better in terms of what we need to declare for our packages (and
CI integration).

> Somehow, one solution would to not provide native-compilation of
> source code that is developed outside the Emacs tree (transient,
> Magit, etc.) and let user locally native-compile them.
I'm not sure that'd work.  Even if you byte-compile that code, you are
still loading the natively-compiled stuff from the Emacs tree.  You
could disable native compilation for the Emacs package itself, but
that'd kinda defeat the purpose of bundling these things with Emacs
(which tbf is an upstream thing).

> Or another solution would to split ’native-comp-eln-load-path’.  Say
> one folder for builtin code that we know is developed outside Emacs
> tree, e.g., transient, Org, etc. And append them in
> EMACSNATIVELOADPATH by default.  When a known package is provided by
> the user, the builtin path is removed from EMACSNATIVELOADPATH (and
> the package path could be added if emacs-build-system native-compile
> them).
This already happens.  The bug is not that built-in stuff is found
prior to non-builtin stuff, it's that it is found *at all*.  Since we
don't do native compilation for packages at the moment, most folks only
get Emacs itself natively compiled.

Cheers

[1] https://issues.guix.gnu.org/72406




^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: [PATCH] gnu: emacs-minimal: Do not native-compile lisp/transient.el.
  2024-07-28 12:41 bug#72333: Magit/Transient error message daniel szmulewicz
                   ` (3 preceding siblings ...)
       [not found] ` <87ikvias2d.fsf@gmail.com>
@ 2024-09-03 14:57 ` Simon Tournier
  2024-09-03 16:49   ` Liliana Marie Prikler
  2024-09-14 17:53 ` bug#72333: Magit/Transient error message Rutherther via Bug reports for GNU Guix
  5 siblings, 1 reply; 18+ messages in thread
From: Simon Tournier @ 2024-09-03 14:57 UTC (permalink / raw)
  To: 72333
  Cc: Simon Tournier, Andrew Tropin, Katherine Cox-Buday,
	Liliana Marie Prikler

Fixes <https://issues.guix.gnu.org/72333>.
Reported by daniel szmulewicz <daniel.szmulewicz@gmail.com>.

* gnu/packages/emacs.scm (emacs-minimal)[arguments]<phases>: Turn off
native-compilation for the file lisp/transient.el.

Change-Id: I27c9d660cbad46be66df641816e4596346969dfc
---
 gnu/packages/emacs.scm | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
index f1ea4fe061..c555ca09f7 100644
--- a/gnu/packages/emacs.scm
+++ b/gnu/packages/emacs.scm
@@ -233,6 +233,16 @@ (define-public emacs-minimal
                 (("\\(tramp-compat-process-running-p \"(.*)\"\\)" all process)
                  (format #f "(or ~a (tramp-compat-process-running-p ~s))"
                          all (string-append "." process "-real"))))))
+          (add-after 'unpack 'do-not-native-compile
+            (lambda _
+              ;; Fixes <https://issues.guix.gnu.org/72333>.  Temporary
+              ;; workaround for native-compilation bug of transient.el.
+              ;; Please remove once the native-compilation for Emacs packages
+              ;; is fully supported.
+              (substitute* "lisp/transient.el"
+                ((";; End:")
+                 ";; no-native-compile: t
+;; End:"))))
           (add-before 'configure 'fix-/bin/pwd
             (lambda _
               ;; Use `pwd', not `/bin/pwd'.

base-commit: 1569b861f504178263b73b4b48563bf3937d01bf
-- 
2.41.0





^ permalink raw reply related	[flat|nested] 18+ messages in thread

* bug#72333: builtin native-compiled is loaded instead of emacs-packages
  2024-09-02 19:22     ` Liliana Marie Prikler
@ 2024-09-03 15:05       ` Simon Tournier
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Tournier @ 2024-09-03 15:05 UTC (permalink / raw)
  To: Liliana Marie Prikler, daniel szmulewicz, 72333,
	Katherine Cox-Buday, Andrew Tropin
  Cc: Alvin Hsu, aurtzy, Richard Sent, Nicolas Goaziou, gemmaro,
	Tomas Volf

Hi Liliana,

On Mon, 02 Sep 2024 at 21:22, Liliana Marie Prikler <liliana.prikler@gmail.com> wrote:

> It is a bug unique to our handling in Guix

Well, waiting a proper fix and a better story for Emacs package native
compilation, I propose to turn off the native-compilation for
lisp/transient.el only; see [1].  This fixes the bug at hand so it
removes the annoyance with a broken Magit.

And it lets the time to discuss a better long-term solution.

1: https://issues.guix.gnu.org/72333#12

Cheers,
simon




^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: [PATCH] gnu: emacs-minimal: Do not native-compile lisp/transient.el.
  2024-09-03 14:57 ` bug#72333: [PATCH] gnu: emacs-minimal: Do not native-compile lisp/transient.el Simon Tournier
@ 2024-09-03 16:49   ` Liliana Marie Prikler
  2024-09-03 19:47     ` Liliana Marie Prikler
  0 siblings, 1 reply; 18+ messages in thread
From: Liliana Marie Prikler @ 2024-09-03 16:49 UTC (permalink / raw)
  To: Simon Tournier, 72333; +Cc: Katherine Cox-Buday, Andrew Tropin

Am Dienstag, dem 03.09.2024 um 16:57 +0200 schrieb Simon Tournier:
> Fixes <https://issues.guix.gnu.org/72333>.
> Reported by daniel szmulewicz <daniel.szmulewicz@gmail.com>.
I think those belong in the trailer, but I can fix that myself.

> * gnu/packages/emacs.scm (emacs-minimal)[arguments]<phases>: Turn off
> native-compilation for the file lisp/transient.el.
> 
> Change-Id: I27c9d660cbad46be66df641816e4596346969dfc
> ---
>  gnu/packages/emacs.scm | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/gnu/packages/emacs.scm b/gnu/packages/emacs.scm
> index f1ea4fe061..c555ca09f7 100644
> --- a/gnu/packages/emacs.scm
> +++ b/gnu/packages/emacs.scm
> @@ -233,6 +233,16 @@ (define-public emacs-minimal
>                  (("\\(tramp-compat-process-running-p \"(.*)\"\\)"
> all process)
>                   (format #f "(or ~a (tramp-compat-process-running-p
> ~s))"
>                           all (string-append "." process "-
> real"))))))
> +          (add-after 'unpack 'do-not-native-compile
> +            (lambda _
> +              ;; Fixes <https://issues.guix.gnu.org/72333>. 
> Temporary
> +              ;; workaround for native-compilation bug of
> transient.el.
> +              ;; Please remove once the native-compilation for Emacs
> packages
> +              ;; is fully supported.
> +              (substitute* "lisp/transient.el"
> +                ((";; End:")
> +                 ";; no-native-compile: t
> +;; End:"))))
Should we do this in a snippet instead?  Otherwise LGTM.

Cheers

^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: [PATCH] gnu: emacs-minimal: Do not native-compile lisp/transient.el.
  2024-09-03 16:49   ` Liliana Marie Prikler
@ 2024-09-03 19:47     ` Liliana Marie Prikler
  2024-09-03 21:00       ` Simon Tournier
  0 siblings, 1 reply; 18+ messages in thread
From: Liliana Marie Prikler @ 2024-09-03 19:47 UTC (permalink / raw)
  To: Simon Tournier, 72333-done; +Cc: Katherine Cox-Buday, Andrew Tropin

Am Dienstag, dem 03.09.2024 um 18:49 +0200 schrieb Liliana Marie
Prikler:
> Am Dienstag, dem 03.09.2024 um 16:57 +0200 schrieb Simon Tournier:
> > Fixes <https://issues.guix.gnu.org/72333>.
> > Reported by daniel szmulewicz <daniel.szmulewicz@gmail.com>.
> I think those belong in the trailer, but I can fix that myself.
Pushed with the mentioned fixes.

> > […]
> Should we do this in a snippet instead?  Otherwise LGTM.
I left it as a phase, so that Emacsen with modified source also
benefit.

Cheers




^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: [PATCH] gnu: emacs-minimal: Do not native-compile lisp/transient.el.
  2024-09-03 19:47     ` Liliana Marie Prikler
@ 2024-09-03 21:00       ` Simon Tournier
  0 siblings, 0 replies; 18+ messages in thread
From: Simon Tournier @ 2024-09-03 21:00 UTC (permalink / raw)
  To: Liliana Marie Prikler; +Cc: 72333-done, Katherine Cox-Buday, Andrew Tropin

On Tue, 3 Sept 2024 at 21:47, Liliana Marie Prikler
<liliana.prikler@gmail.com> wrote:

> > Should we do this in a snippet instead?  Otherwise LGTM.
>
> I left it as a phase, so that Emacsen with modified source also
> benefit.

I think it's better as a phase because of what you wrote and also
because "guix build -S emacs" returns a non (or less) Guix specific
source.

Thanks for quick review and push.  Now it's free my mind and I am
fully able to enjoy Magit v4.0. ;-)

Cheers,
simon




^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: Magit/Transient error message
  2024-07-28 12:41 bug#72333: Magit/Transient error message daniel szmulewicz
                   ` (4 preceding siblings ...)
  2024-09-03 14:57 ` bug#72333: [PATCH] gnu: emacs-minimal: Do not native-compile lisp/transient.el Simon Tournier
@ 2024-09-14 17:53 ` Rutherther via Bug reports for GNU Guix
  2024-09-14 19:12   ` Liliana Marie Prikler
  5 siblings, 1 reply; 18+ messages in thread
From: Rutherther via Bug reports for GNU Guix @ 2024-09-14 17:53 UTC (permalink / raw)
  To: 72333
  Cc: Alvin Hsu, aurtzy, Simon Tournier, Richard Sent,
	Liliana Marie Prikler, daniel szmulewicz, gemmaro, Tomas Volf


From Liliana dmarie Prikler:
> It is a bug unique to our handling in Guix – upstream uses hashes to
> guard against it, but those break grafts.

Do we need to remove the whole hash? The path hash would definitely
break grafts, but why should the content hash break grafts? Having the
content hash would also solve this issue.

Here is the upstream explanation for the hash
https://git.savannah.gnu.org/cgit/emacs.git/tree/src/comp.c?h=194a8f5c1406dd7e762376bdfde78d1b7d01b6b1#n4433
having two parts, path and content hash.

Regards,
Rutherther





^ permalink raw reply	[flat|nested] 18+ messages in thread

* bug#72333: Magit/Transient error message
  2024-09-14 17:53 ` bug#72333: Magit/Transient error message Rutherther via Bug reports for GNU Guix
@ 2024-09-14 19:12   ` Liliana Marie Prikler
  0 siblings, 0 replies; 18+ messages in thread
From: Liliana Marie Prikler @ 2024-09-14 19:12 UTC (permalink / raw)
  To: Rutherther, 72333
  Cc: Alvin Hsu, aurtzy, Simon Tournier, Tomas Volf, daniel szmulewicz,
	gemmaro, Richard Sent

Am Samstag, dem 14.09.2024 um 17:53 +0000 schrieb Rutherther:
> 
> From Liliana dmarie Prikler:
> > It is a bug unique to our handling in Guix – upstream uses hashes
> > to guard against it, but those break grafts.
> 
> Do we need to remove the whole hash? The path hash would definitely
> break grafts, but why should the content hash break grafts? Having
> the content hash would also solve this issue.
> 
> Here is the upstream explanation for the hash
> https://git.savannah.gnu.org/cgit/emacs.git/tree/src/comp.c?h=194a8f5c1406dd7e762376bdfde78d1b7d01b6b1#n4433
> having two parts, path and content hash.
Because the content hash also refers to stuff that we're grafting, e.g.
program names that get compiled to store paths.  It's sadly not that
easy.

Cheers




^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2024-09-14 19:15 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-28 12:41 bug#72333: Magit/Transient error message daniel szmulewicz
2024-07-30 22:20 ` Alvin Hsu
2024-07-30 22:50 ` aurtzy
2024-07-31 16:52   ` Richard Sent
2024-07-31 12:04 ` bug#72333: [PATCH] gnu: emacs-transient: Prioritise loading over built-in gemmaro
2024-07-31 17:10   ` Liliana Marie Prikler
2024-08-17  9:26     ` Tomas Volf
2024-08-17 11:21       ` Liliana Marie Prikler
     [not found] ` <87ikvias2d.fsf@gmail.com>
2024-09-02 18:55   ` bug#72333: builtin native-compiled is loaded instead of emacs-packages Simon Tournier
2024-09-02 19:00     ` Simon Tournier
2024-09-02 19:22     ` Liliana Marie Prikler
2024-09-03 15:05       ` Simon Tournier
2024-09-03 14:57 ` bug#72333: [PATCH] gnu: emacs-minimal: Do not native-compile lisp/transient.el Simon Tournier
2024-09-03 16:49   ` Liliana Marie Prikler
2024-09-03 19:47     ` Liliana Marie Prikler
2024-09-03 21:00       ` Simon Tournier
2024-09-14 17:53 ` bug#72333: Magit/Transient error message Rutherther via Bug reports for GNU Guix
2024-09-14 19:12   ` Liliana Marie Prikler

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).