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

From: Tomas Volf <[email-masked]>
Date: Tue, 25 Mar 2025 22:27:00 +0100

Ludovic Courtès <[email-masked]> writes:

>> I did not know guile-git was supposed to guarantee it.  I assumed it is
>> just a binding to libgit2, so I have expected it to have the same
>> semantics.
>
> Well, ‘eq?’ only makes sense for the Scheme objects that wrap the
> underlying C objects.

In strict sense of the word, sure, but even in C being able to compare
pointers is useful.  My understanding of eq? for objects wrapping C data
structures basically is "pointer equality", so in that sense I have
expected eq? on <commit> to be the equivalent of == on git_commit*.

>
>> I think I would just fix this in Guix, and drop the promise on
>> guile-git's side.  I am obviously unsure whether people rely on it or
>> not, but given it does not work I am tempted to guess.
>
> (guix git) relies on it and I probably wrote pieces of code with that
> assumption in mind.

Fair enough.

>> However I do see the appeal in being able to use eq?.  The correct
>> action probably depends on in what direction you want to take guile-git.
>> Should it stay just a bindings wrapper, or should it provide extra
>> features and guarantees?
>
> It’s not really an extra feature, it’s really a core part of defining
> bindings in my view; ‘define-wrapped-pointer-type’ in Guile implements
> those semantics.

I have read through the documentation for define-wrapped-pointer-type,
and I must say I do not see this part in the documentation.  Only part
talking about eq? is this:

--8<---------------cut here---------------start------------->8---
wrap preserves pointer identity, for two pointer objects p1 and p2 that
are equal?, (eq? (wrap p1) (wrap p2)) ⇒ #t.
--8<---------------cut here---------------end--------------->8---

But the two commit pointers in this case are not equal?, so why should
they eq??

However on my first (and second) reading of the documentation I did not
get it, and expected eq? in the below snippet to return #f (since the
pointer being wrapped does equal? to itself).  So maybe I am missing
something else as well.

--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use (system foreign)
scheme@(guile-user)> ,use (system foreign-library)
scheme@(guile-user)> (define-wrapped-pointer-type foo foo? wrap-foo unwrap-foo #t)
scheme@(guile-user)> (define-wrapped-pointer-type bar bar? wrap-bar unwrap-bar #t)
scheme@(guile-user)> (define open* (foreign-library-function #f "open"))
scheme@(guile-user)> (wrap-foo open*)
$8 = #<foo 7fec7aac1010>
scheme@(guile-user)> (wrap-bar open*)
$9 = #<bar 7fec7d3dd510>
scheme@(guile-user)> (eq? $8 $9)
$10 = #f
scheme@(guile-user)> (equal? open* open*)
$12 = #t
--8<---------------cut here---------------end--------------->8---

>
>> But then again, I do not have much experience with weak tables, and
>> guile-git internals, so maybe I overestimate the complexity.  I would be
>> scared I miss some paths that return commits though.
>
> I’m happy to provide guidance if you want to give it a try, lemme
> know!

One aspect I was thinking about is whether this makes sense to do in
libgit2 directly and contribute to the upstream.  This ("stable" mapping
of hash -> pointer) does seem useful, and there already is
git_commit_free, so maybe it could just reference count?  In that way
all user, not just Guile, would benefit.

Tomas

-- 
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.