Ludovic Courtès <l...@gnu.org> 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.
