GNU bug report logs - #58623
[PATCH 0/3] import/cran: Parameterize for guix-cran.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 58623 in the body.
You can then email your comments to 58623 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: guix-patches <at> gnu.org
Subject: [PATCH 0/3] import/cran: Parameterize for guix-cran.
Date: Wed, 19 Oct 2022 11:28:53 +0200

[Message part 1 (text/plain, inline)]

Hi,

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.

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

[0001-import-cran-Allow-custom-license-prefix.patch (text/plain, attachment)]

[0002-import-cran-Allow-overriding-description-fetch-funct.patch (text/plain, attachment)]

[0003-import-cran-Allow-overriding-tarball-download.patch (text/plain, attachment)]

Message #8 received at 58623 <at> debbugs.gnu.org (full text, mbox):

From: zimoun <zimon.toutoune <at> gmail.com>
To: Lars-Dominik Braun <lars <at> 6xq.net>, 58623 <at> debbugs.gnu.org
Cc: Ricardo Wurmus <rekado <at> elephly.net>, Mathieu Othacehe <othacehe <at> gnu.org>,
 ludo <at> gnu.org, Christopher Baines <mail <at> cbaines.net>,
 Josselin Poiret <dev <at> jpoiret.xyz>
Subject: Re: [bug#58623] [PATCH 0/3] import/cran: Parameterize for guix-cran.
Date: Wed, 02 Nov 2022 19:25:23 +0100

Hi Lars,

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

> 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.


> Subject: [PATCH 1/3] import/cran: Allow custom license prefix.
> X-Debbugs-Cc: zimon.toutoune <at> gmail.com
> X-Debbugs-Cc: dev <at> jpoiret.xyz
> X-Debbugs-Cc: mail <at> cbaines.net
> X-Debbugs-Cc: rekado <at> elephly.net
> X-Debbugs-Cc: othacehe <at> gnu.org
> X-Debbugs-Cc: ludo <at> 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.


> * 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.


> 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.

[...]

> 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

Message #11 received at 58623 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: 58623 <at> debbugs.gnu.org
Subject: Re: [bug#58623] [PATCH 0/3] import/cran: Parameterize for guix-cran.
Date: Sat, 05 Nov 2022 21:47:50 +0100

Hi!

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

> 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:

> +(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.

> +++ 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.

> +(define %fetch-description
> +  (make-parameter fetch-description))

Same comment as above.

> +(define %download-source
> +  (make-parameter download))

Ditto.

Thanks!

Ludo’.

Message #14 received at 58623 <at> debbugs.gnu.org (full text, mbox):

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: rekado <at> elephly.net, 58623 <at> debbugs.gnu.org
Subject: [PATCH v2 0/6] import/cran: Parameterize for guix-cran.
Date: Wed, 30 Nov 2022 17:47:02 +0100

[Message part 1 (text/plain, inline)]

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

[0001-import-utils-Pass-all-arguments-through-to-package-b.patch (text/plain, attachment)]

[0002-import-cran-Allow-custom-license-prefix.patch (text/plain, attachment)]

[0003-import-cran-Allow-overriding-description-fetch-funct.patch (text/plain, attachment)]

[0004-import-cran-Allow-overriding-tarball-download.patch (text/plain, attachment)]

[0005-import-cran-Translate-more-package-dependencies.patch (text/plain, attachment)]

[0006-import-cran-Always-operate-on-source-directory.patch (text/plain, attachment)]

Message #17 received at 58623 <at> debbugs.gnu.org (full text, mbox):

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: rekado <at> elephly.net, 58623 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 0/6] import/cran: Parameterize for guix-cran.
Date: Thu, 1 Dec 2022 12:05:09 +0100

[Message part 1 (text/plain, inline)]

Hi,

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

Cheers,
Lars

[0007-import-cran-Depend-on-gfortran-if-.f-files-are-detec.patch (text/plain, attachment)]

Message #22 received at 58623-done <at> debbugs.gnu.org (full text, mbox):

From: Ricardo Wurmus <rekado <at> elephly.net>
To: 58623-done <at> debbugs.gnu.org
Subject: [PATCH 0/3] import/cran: Parameterize for guix-cran.
Date: Sat, 31 Dec 2022 14:49:08 +0100

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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.