[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
?
Your comment

This issue is archived.

To comment on this conversation send an email to 58623@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 58623
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch