bug#66268: Guix makes invalid assumptions regarding guile-git guarantees leading to guix pull failing

[Simon Tournier]

Hi,

Hum, the updates seem:

 + libgit2 on Feb 17 2023,
 + guile-git on May 15 2022.

(See 8d8e1438ae5a2e50005b500dacd0a26be540fe69 and
b6bfe9ea6a1b19159455b34f1af4ac00ef9b94ab)

And some commits with large body are around:

 + 7b45ead9ec40a5ea1ef8332d55c2bb4beff85eb5 from Jul 18 2023
 + 1e6ddceb8318d413745ca1c9d91fde01b1e0364b from Feb 19 2023
 + 5897d873d0c902f08d13c38500eff11098f2a634 from Aug 10 2022

And I have not investigated more about their commit object size.  Just
counting the number of characters per commit message.  The one you
provided is about 3030, if I am correct.  Here, let filter with the
criteria of 4500, why not. :-)

--8<---------------cut here---------------start------------->8---
$ for ci in $(git log --format=%H --after=2022-05-13); do \
    echo "$(git show -s $ci | wc -c) $ci"                 \
        | awk '$1>4500{print $2 " " $1}'                  \
;done 
7b45ead9ec40a5ea1ef8332d55c2bb4beff85eb5 4997
1e6ddceb8318d413745ca1c9d91fde01b1e0364b 16120
575a03ab3997edee08d20867228e886043d5240f 5511
5897d873d0c902f08d13c38500eff11098f2a634 6258
--8<---------------cut here---------------end--------------->8---

Well, it is probably not a regression.  Or I am missing some details. :-)

I am probably overlooking something, from my understanding, the issue
arises for some corner cases when the bound of the closure does not fit
’eq?’.  For these cases, instead of relying on ’setq’ using ’eq?’, we
could rely on ’set’ using ’equal?’.  No?

On Fri, 29 Sep 2023 at 18:52, wolf <wolf@wolfsden.cz> wrote:

>   ,----
>   | scheme@(guile-user)> (use-modules (git) (guix git))
>   | scheme@(guile-user)> (define %repo (repository-open "/tmp/my-fork"))
>   | scheme@(guile-user)> (define %hash 
> "d51135e8477118dc63a7e5462355cd27e884f4fb")
>   | scheme@(guile-user)> (commit-relation
>   |  (commit-lookup %repo (string->oid %hash))
>   |  (commit-lookup %repo (string->oid %hash)))
>   | $5 = unrelated
>   `----

[...]

>   ,----
>   | $ git merge-base --is-ancestor 9b985229bcd 71f544c102a; echo $?
>   | 0
>   `----

[...]

> 2 Possible solutions
> ====================

Naive question.  Why not rely on “git merge-base --is-ancestor” for
implementing “commit-relation”?

Since f651a359691cbe4750f1fe8d14dd964f7971f91 from Sep 26 2023:

    build: Add dependency on Git.

    * configure.ac: Check for ‘git’ and substitute ‘GIT’.
    * guix/config.scm.in (%git): New variable.
    * guix/self.scm (compiled-guix): Define ‘git’ and pass it to
    ‘make-config.scm’.
    (make-config.scm): Add #:git; emit a ‘%git’ variable.
    * doc/guix.texi (Requirements): Add it.

we can assume Git is available by the code that run “commit-relation”,
no?

The implementation relying on “git merge-base --is-ancestor” does not
have the problem, right?

And from [1], it is 35x faster.  Win-win, no?  Because the fix for ’eq?’
will introduce performance cost and ’commit-relation’ will be even
slower, no?

Well, I do not know.  My words are probably irrelevant.

Cheers,
simon


1: comparing commit-relation using Scheme+libgit2 vs shellout plumbing Git
Simon Tournier <zimon.toutoune@gmail.com>
Tue, 12 Sep 2023 00:48:30 +0200
id:865y4gz5q9.fsf@gmail.com
https://lists.gnu.org/archive/html/guix-devel/2023-09
https://yhetil.org/guix/865y4gz5q9.fsf@gmail.com