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

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

[Thiago Jung Bauermann via Guix-patches via]

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

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

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

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

[Liliana Marie Prikler]

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

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

[Thiago Jung Bauermann via Guix-patches via]

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!

-- 
Thiago

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

[Maxim Cournoyer]

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?).

-- 
Thanks,
Maxim

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

[Thiago Jung Bauermann via Guix-patches via]

Hello Maxim,

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

> 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

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

[Kyle Meyer]

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.

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

[Thiago Jung Bauermann via Guix-patches via]

Hello Kyle,

Kyle Meyer <kyle@kyleam.com> writes:

> 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

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

[Maxim Cournoyer]

Hi,

Kyle Meyer <kyle@kyleam.com> writes:

> 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

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

[zimoun]

Hi Maxim,

Unrelated to this patch.

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

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


1: <https://issues.guix.gnu.org/issue/58690#msgid-ebbe0f9c555b37e7682777804a4af15497d457b3>

Cheers,
simon