[bug#53828] guix: opam: Allow importing local files.

Hi Guix!

This patch lets you import opam packages from a local file. This is a
feature that was requested a few times, and there it is :)

Now, if you have an OCaml package sources that provides an opam file
(usually called opam, or with the .opam extension), you can import the
package, recursively or not with something like:

guix import opam foo -f foo.opam -r

instead of fetching foo from the repositories, it will fetch the data
directly from the given file. WDYT?

Julien Lepiller schreef op zo 06-02-2022 om 22:41 [+0100]:
> Now, if you have an OCaml package sources that provides an opam file
> (usually called opam, or with the .opam extension), you can import
> the
> package, recursively or not with something like:
> 
> guix import opam foo -f foo.opam -r
> 
> instead of fetching foo from the repositories, it will fetch the data
> directly from the given file. WDYT?

I don't know OCaml at all, but that seems potentially useful.
Can we have some tests for this in tests/opam.scm?

Greetings,
Maxime.

Julien schrieb am Sonntag der 06. Februar 2022 um 22:41 +01:

> Hi Guix!
>
> This patch lets you import opam packages from a local file. This is a
> feature that was requested a few times, and there it is :)
>
> Now, if you have an OCaml package sources that provides an opam file
> (usually called opam, or with the .opam extension), you can import the
> package, recursively or not with something like:
>
> guix import opam foo -f foo.opam -r
>
> instead of fetching foo from the repositories, it will fetch the data
> directly from the given file. WDYT?

Why just limit it to local files?  What about accepting a tarball on the
internet or any VCS repository?  This could generally be useful for any
importer.

Also, since we already know the name of the package, maybe specifying
the directory that contains the OCaml project would be enough?

  guix import opam foo --type=local /path/to/directory
  guix import opam foo --type=tarball https://example.org/foo.tar.gz
  guix import opam foo --type=git https://git.example.org/foo
  …

Or am I asking for too much?  :-)

> From b027391357eef4d6fa1bb8741528cf9650aef568 Mon Sep 17 00:00:00 2001
> Message-Id: <b027391357eef4d6fa1bb8741528cf9650aef568.1644183523.git.julien@lepiller.eu>
> From: Julien Lepiller <julien@lepiller.eu>
> Date: Sun, 6 Feb 2022 22:35:14 +0100
> Subject: [PATCH] guix: opam: Allow importing local files.

It should be “import: opam: …”.

> * guix/scripts/import/opam.scm (guix-import-opam): Support `--file` flag.
> * guix/import/utils.scm (recursive-import): Add optional file argument.
> Pass it to the first iteration when available.
> * guix/import/opam.scm (opam-fetch): Fetch from file when available.
> (opam->guix-source): Factorize source out of opam->guix-package.
> (opam->guix-package): Add file argument.  Ensure it does not fail when
> name, version and url are not available.
> ---
>  guix/import/opam.scm         | 101 +++++++++++++++++++----------------
>  guix/import/utils.scm        |  17 +++---
>  guix/scripts/import/opam.scm |  11 +++-
>  3 files changed, 75 insertions(+), 54 deletions(-)
>
> diff --git a/guix/import/opam.scm b/guix/import/opam.scm
> index a6f6fe8c9f..05f79110f8 100644
> --- a/guix/import/opam.scm
> +++ b/guix/import/opam.scm
> @@ -310,8 +310,11 @@ (define (dependency-list->inputs lst)
>    (map string->symbol
>         (ocaml-names->guix-names lst)))
>  
> -(define* (opam-fetch name #:optional (repositories-specs '("opam")))
> -  (or (fold (lambda (repository others)
> +(define* (opam-fetch name #:optional (repositories-specs '("opam")) (file #f))
> +  (or (and file (let ((metadata (get-metadata file)))
> +                  `(("metadata" . ,metadata)
> +                    ("version" . #f))))
> +      (fold (lambda (repository others)
>                (match (find-latest-version name repository)
>                  ((_ version file) `(("metadata" ,@(get-metadata file))
>                                      ("version" . ,version)))
> @@ -320,17 +323,29 @@ (define* (opam-fetch name #:optional (repositories-specs '("opam")))
>              (filter-map get-opam-repository repositories-specs))
>        (warning (G_ "opam: package '~a' not found~%") name)))
>  
> -(define* (opam->guix-package name #:key (repo 'opam) version)
> +(define (opam->guix-source url-dict)
> +  (let ((source-url (and url-dict
> +                         (or (metadata-ref url-dict "src")
> +                             (metadata-ref url-dict "archive")))))
> +    (if source-url
> +        (call-with-temporary-output-file
> +          (lambda (temp port)
> +            (and (url-fetch source-url temp)
> +                 `(origin
> +                    (method url-fetch)
> +                    (uri ,source-url)
> +                    (sha256 (base32 ,(guix-hash-url temp)))))))
> +        'no-source-information)))

I would use ‘and-let*’ instead of ‘let’, and drop the ‘if’ form.

The ability to import from a local file seems to mostly be useful when
the package isn’t meant to be submitted to Guix, right?  Wouldn’t it
make more sense to make the source a ‘local-file’ object instead of just
setting it to #f?

> +(define* (opam->guix-package name #:key (repo '("opam")) (file #f) version)
>    "Import OPAM package NAME from REPOSITORY (a directory name) or, if
> -REPOSITORY is #f, from the official OPAM repository.  Return a 'package' sexp
> +REPOSITORY is #f, from the official OPAM repository.  When FILE is defined,
> +import the packaged defined in the opam FILE instead.  Return a 'package' sexp

s/packaged/package/

>    (and-let* ((with-opam (if (member "opam" repo) repo (cons "opam" repo)))
> -             (opam-file (opam-fetch name with-opam))
> -             (version (assoc-ref opam-file "version"))
> +             (opam-file (opam-fetch name with-opam file))
>               (opam-content (assoc-ref opam-file "metadata"))
> -             (url-dict (metadata-ref opam-content "url"))
> -             (source-url (or (metadata-ref url-dict "src")
> -                             (metadata-ref url-dict "archive")))
> +             (source (opam->guix-source (metadata-ref opam-content "url")))
>               (requirements (metadata-ref opam-content "depends"))
>               (names (dependency-list->names requirements))
>               (dependencies (filter-dependencies names))
> @@ -344,47 +359,41 @@ (define* (opam->guix-package name #:key (repo 'opam) version)
>                                    (not (member name '("dune" "jbuilder"))))
>                                  native-dependencies))))
>          (let ((use-dune? (member "dune" names)))
> -          (call-with-temporary-output-file
> -            (lambda (temp port)
> -              (and (url-fetch source-url temp)
> -                   (values
> -                    `(package
> -                       (name ,(ocaml-name->guix-name name))
> -                       (version ,version)
> -                       (source
> -                         (origin
> -                           (method url-fetch)
> -                           (uri ,source-url)
> -                           (sha256 (base32 ,(guix-hash-url temp)))))
> -                       (build-system ,(if use-dune?
> -                                          'dune-build-system
> -                                          'ocaml-build-system))
> -                       ,@(if (null? inputs)
> -                           '()
> -                           `((propagated-inputs (list ,@inputs))))
> -                       ,@(if (null? native-inputs)
> -                           '()
> -                           `((native-inputs (list ,@native-inputs))))
> -                       ,@(if (equal? name (guix-name->opam-name (ocaml-name->guix-name name)))
> -                           '()
> -                           `((properties
> -                               ,(list 'quasiquote `((upstream-name . ,name))))))
> -                       (home-page ,(metadata-ref opam-content "homepage"))
> -                       (synopsis ,(metadata-ref opam-content "synopsis"))
> -                       (description ,(beautify-description
> -                                      (metadata-ref opam-content "description")))
> -                       (license ,(spdx-string->license
> -                                  (metadata-ref opam-content "license"))))
> -                    (filter
> -                      (lambda (name)
> -                        (not (member name '("dune" "jbuilder"))))
> -                      dependencies))))))))
> +          (values
> +           `(package
> +              (name ,(and name (ocaml-name->guix-name name)))
> +              (version ,(assoc-ref opam-file "version"))
> +              (source ,source)
> +              (build-system ,(if use-dune?
> +                                 'dune-build-system
> +                                 'ocaml-build-system))
> +              ,@(if (null? inputs)
> +                  '()
> +                  `((propagated-inputs (list ,@inputs))))
> +              ,@(if (null? native-inputs)
> +                  '()
> +                  `((native-inputs (list ,@native-inputs))))
> +              ,@(if (and name (equal? name (guix-name->opam-name (ocaml-name->guix-name name))))
> +                  '()
> +                  `((properties
> +                      ,(list 'quasiquote `((upstream-name . ,name))))))
> +              (home-page ,(metadata-ref opam-content "homepage"))
> +              (synopsis ,(metadata-ref opam-content "synopsis"))
> +              (description ,(beautify-description
> +                             (metadata-ref opam-content "description")))
> +              (license ,(spdx-string->license
> +                         (metadata-ref opam-content "license"))))
> +           (filter
> +             (lambda (name)
> +               (not (member name '("dune" "jbuilder"))))
> +             dependencies)))))
>  
> -(define* (opam-recursive-import package-name #:key repo)
> +(define* (opam-recursive-import package-name #:key repo file)
>    (recursive-import package-name
>                      #:repo->guix-package opam->guix-package
>                      #:guix-name ocaml-name->guix-name
> -                    #:repo repo))
> +                    #:repo repo
> +                    #:file file))
>  
>  (define (guix-name->opam-name name)
>    (if (string-prefix? "ocaml-" name)
> diff --git a/guix/import/utils.scm b/guix/import/utils.scm
> index 1c3cfa3e0b..ab35b8a4fc 100644
> --- a/guix/import/utils.scm
> +++ b/guix/import/utils.scm
> @@ -470,7 +470,7 @@ (define (topological-sort nodes
>                     (set-insert (node-name head) visited))))))))
>  
>  (define* (recursive-import package-name
> -                           #:key repo->guix-package guix-name version repo
> +                           #:key repo->guix-package guix-name version repo file
>                             #:allow-other-keys)
>    "Return a list of package expressions for PACKAGE-NAME and all its
>  dependencies, sorted in topological order.  For each package,

The docstrings should also be updated.

On February 7, 2022 9:27:39 AM GMT+01:00, Xinglu Chen <public@yoctocell.xyz> wrote:
>Julien schrieb am Sonntag der 06. Februar 2022 um 22:41 +01:
>
>> Hi Guix!
>>
>> This patch lets you import opam packages from a local file. This is a
>> feature that was requested a few times, and there it is :)
>>
>> Now, if you have an OCaml package sources that provides an opam file
>> (usually called opam, or with the .opam extension), you can import the
>> package, recursively or not with something like:
>>
>> guix import opam foo -f foo.opam -r
>>
>> instead of fetching foo from the repositories, it will fetch the data
>> directly from the given file. WDYT?
>
>Why just limit it to local files?  What about accepting a tarball on the
>internet or any VCS repository?  This could generally be useful for any
>importer.

That's a good idea, but I'm not sure how to recognize these cases.

>
>Also, since we already know the name of the package, maybe specifying
>the directory that contains the OCaml project would be enough?

Sure, that could be useful for your suggestion below to use the local directory as the source.

>
>  guix import opam foo --type=local /path/to/directory
>  guix import opam foo --type=tarball https://example.org/foo.tar.gz
>  guix import opam foo --type=git https://git.example.org/foo
>  …
>
>Or am I asking for too much?  :-)
>
>> From b027391357eef4d6fa1bb8741528cf9650aef568 Mon Sep 17 00:00:00 2001
>> Message-Id: <b027391357eef4d6fa1bb8741528cf9650aef568.1644183523.git.julien@lepiller.eu>
>> From: Julien Lepiller <julien@lepiller.eu>
>> Date: Sun, 6 Feb 2022 22:35:14 +0100
>> Subject: [PATCH] guix: opam: Allow importing local files.
>
>It should be “import: opam: …”.
>
>> * guix/scripts/import/opam.scm (guix-import-opam): Support `--file` flag.
>> * guix/import/utils.scm (recursive-import): Add optional file argument.
>> Pass it to the first iteration when available.
>> * guix/import/opam.scm (opam-fetch): Fetch from file when available.
>> (opam->guix-source): Factorize source out of opam->guix-package.
>> (opam->guix-package): Add file argument.  Ensure it does not fail when
>> name, version and url are not available.
>> ---
>>  guix/import/opam.scm         | 101 +++++++++++++++++++----------------
>>  guix/import/utils.scm        |  17 +++---
>>  guix/scripts/import/opam.scm |  11 +++-
>>  3 files changed, 75 insertions(+), 54 deletions(-)
>>
>> diff --git a/guix/import/opam.scm b/guix/import/opam.scm
>> index a6f6fe8c9f..05f79110f8 100644
>> --- a/guix/import/opam.scm
>> +++ b/guix/import/opam.scm
>> @@ -310,8 +310,11 @@ (define (dependency-list->inputs lst)
>>    (map string->symbol
>>         (ocaml-names->guix-names lst)))
>>  
>> -(define* (opam-fetch name #:optional (repositories-specs '("opam")))
>> -  (or (fold (lambda (repository others)
>> +(define* (opam-fetch name #:optional (repositories-specs '("opam")) (file #f))
>> +  (or (and file (let ((metadata (get-metadata file)))
>> +                  `(("metadata" . ,metadata)
>> +                    ("version" . #f))))
>> +      (fold (lambda (repository others)
>>                (match (find-latest-version name repository)
>>                  ((_ version file) `(("metadata" ,@(get-metadata file))
>>                                      ("version" . ,version)))
>> @@ -320,17 +323,29 @@ (define* (opam-fetch name #:optional (repositories-specs '("opam")))
>>              (filter-map get-opam-repository repositories-specs))
>>        (warning (G_ "opam: package '~a' not found~%") name)))
>>  
>> -(define* (opam->guix-package name #:key (repo 'opam) version)
>> +(define (opam->guix-source url-dict)
>> +  (let ((source-url (and url-dict
>> +                         (or (metadata-ref url-dict "src")
>> +                             (metadata-ref url-dict "archive")))))
>> +    (if source-url
>> +        (call-with-temporary-output-file
>> +          (lambda (temp port)
>> +            (and (url-fetch source-url temp)
>> +                 `(origin
>> +                    (method url-fetch)
>> +                    (uri ,source-url)
>> +                    (sha256 (base32 ,(guix-hash-url temp)))))))
>> +        'no-source-information)))
>
>I would use ‘and-let*’ instead of ‘let’, and drop the ‘if’ form.

That would return #f and make the ocher and-let* in opam->guix-package fail.

>
>The ability to import from a local file seems to mostly be useful when
>the package isn’t meant to be submitted to Guix, right?  Wouldn’t it
>make more sense to make the source a ‘local-file’ object instead of just
>setting it to #f?

Excellent idea.

>
>> +(define* (opam->guix-package name #:key (repo '("opam")) (file #f) version)
>>    "Import OPAM package NAME from REPOSITORY (a directory name) or, if
>> -REPOSITORY is #f, from the official OPAM repository.  Return a 'package' sexp
>> +REPOSITORY is #f, from the official OPAM repository.  When FILE is defined,
>> +import the packaged defined in the opam FILE instead.  Return a 'package' sexp
>
>s/packaged/package/
>
>>    (and-let* ((with-opam (if (member "opam" repo) repo (cons "opam" repo)))
>> -             (opam-file (opam-fetch name with-opam))
>> -             (version (assoc-ref opam-file "version"))
>> +             (opam-file (opam-fetch name with-opam file))
>>               (opam-content (assoc-ref opam-file "metadata"))
>> -             (url-dict (metadata-ref opam-content "url"))
>> -             (source-url (or (metadata-ref url-dict "src")
>> -                             (metadata-ref url-dict "archive")))
>> +             (source (opam->guix-source (metadata-ref opam-content "url")))
>>               (requirements (metadata-ref opam-content "depends"))
>>               (names (dependency-list->names requirements))
>>               (dependencies (filter-dependencies names))
>> @@ -344,47 +359,41 @@ (define* (opam->guix-package name #:key (repo 'opam) version)
>>                                    (not (member name '("dune" "jbuilder"))))
>>                                  native-dependencies))))
>>          (let ((use-dune? (member "dune" names)))
>> -          (call-with-temporary-output-file
>> -            (lambda (temp port)
>> -              (and (url-fetch source-url temp)
>> -                   (values
>> -                    `(package
>> -                       (name ,(ocaml-name->guix-name name))
>> -                       (version ,version)
>> -                       (source
>> -                         (origin
>> -                           (method url-fetch)
>> -                           (uri ,source-url)
>> -                           (sha256 (base32 ,(guix-hash-url temp)))))
>> -                       (build-system ,(if use-dune?
>> -                                          'dune-build-system
>> -                                          'ocaml-build-system))
>> -                       ,@(if (null? inputs)
>> -                           '()
>> -                           `((propagated-inputs (list ,@inputs))))
>> -                       ,@(if (null? native-inputs)
>> -                           '()
>> -                           `((native-inputs (list ,@native-inputs))))
>> -                       ,@(if (equal? name (guix-name->opam-name (ocaml-name->guix-name name)))
>> -                           '()
>> -                           `((properties
>> -                               ,(list 'quasiquote `((upstream-name . ,name))))))
>> -                       (home-page ,(metadata-ref opam-content "homepage"))
>> -                       (synopsis ,(metadata-ref opam-content "synopsis"))
>> -                       (description ,(beautify-description
>> -                                      (metadata-ref opam-content "description")))
>> -                       (license ,(spdx-string->license
>> -                                  (metadata-ref opam-content "license"))))
>> -                    (filter
>> -                      (lambda (name)
>> -                        (not (member name '("dune" "jbuilder"))))
>> -                      dependencies))))))))
>> +          (values
>> +           `(package
>> +              (name ,(and name (ocaml-name->guix-name name)))
>> +              (version ,(assoc-ref opam-file "version"))
>> +              (source ,source)
>> +              (build-system ,(if use-dune?
>> +                                 'dune-build-system
>> +                                 'ocaml-build-system))
>> +              ,@(if (null? inputs)
>> +                  '()
>> +                  `((propagated-inputs (list ,@inputs))))
>> +              ,@(if (null? native-inputs)
>> +                  '()
>> +                  `((native-inputs (list ,@native-inputs))))
>> +              ,@(if (and name (equal? name (guix-name->opam-name (ocaml-name->guix-name name))))
>> +                  '()
>> +                  `((properties
>> +                      ,(list 'quasiquote `((upstream-name . ,name))))))
>> +              (home-page ,(metadata-ref opam-content "homepage"))
>> +              (synopsis ,(metadata-ref opam-content "synopsis"))
>> +              (description ,(beautify-description
>> +                             (metadata-ref opam-content "description")))
>> +              (license ,(spdx-string->license
>> +                         (metadata-ref opam-content "license"))))
>> +           (filter
>> +             (lambda (name)
>> +               (not (member name '("dune" "jbuilder"))))
>> +             dependencies)))))
>>  
>> -(define* (opam-recursive-import package-name #:key repo)
>> +(define* (opam-recursive-import package-name #:key repo file)
>>    (recursive-import package-name
>>                      #:repo->guix-package opam->guix-package
>>                      #:guix-name ocaml-name->guix-name
>> -                    #:repo repo))
>> +                    #:repo repo
>> +                    #:file file))
>>  
>>  (define (guix-name->opam-name name)
>>    (if (string-prefix? "ocaml-" name)
>> diff --git a/guix/import/utils.scm b/guix/import/utils.scm
>> index 1c3cfa3e0b..ab35b8a4fc 100644
>> --- a/guix/import/utils.scm
>> +++ b/guix/import/utils.scm
>> @@ -470,7 +470,7 @@ (define (topological-sort nodes
>>                     (set-insert (node-name head) visited))))))))
>>  
>>  (define* (recursive-import package-name
>> -                           #:key repo->guix-package guix-name version repo
>> +                           #:key repo->guix-package guix-name version repo file
>>                             #:allow-other-keys)
>>    "Return a list of package expressions for PACKAGE-NAME and all its
>>  dependencies, sorted in topological order.  For each package,
>
>The docstrings should also be updated.

Hi,

Julien schrieb am Montag der 07. Februar 2022 um 09:46 +01:

>>> +(define (opam->guix-source url-dict)
>>> +  (let ((source-url (and url-dict
>>> +                         (or (metadata-ref url-dict "src")
>>> +                             (metadata-ref url-dict "archive")))))
>>> +    (if source-url
>>> +        (call-with-temporary-output-file
>>> +          (lambda (temp port)
>>> +            (and (url-fetch source-url temp)
>>> +                 `(origin
>>> +                    (method url-fetch)
>>> +                    (uri ,source-url)
>>> +                    (sha256 (base32 ,(guix-hash-url temp)))))))
>>> +        'no-source-information)))
>>
>>I would use ‘and-let*’ instead of ‘let’, and drop the ‘if’ form.
>
> That would return #f and make the ocher and-let* in opam->guix-package
> fail.

Right, I missed the ‘and-let*’ in ‘opam->guix-package’.  However, seeing
as the ‘source’ variable is only used in the body of the ‘and-let*’, I
think it would make sense to not even bind the result of
‘opam->guix-source’ to any variable, instead using it directly in the
‘package’ form.

  `(package
     …
     (source ,source)
     …)

becomes

  `(package
     …
     (source ,(opam->guix-source (metadata-ref opam-content "url")))
     …)

WDYT?