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

> Hi!
>
> Christopher Baines <mail@cbaines.net> skribis:
>
>> * gnu/packages/package-management.scm (guix-build-coordinator): New variable.
>
> [...]
>
>> +           (lambda* (#:key inputs outputs #:allow-other-keys)
>> +             (let* ((out (assoc-ref outputs "out"))
>> +                    (bin (string-append out "/bin"))
>> +                    (guile (assoc-ref inputs "guile"))
>> +                    (guile-effective-version
>> +                     (read-line
>> +                      (open-pipe* OPEN_READ
>> +                                  (string-append guile "/bin/guile")
>> +                                  "-c" "(display (effective-version))")))
>
> Maybe use ‘target-guile-effective-version’ from (guix build
> guile-build-system).

I've changed this. I'll also try and remember and update the
guix-data-service package accordingly, as that's where I copied this
from.

>> +                  (wrap-program (string-append bin "/" file)
>> +                    `("PATH" ":" prefix
>> +                      (,bin ,(assoc-ref inputs "sqitch")))
>> +                    `("PERL5LIB" ":" prefix
>> +                      (,(getenv "PERL5LIB")))
>
> Do we really need PERL5LIB here?  Shouldn’t it be done in the wrapper of
> ‘sqitch’ directly?

Hmm, when I packaged sqitch, I didn't add all the database drivers which
are optional dependencies, but perl-dbd-pg did end up in there somehow
(probably because that's needed by the guix-data-service).

I've now gone ahead and added the database drivers which Guix has
packages for [1], it does increase the closure size a bit, but it was
quite big anyway.

1:
http://git.savannah.gnu.org/cgit/guix.git/commit/?id=6a1c3a908c8aadfde5b5fe405132839683a01776

All that means I can remove the wrapping here.

>> +                (scandir bin
>> +                         (match-lambda
>> +                           ((or "." "..") #f)
>> +                           (_ #t))))
>
> I think you could just use (find-files bin).

Done.

>> +    (inputs
>> +     `(("guile" ,guile-3.0-latest)
>
> Rather ,@(assoc-ref (package-native-inputs guix) "guile").

Done.

>> +       ("sqitch" ,sqitch)
>> +       ("perl-dbd-sqlite" ,perl-dbd-sqlite)))
>
> Shouldn’t perl-dbd-sqlite be a dependency of sqitch?

See above.

>> +    (home-page "https://git.cbaines.net/guile/guix/build-coordinator")
>> +    (synopsis "")
>> +    (description
>> +     "")
>
> Missing!  :-)

Ah, yeah, I should have remembered to lint the package. I've attempted
to write something now, I'm still working on trying to explain what the
Guix Build Coordinator is meant to be.

> LGTM with changes along these lines!

Thanks!