[bug#68935,1/3] svn-fetch: Require svn-command argument.
Commit Message
* guix/build/svn.scm (svn-fetch): Require svn-command argument.
* guix/svn-download.scm: Pass svn-command to each (svn-fetch).
Change-Id: Ibc7d3a79e45374588f71ca4d4ac2685c60ff3c00
---
guix/build/svn.scm | 5 ++---
guix/svn-download.scm | 35 ++++++++++++++++++++++++++---------
2 files changed, 28 insertions(+), 12 deletions(-)
Comments
Hi Herman,
Herman Rimm <herman@rimm.ee> 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’.
@@ -31,9 +31,8 @@ (define-module (guix build svn)
;;;
;;; Code:
-(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))
"Fetch REVISION from URL into DIRECTORY. REVISION must be an integer, and a
@@ -2,6 +2,7 @@
;;; Copyright © 2014-2016, 2019, 2021-2023 Ludovic Courtès <ludo@gnu.org>
;;; Copyright © 2014 Sree Harsha Totakura <sreeharsha@totakura.in>
;;; Copyright © 2017, 2019, 2021 Ricardo Wurmus <rekado@elephly.net>
+;;; Copyright © 2024 Herman Rimm <herman@rimm.ee>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -105,7 +106,7 @@ (define* (svn-fetch ref hash-algo hash
(or (svn-fetch (getenv "svn url")
(string->number (getenv "svn revision"))
#$output
- #:svn-command #+(file-append svn "/bin/svn")
+ #+(file-append svn "/bin/svn")
#:recursive? (match (getenv "svn recursive?")
("yes" #t)
(_ #f))
@@ -195,7 +196,7 @@ (define* (svn-multi-fetch ref hash-algo hash
(if (string-suffix? "/" location)
(string-append #$output "/" location)
(string-append #$output "/" (dirname location)))
- #:svn-command #+(file-append svn "/bin/svn")
+ #+(file-append svn "/bin/svn")
#:recursive? (match (getenv "svn recursive?")
("yes" #t)
(_ #f))
@@ -245,25 +246,34 @@ (define* (svn-multi-fetch ref hash-algo hash
(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")))))))
(define* (download-multi-svn-to-store store ref
#:optional (name (basename (svn-multi-reference-url ref)))
- #:key (log (current-error-port)))
+ #:key (log (current-error-port))
+ (svn (subversion-package)))
"Download from REF, a <svn-multi-reference> object to STORE. Write progress
reports to LOG."
(call-with-temporary-directory
@@ -272,16 +282,23 @@ (define* (download-multi-svn-to-store store ref
(let ((dir (string-append temp "/" (dirname location))))
(mkdir-p dir))
(parameterize ((current-output-port log))
- (build:svn-fetch (string-append (svn-multi-reference-url ref)
- "/" location)
+ (with-imported-modules
+ (source-module-closure '((guix build svn)
+ (guix build utils)))
+ #~(begin
+ (use-modules (guix build svn)
+ (guix build utils))
+ (svn-fetch (string-append (svn-multi-reference-url ref)
+ "/" location)
(svn-multi-reference-revision ref)
(if (string-suffix? "/" location)
(string-append temp "/" location)
(string-append temp "/" (dirname location)))
+ #+(file-append svn "/bin/svn")
#:recursive?
(svn-multi-reference-recursive? ref)
#:user-name (svn-multi-reference-user-name ref)
- #:password (svn-multi-reference-password ref))))
+ #:password (svn-multi-reference-password ref))))))
(svn-multi-reference-locations ref))
(add-to-store store name #t "sha256" temp)))))