GNU bug report logs - #77875
[PATCH 0/2] Use 'graph-descendant?' from Guile-Git instead of custom code

Previous Next

Package: guix-patches;

Reported by: Ludovic Courtès <ludo <at> gnu.org>

Date: Thu, 17 Apr 2025 20:21:01 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Tomas Volf <~@wolfsden.cz>
Cc: 77875 <at> debbugs.gnu.org
Subject: [bug#77875] [PATCH 1/2] git: Use ‘graph-descendant?’ from Guile-Git >= 0.10.0 when available.
Date: Mon, 21 Apr 2025 23:26:56 +0200
Hi Tomas,

Tomas Volf <~@wolfsden.cz> writes:

> Ludovic Courtès <ludo <at> gnu.org> writes:
>
>> Fixes <https://issues.guix.gnu.org/66268>.
>>
>> Fixes a bug whereby ‘commit-relation’ and ‘commit-descendant?’ would
>> provide an incorrect result when two distinct <commit> objects would
>> exist for the same commit, which can happen when the commit’s metadata
>> is beyond 4 KiB, as of libgit2 1.8/1.9.
>
> Ooh, so this *used to work* in <1.8, interesting, I had no idea. :)

No no; what I meant is that this is known to be the case with 1.8 and
1.9, but I haven't checked with other versions.

>> +  (if (resolve-module '(guix graph) #:ensure #f)  ;Guile-Git >= 0.10.0
>
> Two notes here:
>
> 1. Should this not be '(git graph)?

Oops, yes!  Fixed locally.

> 2. I thought that when you do `guix pull`, the new Guix is built against
> the packages in the newly pulled version.  So everyone who pulls these
> changes, should also pull the guile-git 0.10.0 no?  It should not be
> possible to end up in "new guix, old guile-git" situation.  So I do not
> understand why you need both branches.

That's correct.  The second branch is a convenience for developers as I
wrote, typically for someone developing in “guix shell” but from a Guix
that doesn’t have 0.10.0 yet.  We can remove that second branch in a few
weeks.

> How is the `graph-descendant?' set?

It’s from the (git) module, automatically pulled in when available.

>> +      (lambda (new old)
>> +        "Return true if NEW is the descendant of one of OLD, a list of
>> +commits."
>> +        (let ((repository (commit-owner new))
>> +              (new (commit-id new)))
>> +          (any (lambda (old)
>> +                 (let ((old (commit-id old)))
>> +                   (or (graph-descendant? repository new old)
>> +                       (oid=? old new))))
>> +               old)))
>
> I would be tempted to write this in terms of commit-relation and memq
> with '(self descendant).  Beside being slightly easier to grasp (for
> me), it would allow you to have just a single branch, since
> commit-relation is available regardless of guile-git version.  Not sure
> if there would be performance impact in <0.10.0 though.

I thought about this but I think the implementation above is slightly
more efficient, and since the "else" branch will be removed soon, let's
not bother.

Sending v2 right away.

Thanks,
Ludo'.




This bug report was last modified 86 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.