[bug#37322,1/2] adds the capability of importing a specified version to the crate importer
diff mbox series

Message ID 20190906153211.8613-1-mjbecze@riseup.net
State New
Headers show
Series
  • [bug#37322,1/2] adds the capability of importing a specified version to the crate importer
Related show

Commit Message

Martin Becze Sept. 6, 2019, 3:32 p.m. UTC
---
 guix/import/crate.scm | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Comments

Ludovic Courtès Sept. 8, 2019, 12:27 p.m. UTC | #1
Hello Martin,

So the goal is to allow users to run “guix import rust-foo@1.2.3”,
right?  That’s a good idea.

Martin Becze <mjbecze@riseup.net> skribis:

> diff --git a/guix/import/crate.scm b/guix/import/crate.scm
> index f6057dbf8b..3266ebdfec 100644
> --- a/guix/import/crate.scm
> +++ b/guix/import/crate.scm
> @@ -1,6 +1,7 @@
>  ;;; GNU Guix --- Functional package management for GNU
>  ;;; Copyright © 2016 David Craven <david@craven.ch>
>  ;;; Copyright © 2019 Ludovic Courtès <ludo@gnu.org>
> +;;; Copyright © 2019 Martin Becze <mjbecze@riseup.net>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -181,8 +182,8 @@ and LICENSE."
>    ;; This regexp matches that.
>    (make-regexp "^(.*) OR (.*)$"))
>  
> -(define (crate->guix-package crate-name)
> -  "Fetch the metadata for CRATE-NAME from crates.io, and return the
> +(define (crate->guix-package crate-name@version)
> +  "Fetch the metadata for CRATE-NAME@VERSION from crates.io, and return the

The idea is that the (guix import …) modules do not contain UI-related
stuff, and that UI-related stuff is instead isolated in (guix scripts
import …).

Thus, splitting the “crate@version” string should happen in (guix
scripts import crate) rather than in this module.

Instead, you could simply change ‘crate->guix-package’ so that it takes
an optional ‘version’ field:

  (define* (crate->guix-package crate-name #:optional version)
    …)

In a second step, you’d change (guix scripts import crate) so that it
passes that version string to ‘crate->guix-package’.

How does that sound?

> +  (define crate-name-version-list
> +    (let ((lnv (string-split crate-name@version #\@)))
> +      (if (= 1 (length lnv))
> +	  (append lnv '(#f))
> +	  lnv)))
> +
> +  (define crate-name
> +    (car crate-name-version-list))
> +
> +
>    (define crate
>      (lookup-crate crate-name))
>  
> +  (define crate-version-string
> +    (let ((version (cadr crate-name-version-list)))
> +      (if version
> +        version
> +        (crate-latest-version crate))))

We have a policy of not using ‘cadr’ and friends:

  https://guix.gnu.org/manual/en/html_node/Data-Types-and-Pattern-Matching.html

So the code above would typically look like:

  (define-values (name version)
    (match (string-split crate-spec #\@)
      ((name version) (values name version))
      ((name)         (values name #f))))

Could you send an updated patch (actually two patches)?

Thanks for working on the importer!

Ludo’.
Martin Becze Sept. 9, 2019, 3:32 p.m. UTC | #2
yep! thats the idea. I'll fix up the guile soon.

On 9/8/19 2:27 PM, Ludovic Courtès wrote:
> Hello Martin,
>
> So the goal is to allow users to run “guix import rust-foo@1.2.3”,
> right?  That’s a good idea.
>
> Martin Becze <mjbecze@riseup.net> skribis:
>
>> diff --git a/guix/import/crate.scm b/guix/import/crate.scm
>> index f6057dbf8b..3266ebdfec 100644
>> --- a/guix/import/crate.scm
>> +++ b/guix/import/crate.scm
>> @@ -1,6 +1,7 @@
>>  ;;; GNU Guix --- Functional package management for GNU
>>  ;;; Copyright © 2016 David Craven <david@craven.ch>
>>  ;;; Copyright © 2019 Ludovic Courtès <ludo@gnu.org>
>> +;;; Copyright © 2019 Martin Becze <mjbecze@riseup.net>
>>  ;;;
>>  ;;; This file is part of GNU Guix.
>>  ;;;
>> @@ -181,8 +182,8 @@ and LICENSE."
>>    ;; This regexp matches that.
>>    (make-regexp "^(.*) OR (.*)$"))
>>  
>> -(define (crate->guix-package crate-name)
>> -  "Fetch the metadata for CRATE-NAME from crates.io, and return the
>> +(define (crate->guix-package crate-name@version)
>> +  "Fetch the metadata for CRATE-NAME@VERSION from crates.io, and return the
> The idea is that the (guix import …) modules do not contain UI-related
> stuff, and that UI-related stuff is instead isolated in (guix scripts
> import …).
>
> Thus, splitting the “crate@version” string should happen in (guix
> scripts import crate) rather than in this module.
>
> Instead, you could simply change ‘crate->guix-package’ so that it takes
> an optional ‘version’ field:
>
>   (define* (crate->guix-package crate-name #:optional version)
>     …)
>
> In a second step, you’d change (guix scripts import crate) so that it
> passes that version string to ‘crate->guix-package’.
>
> How does that sound?
>
>> +  (define crate-name-version-list
>> +    (let ((lnv (string-split crate-name@version #\@)))
>> +      (if (= 1 (length lnv))
>> +	  (append lnv '(#f))
>> +	  lnv)))
>> +
>> +  (define crate-name
>> +    (car crate-name-version-list))
>> +
>> +
>>    (define crate
>>      (lookup-crate crate-name))
>>  
>> +  (define crate-version-string
>> +    (let ((version (cadr crate-name-version-list)))
>> +      (if version
>> +        version
>> +        (crate-latest-version crate))))
> We have a policy of not using ‘cadr’ and friends:
>
>   https://guix.gnu.org/manual/en/html_node/Data-Types-and-Pattern-Matching.html
>
> So the code above would typically look like:
>
>   (define-values (name version)
>     (match (string-split crate-spec #\@)
>       ((name version) (values name version))
>       ((name)         (values name #f))))
>
> Could you send an updated patch (actually two patches)?
>
> Thanks for working on the importer!
>
> Ludo’.

Patch
diff mbox series

diff --git a/guix/import/crate.scm b/guix/import/crate.scm
index f6057dbf8b..3266ebdfec 100644
--- a/guix/import/crate.scm
+++ b/guix/import/crate.scm
@@ -1,6 +1,7 @@ 
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2016 David Craven <david@craven.ch>
 ;;; Copyright © 2019 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2019 Martin Becze <mjbecze@riseup.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -181,8 +182,8 @@  and LICENSE."
   ;; This regexp matches that.
   (make-regexp "^(.*) OR (.*)$"))
 
-(define (crate->guix-package crate-name)
-  "Fetch the metadata for CRATE-NAME from crates.io, and return the
+(define (crate->guix-package crate-name@version)
+  "Fetch the metadata for CRATE-NAME@VERSION from crates.io, and return the
 `package' s-expression corresponding to that package, or #f on failure."
   (define (string->license string)
     (match (regexp-exec %dual-license-rx string)
@@ -193,13 +194,29 @@  and LICENSE."
   (define (normal-dependency? dependency)
     (eq? (crate-dependency-kind dependency) 'normal))
 
+  (define crate-name-version-list
+    (let ((lnv (string-split crate-name@version #\@)))
+      (if (= 1 (length lnv))
+	  (append lnv '(#f))
+	  lnv)))
+
+  (define crate-name
+    (car crate-name-version-list))
+
+
   (define crate
     (lookup-crate crate-name))
 
+  (define crate-version-string
+    (let ((version (cadr crate-name-version-list)))
+      (if version
+        version
+        (crate-latest-version crate))))
+
   (and crate
        (let* ((version        (find (lambda (version)
                                       (string=? (crate-version-number version)
-                                                (crate-latest-version crate)))
+                                                crate-version-string))
                                     (crate-versions crate)))
               (dependencies   (crate-version-dependencies version))
               (dep-crates     (filter normal-dependency? dependencies))