[PATCH 0/3] import/cran: Parameterize for guix-cran.

  • Done
  • quality assurance status badge
Details
4 participants
  • Lars-Dominik Braun
  • Ludovic Courtès
  • Ricardo Wurmus
  • zimoun
Owner
unassigned
Submitted by
Lars-Dominik Braun
Severity
normal
L
L
Lars-Dominik Braun wrote on 19 Oct 2022 11:28
(address . guix-patches@gnu.org)
Y0/DVVjwYCtiDRxd@noor.fritz.box
Hi,

the attached patches are required for guix-cran
ability to add a prefix to licenses, but it was not exposed. Additionally
I need to parameterize fetch/download functions, so I can cache the
tarballs/DESCRIPTION files.

Cheers,
Lars


Lars-Dominik Braun (3):
import/cran: Allow custom license prefix.
import/cran: Allow overriding description fetch function.
import/cran: Allow overriding tarball download.

guix/import/cran.scm | 30 +++++++++++++++++++++---------
guix/scripts/import/cran.scm | 18 ++++++++++++++++--
2 files changed, 37 insertions(+), 11 deletions(-)

--
2.37.3
From 758a4f70fda5758449747e14db1991f6243174b1 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Tue, 18 Oct 2022 12:45:15 +0200
Subject: [PATCH 1/3] import/cran: Allow custom license prefix.
X-Debbugs-Cc: zimon.toutoune@gmail.com
X-Debbugs-Cc: dev@jpoiret.xyz
X-Debbugs-Cc: mail@cbaines.net
X-Debbugs-Cc: rekado@elephly.net
X-Debbugs-Cc: othacehe@gnu.org
X-Debbugs-Cc: ludo@gnu.org

* guix/import/cran.scm (%license-prefix): New parameter.
(string->license): Use it.
* guix/scripts/import/cran.scm (%options): Add new parameter -p/--license-prefix.
(show-help): Document it.
(parse-options): Pass it as a parameter to importer.
---
guix/import/cran.scm | 10 +++++++---
guix/scripts/import/cran.scm | 18 ++++++++++++++++--
2 files changed, 23 insertions(+), 5 deletions(-)

Toggle diff (80 lines)
diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index 17e33d5f52..d13231f633 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -55,6 +55,7 @@ (define-module (guix import cran)
#:use-module (guix packages)
#:use-module (gnu packages)
#:export (%input-style
+ %license-prefix
cran->guix-package
bioconductor->guix-package
@@ -82,6 +83,9 @@ (define-module (guix import cran)
(define %input-style
(make-parameter 'variable)) ; or 'specification
+(define %license-prefix
+ (make-parameter identity))
+
(define (string->licenses license-string)
(let ((licenses
(map string-trim-both
@@ -89,9 +93,9 @@ (define (string->licenses license-string)
(char-set-complement (char-set #\|))))))
(string->license licenses)))
-(define string->license
- (let ((prefix identity))
- (match-lambda
+(define (string->license license-string)
+ (let ((prefix (%license-prefix)))
+ (match license-string
("AGPL-3" (prefix 'agpl3))
("AGPL (>= 3)" (prefix 'agpl3+))
("Artistic-2.0" (prefix 'artistic2.0))
diff --git a/guix/scripts/import/cran.scm b/guix/scripts/import/cran.scm
index 2934d4300a..3186bf9248 100644
--- a/guix/scripts/import/cran.scm
+++ b/guix/scripts/import/cran.scm
@@ -53,6 +53,9 @@ (define (show-help)
(display (G_ "
-s, --style=STYLE choose output style, either specification or variable"))
(display (G_ "
+ -p, --license-prefix=PREFIX
+ add custom prefix to licenses, useful for prefixed import of (guix licenses)"))
+ (display (G_ "
-V, --version display version information and exit"))
(newline)
(show-bug-report-information))
@@ -74,6 +77,10 @@ (define %options
(lambda (opt name arg result)
(alist-cons 'style (string->symbol arg)
(alist-delete 'style result))))
+ (option '(#\p "license-prefix") #t #f
+ (lambda (opt name arg result)
+ (alist-cons 'license-prefix arg
+ (alist-delete 'license-prefix result))))
(option '(#\r "recursive") #f #f
(lambda (opt name arg result)
(alist-cons 'recursive #t result)))
@@ -95,8 +102,15 @@ (define (parse-options)
(('argument . value)
value)
(_ #f))
- (reverse opts))))
- (parameterize ((%input-style (assoc-ref opts 'style)))
+ (reverse opts)))
+ (prefix (assoc-ref opts 'license-prefix))
+ (prefix-proc (if (string? prefix)
+ (lambda (symbol)
+ (string->symbol
+ (string-append prefix (symbol->string symbol))))
+ identity)))
+ (parameterize ((%input-style (assoc-ref opts 'style))
+ (%license-prefix prefix-proc))
(match args
((spec)
(let ((name version (package-name->name+version spec)))
--
2.37.3
From 19b0e079f409b90a51620454a1d3026d379c3fb1 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Tue, 18 Oct 2022 12:45:45 +0200
Subject: [PATCH 2/3] import/cran: Allow overriding description fetch function.
X-Debbugs-Cc: zimon.toutoune@gmail.com
X-Debbugs-Cc: dev@jpoiret.xyz
X-Debbugs-Cc: mail@cbaines.net
X-Debbugs-Cc: rekado@elephly.net
X-Debbugs-Cc: othacehe@gnu.org
X-Debbugs-Cc: ludo@gnu.org

* guix/import/cran.scm (%fetch-description): New parameter.
(cran->guix-package): Use it.
(upstream-name): Use it.
---
guix/import/cran.scm | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

Toggle diff (42 lines)
diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index d13231f633..05374b5317 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -56,6 +56,7 @@ (define-module (guix import cran)
#:use-module (gnu packages)
#:export (%input-style
%license-prefix
+ %fetch-description
cran->guix-package
bioconductor->guix-package
@@ -350,6 +351,9 @@ (define* (fetch-description repository name #:optional version)
`(hg-changeset . ,changeset)
meta)))))))))
+(define %fetch-description
+ (make-parameter fetch-description))
+
(define (listify meta field)
"Look up FIELD in the alist META. If FIELD contains a comma-separated
string, turn it into a list and strip off parenthetic expressions. Return the
@@ -640,7 +644,7 @@ (define cran->guix-package
(lambda* (package-name #:key (repo 'cran) version)
"Fetch the metadata for PACKAGE-NAME from REPO and return the `package'
s-expression corresponding to that package, or #f on failure."
- (let ((description (fetch-description repo package-name version)))
+ (let ((description ((%fetch-description) repo package-name version)))
(if description
(description->package repo description)
(case repo
@@ -694,7 +698,7 @@ (define upstream-name
(package->upstream-name pkg))
(define meta
- (fetch-description 'cran upstream-name))
+ ((%fetch-description) 'cran upstream-name))
(and meta
(let ((version (assoc-ref meta "Version")))
--
2.37.3
From 89e46f83c2a39a63326bb4faedf36fb678c03a03 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Tue, 18 Oct 2022 12:45:56 +0200
Subject: [PATCH 3/3] import/cran: Allow overriding tarball download.
X-Debbugs-Cc: zimon.toutoune@gmail.com
X-Debbugs-Cc: dev@jpoiret.xyz
X-Debbugs-Cc: mail@cbaines.net
X-Debbugs-Cc: rekado@elephly.net
X-Debbugs-Cc: othacehe@gnu.org
X-Debbugs-Cc: ludo@gnu.org

* guix/import/cran.scm (%download-source): New parameter.
(description->package): Use it.
---
guix/import/cran.scm | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

Toggle diff (39 lines)
diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index 05374b5317..2a12963532 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -57,6 +57,7 @@ (define-module (guix import cran)
#:export (%input-style
%license-prefix
%fetch-description
+ %download-source
cran->guix-package
bioconductor->guix-package
@@ -265,6 +266,9 @@ (define download
;; of the URLs is the /Archive CRAN URL.
(any (cut download-to-store store <>) urls)))))))))
+(define %download-source
+ (make-parameter download))
+
(define (fetch-description-from-tarball url)
"Fetch the tarball at URL, extra its 'DESCRIPTION' file, parse it, and
return the resulting alist."
@@ -547,10 +551,10 @@ (define (description->package repository meta)
(_ #f)))))
(git? (if (assoc-ref meta 'git) #true #false))
(hg? (if (assoc-ref meta 'hg) #true #false))
- (source (download source-url #:method (cond
- (git? 'git)
- (hg? 'hg)
- (else #f))))
+ (source ((%download-source) source-url #:method (cond
+ (git? 'git)
+ (hg? 'hg)
+ (else #f))))
(sysdepends (append
(if (needs-zlib? source (not (or git? hg?))) '("zlib") '())
(filter (lambda (name)
--
2.37.3
Z
Z
zimoun wrote on 2 Nov 2022 19:25
8735b1b4do.fsf@gmail.com
Hi Lars,

On mer., 19 oct. 2022 at 11:28, Lars-Dominik Braun <lars@6xq.net> wrote:

Toggle quote (6 lines)
> the attached patches are required for guix-cran
> (https://github.com/guix-science/guix-cran). import/cran already has the
> ability to add a prefix to licenses, but it was not exposed. Additionally
> I need to parameterize fetch/download functions, so I can cache the
> tarballs/DESCRIPTION files.

This is really cool! Thanks for working on that.


Toggle quote (8 lines)
> Subject: [PATCH 1/3] import/cran: Allow custom license prefix.
> X-Debbugs-Cc: zimon.toutoune@gmail.com
> X-Debbugs-Cc: dev@jpoiret.xyz
> X-Debbugs-Cc: mail@cbaines.net
> X-Debbugs-Cc: rekado@elephly.net
> X-Debbugs-Cc: othacehe@gnu.org
> X-Debbugs-Cc: ludo@gnu.org

Because the patch had been sent as attachment, I have not received this
X-Debbugs-CC, IIRC. Well, I have added these CC.


Toggle quote (6 lines)
> * guix/import/cran.scm (%license-prefix): New parameter.
> (string->license): Use it.
> * guix/scripts/import/cran.scm (%options): Add new parameter -p/--license-prefix.
> (show-help): Document it.
> (parse-options): Pass it as a parameter to importer.

LGTM. Maybe a line in the manual could help.


Toggle quote (6 lines)
> Subject: [PATCH 2/3] import/cran: Allow overriding description fetch function.
>
> * guix/import/cran.scm (%fetch-description): New parameter.
> (cran->guix-package): Use it.
> (upstream-name): Use it.

[...]

Toggle quote (8 lines)
> Subject: [PATCH 3/3] import/cran: Allow overriding tarball download.
>
> * guix/import/cran.scm (%download-source): New parameter.
> (description->package): Use it.
> ---
> guix/import/cran.scm | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)

Well, I miss what it changes – I have nothing special to comment, so
LGTM. :-)


Cheers,
simon
L
L
Ludovic Courtès wrote on 5 Nov 2022 21:47
(name . Lars-Dominik Braun)(address . lars@6xq.net)(address . 58623@debbugs.gnu.org)
87iljti0w9.fsf@gnu.org
Hi!

Lars-Dominik Braun <lars@6xq.net> skribis:

Toggle quote (6 lines)
> the attached patches are required for guix-cran
> (https://github.com/guix-science/guix-cran). import/cran already has the
> ability to add a prefix to licenses, but it was not exposed. Additionally
> I need to parameterize fetch/download functions, so I can cache the
> tarballs/DESCRIPTION files.

Nice! Minor comments:

Toggle quote (3 lines)
> +(define %license-prefix
> + (make-parameter identity))

Overall, unless it’s impractical, I’d suggest using explicit keyword
parameters instead of SRFI-39 parameters (like this one). That makes
things clearer. (That’s essentially lexical scope vs. dynamic scope.)

Also, when introducing a public global variable, please add a short
comment below the ‘define’ line explaining what it does.

Toggle quote (8 lines)
> +++ b/guix/scripts/import/cran.scm
> @@ -53,6 +53,9 @@ (define (show-help)
> (display (G_ "
> -s, --style=STYLE choose output style, either specification or variable"))
> (display (G_ "
> + -p, --license-prefix=PREFIX
> + add custom prefix to licenses, useful for prefixed import of (guix licenses)"))

I agree with zimoun that this should be documented in the manual. I’d
also remove everything after the comma.

Toggle quote (3 lines)
> +(define %fetch-description
> + (make-parameter fetch-description))

Same comment as above.

Toggle quote (3 lines)
> +(define %download-source
> + (make-parameter download))

Ditto.

Thanks!

Ludo’.
L
L
Lars-Dominik Braun wrote on 30 Nov 2022 17:47
[PATCH v2 0/6] import/cran: Parameterize for guix-cran.
(name . Ludovic Courtès)(address . ludo@gnu.org)
Y4eJBoocw9aAgI6R@noor.fritz.box
Hi Ludo,

here’s a v2, which hopefully addresses your comments. Passing in
arguments required some refactoring in import/utils.scm. I also added
another commit, which speeds up imports significantly. There I tried
to use VALUES (and LET*-VALUES), but ultimately failed and fell back to
LIST and CAR/CADR. There’s probably a better solution?

Cheers,
Lars

Lars-Dominik Braun (6):
import/utils: Pass all arguments through to package builder.
import/cran: Allow custom license prefix.
import/cran: Allow overriding description fetch function.
import/cran: Allow overriding tarball download.
import/cran: Translate more package dependencies.
import/cran: Always operate on source directory.

doc/guix.texi | 4 +
guix/import/cran.scm | 156 +++++++++++++++++------------------
guix/import/crate.scm | 3 +-
guix/import/egg.scm | 3 +-
guix/import/elm.scm | 2 +-
guix/import/gem.scm | 3 +-
guix/import/gnu.scm | 3 +-
guix/import/go.scm | 5 +-
guix/import/hackage.scm | 5 +-
guix/import/hexpm.scm | 2 +-
guix/import/minetest.scm | 5 +-
guix/import/opam.scm | 2 +-
guix/import/pypi.scm | 2 +-
guix/import/stackage.scm | 5 +-
guix/import/texlive.scm | 4 +-
guix/import/utils.scm | 10 +--
guix/scripts/import/cran.scm | 21 ++++-
17 files changed, 130 insertions(+), 105 deletions(-)

--
2.37.4
From fd4a29319686f99bb5d312baefe687dcef3b3f88 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Tue, 18 Oct 2022 12:45:45 +0200
Subject: [PATCH v2 3/6] import/cran: Allow overriding description fetch
function.

* guix/import/cran.scm (cran->guix-package): New parameter FETCH-DESCRIPTION.
---
guix/import/cran.scm | 1 +
1 file changed, 1 insertion(+)

Toggle diff (14 lines)
diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index c24862129f..b2c58ee5ec 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -640,6 +640,7 @@ (define* (description->package repository meta #:key (license-prefix identity))
(define cran->guix-package
(memoize
(lambda* (package-name #:key (repo 'cran) version (license-prefix identity)
+ (fetch-description fetch-description)
#:allow-other-keys)
"Fetch the metadata for PACKAGE-NAME from REPO and return the `package'
s-expression corresponding to that package, or #f on failure."
--
2.37.4
From 217f9f6af608324e593ce114108b97b65182339d Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Tue, 18 Oct 2022 12:45:56 +0200
Subject: [PATCH v2 4/6] import/cran: Allow overriding tarball download.

* guix/import/cran.scm (description->package): New parameter DOWNLOAD-SOURCE.
(cran->guix-package): Ditto.
---
guix/import/cran.scm | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

Toggle diff (48 lines)
diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index b2c58ee5ec..a89deb8e55 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -503,7 +503,8 @@ (define (needs-pkg-config? thing tarball?)
(define (needs-knitr? meta)
(member "knitr" (listify meta "VignetteBuilder")))
-(define* (description->package repository meta #:key (license-prefix identity))
+(define* (description->package repository meta #:key (license-prefix identity)
+ (download-source download))
"Return the `package' s-expression for an R package published on REPOSITORY
from the alist META, which was derived from the R package's DESCRIPTION file."
(let* ((base-url (case repository
@@ -545,10 +546,10 @@ (define* (description->package repository meta #:key (license-prefix identity))
(_ #f)))))
(git? (if (assoc-ref meta 'git) #true #false))
(hg? (if (assoc-ref meta 'hg) #true #false))
- (source (download source-url #:method (cond
- (git? 'git)
- (hg? 'hg)
- (else #f))))
+ (source (download-source source-url #:method (cond
+ (git? 'git)
+ (hg? 'hg)
+ (else #f))))
(sysdepends (append
(if (needs-zlib? source (not (or git? hg?))) '("zlib") '())
(filter (lambda (name)
@@ -641,13 +642,15 @@ (define cran->guix-package
(memoize
(lambda* (package-name #:key (repo 'cran) version (license-prefix identity)
(fetch-description fetch-description)
+ (download-source download)
#:allow-other-keys)
"Fetch the metadata for PACKAGE-NAME from REPO and return the `package'
s-expression corresponding to that package, or #f on failure."
(let ((description (fetch-description repo package-name version)))
(if description
(description->package repo description
- #:license-prefix license-prefix)
+ #:license-prefix license-prefix
+ #:download-source download-source)
(case repo
((git)
;; Retry import from Bioconductor
--
2.37.4
From 28727bc88843b9cf31f1b2eaba5be039ae47856e Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Sat, 22 Oct 2022 10:37:50 +0200
Subject: [PATCH v2 5/6] import/cran: Translate more package dependencies.

Assumes we use package variable names, not package specification names.

* guix/import/cran.scm (invalid-packages): Add more invalid names.
(transform-sysname): Transform more package names.
---
guix/import/cran.scm | 25 +++++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)

Toggle diff (53 lines)
diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index a89deb8e55..b10d9f391b 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -394,10 +394,13 @@ (define invalid-packages
"c++11"
"c++14"
"c++17"
+ "c99"
"getopt::long"
"posix.1-2001"
+ "gnu"
"linux"
"none"
+ "posix.1-2001"
"windows"
"xcode"
"xquartz"))
@@ -405,12 +408,30 @@ (define invalid-packages
(define (transform-sysname sysname)
"Return a Guix package name for the common package name SYSNAME."
(match sysname
+ ("booktabs" "texlive-booktabs")
+ ("bowtie2" "bowtie")
+ ("cat" "coreutils")
("java" "openjdk")
+ ("exiftool" "perl-image-exiftool")
("fftw3" "fftw")
- ("tcl/tk" "tcl")
- ("booktabs" "texlive-booktabs")
("freetype2" "freetype")
+ ("gettext" "gnu-gettext")
+ ("gmake" "gnu-make")
+ ("libarchive-devel" "libarchive")
+ ("libarchive_dev" "libarchive")
+ ("libbz2" "bzip2")
+ ("libexpat" "expat")
+ ("liblz4" "lz4")
+ ("liblzma" "xz")
+ ("libzstd" "zstd")
+ ("libxml2-devel" "libxml2")
+ ("libz" "zlib")
+ ("pandoc-citeproc" "pandoc")
+ ("python3" "python-3")
("sqlite3" "sqlite")
+ ("svn" "subversion")
+ ("tcl/tk" "tcl")
+ ("whoami" "coreutils")
(_ sysname)))
(define cran-guix-name (cut guix-name "r-" <>))
--
2.37.4
L
L
Lars-Dominik Braun wrote on 1 Dec 2022 12:05
(name . Ludovic Courtès)(address . ludo@gnu.org)
Y4iKZciaby7GYVjx@noor.fritz.box
Hi,

looks like I missed an inconsistency, which is fixed by the attched, additional patch.

Cheers,
Lars
From f980c02442ab1cc01787a6cc8462f747ccc5ff11 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 1 Dec 2022 12:00:53 +0100
Subject: [PATCH v2 7/7] import/cran: Depend on gfortran if .f files are
detected too.

There was an inconsistency between tarball-needs-fortran? and
directory-needs-fortran?.

* guix/import/cran.scm (directory-needs-fortran?): Match .f files too.
---
guix/import/cran.scm | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

Toggle diff (15 lines)
diff --git a/guix/import/cran.scm b/guix/import/cran.scm
index f130543c4c..c71ff957b6 100644
--- a/guix/import/cran.scm
+++ b/guix/import/cran.scm
@@ -438,7 +438,7 @@ (define cran-guix-name (cut guix-name "r-" <>))
(define (directory-needs-fortran? dir)
"Check if the directory DIR contains Fortran source files."
- (match (find-files dir "\\.f(90|95)$")
+ (match (find-files dir "\\.f(90|95)?$")
(() #f)
(_ #t)))
--
2.37.4
R
R
Ricardo Wurmus wrote on 31 Dec 2022 14:49
[PATCH 0/3] import/cran: Parameterize for guix-cran.
(address . 58623-done@debbugs.gnu.org)
878rin7kca.fsf@elephly.net
Thank you for the patches! I made minor changes to the commit messages
(to replace “parameter” with “argument”), used multiple values with
SRFI-71 instead of cdr+cadr, adjusted expectations in a test, and fixed
a small bug in a follow-up commit.

--
Ricardo
Closed
?