From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxim Cournoyer Subject: bug#24450: [PATCHv2] Re: pypi importer outputs strange character series in optional dependency case. Date: Tue, 11 Jun 2019 09:39:48 +0900 Message-ID: <874l4x550r.fsf@gmail.com> References: <87pnod7ot4.fsf@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Return-path: Received: from eggs.gnu.org ([2001:470:142:3::10]:44976) by lists.gnu.org with esmtp (Exim 4.86_2) (envelope-from ) id 1haUqW-00051z-3H for bug-guix@gnu.org; Mon, 10 Jun 2019 20:41:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1haUqU-0003BA-M3 for bug-guix@gnu.org; Mon, 10 Jun 2019 20:41:04 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:44162) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1haUqU-0003B4-Iq for bug-guix@gnu.org; Mon, 10 Jun 2019 20:41:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1haUqU-0004GP-E0 for bug-guix@gnu.org; Mon, 10 Jun 2019 20:41:02 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: (Ricardo Wurmus's message of "Tue, 28 May 2019 13:04:44 +0200") List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: Ricardo Wurmus Cc: 24450@debbugs.gnu.org Hello! Ricardo Wurmus writes: > Patch number 6: > >> From fb0547ef225103c0f8355a7eccc41e0d028f6563 Mon Sep 17 00:00:00 2001 >> From: Maxim Cournoyer >> 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 META= DATA >> file should be used instead (see: https://github.com/pypa/wheel/issues/1= 95). > >> This change updates our PyPI importer so that it uses the later. > > Typo: should be =E2=80=9Clatter=E2=80=9D instead of =E2=80=9Clater=E2=80= =9D. Fixed. >> * guix/import/pypi.scm (define-module): Remove unnecessary modules and e= xport >> the PARSE-WHEEL-METADATA method. > > Please remove the indentation here. Also, please don=E2=80=99t use =E2= =80=9Cmethod=E2=80=9D > (because it=E2=80=99s not); use =E2=80=9Cprocedure=E2=80=9D instead. Done. Thanks for fixing my terminology :-). >> (parse-wheel-metadata): Add method. > > Same here. Done. >> + (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 =3D=3D " line))) > > The use of =E2=80=9Cregexp-match?=E2=80=9D here isn=E2=80=99t strictly ne= cessary as the return > value is true-ish anyway. Done. >> + (call-with-input-file metadata >> + (lambda (port) >> + (let loop ((requirements '())) >> + (let ((line (read-line port))) >> + ;; Stop at the first 'Provides-Extra' section: the non-option= al >> + ;; 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 =E2=80=9Cif=E2=80=9D = and "cond=E2=80=9C. Oh, I get it now, I think: --8<---------------cut here---------------start------------->8--- =20 (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))))))))) + (match (read-line port) + (line + ;; Stop at the first 'Provides-Extra' section: the non-optional + ;; requirements appear before the optional ones. + (cond + ((eof-object? line) + (reverse (delete-duplicates requirements))) + ((and (requires-dist-header? line) (not (extra? line))) + (loop (cons (specification->requirement-name + (requires-dist-value line)) + requirements))) + (else + (loop requirements))))))))) =20 (define (guess-requirements source-url wheel-url archive) "Given SOURCE-URL, WHEEL-URL and a ARCHIVE of the package, return a list --8<---------------cut here---------------end--------------->8--- >> (define (read-wheel-metadata wheel-archive) >> ;; Given WHEEL-ARCHIVE, a ZIP Python wheel archive, return the pack= age's >> - ;; requirements. >> + ;; requirements, or #f if the metadata file contained therein could= n'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_require= s")) >> - (requirements (if run_requires >> - (hash-ref (list-ref run_req= uires 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 metada= ta)) >> + (parse-wheel-metadata (string-append dir "/" metadata)) >> + (begin >> + (warning >> + (G_ "Failed to extract file: ~a from wheel.~%") metadat= a) >> + #f)))))) > > The old approach took care of removing the unpacked archive no matter > what happened. The new code doesn=E2=80=99t do that. The temporary directory where the archive is unpacked should be cleared when leaving upon leaving its scope; the docstring of "call-with-temporary-directory" says: "Call PROC with a name of a temporary directory; close the directory and delete it when leaving the dynamic extent of this call." >> --- a/tests/pypi.scm >> +++ b/tests/pypi.scm > > Thanks for the tests! :-) Maxim