Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (38 lines)
> Hi Tomas,
>
> Somehow nobody acted over this bug report for a long time. Terrible.
>
> wolf <wolf@wolfsden.cz> skribis:
>
>> There is an assumption made by Guix regarding guile-git, which is not
>> true. The problem is demonstrated using my fork, since that is where
>> I encountered it first, but official Guix will hit the same problem
>> sooner or later. I will also provide an independent repository for
>> the verification.
>>
>> Guix made a design decision to compare commit objects using eq?, based
>> on the assumption that guile-git will return the same object for the
>> same commit. However that assumption is wrong and can lead to fun
>> issues like:
>>
>> ,----
>> | 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
>> `----
>
> Ouch.
>
> ‘tests/commit.scm’ in Guile-Git checks that behavior, but maybe it’s
> just a lucky case.
>
> Two questions/comments:
>
> 1. Come up with a similar test (for Guile-Git) where commits are *not*
> ‘eq?’? (It it enough to create a commit with a log that’s beyond
> 4 KiB?)
Toggle snippet (21 lines)
#!/bin/sh
set -eu
d=/tmp/ttest
rm -rf "$d"
mkdir -p "$d"
cd "$d"
git init -q
git commit -q --allow-empty -m "$(seq 0 8192)"
h=$(git rev-parse HEAD)
d="$d" h="$h" guile -c '
(use-modules (git) (guix git))
(define %repo (repository-open (getenv "d")))
(pk (commit-relation (commit-lookup %repo (string->oid (getenv "h")))
(commit-lookup %repo (string->oid (getenv "h")))))
'
Toggle snippet (5 lines)
$ /tmp/x.sh
;;; (unrelated)
IIRC from all the back then, the 4kB is a (default) limit for commit
being cache-able. It technically is on all of commit data, not just the
commit message, but the message is the easiest to influence.
Toggle quote (8 lines)
>
> 2. Since Guile-Git has been pretending to provide that eq?-ness
> guarantee, I’m tempted to fix the problem in Guile-Git, by having a
> per-repository lookup table (a weak-value hash table mapping OIDs
> to commits).
>
> How does that sound?
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.
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. I also took a
quick look, and I did not find this promise actually being documented
anywhere.
The patch on Guix side is pretty simple[0] (ignore the licensing, I have
no problem changing the license if you decide to used it).
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?
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.
This probably was not very useful answer. :)
Tomas
--
There are only two hard things in Computer Science:
cache invalidation, naming things and off-by-one errors.
-----BEGIN PGP SIGNATURE-----
iQJCBAEBCgAsFiEEt4NJs4wUfTYpiGikL7/ufbZ/wakFAmfPPWkOHH5Ad29sZnNk
ZW4uY3oACgkQL7/ufbZ/wanXPA//RHhy34qLgsjyR+eXfBCEWsb/ZuFWSITQY3s3
lEBTtocP0nhr9eOJzG9LrL79ce2g3GC/9mhC2sc6+eUgt1lZJDPBbyokc0iXDplW
zK+Th8iZcAg/2xew5jog/ZHfwwibJ22r7uVoUAkIx98uqcL0NnP5rxzbAYkCwO6c
vHUvY/RxvO/6Vq3+zVNjfACERWJ78iayGuD8RXOij/AY3UYK62TzItNHKh/thE3j
fI6cHztUp+TZURXLyoYN+Bx2xmswmJSPriBv7dYseR/teQMQegtZNyZiU2LGXBBp
QREcxmbPLmuAkqfNMiHoUQqaihjMITlDLICXI4B0rwHe9uAiwTvrlERTZDFTsQrh
BjV26EgGv/ANOx1ZM93JMcdtYfB1wI0UO+VPTWQi0UxsrzNlLhyuyOHGLEIrdR2A
A7jyofdMw2/yc1wyTkqJWd4FbWHB8NdZpPJB5Pzo9ZcV396d7La+m7LZ6MfymX6N
NC/CvXj4qcOqITmX/5GPqXL3icPJFhzkLB6yh7KK8EAd87sRbOn/1L79KLxbcKEs
I+GecPGps5Tnt/7PRkDI6UIS3cNI6s+xgaMmbryUkFW5HET6nHuGv6lfCSE4uEnD
EDMFa4wxjul50cUB2LpbjzlGofqlQW9wxBpzDzyr5A1JsSoRIRbknEBaOO8jJqbz
xWuUQy8=
=iADa
-----END PGP SIGNATURE-----
