bug#24450: [PATCHv2] Re: pypi importer outputs strange character series in optional dependency case.

[Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>]

Patch number 6:

> From fb0547ef225103c0f8355a7eccc41e0d028f6563 Mon Sep 17 00:00:00 2001
> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
> Date: Thu, 28 Mar 2019 00:26:03 -0400
> Subject: [PATCH 6/9] import: pypi: Parse wheel METADATA instead of
>  metadata.json.

> With newer Wheel releases, there is no more metadata.json file; the METADATA
> file should be used instead (see: https://github.com/pypa/wheel/issues/195).

> This change updates our PyPI importer so that it uses the later.

Typo: should be “latter” instead of “later”.

> * guix/import/pypi.scm (define-module): Remove unnecessary modules and export
>   the PARSE-WHEEL-METADATA method.

Please remove the indentation here.  Also, please don’t use “method”
(because it’s not); use “procedure” instead.

> (parse-wheel-metadata): Add method.

Same here.

> +  (define (requires-dist-header? line)
> +    ;; Return #t if the given LINE is a Requires-Dist header.
> +    (regexp-match? (string-match "^Requires-Dist: " line)))
> +
> +  (define (requires-dist-value line)
> +    (string-drop line (string-length "Requires-Dist: ")))
> +
> +  (define (extra? line)
> +    ;; Return #t if the given LINE is an "extra" requirement.
> +    (regexp-match? (string-match "extra == " line)))

The use of “regexp-match?” here isn’t strictly necessary as the return
value is true-ish anyway.

> +  (call-with-input-file metadata
> +    (lambda (port)
> +      (let loop ((requirements '()))
> +        (let ((line (read-line port)))
> +          ;; Stop at the first 'Provides-Extra' section: the non-optional
> +          ;; requirements appear before the optional ones.
> +          (if (eof-object? line)
> +              (reverse (delete-duplicates requirements))
> +              (cond
> +               ((and (requires-dist-header? line) (not (extra? line)))
> +                (loop (cons (specification->requirement-name
> +                             (requires-dist-value line))
> +                            requirements)))
> +               (else
> +                (loop requirements)))))))))
> +

As before you can simplify the nested let and merge “if” and "cond“.

>    (define (read-wheel-metadata wheel-archive)
>      ;; Given WHEEL-ARCHIVE, a ZIP Python wheel archive, return the package's
> -    ;; requirements.
> +    ;; requirements, or #f if the metadata file contained therein couldn't be
> +    ;; extracted.
>      (let* ((dirname (wheel-url->extracted-directory wheel-url))
> -           (json-file (string-append dirname "/metadata.json")))
> -      (and (zero? (system* "unzip" "-q" wheel-archive json-file))
> -           (dynamic-wind
> -             (const #t)
> -             (lambda ()
> -               (call-with-input-file json-file
> -                 (lambda (port)
> -                   (let* ((metadata (json->scm port))
> -                          (run_requires (hash-ref metadata "run_requires"))
> -                          (requirements (if run_requires
> -                                            (hash-ref (list-ref run_requires 0)
> -                                                       "requires")
> -                                            '())))
> -                     (map specification->requirement-name requirements)))))
> -             (lambda ()
> -               (delete-file json-file)
> -               (rmdir dirname))))))
> +           (metadata (string-append dirname "/METADATA")))
> +      (call-with-temporary-directory
> +       (lambda (dir)
> +         (if (zero? (system* "unzip" "-q" wheel-archive "-d" dir metadata))
> +             (parse-wheel-metadata (string-append dir "/" metadata))
> +             (begin
> +               (warning
> +                (G_ "Failed to extract file: ~a from wheel.~%") metadata)
> +               #f))))))

The old approach took care of removing the unpacked archive no matter
what happened.  The new code doesn’t do that.

> --- a/tests/pypi.scm
> +++ b/tests/pypi.scm

Thanks for the tests!

--
Ricardo