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 16:48:57 +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]:52805) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hVdQd-0000rU-MR for bug-guix@gnu.org; Tue, 28 May 2019 10:50:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hVdQX-00076J-OV for bug-guix@gnu.org; Tue, 28 May 2019 10:50:11 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:43262) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hVdQP-00070T-Tq for bug-guix@gnu.org; Tue, 28 May 2019 10:50:06 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hVdQP-000215-OU for bug-guix@gnu.org; Tue, 28 May 2019 10:50:01 -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 > From cfde6e09f8f8c692fe252d76ed27e8c50a9e5377 Mon Sep 17 00:00:00 2001 > From: Maxim Cournoyer > Date: Sat, 30 Mar 2019 23:13:26 -0400 > Subject: [PATCH 8/9] import: pypi: Scan source archive to find requires.t= xt > file. > * guix/import/pypi.scm (use-modules): Use invoke from (guix build utils). > (guess-requirements)[archive-root-directory]: Remove procedure. Oh, I guess I reviewed this procedure in vain :( Please modify the commits so that added procedures are not removed in later commits. This is easier on the reviewer and makes for a clearer commit history. > (define (guess-requirements-from-source) > ;; Return the package's requirements by guessing them from the sourc= e. > - (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 (%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))))) > - (if (zero? exit-code) > - (parse-requires.txt (string-append dir "/" requires.t= xt)) > - (begin > - (warning > - (G_ "Failed to extract file: ~a from source.~%") > - requires.txt) > - (list '() '())))))) > + (if (compressed-file? source-url) > + (call-with-temporary-directory > + (lambda (dir) > + (parameterize ((current-error-port (%make-void-port "rw+")) > + (current-output-port (%make-void-port "rw+"))) > + (if (string=3D? "zip" (file-extension source-url)) > + (invoke "unzip" archive "-d" dir) > + (invoke "tar" "xf" archive "-C" dir))) > + (let ((requires.txt-files > + (find-files dir (lambda (abs-file-name _) > + (string-match "\\.egg-info/requires.txt$" > + abs-file-name))))) > + (if (> (length requires.txt-files) 0) Let=E2=80=99s work on the empty list directly. Here =E2=80=9Cmatch=E2=80= =9D would be better. > + (begin > + (parse-requires.txt (first requires.txt-files))) No need for =E2=80=9Cbegin=E2=80=9D here. > + (begin (warning (G_ "Cannot guess requirements from sou= rce archive:\ > + no requires.txt file found.~%")) > + (list '() '())))))) I know that this is from an earlier commit, but I don=E2=80=99t like the lo= ok of =E2=80=9C(list '() '())=E2=80=9D at all :) > + (begin > + (warning (G_ "Unsupported archive format; \ > +cannot determine package dependencies from source archive: ~a~%") > + (basename source-url)) > (list '() '())))) Same here. Certainly there=E2=80=99s a better return value. -- Ricardo