> 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.txt > 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 source. > - (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 dirname #\-))) > - (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-port "rw+"))) > - (if (string=? "zip" extension) > - (system* "unzip" archive "-d" dir requires.txt) > - (system* "tar" "xf" archive "-C" dir requires.txt))))) > - (if (zero? exit-code) > - (parse-requires.txt (string-append dir "/" requires.txt)) > - (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=? "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’s work on the empty list directly. Here “match” would be better. > + (begin > + (parse-requires.txt (first requires.txt-files))) No need for “begin” here. > + (begin (warning (G_ "Cannot guess requirements from source archive:\ > + no requires.txt file found.~%")) > + (list '() '())))))) I know that this is from an earlier commit, but I don’t like the look of “(list '() '())” at all :) > + (begin > + (warning (G_ "Unsupported archive format; \ > +cannot determine package dependencies from source archive: ~a~%") > + (basename source-url)) > (list '() '())))) Same here. Certainly there’s a better return value. -- Ricardo