On 2023-09-30 17:48:55 +0200, Simon Tournier wrote:
> Hi,
>
> Hum, the updates seem:
>
> + libgit2 on Feb 17 2023,
> + guile-git on May 15 2022.
>
> (See 8d8e1438ae5a2e50005b500dacd0a26be540fe69 and
> b6bfe9ea6a1b19159455b34f1af4ac00ef9b94ab)
>
> And some commits with large body are around:
>
> + 7b45ead9ec40a5ea1ef8332d55c2bb4beff85eb5 from Jul 18 2023
> + 1e6ddceb8318d413745ca1c9d91fde01b1e0364b from Feb 19 2023
> + 5897d873d0c902f08d13c38500eff11098f2a634 from Aug 10 2022
>
> And I have not investigated more about their commit object size. Just
> counting the number of characters per commit message. The one you
> provided is about 3030, if I am correct. Here, let filter with the
> criteria of 4500, why not. :-)
>
> --8<---------------cut here---------------start------------->8---
> $ for ci in $(git log --format=%H --after=2022-05-13); do \
> echo "$(git show -s $ci | wc -c) $ci" \
> | awk '$1>4500{print $2 " " $1}' \
> ;done
> 7b45ead9ec40a5ea1ef8332d55c2bb4beff85eb5 4997
> 1e6ddceb8318d413745ca1c9d91fde01b1e0364b 16120
> 575a03ab3997edee08d20867228e886043d5240f 5511
> 5897d873d0c902f08d13c38500eff11098f2a634 6258
> --8<---------------cut here---------------end--------------->8---
>
> Well, it is probably not a regression. Or I am missing some details. :-)Thank you for raising this up and making me look into it closer. The issue (commits not being eq? to themselves) does happen for those listed above, however commit-relation still works fine. I am unsure why, I spent most of today on it and did not manage to find clear rules. However the fact remains that when one is on d51135e8477118dc63a7e5462355cd27e884f4fb, guix pull to 4dbd25fa0e09b40ba2ab01d1e64fa36db652b501 does fail. I pushed those commits into https://git.sr.ht/~graywolf/guix-guile-git-repro as branch xxx in case anyone is curious and wants to investigate. > > I am probably overlooking something, from my understanding, the issue > arises for some corner cases when the bound of the closure does not fit > ’eq?’. For these cases, instead of relying on ’setq’ using ’eq?’, we > could rely on ’set’ using ’equal?’. No? I do not believe so, the mismatch happens even for equal?. I do not know enough about scheme to know whether guile-git could override equal? for the commit record type, but it does not seem to do so. scheme@(guile-user)> (use-modules (git) (guix git)) scheme@(guile-user)> (define %repo (repository-open "/home/wolf/src/guix")) scheme@(guile-user)> (define %hash "1e6ddceb8318d413745ca1c9d91fde01b1e0364b") scheme@(guile-user)> (equal? (commit-lookup %repo (string->oid %hash)) (commit-lookup %repo (string->oid %hash))) $1 = #f > > On Fri, 29 Sep 2023 at 18:52, wolf <[email protected]> wrote: > > > ,---- > > | 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 > > `---- > > [...] > > > ,---- > > | $ git merge-base --is-ancestor 9b985229bcd 71f544c102a; echo $? > > | 0 > > `---- > > [...] > > > 2 Possible solutions > > ==================== > > Naive question. Why not rely on “git merge-base --is-ancestor” for > implementing “commit-relation”? > > Since f651a359691cbe4750f1fe8d14dd964f7971f91 from Sep 26 2023: > > build: Add dependency on Git. > > * configure.ac: Check for ‘git’ and substitute ‘GIT’. > * guix/config.scm.in (%git): New variable. > * guix/self.scm (compiled-guix): Define ‘git’ and pass it to > ‘make-config.scm’. > (make-config.scm): Add #:git; emit a ‘%git’ variable. > * doc/guix.texi (Requirements): Add it. > > we can assume Git is available by the code that run “commit-relation”, > no? > > The implementation relying on “git merge-base --is-ancestor” does not > have the problem, right? Well, that is true. I forgot to mention this option, because there did not seem to be a consensus regarding replacing more parts of guile-git with git proper. But this would likely be the best solution if that approach is acceptable. > > And from [1], it is 35x faster. Win-win, no? Because the fix for ’eq?’ > will introduce performance cost and ’commit-relation’ will be even > slower, no? For what it is worth, the implementation using <commit-set> (I needed it for my fork to be able to pull) is not noticeably slower. The slow down is likely measurable, but since I did not even notice it I did not bother measuring it. Not that I am arguing against using git. > > Well, I do not know. My words are probably irrelevant. > > Cheers, > simon > > > 1: comparing commit-relation using Scheme+libgit2 vs shellout plumbing Git > Simon Tournier <[email protected]> > Tue, 12 Sep 2023 00:48:30 +0200 > id:[email protected] > https://lists.gnu.org/archive/html/guix-devel/2023-09 > https://yhetil.org/guix/[email protected] -- There are only two hard things in Computer Science: cache invalidation, naming things and off-by-one errors.
signature.asc
Description: PGP signature
