From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ricardo Wurmus Subject: bug#24450: [PATCHv2] Re: pypi importer outputs strange character series in optional dependency case. Date: Tue, 28 May 2019 12:23:53 +0200 Message-ID: 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 ([209.51.188.92]:51327) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hVZHG-0003hF-Id for bug-guix@gnu.org; Tue, 28 May 2019 06:24:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hVZH9-0005N2-Pw for bug-guix@gnu.org; Tue, 28 May 2019 06:24:14 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:41594) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hVZH0-0005Je-JX for bug-guix@gnu.org; Tue, 28 May 2019 06:24:05 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hVZH0-0006cg-Da for bug-guix@gnu.org; Tue, 28 May 2019 06:24:02 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-Reply-To: <87pnod7ot4.fsf@gmail.com> 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: Maxim Cournoyer Cc: 24450@debbugs.gnu.org On to the next: > From 73e27235cac1275ba7671fd2364325cf5788cb3c Mon Sep 17 00:00:00 2001 > From: Maxim Cournoyer > Date: Thu, 28 Mar 2019 00:26:02 -0400 > Subject: [PATCH 5/9] import: pypi: Support more types of archives. > > This change enables the PyPI importer to look for requirements in a source > archive of a different type than "tar.gz" or "tar.bz2". Okay. > * guix/import/pypi.scm: (guess-requirements)[tarball-directory]: Rename t= o... > [archive-root-directory]: this. Use COMPRESSED-FILED? to determine if an > archive is supported or not. Nitpick: please use =E2=80=9C...this.=E2=80=9D and leave two spaces between= sentences. Typo: it should be COMPRESSED-FILE? > [guess-requirements-from-source]: Adapt to use the new method, and use un= zip > to extract ZIP archives. s/method/procedure/ Please also mention that =E2=80=9Ccompute-inputs=E2=80=9D has been adjusted. > - (define (tarball-directory url) > - ;; Given the URL of the package's tarball, return the name of the di= rectory > + (define (archive-root-directory url) > + ;; Given the URL of the package's archive, return the name of the di= rectory > ;; that will be created upon decompressing it. If the filetype is not > ;; supported, return #f. > - ;; TODO: Support more archive formats. > - (let ((basename (substring url (+ 1 (string-rindex url #\/))))) > - (cond > - ((string-suffix? ".tar.gz" basename) > - (string-drop-right basename 7)) > - ((string-suffix? ".tar.bz2" basename) > - (string-drop-right basename 8)) > - (else > + (if (compressed-file? url) > + (let ((root-directory (file-sans-extension (basename url)))) > + (if (string=3D? "tar" (file-extension root-directory)) > + (file-sans-extension root-directory) > + root-directory)) > (begin > - (warning (G_ "Unsupported archive format: \ > -cannot determine package dependencies")) > - #f))))) > + (warning (G_ "Unsupported archive format (~a): \ > +cannot determine package dependencies") (file-extension url)) > + #f))) I think the double application of file-sans-extension and the intermediate variable name =E2=80=9Croot-directory=E2=80=9D for something t= hat is a file is a little confusing, but I don=E2=80=99t have a better proposal (other th= an to replace file-extension and file-sans-extension with a match expression). > (define (read-wheel-metadata wheel-archive) > ;; Given WHEEL-ARCHIVE, a ZIP Python wheel archive, return the packa= ge's > @@ -246,16 +243,20 @@ cannot determine package dependencies")) > (define (guess-requirements-from-source) > ;; Return the package's requirements by guessing them from the sourc= e. > - (let ((dirname (tarball-directory source-url))) > + (let ((dirname (archive-root-directory source-url)) > + (extension (file-extension source-url))) > (if (string? dirname) > (call-with-temporary-directory > (lambda (dir) > (let* ((pypi-name (string-take dirname (string-rindex dirna= me #\-))) > (requires.txt (string-append dirname "/" pypi-name > ".egg-info" "/requires.= txt")) > - (exit-code (parameterize ((current-error-port (%make= -void-port "rw+")) > - (current-output-port (%mak= e-void-port "rw+"))) > - (system* "tar" "xf" tarball "-C" dir re= quires.txt)))) > + (exit-code > + (parameterize ((current-error-port (%make-void-port= "rw+")) > + (current-output-port (%make-void-por= t "rw+"))) > + (if (string=3D? "zip" extension) > + (system* "unzip" archive "-d" dir requires.tx= t) > + (system* "tar" "xf" archive "-C" dir requires= .txt))))) I guess this is why I=E2=80=99m not too happy with this: we=E2=80=99re chec= king in multiple places if the format is supported but then forget about this again until the next time we need to do something to the file. I wonder if we could do better and answer the question just once. -- Ricardo