[PATCH] gnu: emacs-magit: Substitute git executable path.

  • Done
  • quality assurance status badge
Details
5 participants
  • Thiago Jung Bauermann
  • Kyle Meyer
  • Liliana Marie Prikler
  • Maxim Cournoyer
  • zimoun
Owner
unassigned
Submitted by
Thiago Jung Bauermann
Severity
normal
T
T
Thiago Jung Bauermann wrote on 13 Nov 2022 22:11
(address . guix-patches@gnu.org)(name . Thiago Jung Bauermann)(address . bauermann@kolabnow.com)
20221113211111.59581-1-bauermann@kolabnow.com
Magit has a strong dependency on Git so it should directly reference the
git executable rather than expect it to be available in the profile or
environment.

This also fixes a build failure in emacs-forge.

* gnu/packages/emacs-xyz.scm (emacs-magit)[arguments]<#:phases>: Substitute
git path in the ‘magit-git-executable’ variable.
---
gnu/packages/emacs-xyz.scm | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

Toggle diff (25 lines)
diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
index 8ff6275bad96..1406f8a715f2 100644
--- a/gnu/packages/emacs-xyz.scm
+++ b/gnu/packages/emacs-xyz.scm
@@ -117,6 +117,7 @@
;;; Copyright © 2022 Jose G Perez Taveras <josegpt27@gmail.com>
;;; Copyright © 2022 Hilton Chain <hako@ultrarare.space>
;;; Copyright © 2022 Nicolas Graves <ngraves@ngraves.fr>
+;;; Copyright © 2022 Thiago Jung Bauermann <bauermann@kolabnow.com>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -1031,7 +1032,11 @@ (define-public emacs-magit
("magit-version" #$version))))
(add-after 'set-magit-version 'patch-exec-paths
(lambda* (#:key inputs #:allow-other-keys)
- (make-file-writable "lisp/magit-sequence.el")
+ (for-each make-file-writable
+ (list "lisp/magit-git.el" "lisp/magit-sequence.el"))
+ (emacs-substitute-variables "lisp/magit-git.el"
+ ("magit-git-executable"
+ (search-input-file inputs "/bin/git")))
(emacs-substitute-variables "lisp/magit-sequence.el"
("magit-perl-executable"
(search-input-file inputs "/bin/perl")))))
L
L
Liliana Marie Prikler wrote on 13 Nov 2022 22:24
2f939a4eb14313d5e2c656951ebe91bba31c8504.camel@gmail.com
Am Sonntag, dem 13.11.2022 um 18:11 -0300 schrieb Thiago Jung
Bauermann:
Toggle quote (41 lines)
> Magit has a strong dependency on Git so it should directly reference
> the
> git executable rather than expect it to be available in the profile
> or
> environment.
>
> This also fixes a build failure in emacs-forge.
>
> * gnu/packages/emacs-xyz.scm (emacs-magit)[arguments]<#:phases>:
> Substitute
> git path in the ‘magit-git-executable’ variable.
> ---
>  gnu/packages/emacs-xyz.scm | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-xyz.scm
> index 8ff6275bad96..1406f8a715f2 100644
> --- a/gnu/packages/emacs-xyz.scm
> +++ b/gnu/packages/emacs-xyz.scm
> @@ -117,6 +117,7 @@
>  ;;; Copyright © 2022 Jose G Perez Taveras <josegpt27@gmail.com>
>  ;;; Copyright © 2022 Hilton Chain <hako@ultrarare.space>
>  ;;; Copyright © 2022 Nicolas Graves <ngraves@ngraves.fr>
> +;;; Copyright © 2022 Thiago Jung Bauermann <bauermann@kolabnow.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -1031,7 +1032,11 @@ (define-public emacs-magit
>                    ("magit-version" #$version))))
>              (add-after 'set-magit-version 'patch-exec-paths
>                (lambda* (#:key inputs #:allow-other-keys)
> -                (make-file-writable "lisp/magit-sequence.el")
> +                (for-each make-file-writable
> +                          (list "lisp/magit-git.el" "lisp/magit-
> sequence.el"))
> +                (emacs-substitute-variables "lisp/magit-git.el"
> +                  ("magit-git-executable"
> +                   (search-input-file inputs "/bin/git")))
>                  (emacs-substitute-variables "lisp/magit-sequence.el"
>                    ("magit-perl-executable"
>                     (search-input-file inputs "/bin/perl")))))
LGTM, will push once I'm done with some other stuff.
L
L
Liliana Marie Prikler wrote on 13 Nov 2022 22:55
93a8d2f90b5b25e8bfce44b114015a50ec091ec3.camel@gmail.com
Am Sonntag, dem 13.11.2022 um 22:24 +0100 schrieb Liliana Marie
Prikler:
Toggle quote (48 lines)
> Am Sonntag, dem 13.11.2022 um 18:11 -0300 schrieb Thiago Jung
> Bauermann:
> > Magit has a strong dependency on Git so it should directly
> > reference
> > the
> > git executable rather than expect it to be available in the profile
> > or
> > environment.
> >
> > This also fixes a build failure in emacs-forge.
> >
> > * gnu/packages/emacs-xyz.scm (emacs-magit)[arguments]<#:phases>:
> > Substitute
> > git path in the ‘magit-git-executable’ variable.
> > ---
> >  gnu/packages/emacs-xyz.scm | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/gnu/packages/emacs-xyz.scm b/gnu/packages/emacs-
> > xyz.scm
> > index 8ff6275bad96..1406f8a715f2 100644
> > --- a/gnu/packages/emacs-xyz.scm
> > +++ b/gnu/packages/emacs-xyz.scm
> > @@ -117,6 +117,7 @@
> >  ;;; Copyright © 2022 Jose G Perez Taveras <josegpt27@gmail.com>
> >  ;;; Copyright © 2022 Hilton Chain <hako@ultrarare.space>
> >  ;;; Copyright © 2022 Nicolas Graves <ngraves@ngraves.fr>
> > +;;; Copyright © 2022 Thiago Jung Bauermann
> > <bauermann@kolabnow.com>
> >  ;;;
> >  ;;; This file is part of GNU Guix.
> >  ;;;
> > @@ -1031,7 +1032,11 @@ (define-public emacs-magit
> >                    ("magit-version" #$version))))
> >              (add-after 'set-magit-version 'patch-exec-paths
> >                (lambda* (#:key inputs #:allow-other-keys)
> > -                (make-file-writable "lisp/magit-sequence.el")
> > +                (for-each make-file-writable
> > +                          (list "lisp/magit-git.el" "lisp/magit-
> > sequence.el"))
> > +                (emacs-substitute-variables "lisp/magit-git.el"
> > +                  ("magit-git-executable"
> > +                   (search-input-file inputs "/bin/git")))
> >                  (emacs-substitute-variables "lisp/magit-
> > sequence.el"
> >                    ("magit-perl-executable"
> >                     (search-input-file inputs "/bin/perl")))))
> LGTM, will push once I'm done with some other stuff.
Aaaand it's done.
Closed
T
T
Thiago Jung Bauermann wrote on 13 Nov 2022 23:21
(name . Liliana Marie Prikler)(address . liliana.prikler@gmail.com)(address . 59253-done@debbugs.gnu.org)
87r0y6pkac.fsf@kolabnow.com
Hello Liliana,

Liliana Marie Prikler <liliana.prikler@gmail.com> writes:

Toggle quote (19 lines)
> Am Sonntag, dem 13.11.2022 um 22:24 +0100 schrieb Liliana Marie
> Prikler:
>> Am Sonntag, dem 13.11.2022 um 18:11 -0300 schrieb Thiago Jung
>> Bauermann:
>> > Magit has a strong dependency on Git so it should directly
>> > reference
>> > the
>> > git executable rather than expect it to be available in the profile
>> > or
>> > environment.
>> >
>> > This also fixes a build failure in emacs-forge.
>> >
>> > * gnu/packages/emacs-xyz.scm (emacs-magit)[arguments]<#:phases>:
>> > Substitute
>> > git path in the ‘magit-git-executable’ variable.
>> LGTM, will push once I'm done with some other stuff.
> Aaaand it's done.

Wow, that was quick. Thank you!

--
Thiago
Closed
M
M
Maxim Cournoyer wrote on 14 Nov 2022 04:01
(name . Thiago Jung Bauermann via Guix-patches via)(address . guix-patches@gnu.org)
87y1seqlwm.fsf@gmail.com
Hi,

Thiago Jung Bauermann via Guix-patches via <guix-patches@gnu.org>
writes:

Toggle quote (25 lines)
> Hello Liliana,
>
> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>
>> Am Sonntag, dem 13.11.2022 um 22:24 +0100 schrieb Liliana Marie
>> Prikler:
>>> Am Sonntag, dem 13.11.2022 um 18:11 -0300 schrieb Thiago Jung
>>> Bauermann:
>>> > Magit has a strong dependency on Git so it should directly
>>> > reference
>>> > the
>>> > git executable rather than expect it to be available in the profile
>>> > or
>>> > environment.
>>> >
>>> > This also fixes a build failure in emacs-forge.
>>> >
>>> > * gnu/packages/emacs-xyz.scm (emacs-magit)[arguments]<#:phases>:
>>> > Substitute
>>> > git path in the ‘magit-git-executable’ variable.
>>> LGTM, will push once I'm done with some other stuff.
>> Aaaand it's done.
>
> Wow, that was quick. Thank you!

Won't this make magit unusable from TRAMP (which already seems broken
for me since the time we enabled native comp -- anyone else?).

--
Thanks,
Maxim
Closed
T
T
Thiago Jung Bauermann wrote on 14 Nov 2022 04:09
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
87h6z2p6jc.fsf@kolabnow.com
Hello Maxim,

Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:

Toggle quote (33 lines)
> Hi,
>
> Thiago Jung Bauermann via Guix-patches via <guix-patches@gnu.org>
> writes:
>
>> Hello Liliana,
>>
>> Liliana Marie Prikler <liliana.prikler@gmail.com> writes:
>>
>>> Am Sonntag, dem 13.11.2022 um 22:24 +0100 schrieb Liliana Marie
>>> Prikler:
>>>> Am Sonntag, dem 13.11.2022 um 18:11 -0300 schrieb Thiago Jung
>>>> Bauermann:
>>>> > Magit has a strong dependency on Git so it should directly
>>>> > reference
>>>> > the
>>>> > git executable rather than expect it to be available in the profile
>>>> > or
>>>> > environment.
>>>> >
>>>> > This also fixes a build failure in emacs-forge.
>>>> >
>>>> > * gnu/packages/emacs-xyz.scm (emacs-magit)[arguments]<#:phases>:
>>>> > Substitute
>>>> > git path in the ‘magit-git-executable’ variable.
>>>> LGTM, will push once I'm done with some other stuff.
>>> Aaaand it's done.
>>
>> Wow, that was quick. Thank you!
>
> Won't this make magit unusable from TRAMP (which already seems broken
> for me since the time we enabled native comp -- anyone else?).

Hm, interesting point. I don't know. I don't use TRAMP because it was
too slow for me in the couple of times I've tried.

magit/lisp/magit-git.el does have this custom variable though, which
this patch didn't change:

#+BEGIN_SRC elisp
(defcustom magit-remote-git-executable "git"
"The Git executable used by Magit on remote machines.
On the local host `magit-git-executable' is used instead.
Consider customizing `tramp-remote-path' instead of this
option."
:package-version '(magit . "3.2.0")
:group 'magit-process
:type 'string)
#+END_SRC elisp

So I'd say it's possible that my patch doesn't affect whether Magit
works from TRAMP…

--
Thanks
Thiago
Closed
K
K
Kyle Meyer wrote on 15 Nov 2022 01:52
(name . Thiago Jung Bauermann)(address . bauermann@kolabnow.com)
87cz9pvy23.fsf@kyleam.com
Thiago Jung Bauermann via Guix-patches via writes:

Toggle quote (1 lines)
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
[...]
Toggle quote (23 lines)
>> Won't this make magit unusable from TRAMP (which already seems broken
>> for me since the time we enabled native comp -- anyone else?).
>
> Hm, interesting point. I don't know. I don't use TRAMP because it was
> too slow for me in the couple of times I've tried.
>
> magit/lisp/magit-git.el does have this custom variable though, which
> this patch didn't change:
>
> #+BEGIN_SRC elisp
> (defcustom magit-remote-git-executable "git"
> "The Git executable used by Magit on remote machines.
> On the local host `magit-git-executable' is used instead.
> Consider customizing `tramp-remote-path' instead of this
> option."
> :package-version '(magit . "3.2.0")
> :group 'magit-process
> :type 'string)
> #+END_SRC elisp
>
> So I'd say it's possible that my patch doesn't affect whether Magit
> works from TRAMP…

That's right. magit-remote-git-executable is the executable that will
be used when operating remotely via TRAMP, so your patch shouldn't
affect it (and if it does, it's unintended behavior on Magit's end).

Note that, as the :package-version value suggests, this split happened
in a relatively recent release, so previous some previous discussions on
the list (e.g., bug#30434) are now out of date.
T
T
Thiago Jung Bauermann wrote on 15 Nov 2022 05:30
(name . Kyle Meyer)(address . kyle@kyleam.com)
87cz9oq1nb.fsf@kolabnow.com
Hello Kyle,

Kyle Meyer <kyle@kyleam.com> writes:

Toggle quote (35 lines)
> Thiago Jung Bauermann via Guix-patches via writes:
>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
> [...]
>>> Won't this make magit unusable from TRAMP (which already seems broken
>>> for me since the time we enabled native comp -- anyone else?).
>>
>> Hm, interesting point. I don't know. I don't use TRAMP because it was
>> too slow for me in the couple of times I've tried.
>>
>> magit/lisp/magit-git.el does have this custom variable though, which
>> this patch didn't change:
>>
>> #+BEGIN_SRC elisp
>> (defcustom magit-remote-git-executable "git"
>> "The Git executable used by Magit on remote machines.
>> On the local host `magit-git-executable' is used instead.
>> Consider customizing `tramp-remote-path' instead of this
>> option."
>> :package-version '(magit . "3.2.0")
>> :group 'magit-process
>> :type 'string)
>> #+END_SRC elisp
>>
>> So I'd say it's possible that my patch doesn't affect whether Magit
>> works from TRAMP…
>
> That's right. magit-remote-git-executable is the executable that will
> be used when operating remotely via TRAMP, so your patch shouldn't
> affect it (and if it does, it's unintended behavior on Magit's end).
>
> Note that, as the :package-version value suggests, this split happened
> in a relatively recent release, so previous some previous discussions on
> the list (e.g., bug#30434) are now out of date.

That's great! Thank you for the clarification.

--
Thanks
Thiago
M
M
Maxim Cournoyer wrote on 15 Nov 2022 14:49
(name . Kyle Meyer)(address . kyle@kyleam.com)
87tu30pbu9.fsf@gmail.com
Hi,

Kyle Meyer <kyle@kyleam.com> writes:

Toggle quote (35 lines)
> Thiago Jung Bauermann via Guix-patches via writes:
>
>> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
> [...]
>>> Won't this make magit unusable from TRAMP (which already seems broken
>>> for me since the time we enabled native comp -- anyone else?).
>>
>> Hm, interesting point. I don't know. I don't use TRAMP because it was
>> too slow for me in the couple of times I've tried.
>>
>> magit/lisp/magit-git.el does have this custom variable though, which
>> this patch didn't change:
>>
>> #+BEGIN_SRC elisp
>> (defcustom magit-remote-git-executable "git"
>> "The Git executable used by Magit on remote machines.
>> On the local host `magit-git-executable' is used instead.
>> Consider customizing `tramp-remote-path' instead of this
>> option."
>> :package-version '(magit . "3.2.0")
>> :group 'magit-process
>> :type 'string)
>> #+END_SRC elisp
>>
>> So I'd say it's possible that my patch doesn't affect whether Magit
>> works from TRAMP…
>
> That's right. magit-remote-git-executable is the executable that will
> be used when operating remotely via TRAMP, so your patch shouldn't
> affect it (and if it does, it's unintended behavior on Magit's end).
>
> Note that, as the :package-version value suggests, this split happened
> in a relatively recent release, so previous some previous discussions on
> the list (e.g., bug#30434) are now out of date.

Excellent. Thanks for the explanation!

--
Thanks,
Maxim
Z
Z
zimoun wrote on 15 Nov 2022 09:52
86v8ngioqp.fsf@gmail.com
Hi Maxim,

Unrelated to this patch.

On Sun, 13 Nov 2022 at 22:01, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

Toggle quote (3 lines)
> Won't this make magit unusable from TRAMP (which already seems broken
> for me since the time we enabled native comp -- anyone else?).

For TRAMP precisely, I do not know, but ESS has troubles with TRAMP, see bug#58690.



Cheers,
simon
?