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

Previous Next

Full log

View this message in rfc822 format

From: Tomas Volf <~@wolfsden.cz>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: wolf <wolf <at> wolfsden.cz>, 66268 <at> debbugs.gnu.org
Subject: bug#66268: Guix makes invalid assumptions regarding guile-git guarantees leading to guix pull failing
Date: Mon, 10 Mar 2025 20:28:41 +0100

[Message part 1 (text/plain, inline)]

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

> Hi Tomas,
>
> Somehow nobody acted over this bug report for a long time.  Terrible.
>
> wolf <wolf <at> 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?)

Seems to be the case:

--8<---------------cut here---------------start------------->8---
#!/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")))))
'
--8<---------------cut here---------------end--------------->8---

--8<---------------cut here---------------start------------->8---
$ /tmp/x.sh

;;; (unrelated)
--8<---------------cut here---------------end--------------->8---

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.

>
>   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

0: https://git.wolfsden.cz/guix/tree/etc/0001-git-Fix-usage-of-guile-git.patch

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

[signature.asc (application/pgp-signature, inline)]

Previous Next

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