On to the next: > From 73e27235cac1275ba7671fd2364325cf5788cb3c Mon Sep 17 00:00:00 2001 > From: Maxim Cournoyer <[email protected]> > 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 to... > [archive-root-directory]: this. Use COMPRESSED-FILED? to determine if an > archive is supported or not. Nitpick: please use “...this.” and leave two spaces between sentences. Typo: it should be COMPRESSED-FILE? > [guess-requirements-from-source]: Adapt to use the new method, and use unzip > to extract ZIP archives. s/method/procedure/ Please also mention that “compute-inputs” has been adjusted. > - (define (tarball-directory url) > - ;; Given the URL of the package's tarball, return the name of the > directory > + (define (archive-root-directory url) > + ;; Given the URL of the package's archive, return the name of the > directory > ;; 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=? "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 “root-directory” for something that is a file is a little confusing, but I don’t have a better proposal (other than 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 package's > @@ -246,16 +243,20 @@ cannot determine package dependencies")) > (define (guess-requirements-from-source) > ;; Return the package's requirements by guessing them from the source. > - (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 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+"))) > - (system* "tar" "xf" tarball "-C" dir > 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))))) I guess this is why I’m not too happy with this: we’re checking 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
