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 <w...@wolfsden.cz> 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 <zimon.touto...@gmail.com> > Tue, 12 Sep 2023 00:48:30 +0200 > id:865y4gz5q9....@gmail.com > https://lists.gnu.org/archive/html/guix-devel/2023-09 > https://yhetil.org/guix/865y4gz5q9....@gmail.com -- There are only two hard things in Computer Science: cache invalidation, naming things and off-by-one errors.
signature.asc
Description: PGP signature