Subject: [bug#38408] [PATCH v16 3/6] import: crate: Use guile-semver to resolve module versions.

From: Hartmut Goebel <[email-masked]>
Date: Tue, 10 Nov 2020 23:13:04 +0100

Am 10.11.20 um 22:39 schrieb Hartmut Goebel:
> From: Martin Becze <[email-masked]>
>
> * guix/import/crate.scm: Add guile-semver as a soft dependency.
> […]

Please add a brief change description for crate-recursive-import.

> * guix/import/utils.scm (package-names->package-inputs): Implement
>   handling of (name version) pairs.
> * guix/scripts/import/crate.scm (guix-import-crate): Move
>   'package-definition' from here to guix/import/crate.scm.

I could not spot this change. has this been reverted in one of the many
version of this series?

Also this file contains a change not described here.

> -(define* (crate->guix-package crate-name #:optional version)
> +(define* (crate->guix-package crate-name #:key version repo)
>    "Fetch the metadata for CRATE-NAME from crates.io, and return the
>  `package' s-expression corresponding to that package, or #f on failure.
>  When VERSION is specified, attempt to fetch that version; otherwise fetch the
>  latest version of CRATE-NAME."

Does this still fetch the latest version? Or is it the latest version
matching a semver range, or the latest non-beta version?

> @@ -231,15 +264,12 @@ latest version of CRATE-NAME."
>                             #:description (crate-description crate)
>                             #:license (and=> (crate-version-license version*)
>                                              string->license))
> -          (append cargo-inputs cargo-development-inputs)))))
> -
> -(define* (crate-recursive-import crate-name #:optional version)
> -  (recursive-import crate-name #f
> -                    #:repo->guix-package
> -                    (lambda (name repo)
> -                      (let ((version (and (string=? name crate-name)
> -                                          version)))
> -                        (crate->guix-package name version)))
> +          cargo-inputs))))

Is this change in the last line intended? Another patch of this series
reverts this change.

> @@ -131,15 +137,18 @@
>    \"dependencies\": [
>       {
>         \"crate_id\": \"intermediate-2\",
> -       \"kind\": \"normal\"
> +       \"kind\": \"normal\",
> +       \"req\": \"1.0.0\"
>       },
>       {
>         \"crate_id\": \"leaf-alice\",
> -       \"kind\": \"normal\"
> +       \"kind\": \"normal\",
> +       \"req\": \"1.0.0\"
>       },
>       {
>         \"crate_id\": \"leaf-bob\",
> -       \"kind\": \"normal\"
> +       \"kind\": \"normal\",
> +       \"req\": \"1.0.0\"
>       }
>    ]
>  }")

All packages in this test have version 1.0.0 and the requirement is
1.0.0, too. So semver resolution is not actually tested by these
packages. I suggest to add more available versions to some of the
packages and actually use a different semver operators.

To avoid confusion, maybe it makes sense to rename packages to get rid
of the trailing number. E.g. "intermediate-1" could become "intermediate-A"

Please also make sure, there is a test-case with some 0.x version, since
crates.io documentations says [1]: "This compatibility convention is
different from SemVer in the way it treats versions before 1.0.0. …"

[1]
https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#caret-requirements

-- 
Regards
Hartmut Goebel

| Hartmut Goebel          | [email-masked]               |
| www.crazy-compilers.com | compilers which you thought are impossible |