hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Simon Tournier]

Hi,

On Mon, 11 Sep 2023 at 16:23, Ludovic Courtès <ludo@gnu.org> wrote:

> Note that the patch series adds a hard dependency on Git.
> This is because the existing ‘git-fetch’ code depends on Git,
> which is itself motivated by the fact that Git supports
> shallow clones and libgit2/Guile-Git doesn’t.

Going this path, I appears to me worth to revisit the proposal:

        RFC: libgit2 is slow/inefficient; switch to git command?
        Maxim Cournoyer <maxim.cournoyer@gmail.com>
        Mon, 21 Nov 2022 21:21:02 -0500
        id:87cz9fpw4x.fsf@gmail.com
        https://yhetil.org/guix/87cz9fpw4x.fsf@gmail.com
        https://lists.gnu.org/archive/html/guix-devel/2022-11

I know it is not an option for now to parse the output of ’git’ commands
in order to keep the features of (guix git), (guix channels), etc.

However, this discussion was mentioning an implementation of
clone/checkout in pure Racket supporting shallow checkout.  Considering
the current level of integration, I thought the next Big Plan™ was to
gradually move bits of Guile-Git to being pure Scheme, maybe based on
the Racket implementation of ’clone’ as a starting point.

Personally, I do not have a strong opinion about the Big Plan™.  I note
that the introduction of Git as a hard dependency is a slippery slope
considering the current state of libgit2.  Here, it starts with “git
clone”, then “git gc” (unsupported by libgit2) is also in the pipes
(#65720 [1]).  And after timing, I am almost sure that many operations
using Guile-Git will be slower than their plain Git counter-parts.  And
we will start to parse the output of ’git’ plumbing commands.  Slippery
slope for pushing Guile-Git out, no?

And I do not speak about the closure.  Is it possible to extract the
command ’git-clone’ from git-minimal?  It would reduce the size, no?


Cheers,
simon

1: https://issues.guix.gnu.org/65720

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[wolf]

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

On 2023-09-11 17:17:24 +0200, Simon Tournier wrote:
> Hi,
> 
> On Mon, 11 Sep 2023 at 16:23, Ludovic Courtès <ludo@gnu.org> wrote:
> 
> > Note that the patch series adds a hard dependency on Git.
> > This is because the existing ‘git-fetch’ code depends on Git,
> > which is itself motivated by the fact that Git supports
> > shallow clones and libgit2/Guile-Git doesn’t.
> 
> Going this path, I appears to me worth to revisit the proposal:
> 
>         RFC: libgit2 is slow/inefficient; switch to git command?
>         Maxim Cournoyer <maxim.cournoyer@gmail.com>
>         Mon, 21 Nov 2022 21:21:02 -0500
>         id:87cz9fpw4x.fsf@gmail.com
>         https://yhetil.org/guix/87cz9fpw4x.fsf@gmail.com
>         https://lists.gnu.org/archive/html/guix-devel/2022-11
> 
> I know it is not an option for now to parse the output of ’git’ commands
> in order to keep the features of (guix git), (guix channels), etc.
> 
> However, this discussion was mentioning an implementation of
> clone/checkout in pure Racket supporting shallow checkout.  Considering
> the current level of integration, I thought the next Big Plan™ was to
> gradually move bits of Guile-Git to being pure Scheme, maybe based on
> the Racket implementation of ’clone’ as a starting point.
> 
> Personally, I do not have a strong opinion about the Big Plan™.  I note
> that the introduction of Git as a hard dependency is a slippery slope
> considering the current state of libgit2.  Here, it starts with “git
> clone”, then “git gc” (unsupported by libgit2) is also in the pipes
> (#65720 [1]).  And after timing, I am almost sure that many operations
> using Guile-Git will be slower than their plain Git counter-parts.  And
> we will start to parse the output of ’git’ plumbing commands.

If you don't mind me asking, why is that so problematic approach?  Git's
plumbing commands are intended to be used in scripts, so I am unsure what the
problem is.

I cannot recall a single instance when some tooling I have at home or wrote for
work stopped working due to git changing.  I guess there likely are such
instances, but would be interested in examples if someone has a list.

> Slippery slope for pushing Guile-Git out, no?

Guile-git does not seem to be in the best place currently, when I was putting
together the script I sent to #65720, I tried to use the info manual, and the
best way to describe it is "incomplete".  Some behaviors are... surprising.

Arguably that is the impression I got based on one morning of trying to use it,
so it is probably inaccurate description and lacks some details.

> 
> And I do not speak about the closure.  Is it possible to extract the
> command ’git-clone’ from git-minimal?  It would reduce the size, no?
> 
> 
> Cheers,
> simon
> 
> 1: https://issues.guix.gnu.org/65720
> 

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

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Maxim Cournoyer]

Hi wolf,

wolf <wolf@wolfsden.cz> writes:

[...]

>> Personally, I do not have a strong opinion about the Big Plan™.  I note
>> that the introduction of Git as a hard dependency is a slippery slope
>> considering the current state of libgit2.  Here, it starts with “git
>> clone”, then “git gc” (unsupported by libgit2) is also in the pipes
>> (#65720 [1]).  And after timing, I am almost sure that many operations
>> using Guile-Git will be slower than their plain Git counter-parts.  And
>> we will start to parse the output of ’git’ plumbing commands.
>
> If you don't mind me asking, why is that so problematic approach?  Git's
> plumbing commands are intended to be used in scripts, so I am unsure what the
> problem is.

In the grand scheme of things (pun intended), we'd like every
programming to be feasible via nice Scheme APIs, which is what Guile-Git
provides to work with git repositories.  The appeal is to have a single
language to rule them all, reducing friction among Guix contributors.
The alternative here is to have an API reduced to invoking system
commands with string arguments, which is less expressive and lacks
elegance.

-- 
Thanks,
Maxim

comparing commit-relation using Scheme+libgit2 vs shellout plumbing Git

[Simon Tournier]

Hi,

On Mon, 11 Sep 2023 at 14:26, Maxim Cournoyer <maxim.cournoyer@gmail.com> wrote:

> In the grand scheme of things (pun intended), we'd like every
> programming to be feasible via nice Scheme APIs, which is what Guile-Git
> provides to work with git repositories.  The appeal is to have a single
> language to rule them all, reducing friction among Guix contributors.
> The alternative here is to have an API reduced to invoking system
> commands with string arguments, which is less expressive and lacks
> elegance.

As Maxim noticed in the message that I am proposing to revisit, it seems
that libgit2 comes with some performance penalties.  As wolf is
illustrating in the message:

        bug#65720: Guile-Git-managed checkouts grow way too much
        wolf <wolf@wolfsden.cz>
        Mon, 11 Sep 2023 16:42:59 +0200
        id:ZP8nc1m8rN_34XV-@ws
        https://issues.guix.gnu.org//65720
        https://issues.guix.gnu.org/msgid/ZP8nc1m8rN_34XV-@ws
        https://yhetil.org/guix/ZP8nc1m8rN_34XV-@ws

it might be possible to use an invocation of plain Git command which is
much faster in this case.  Well, that’s need to be investigated, IMHO.

For instance, instead of the current ’commit-relation’ implementation,

        (define (commit-relation old new)
          "Return a symbol denoting the relation between OLD and NEW, two commit
        objects: 'ancestor (meaning that OLD is an ancestor of NEW), 'descendant, or
        'unrelated, or 'self (OLD and NEW are the same commit)."
          (if (eq? old new)
              'self
              (let ((newest (commit-closure new)))
                (if (set-contains? newest old)
                    'ancestor
                    (let* ((seen   (list->setq (commit-parents new)))
                           (oldest (commit-closure old seen)))
                      (if (set-contains? oldest new)
                          'descendant
                          'unrelated))))))

which relies on ’commit-closure’, they propose to use a plumbing Git
commands, as:

        (define (shelling-commit-relation old new)
          (let ((h-old (oid->string (commit-id old)))
                (h-new (oid->string (commit-id new))))
            (cond ((eq? old new)
                   'self)
                  ((zero? (git-C %repo "merge-base" "--is-ancestor" h-old h-new))
                   'ancestor)
                  ((zero? (git-C %repo "merge-base" "--is-ancestor" h-new h-old))
                   'descendant)
                  (else
                   'unrelated))))

Well, this needs to be checked (read the Git documentation which is
probable harder than read some Scheme implementation ;-)) in order to
see if these invocations are doing the same.


>> I’m quite confident this would be slow
>
> My version is ~2000x faster compared to (guix git):
>
>     Guix: 1048.620992ms
>     Git:  0.532143ms

On my machine, I get something less spectacular for a history with 1000
commits in between.

--8<---------------cut here---------------start------------->8---
scheme@(guix-user)> ,time (commit-relation* 1000th newest)
$1 = ancestor
;; 0.128948s real time, 0.082921s run time.  0.046578s spent in GC.
scheme@(guix-user)> ,time (commit-relation 1000th newest)
$2 = ancestor
;; 4.588075s real time, 5.521358s run time.  1.404764s spent in GC.
--8<---------------cut here---------------end--------------->8---

I did something very similar as wolf is proposing and named it
’commit-relation*’.

Well, considering the implementation of ’commit-relation’, I think the
slowness is expected compared to the plain plumbing Git command.
Basically, ’commit-closure’ walks the Git history and for sure the loop
cannot be as efficient as an optimized Git specific implementation.

Hum, I think the most annoying is the time spent in GC.  Basically,
’commit-closure’ is building a set with many visited elements and that
set must be garbage collected.  And this GC time is not nothing compared
to the whole time, IMHO.

I agree with the grand scheme of things and that’s why I started this
thread. :-) However, for what it is worth, today I am less convinced
that manipulating libgit2 is able to provide “not-so-worse” performance
compared to what plain plumbing Git commands could offer.

Cheers,
simon

Re: comparing commit-relation using Scheme+libgit2 vs shellout plumbing Git

[Attila Lendvai]

is the decision between libgit2 and invoking git really such a big commitment?

let's make sure the entire guix codebase uses a single git related API, and then we can easily switch back and forth between the two.

on another note, i'm surprised that the reference implementation of git itself doesn't have a lib, and libgit2 even had to be written. even this may change in the future.

-- 
• attila lendvai
• PGP: 963F 5D5F 45C7 DFCD 0A39
--
“I learned long ago, never to wrestle with a pig. You get dirty, and besides, the pig likes it.”
	— George Bernard Shaw (1856–1950)

Re: comparing commit-relation using Scheme+libgit2 vs shellout plumbing Git

[Ludovic Courtès]

Simon Tournier <zimon.toutoune@gmail.com> skribis:

> On my machine, I get something less spectacular for a history with 1000
> commits in between.
>
> scheme@(guix-user)> ,time (commit-relation* 1000th newest)
> $1 = ancestor
> ;; 0.128948s real time, 0.082921s run time.  0.046578s spent in GC.
> scheme@(guix-user)> ,time (commit-relation 1000th newest)
> $2 = ancestor
> ;; 4.588075s real time, 5.521358s run time.  1.404764s spent in GC.
>
> I did something very similar as wolf is proposing and named it
> ’commit-relation*’.

That’s an order of magnitude.  Probably it could be a bit less if we put
some effort in it (‘commit-relation’ is implemented in a fairly naive
way.)

That said, ‘commit-relation’ is just one example.  I’d encourage
interested people to look at (guix git-authenticate) to get a feel of
what we need.  Most of it is quite pedestrian, like
‘load-keyring-from-reference’ or ‘commit-signing-key’, but I don’t think
we can get a decent throughput if we shell out for all these things
(assuming ‘git’ can even give us raw data).

Ludo’.

Re: comparing commit-relation using Scheme+libgit2 vs shellout plumbing Git

[Simon Tournier]

Hi Ludo,

On Thu, 14 Sep 2023 at 12:30, Ludovic Courtès <ludo@gnu.org> wrote:

>                                                        but I don’t think
> we can get a decent throughput if we shell out for all these things
> (assuming ‘git’ can even give us raw data).

Do you consider that Magit does not have a decent throughput?
Do you consider that Git-Annex does not have a decent throughput?

To my knowledge, they shell out Git plumbing commands; one using Emacs
Lisp and the other Haskell.

And some porcelain Git commands that we all are using daily are Bash
scripts calling plumbing Git commands that shell out.  (Or were Bash
scripts before being replaced by C builtin).

For example, git-rebase, git-pull, git-log, etc.

https://github.com/git/git/commit/55071ea248ef8040e4b29575376273e4dd061683
https://github.com/git/git/commit/b1456605c26eb6bd991b70b0ca0a3ce0f02473e9
https://github.com/git/git/commit/e3a125a94d34d22a8ca53e84949a1bb38cd6e425

The task is probably complex and boring, I agree.  However, I am not
convinced the issue is about a “decent throughput”.  The best would be
to have the same performance using libgit2 and using plumbing Git
command for one example: say ’commit-relation’.  Or another one. :-)

Otherwise, I believe what I am seeing. ;-)

Cheers,
simon

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Simon Tournier]

Re,

On Mon, 11 Sep 2023 at 17:17, Simon Tournier <zimon.toutoune@gmail.com> wrote:
> On Mon, 11 Sep 2023 at 16:23, Ludovic Courtès <ludo@gnu.org> wrote:
>
>> Note that the patch series adds a hard dependency on Git.
>> This is because the existing ‘git-fetch’ code depends on Git,
>> which is itself motivated by the fact that Git supports
>> shallow clones and libgit2/Guile-Git doesn’t.

[...]

> Personally, I do not have a strong opinion about the Big Plan™.  I note
> that the introduction of Git as a hard dependency is a slippery slope
> considering the current state of libgit2.  Here, it starts with “git
> clone”, then “git gc” (unsupported by libgit2) is also in the pipes
> (#65720 [1]).  And after timing, I am almost sure that many operations
> using Guile-Git will be slower than their plain Git counter-parts.  And
> we will start to parse the output of ’git’ plumbing commands.  Slippery
> slope for pushing Guile-Git out, no?

For example, having plain Git command with shallow clone would allow to
save resource when cloning the first time the Guix checkout cache.

Compare,

--8<---------------cut here---------------start------------->8---
$ git clone https://git.savannah.gnu.org/git/guix.git guix-full
Cloning into 'guix-full'...                                                                                                                                                                
remote: Counting objects: 696917, done.                                                                                                                                                    
remote: Compressing objects: 100% (143179/143179), done.                                                                                                                                   
remote: Total 696917 (delta 552872), reused 696909 (delta 552867)                                                                                                                          
Receiving objects: 100% (696917/696917), 347.14 MiB | 29.31 MiB/s, done.                                                                                                                   
Resolving deltas: 100% (552872/552872), done.
--8<---------------cut here---------------end--------------->8---

and,

--8<---------------cut here---------------start------------->8---
$ git clone --shallow-since=2019-04-30 https://git.savannah.gnu.org/git/guix.git guix-oldest
Cloning into 'guix-oldest'...
remote: Counting objects: 426879, done.
remote: Compressing objects: 100% (87246/87246), done.
remote: Total 426879 (delta 342111), reused 423970 (delta 339518)
Receiving objects: 100% (426879/426879), 259.75 MiB | 11.26 MiB/s, done.
Resolving deltas: 100% (342111/342111), done.
Checking connectivity: 426863, done.
--8<---------------cut here---------------end--------------->8---

(Here, 2019-04-30 is the date that contains the %oldest-possible-commit
"6298c3ffd9654d3231a6f25390b056483e8f407c" v1.0.)

Well, ’shallow’ probably implies an overload on server side.  But that
is far less than the bits it saves: 87.39 MiB (= 347.14 - 259.75).  It
saves something like 25% to download, if I read correctly.  I let you do
some maths for the improvement you will get. :-)

And there is no integration with Guile-Git, if I read correctly.  The
job is done by the procedure ’clone*’ from the module (guix git).  This
procedure call the Guile-Git procedure ’clone’ which could drop-in
replaced by (invoke git-command "clone --shallow-since=2019-04-30" …).


Cheers,
simon

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Maxim Cournoyer]

Hi,

Simon Tournier <zimon.toutoune@gmail.com> writes:

> Hi,
>
> On Mon, 11 Sep 2023 at 16:23, Ludovic Courtès <ludo@gnu.org> wrote:
>
>> Note that the patch series adds a hard dependency on Git.
>> This is because the existing ‘git-fetch’ code depends on Git,
>> which is itself motivated by the fact that Git supports
>> shallow clones and libgit2/Guile-Git doesn’t.

That's no longer true, as of libgit2 v1.7.0 [0].  I already have it
packaged in a branch, but I need to iron out dependent breakages.

[0]  https://github.com/libgit2/libgit2/releases/tag/v1.7.0

So given there's no technical reasons not to use libgit2, I'd use that
and keep the closure size down.

-- 
Thanks,
Maxim

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Josselin Poiret]

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

Hi,

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

> That's no longer true, as of libgit2 v1.7.0 [0].  I already have it
> packaged in a branch, but I need to iron out dependent breakages.
>
> [0]  https://github.com/libgit2/libgit2/releases/tag/v1.7.0
>
> So given there's no technical reasons not to use libgit2, I'd use that
> and keep the closure size down.

There's still the `git gc` problem though.

My opinion is that the preferred API for Git is still the UNIX one via
plumbing commands.  Anything else is trying to catch up to it, and then
we get into this conundrum that we want to do everything in Scheme, but
we're unable to do it as well as Git itself.  If I had to choose, a
Guile library wrapping the Git commands would be the best, especially
since we're managing long-living checkouts, something libgit2 doen't
seem too interested in.

Best,
-- 
Josselin Poiret

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

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Maxim Cournoyer]

Hi,

Josselin Poiret <dev@jpoiret.xyz> writes:

> Hi,
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
>
>> That's no longer true, as of libgit2 v1.7.0 [0].  I already have it
>> packaged in a branch, but I need to iron out dependent breakages.
>>
>> [0]  https://github.com/libgit2/libgit2/releases/tag/v1.7.0
>>
>> So given there's no technical reasons not to use libgit2, I'd use that
>> and keep the closure size down.
>
> There's still the `git gc` problem though.

It's klunky, but a workaround is to locally clone the checkout anew
using libgit2, as suggested here [0].  We could also try to contribute
to libgit2 toward adding proper support for a 'gc' action.

[0]  https://github.com/libgit2/libgit2/issues/3247#issuecomment-486585944

-- 
Thanks,
Maxim

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[wolf]

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

On 2023-09-12 08:56:15 -0400, Maxim Cournoyer wrote:
> Hi,
> 
> Josselin Poiret <dev@jpoiret.xyz> writes:
> 
> > Hi,
> >
> > Maxim Cournoyer <maxim.cournoyer@gmail.com> writes:
> >
> >> That's no longer true, as of libgit2 v1.7.0 [0].  I already have it
> >> packaged in a branch, but I need to iron out dependent breakages.
> >>
> >> [0]  https://github.com/libgit2/libgit2/releases/tag/v1.7.0
> >>
> >> So given there's no technical reasons not to use libgit2, I'd use that
> >> and keep the closure size down.
> >
> > There's still the `git gc` problem though.
> 
> It's klunky, but a workaround is to locally clone the checkout anew using
> libgit2, as suggested here [0].

While the comment does suggest a workaround, I would recommend to read the
comment right below it before using it.  I am not sure if Guix would be affected
by those mentioned concurrency issues, but sounds like something that needs to
be understood in detail.

> We could also try to contribute to libgit2 toward adding proper support for a
> 'gc' action.
> 
> [0]  https://github.com/libgit2/libgit2/issues/3247#issuecomment-486585944
> 
> -- 
> Thanks,
> Maxim
> 

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

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Ludovic Courtès]

Hi,

Josselin Poiret <dev@jpoiret.xyz> skribis:

> My opinion is that the preferred API for Git is still the UNIX one via
> plumbing commands.  Anything else is trying to catch up to it, and then
> we get into this conundrum that we want to do everything in Scheme, but
> we're unable to do it as well as Git itself.  If I had to choose, a
> Guile library wrapping the Git commands would be the best, especially
> since we're managing long-living checkouts, something libgit2 doen't
> seem too interested in.

I have mixed feelings here.  Clearly, I don’t think a Unix command can
ever be as rich and efficient as a “proper library”.

Are alternative Git implementations doomed to always try to catch up?
My intuition would be “no”, because not so much changes in Git as an
on-disk format and protocol.

There is one big change coming up though: SHA256 support (now officially
supported in Git).  Is it being discussed in libgit2?

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

> Josselin Poiret <dev@jpoiret.xyz> writes:

[...]

>> There's still the `git gc` problem though.
>
> It's klunky, but a workaround is to locally clone the checkout anew
> using libgit2, as suggested here [0].
>
> [0]  https://github.com/libgit2/libgit2/issues/3247#issuecomment-486585944

That doesn’t work, at least with libgit2 1.3.2:

  https://issues.guix.gnu.org/65720#7

> We could also try to contribute to libgit2 toward adding proper
> support for a 'gc' action.

I share this sentiment: if we’re gonna depend on it, we’ve gotta invest
in it.  We’re benefiting from it so we shouldn’t be mere consumers.

I have to admit I don’t see myself doing it right now, but I would
definitely encourage others to do so.

Now, as a corollary to what I wrote above: if we don’t invest in it, we
should be prepared to drop it.

Ludo’.

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Ludovic Courtès]

Hi!

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

> So given there's no technical reasons not to use libgit2, I'd use that
> and keep the closure size down.

For the record, that’s a 6% increase:

--8<---------------cut here---------------start------------->8---
$ guix size guix | tail -1
total: 633.0 MiB
$ guix size guix git-minimal | tail -1
total: 675.7 MiB
--8<---------------cut here---------------end--------------->8---

(Of course it all adds up; I’m not saying we can dismiss it.)

In the context of <https://issues.guix.gnu.org/65866> plus the lack of
GC in libgit2 discussed in <https://issues.guix.gnu.org/65720>, my
inclination is to include that hard dependency on Git.

That’s not a happy choice for me, but it has the advantage of solving
two immediate problems.

I would revisit it as soon as libgit2 supports shallow clones (which is
coming, as you write) and GC (or a workaround to that effect).  SHA256
may also soon be a requirement: we’ll need to be able to clone repos
that use it.

How does that sound?

Ludo’.

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Simon Tournier]

Hi,

On Thu, 14 Sept 2023 at 18:51, Ludovic Courtès <ludo@gnu.org> wrote:

> For the record, that’s a 6% increase:
>
> --8<---------------cut here---------------start------------->8---
> $ guix size guix | tail -1
> total: 633.0 MiB
> $ guix size guix git-minimal | tail -1
> total: 675.7 MiB
> --8<---------------cut here---------------end--------------->8---
>
> (Of course it all adds up; I’m not saying we can dismiss it.)
>
> In the context of <https://issues.guix.gnu.org/65866> plus the lack of
> GC in libgit2 discussed in <https://issues.guix.gnu.org/65720>, my
> inclination is to include that hard dependency on Git.

And considering bug#65924 [1], it is not 6% but more.  Because
currently git-minimal is broken and coreutils and potentially
util-linux would also be part of the closure.

--8<---------------cut here---------------start------------->8---
$ guix size guix | tail -1
total: 633.0 MiB
$ guix size guix git-minimal coreutils | tail -1
total: 692.7 MiB
$ guix size guix git-minimal coreutils util-linux | tail -1
total: 706.6 MiB
--8<---------------cut here---------------end--------------->8---

Therefore. it is 9.4% or worse 11.6%.

1: bug#65924: git searches coreutils and util-linux commands in PATH
Maxim Cournoyer <maxim.cournoyer@gmail.com>
Wed, 13 Sep 2023 14:00:09 -0400
id:87fs3iuf6e.fsf@gmail.com
https://yhetil.org/guix/87fs3iuf6e.fsf@gmail.com
https://issues.guix.gnu.org/msgid/87fs3iuf6e.fsf@gmail.com


> That’s not a happy choice for me, but it has the advantage of solving
> two immediate problems.

Three. :-)

Once git-minimal and plumbing Git commands around, some slow
procedures using libgit2 will be replaced by faster ones.  And I also
have in mind some Git repository normalization that differs from SWH;
having plain Git commands would also easy that part.

Just to point that the introduction of git-minimal as hard dependency
of Guix means that libgit2 will be slowly removed from the picture,
IMHO.

Cheers,
simon

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Maxim Cournoyer]

Hi Ludovic,

Ludovic Courtès <ludo@gnu.org> writes:

> Hi!
>
> Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
>
>> So given there's no technical reasons not to use libgit2, I'd use that
>> and keep the closure size down.
>
> For the record, that’s a 6% increase:
>
> $ guix size guix | tail -1
> total: 633.0 MiB
> $ guix size guix git-minimal | tail -1
> total: 675.7 MiB
>
> (Of course it all adds up; I’m not saying we can dismiss it.)

As Simon pointed out, it'd be more after wrapping 'git' with coreutils
and possible util-linux on its PATH.

> In the context of <https://issues.guix.gnu.org/65866> plus the lack of
> GC in libgit2 discussed in <https://issues.guix.gnu.org/65720>, my
> inclination is to include that hard dependency on Git.
>
> That’s not a happy choice for me, but it has the advantage of solving
> two immediate problems.
>
> I would revisit it as soon as libgit2 supports shallow clones (which is
> coming, as you write)

This isn't "coming", it's already been released :-).

> and GC (or a workaround to that effect).  SHA256 may also soon be a
> requirement: we’ll need to be able to clone repos that use it.
>
> How does that sound?

Yeah, 'git gc' is lacking from libgit2.  I'm not against adding
dependency on the real 'git' CLI, but at that point, as Simon mentioned,
I see little reason to keep libgit2 around for much longer, given it
performs worst than git CLI in every aspect.  I doubt forking processes
on GNU/Linux would cause a performance hit compared to using libgit2,
especially given how optimized git appears to be (at least compared to
libgit2).

So, I think we need to agree on the future of libgit2 in the big picture
and decide to invest in it or let it in favor of just using git.

-- 
Thanks,
Maxim

[bug#65866] hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Ludovic Courtès]

Hello!

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

> Yeah, 'git gc' is lacking from libgit2.  I'm not against adding
> dependency on the real 'git' CLI, but at that point, as Simon mentioned,
> I see little reason to keep libgit2 around for much longer, given it
> performs worst than git CLI in every aspect.  I doubt forking processes
> on GNU/Linux would cause a performance hit compared to using libgit2,
> especially given how optimized git appears to be (at least compared to
> libgit2).

As I wrote, as an example, I don’t think that there could be a practical
implementation of (guix git-authenticate) shelling out to ‘git’.

Anyhow, how about this plan:

  1. Merge <https://issues.guix.gnu.org/65866> with the hard Git
     dependency.

  2. When libgit2 1.7 with shallow clones is available in Guix, work on
     a patch to use Guile-Git for clones and evaluate it.

  3. Brainstorm on ways to address lack of GC support based on a closer
     analysis of disk usage for Guix’s cached checkouts.

Deal?

Ludo’.

PS: I don’t buy the “libgit2 will disappear from Guix” argument because
    it’s not a natural phenomenon that we’re observing but a willful
    construction.

[bug#65866] hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Simon Tournier]

Hi Ludo,

On Mon, 18 Sept 2023 at 15:56, Ludovic Courtès <ludo@gnu.org> wrote:

> Anyhow, how about this plan:
>
>   1. Merge <https://issues.guix.gnu.org/65866> with the hard Git
>      dependency.

Is #65866 fixing bug#63331 (Guile-GnuTLS/Git circular dependency) [1]?

Does the merge of #65866 lead to close #63331?  Because you wrote,

    This patch series is a first step towards getting Git out of
    derivation graphs when it’s only used to fetch source code
    (origins with ‘git-fetch’), with the goal of fixing:

so...

>   2. When libgit2 1.7 with shallow clones is available in Guix, work on
>      a patch to use Guile-Git for clones and evaluate it.

...we could also suggest to continue and have a complete fix of #63331
before merging #65866.  It avoids to introduce a hard dependency which
will be difficult to remove and let the time for this evaluation of
libgit-2.1.7, no?

For what my opinion is worth, I have nothing for introducing a hard
dependency to Git but we have to be clear that 1. once introduced it
will hard to remove, 2. we will merge patches using faster Git
plumbing equivalent implementation and so 3. it will push out
Guile-Git.


> As I wrote, as an example, I don’t think that there could be a practical
> implementation of (guix git-authenticate) shelling out to ‘git’.

[...]

> PS: I don’t buy the “libgit2 will disappear from Guix” argument because
>     it’s not a natural phenomenon that we’re observing but a willful
>     construction.

As I wrote elsewhere, Git-Annex (or Magit) are shelling out to 'git',
IIRC.  Well, personally I do not consider that Git-Annex is slow or
that Git-Annex does not implement features as complex as (guix
git-authenticate).

After reading [2],

     I cannot imagine a viable implementation of things like ‘commit-closure’
     and ‘commit-relation’ from (guix git) done by shelling out to ‘git’.
     I’m quite confident this would be slow and brittle.

wolf came 3 days later [3] with a first rough implementation for
'commit-relation' using Git plumbing which is much more faster than
the one implemented with Guile-Git.  Even, speaking specifically about
'commit-relation', I challenge whoever to beat "git merge-base
--is-ancestor"; life is risky: I bet my round of beers in Brussels in
the next Guix Days. :-)  Reading the C implementation of
"merge-base.c" [4] and following the various procedures, it appears to
me impossible to beat it; bah using Guile-Git cumulates various
penalties from talking to libgit2 to the Garbage Collector of Guile.

The question does not appear to me if you buy it or not. :-)  The
question is instead: do we merge code that uses Git plumbing shelling
out that is faster than the current implementation using Guile-Git?

Cheers,
simon

1: https://issues.guix.gnu.org/issue/63331

2: bug#65720: Guile-Git-managed checkouts grow way too much
Ludovic Courtès <ludo@gnu.org>
Fri, 08 Sep 2023 19:08:05 +0200
id:87pm2s385m.fsf@gnu.org
https://issues.guix.gnu.org//65720
https://issues.guix.gnu.org/msgid/87pm2s385m.fsf@gnu.org
https://yhetil.org/guix/87pm2s385m.fsf@gnu.org

3: bug#65720: Guile-Git-managed checkouts grow way too much
wolf <wolf@wolfsden.cz>
Mon, 11 Sep 2023 16:42:59 +0200
id:ZP8nc1m8rN_34XV-@ws
https://issues.guix.gnu.org//65720
https://issues.guix.gnu.org/msgid/ZP8nc1m8rN_34XV-@ws
https://yhetil.org/guix/ZP8nc1m8rN_34XV-@ws

4: https://github.com/git/git/blob/bda494f4043963b9ec9a1ecd4b19b7d1cd9a0518/builtin/merge-base.c#L103-L115

Re: bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts

[Ludovic Courtès]

Hi Simon,

Simon Tournier <zimon.toutoune@gmail.com> skribis:

> On Mon, 18 Sept 2023 at 15:56, Ludovic Courtès <ludo@gnu.org> wrote:
>
>> Anyhow, how about this plan:
>>
>>   1. Merge <https://issues.guix.gnu.org/65866> with the hard Git
>>      dependency.
>
> Is #65866 fixing bug#63331 (Guile-GnuTLS/Git circular dependency) [1]?

Yes, as written in the cover letter.

[...]

>>   2. When libgit2 1.7 with shallow clones is available in Guix, work on
>>      a patch to use Guile-Git for clones and evaluate it.
>
> ...we could also suggest to continue and have a complete fix of #63331
> before merging #65866.

Sorry, I don’t understand.  As I wrote in the cover letter, this patch
series is the complete fix for <https://issues.guix.gnu.org/63331>.

> It avoids to introduce a hard dependency which will be difficult to
> remove and let the time for this evaluation of libgit-2.1.7, no?

What this patch series sets in stone is “builtin:git-download” and its
semantics.

Its implementation can change over time though: it can switch to
libgit2, to OCaml-Git, or anything that pleases us.  These are
implementation details not visible from the outside.

>> As I wrote, as an example, I don’t think that there could be a practical
>> implementation of (guix git-authenticate) shelling out to ‘git’.
>
> [...]
>
>> PS: I don’t buy the “libgit2 will disappear from Guix” argument because
>>     it’s not a natural phenomenon that we’re observing but a willful
>>     construction.
>
> As I wrote elsewhere, Git-Annex (or Magit) are shelling out to 'git',
> IIRC.  Well, personally I do not consider that Git-Annex is slow or
> that Git-Annex does not implement features as complex as (guix
> git-authenticate).
>
> After reading [2],
>
>      I cannot imagine a viable implementation of things like ‘commit-closure’
>      and ‘commit-relation’ from (guix git) done by shelling out to ‘git’.
>      I’m quite confident this would be slow and brittle.
>
> wolf came 3 days later [3] with a first rough implementation for
> 'commit-relation' using Git plumbing which is much more faster than
> the one implemented with Guile-Git.

Yes, point taken.  It’s not so much about whether Git-Annex is “less
complex”, it’s about the level of integration needed.  But you don’t
have to take my word for it.

We’ve spent lots of words on the issue of a dependency on Git, and yet
this patch series doesn’t actually change much in that regard:
‘git-fetch’ already uses Git.

I suggest that we focus on the various sub-problems we’re trying to
solve without losing sight of the big picture, yet without conflating
them all.

Ludo’.

Re: bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts

[Simon Tournier]

Hi Ludo,

On Tue, 19 Sep 2023 at 16:43, Ludovic Courtès <ludo@gnu.org> wrote:

>>>   1. Merge <https://issues.guix.gnu.org/65866> with the hard Git
>>>      dependency.
>>
>> Is #65866 fixing bug#63331 (Guile-GnuTLS/Git circular dependency) [1]?
>
> Yes, as written in the cover letter.

[...]

>                            As I wrote in the cover letter, this patch
> series is the complete fix for <https://issues.guix.gnu.org/63331>.

Thanks for clarifying the cover letter:

        This patch series is a first step towards getting Git out of
        derivation graphs when it’s only used to fetch source code
        (origins with ‘git-fetch’), with the goal of fixing:

          https://issues.guix.gnu.org/63331

Because I am not native, my dictionary says, Goal: Something that is
your goal is something that you hope to achieve, especially when much
time and effort will be needed.

Sorry if, from the cover letter and my vague understanding of the code,
it was not obvious for me that merging #65866 directly close #63331.
From my understanding, #65866 was one step toward closing #63331 and not
the complete fix.

Anyway. :-)


> I suggest that we focus on the various sub-problems we’re trying to
> solve without losing sight of the big picture, yet without conflating
> them all.

The way we are trying to focus or solve these various sub-problems
depends on what we have at hand (the big picture).  Having an hard
dependency of Git means these immediate improvements by drop-in
replacements:

 + git clone => 3x faster for full Guix repository
 + shallow clone => 25% of improvements
 + git fetch => no worry much about gc
 + commit-relation => 35x faster

for an increase of the closure between 9% and 12%.  All these numbers
are for my machine and I guess they would be the order on average.

That said, I expressed my concerns about the “big picture” and
libgit2. :-)

Cheers,
simon

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Vagrant Cascadian]

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

On 2023-09-11, Simon Tournier wrote:
> On Mon, 11 Sep 2023 at 16:23, Ludovic Courtès <ludo@gnu.org> wrote:
>> Note that the patch series adds a hard dependency on Git.
>> This is because the existing ‘git-fetch’ code depends on Git,
>> which is itself motivated by the fact that Git supports
>> shallow clones and libgit2/Guile-Git doesn’t.
...
> Personally, I do not have a strong opinion about the Big Plan™.  I note
> that the introduction of Git as a hard dependency is a slippery slope
> considering the current state of libgit2.  Here, it starts with “git
> clone”, then “git gc” (unsupported by libgit2) is also in the pipes
> (#65720 [1]).

What about making git an optional dependency, and only calling out to
"git gc" if git is available in PATH? Maybe possible also with shallow
clones?

Then you have the best/worst of both worlds! Speaking to the worst, you
have at least two disparate codepaths for a seemingly similar operation,
and that might be annoying...

live well,
  vagrant

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

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Csepp]

Vagrant Cascadian <vagrant@debian.org> writes:

> [[PGP Signed Part:Undecided]]
> On 2023-09-11, Simon Tournier wrote:
>> On Mon, 11 Sep 2023 at 16:23, Ludovic Courtès <ludo@gnu.org> wrote:
>>> Note that the patch series adds a hard dependency on Git.
>>> This is because the existing ‘git-fetch’ code depends on Git,
>>> which is itself motivated by the fact that Git supports
>>> shallow clones and libgit2/Guile-Git doesn’t.
> ...
>> Personally, I do not have a strong opinion about the Big Plan™.  I note
>> that the introduction of Git as a hard dependency is a slippery slope
>> considering the current state of libgit2.  Here, it starts with “git
>> clone”, then “git gc” (unsupported by libgit2) is also in the pipes
>> (#65720 [1]).
>
> What about making git an optional dependency, and only calling out to
> "git gc" if git is available in PATH? Maybe possible also with shallow
> clones?
>
> Then you have the best/worst of both worlds! Speaking to the worst, you
> have at least two disparate codepaths for a seemingly similar operation,
> and that might be annoying...
>
> live well,
>   vagrant
>
> [[End of PGP Signed Part]]

For what it's worth, I wrote a small (incomplete) tool for some commit
analysis that used specific --format arguments that were easy to parse.
It's not especially difficult.

Re: hard dependency on Git? (was bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts)

[Simon Tournier]

Hi Vagrant,

On Mon, 11 Sep 2023 at 12:35, Vagrant Cascadian <vagrant@debian.org> wrote:

> What about making git an optional dependency, and only calling out to
> "git gc" if git is available in PATH?

Somehow, that’s more or less the case, IIUC,

--8<---------------cut here---------------start------------->8---
15 candidates:
./build/android-repo.scm:57:      (invoke git-repo-command "init" "-u" manifest-url "-b" manifest-revision
./build/android-repo.scm:59:      (invoke git-repo-command "sync" "-c" "--fail-fast" "-v" "-j"
./build/git.scm:55:      (invoke git-command "init" "--initial-branch=main")
./build/git.scm:56:      (invoke git-command "remote" "add" "origin" url)
./build/git.scm:58:          (invoke git-command "checkout" "FETCH_HEAD")
./build/git.scm:62:            (invoke git-command "fetch" "origin")
./build/git.scm:63:            (invoke git-command "checkout" commit)))
./build/git.scm:66:        (invoke git-command "submodule" "update" "--init" "--recursive")
./git-download.scm:175:                         (invoke "git" "-C" #$output "init")
./git-download.scm:176:                         (invoke "git" "-C" #$output "config" "--local"
./git-download.scm:178:                         (invoke "git" "-C" #$output "config" "--local"
./git-download.scm:180:                         (invoke "git" "-C" #$output "add" ".")
./git-download.scm:181:                         (invoke "git" "-C" #$output "commit" "-am" "init")
./git-download.scm:182:                         (invoke "git" "-C" #$output "read-tree" "--empty")
./git-download.scm:183:                         (invoke "git" "-C" #$output "reset" "--hard")
--8<---------------cut here---------------end--------------->8---

An “optional dependency”, is it not already the case?

I read hard-dependency as the idea behind a change like [1].  For
instance, something like:

--8<---------------cut here---------------start------------->8---
diff --git a/guix/self.scm b/guix/self.scm
index 81a36e007f..41c5f40786 100644
--- a/guix/self.scm
+++ b/guix/self.scm
@@ -68,6 +68,7 @@ (define %packages
       ("gzip"               . ,(ref 'compression 'gzip))
       ("bzip2"              . ,(ref 'compression 'bzip2))
       ("xz"                 . ,(ref 'compression 'xz))
+      ("git-minimal"        . ,(ref 'version-control 'git-minimal))
       ("po4a"               . ,(ref 'gettext 'po4a))
       ("gettext-minimal"    . ,(ref 'gettext 'gettext-minimal))
       ("gcc-toolchain"      . ,(ref 'commencement 'gcc-toolchain))
@@ -825,6 +826,9 @@ (define* (compiled-guix source #:key
   (define guile-lzma
     (specification->package "guile-lzma"))

+  (define git
+    (specification->package "git-minimal"))
+
--8<---------------cut here---------------end--------------->8---

In the context of the proposal patch#65866 [1], this hard-dependency
makes sense.  From my point of view, once we have git-minimal as a
hard-dependency, I do not see the point to keep slower Git operations
using libgit2; as ’commit-relation’ for one example.

But maybe I am missing something.

Cheers,
simon


1: [bug#65866] [PATCH 5/8] build: Add dependency on Git.
Ludovic Courtès <ludo@gnu.org>
Mon, 11 Sep 2023 16:25:23 +0200
id:4eca94501c2c1e9986e1f718eeccb3eb9276dcd4.1694441831.git.ludo@gnu.org
https://issues.guix.gnu.org//65866
https://issues.guix.gnu.org/msgid/4eca94501c2c1e9986e1f718eeccb3eb9276dcd4.1694441831.git.ludo@gnu.org
https://yhetil.org/guix/4eca94501c2c1e9986e1f718eeccb3eb9276dcd4.1694441831.git.ludo@gnu.org

[bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts

[Ludovic Courtès]

Hi Maxim,

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

>> +(define* (perform-git-download drv #:optional output
>> +                               #:key print-build-trace?)
>> +  "Perform the download described by DRV, a fixed-output derivation, to
>> +OUTPUT.
>> +
>> +Note: Unless OUTPUT is #f, we don't read the value of 'out' in DRV since the
>> +actual output is different from that when we're doing a 'bmCheck' or
>
> I'd drop the 'we's and use impersonal imperative tense or at least
> 's/when we're doing/when doing/'.

Noted.  (That’s actually copied from ‘perform-download’; I’ll fix it
there as well.)

>> +'bmRepair' build."
>> +  (derivation-let drv ((output* "out")
>
> I'd name this variable just 'out', for consistency with the others.

No because there’s also a parameter called ‘output’ and there’s
(or output output*).  But lemme see, I should remove this optional
‘output’ parameter.

>> +;; 'with-temporary-git-repository' relies on the 'git' command.
>> +(unless (which (git-command)) (test-skip 1))
>
> I'd expect the 'git' command to now be required by Autoconf at build
> time, which should mean checking it here is not useful/required?

That comes in a subsequent patch.

>> +(test-assert "'git-download' built-in builder, not found"
>> +  (let* ((drv (derivation %store "git-download"
>> +                          "builtin:git-download" '()
>> +                          #:env-vars
>> +                          `(("url" . "file:///does-not-exist.git")
>> +                            ("commit"
>> +                             . "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))
>> +                          #:hash-algo 'sha256
>> +                          #:hash (gcrypt:sha256 #vu8())
>> +                          #:recursive? #t)))
>> +    (guard (c ((store-protocol-error? c)
>> +               (string-contains (store-protocol-error-message c) "failed")))
>> +      (build-derivations %store (list drv))
>> +      #f)))
>> +
>
> Maybe the error message compared could be more precised, if it already
> contains the necessary details?

Unfortunately it doesn’t (same strategy as with the existing
“builtin:download” tests.)

Thanks for your feedback!

Ludo’.

[bug#65866] [PATCH v2 0/8] Add built-in builder for Git checkouts

[Ludovic Courtès]

Hello,

Changes compared to v1:

  • The ‘output’ parameter of the ‘perform-download’ and
    ‘perform-git-download’ procedures is now mandatory.
    Consequently, the confusing ‘output*’ variable is gone.

  • The docstring of these two procedures has been clarified
    accordingly.

  • I think there’s no third item.

Let me know if I missed something!

Thanks,
Ludo’.

Ludovic Courtès (8):
  git-download: Move fallback code to (guix build git).
  git-download: Honor the ‘GUIX_DOWNLOAD_FALLBACK_TEST’ environment
    variable.
  perform-download: Remove unused one-argument clause.
  daemon: Add “git-download” built-in builder.
  build: Add dependency on Git.
  perform-download: Use the ‘git’ command captured at configure time.
  git-download: Use “builtin:git-download” when available.
  tests: Assume ‘git’ is always available.

 configure.ac                      |   7 ++
 doc/guix.texi                     |   1 +
 guix/build/git.scm                |  44 ++++++++++-
 guix/config.scm.in                |   6 +-
 guix/git-download.scm             | 122 ++++++++++++++++++------------
 guix/scripts/perform-download.scm |  67 +++++++++++-----
 guix/self.scm                     |  10 ++-
 nix/libstore/builtins.cc          |   5 +-
 tests/builders.scm                |  29 ++++++-
 tests/channels.scm                |   7 +-
 tests/derivations.scm             |  94 ++++++++++++++++++++++-
 tests/git-authenticate.scm        |   1 -
 tests/git.scm                     |  10 ---
 tests/import-git.scm              |  18 -----
 14 files changed, 309 insertions(+), 112 deletions(-)


base-commit: 3d8d67ef6928f5d81118c97f03372cd341eab8b0
-- 
2.41.0

[bug#65866] [PATCH v2 7/8] git-download: Use “builtin:git-download” when available.

[Ludovic Courtès]

Fixes <https://issues.guix.gnu.org/63331>.

Longer-term this will remove Git from the derivation graph when its sole
use is to perform a checkout for a fixed-output derivation, thereby
breaking dependency cycles that can arise in these situations.

* guix/git-download.scm (git-fetch): Rename to…
(git-fetch/in-band): … this.  Deal with GIT or GUILE being #f.
(git-fetch/built-in, built-in-builders*, git-fetch): New procedures.
* tests/builders.scm ("git-fetch, file URI"): New test.
---
 guix/git-download.scm | 68 +++++++++++++++++++++++++++++++++++++------
 tests/builders.scm    | 29 +++++++++++++++++-
 2 files changed, 87 insertions(+), 10 deletions(-)

diff --git a/guix/git-download.scm b/guix/git-download.scm
index f1f19397c6..505dff0a89 100644
--- a/guix/git-download.scm
+++ b/guix/git-download.scm
@@ -27,6 +27,7 @@ (define-module (guix git-download)
   #:use-module (guix records)
   #:use-module (guix packages)
   #:use-module (guix modules)
+  #:use-module ((guix derivations) #:select (raw-derivation))
   #:autoload   (guix build-system gnu) (standard-packages)
   #:autoload   (guix download) (%download-fallback-test)
   #:autoload   (git bindings)   (libgit2-init!)
@@ -78,15 +79,19 @@ (define (git-package)
   (let ((distro (resolve-interface '(gnu packages version-control))))
     (module-ref distro 'git-minimal)))
 
-(define* (git-fetch ref hash-algo hash
-                    #:optional name
-                    #:key (system (%current-system)) (guile (default-guile))
-                    (git (git-package)))
-  "Return a fixed-output derivation that fetches REF, a <git-reference>
-object.  The output is expected to have recursive hash HASH of type
-HASH-ALGO (a symbol).  Use NAME as the file name, or a generic name if #f."
+(define* (git-fetch/in-band ref hash-algo hash
+                            #:optional name
+                            #:key (system (%current-system))
+                            (guile (default-guile))
+                            (git (git-package)))
+  "Return a fixed-output derivation that performs a Git checkout of REF, using
+GIT and GUILE (thus, said derivation depends on GIT and GUILE).
+
+This method is deprecated in favor of the \"builtin:git-download\" builder.
+It will be removed when versions of guix-daemon implementing
+\"builtin:git-download\" will be sufficiently widespread."
   (define inputs
-    `(("git" ,git)
+    `(("git" ,(or git (git-package)))
 
       ;; When doing 'git clone --recursive', we need sed, grep, etc. to be
       ;; available so that 'git submodule' works.
@@ -154,7 +159,8 @@ (define* (git-fetch ref hash-algo hash
                                      #:recursive? recursive?
                                      #:git-command "git")))))
 
-  (mlet %store-monad ((guile (package->derivation guile system)))
+  (mlet %store-monad ((guile (package->derivation (or guile (default-guile))
+                                                  system)))
     (gexp->derivation (or name "git-checkout") build
 
                       ;; Use environment variables and a fixed script name so
@@ -181,6 +187,50 @@ (define* (git-fetch ref hash-algo hash
                       #:recursive? #t
                       #:guile-for-build guile)))
 
+(define* (git-fetch/built-in ref hash-algo hash
+                             #:optional name
+                             #:key (system (%current-system)))
+  "Return a fixed-output derivation without any dependency that performs a Git
+checkout of REF, using the \"builtin:git-download\" derivation builder."
+  (raw-derivation (or name "git-checkout") "builtin:git-download" '()
+                  #:system system
+                  #:hash-algo hash-algo
+                  #:hash hash
+                  #:recursive? #t
+                  #:env-vars
+                  `(("url" . ,(object->string
+                               (match (%download-fallback-test)
+                                 ('content-addressed-mirrors
+                                  "https://example.org/does-not-exist")
+                                 (_
+                                  (git-reference-url ref)))))
+                    ("commit" . ,(git-reference-commit ref))
+                    ("recursive?" . ,(object->string
+                                      (git-reference-recursive? ref))))
+                  #:leaked-env-vars '("http_proxy" "https_proxy"
+                                      "LC_ALL" "LC_MESSAGES" "LANG"
+                                      "COLUMNS")
+                  #:local-build? #t))
+
+(define built-in-builders*
+  (store-lift built-in-builders))
+
+(define* (git-fetch ref hash-algo hash
+                    #:optional name
+                    #:key (system (%current-system))
+                    guile git)
+  "Return a fixed-output derivation that fetches REF, a <git-reference>
+object.  The output is expected to have recursive hash HASH of type
+HASH-ALGO (a symbol).  Use NAME as the file name, or a generic name if #f."
+  (mlet %store-monad ((builtins (built-in-builders*)))
+    (if (member "git-download" builtins)
+        (git-fetch/built-in ref hash-algo hash name
+                            #:system system)
+        (git-fetch/in-band ref hash-algo hash name
+                           #:system system
+                           #:guile guile
+                           #:git git))))
+
 (define (git-version version revision commit)
   "Return the version string for packages using git-download."
   ;; git-version is almost exclusively executed while modules are being loaded.
diff --git a/tests/builders.scm b/tests/builders.scm
index 0b5577c7a3..619caa5f31 100644
--- a/tests/builders.scm
+++ b/tests/builders.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2012, 2013, 2014, 2015, 2018, 2019, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2012-2015, 2018-2019, 2021, 2023 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2021 Lars-Dominik Braun <lars@6xq.net>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -20,6 +20,7 @@
 
 (define-module (tests builders)
   #:use-module (guix download)
+  #:use-module (guix git-download)
   #:use-module (guix build-system)
   #:use-module (guix build-system gnu)
   #:use-module (guix build gnu-build-system)
@@ -31,9 +32,12 @@ (define-module (tests builders)
   #:use-module (guix base32)
   #:use-module (guix derivations)
   #:use-module (gcrypt hash)
+  #:use-module ((guix hash) #:select (file-hash*))
   #:use-module (guix tests)
+  #:use-module (guix tests git)
   #:use-module (guix packages)
   #:use-module (gnu packages bootstrap)
+  #:use-module ((ice-9 ftw) #:select (scandir))
   #:use-module (ice-9 match)
   #:use-module (ice-9 textual-ports)
   #:use-module (srfi srfi-1)
@@ -84,6 +88,29 @@ (define url-fetch*
     (and (file-exists? out)
          (valid-path? %store out))))
 
+(test-equal "git-fetch, file URI"
+  '("." ".." "a.txt" "b.scm")
+  (let ((nonce (random-text)))
+    (with-temporary-git-repository directory
+        `((add "a.txt" ,nonce)
+          (add "b.scm" "#t")
+          (commit "Commit.")
+          (tag "v1.0.0" "The tag."))
+      (run-with-store %store
+        (mlet* %store-monad ((hash
+                              -> (file-hash* directory
+                                             #:algorithm (hash-algorithm sha256)
+                                             #:recursive? #t))
+                             (drv (git-fetch
+                                   (git-reference
+                                    (url (string-append "file://" directory))
+                                    (commit "v1.0.0"))
+                                   'sha256 hash
+                                   "git-fetch-test")))
+          (mbegin %store-monad
+            (built-derivations (list drv))
+            (return (scandir (derivation->output-path drv)))))))))
+
 (test-assert "gnu-build-system"
   (build-system? gnu-build-system))
 
-- 
2.41.0

[bug#65866] [PATCH v2 7/8] git-download: Use “builtin:git-download” when available.

[Simon Tournier]

Hi Ludo,

On Sat, 23 Sep 2023 at 00:28, Ludovic Courtès <ludo@gnu.org> wrote:

> +(define* (git-fetch/built-in ref hash-algo hash
> +                             #:optional name
> +                             #:key (system (%current-system)))
> +  "Return a fixed-output derivation without any dependency that performs a Git
> +checkout of REF, using the \"builtin:git-download\" derivation builder."

I do not understand what means “without any dependency” here.  I would
drop it.

Cheers,
simon

[bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts

[Ludovic Courtès]

Simon Tournier <zimon.toutoune@gmail.com> skribis:

> On Sat, 23 Sep 2023 at 00:28, Ludovic Courtès <ludo@gnu.org> wrote:
>
>> +(define* (git-fetch/built-in ref hash-algo hash
>> +                             #:optional name
>> +                             #:key (system (%current-system)))
>> +  "Return a fixed-output derivation without any dependency that performs a Git
>> +checkout of REF, using the \"builtin:git-download\" derivation builder."
>
> I do not understand what means “without any dependency” here.

It means the derivation does not depend on anything (it has zero
“sources” and zero “input derivations”).  That’s fundamental here, which
is why I made it explicit.

Ludo’.

[bug#65866] [PATCH 0/8] Add built-in builder for Git checkouts

[Simon Tournier]

Re,

On Mon, 25 Sep 2023 at 11:23, Ludovic Courtès <ludo@gnu.org> wrote:

>>> +  "Return a fixed-output derivation without any dependency that performs a Git
>>> +checkout of REF, using the \"builtin:git-download\" derivation builder."
>>
>> I do not understand what means “without any dependency” here.
>
> It means the derivation does not depend on anything (it has zero
> “sources” and zero “input derivations”).  That’s fundamental here, which
> is why I made it explicit.

For instance, in ’built-in-download’, the docstring reads:

--8<---------------cut here---------------start------------->8---
This is an \"out-of-band\" download in that the returned derivation does not
explicitly depend on Guile, GnuTLS, etc.  Instead, the daemon performs the
download by itself using its own dependencies.
--8<---------------cut here---------------end--------------->8---

which I find “clearer” than “without any dependency”.

Well, enough for nitpicking. :-)

Cheers,
simon

bug#65866: [PATCH 0/8] Add built-in builder for Git checkouts

[Ludovic Courtès]

Hi,

Simon Tournier <zimon.toutoune@gmail.com> skribis:

> Re,
>
> On Mon, 25 Sep 2023 at 11:23, Ludovic Courtès <ludo@gnu.org> wrote:
>
>>>> +  "Return a fixed-output derivation without any dependency that performs a Git
>>>> +checkout of REF, using the \"builtin:git-download\" derivation builder."
>>>
>>> I do not understand what means “without any dependency” here.
>>
>> It means the derivation does not depend on anything (it has zero
>> “sources” and zero “input derivations”).  That’s fundamental here, which
>> is why I made it explicit.
>
> For instance, in ’built-in-download’, the docstring reads:
>
> This is an \"out-of-band\" download in that the returned derivation does not
> explicitly depend on Guile, GnuTLS, etc.  Instead, the daemon performs the
> download by itself using its own dependencies.
>
> which I find “clearer” than “without any dependency”.

Yes, good idea.

I changed the docstring as you suggest and pushed the whole thing:

  ba21eeb565 tests: Assume ‘git’ is always available.
  13b0cf85eb git-download: Use “builtin:git-download” when available.
  c4a1d69a69 perform-download: Use the ‘git’ command captured at configure time.
  f651a35969 build: Add dependency on Git.
  95f2123135 daemon: Add “git-download” built-in builder.
  9d0e2002a5 perform-download: Remove unused one-argument clause.
  c7ed1e0160 git-download: Honor the ‘GUIX_DOWNLOAD_FALLBACK_TEST’ environment variable.
  811b249397 git-download: Move fallback code to (guix build git).

I’ll update the ‘guix’ package soon so we can start using the new
builtin.

Thanks to the two of you!

Ludo’.