Subject: [bug#68935] [PATCH 1/3] svn-fetch: Require svn-command argument.

From: Ludovic Courtès <[email-masked]>
Date: Wed, 07 Feb 2024 22:34:01 +0100

Hi Herman,

Herman Rimm <[email-masked]> skribis:

> * guix/build/svn.scm (svn-fetch): Require svn-command argument.
> * guix/svn-download.scm: Pass svn-command to each (svn-fetch).
>
> Change-Id: Ibc7d3a79e45374588f71ca4d4ac2685c60ff3c00

Usually we write all the entities (procedures, variables, etc.)
modified in the commit log to help review.

> -(define* (svn-fetch url revision directory
> -                    #:key (svn-command "svn")
> -                    (recursive? #t)
> +(define* (svn-fetch url revision directory svn-command
> +                    #:key (recursive? #t)
>                      (user-name #f)
>                      (password #f))

What’s the rationale for this change?

It’s a relatively common pattern in Guix build-side code to assume that
commands can be found in $PATH while still providing a way to specify
the command’s absolute file name.

>  (define* (download-svn-to-store store ref
>                                  #:optional (name (basename (svn-reference-url ref)))
> -                                #:key (log (current-error-port)))
> +                                #:key (log (current-error-port))
> +                                (svn (subversion-package)))
>    "Download from REF, a <svn-reference> object to STORE.  Write progress
>  reports to LOG."
>    (call-with-temporary-directory
>     (lambda (temp)
>       (let ((result
>              (parameterize ((current-output-port log))
> -              (build:svn-fetch (svn-reference-url ref)
> +              (with-imported-modules
> +                  (source-module-closure '((guix build svn)
> +                                           (guix build utils)))
> +                #~(begin
> +                    (use-modules (guix build svn)
> +                                 (guix build utils))
> +                    (svn-fetch (svn-reference-url ref)
>                                 (svn-reference-revision ref)
>                                 (string-append temp "/svn")
> +                               #+(file-append svn "/bin/svn")
>                                 #:user-name (svn-reference-user-name ref)
> -                               #:password (svn-reference-password ref)))))
> +                               #:password (svn-reference-password ref)))))))
>         (and result
>              (add-to-store store name #t "sha256"
>                            (string-append temp "/svn")))))))

This doesn’t have the desired effect: ‘download-svn-to-store’ will now
always return a gexp instead of returning either #f or a store file
name.

This procedure and ‘download-multi-svn-to-store’ (used by the texlive
importer) really expect “svn” to be in $PATH.  It cannot really be
avoided AFAICS.

Thanks,
Ludo’.