GNU bug report logs -
#65866
[PATCH 0/8] Add built-in builder for Git checkouts
Previous Next
Reported by: Ludovic Courtès <ludo <at> gnu.org>
Date: Mon, 11 Sep 2023 14:25:01 UTC
Severity: normal
Tags: patch
Done: Ludovic Courtès <ludo <at> gnu.org>
Bug is archived. No further changes may be made.
Full log
Message #86 received at 65866 <at> debbugs.gnu.org (full text, mbox):
Hi Maxim,
Maxim Cournoyer <maxim.cournoyer <at> gmail.com> skribis:
>> +(define* (perform-git-download drv #:optional output
>> + #:key print-build-trace?)
>> + "Perform the download described by DRV, a fixed-output derivation, to
>> +OUTPUT.
>> +
>> +Note: Unless OUTPUT is #f, we don't read the value of 'out' in DRV since the
>> +actual output is different from that when we're doing a 'bmCheck' or
>
> I'd drop the 'we's and use impersonal imperative tense or at least
> 's/when we're doing/when doing/'.
Noted. (That’s actually copied from ‘perform-download’; I’ll fix it
there as well.)
>> +'bmRepair' build."
>> + (derivation-let drv ((output* "out")
>
> I'd name this variable just 'out', for consistency with the others.
No because there’s also a parameter called ‘output’ and there’s
(or output output*). But lemme see, I should remove this optional
‘output’ parameter.
>> +;; 'with-temporary-git-repository' relies on the 'git' command.
>> +(unless (which (git-command)) (test-skip 1))
>
> I'd expect the 'git' command to now be required by Autoconf at build
> time, which should mean checking it here is not useful/required?
That comes in a subsequent patch.
>> +(test-assert "'git-download' built-in builder, not found"
>> + (let* ((drv (derivation %store "git-download"
>> + "builtin:git-download" '()
>> + #:env-vars
>> + `(("url" . "file:///does-not-exist.git")
>> + ("commit"
>> + . "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"))
>> + #:hash-algo 'sha256
>> + #:hash (gcrypt:sha256 #vu8())
>> + #:recursive? #t)))
>> + (guard (c ((store-protocol-error? c)
>> + (string-contains (store-protocol-error-message c) "failed")))
>> + (build-derivations %store (list drv))
>> + #f)))
>> +
>
> Maybe the error message compared could be more precised, if it already
> contains the necessary details?
Unfortunately it doesn’t (same strategy as with the existing
“builtin:download” tests.)
Thanks for your feedback!
Ludo’.
This bug report was last modified 1 year and 203 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.