From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
To: Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de>
Cc: 24450@debbugs.gnu.org
Subject: bug#24450: [PATCHv2] Re: pypi importer outputs strange character series in optional dependency case.
Date: Sun, 16 Jun 2019 14:10:52 +0900 [thread overview]
Message-ID: <878su22jz7.fsf@gmail.com> (raw)
In-Reply-To: <idjy32q8wiu.fsf@bimsb-sys02.mdc-berlin.net> (Ricardo Wurmus's message of "Tue, 28 May 2019 16:48:57 +0200")
Hello Ricardo!
Ricardo Wurmus <ricardo.wurmus@mdc-berlin.de> writes:
>> From cfde6e09f8f8c692fe252d76ed27e8c50a9e5377 Mon Sep 17 00:00:00 2001
>> From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
>> 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.
Indeed; I'll be more careful about this is the future; sorry!
I've squashed this commit along with the one enabling more archive types
support, as this is where the modified (and later removed) procedure
originated.
>> (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.
Done, like this:
--8<---------------cut here---------------start------------->8---
- (if (> (length requires.txt-files) 0)
- (parse-requires.txt (first requires.txt-files))
- (begin (warning (G_ "Cannot guess requirements from source archive:\
+ (match requires.txt-files
+ (()
+ (warning (G_ "Cannot guess requirements from source archive:\
no requires.txt file found.~%"))
- '())))))
+ '())
+ (else (parse-requires.txt (first requires.txt-files)))))))
--8<---------------cut here---------------end--------------->8---
>> + (begin
>> + (parse-requires.txt (first requires.txt-files)))
>
> No need for “begin” here.
Fixed.
>> + (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.
This might look ugly, but I can't think of a better return value, since
using anything else would mean having to introduce extra logic in the
callers, while it is now a correct value that needs no special case.
Maxim
next prev parent reply other threads:[~2019-06-16 5:12 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-09-16 20:00 bug#24450: pypi importer outputs strange character series in optional dependency case ng0
2019-03-29 4:24 ` Maxim Cournoyer
2019-06-16 17:02 ` ng0
2019-06-26 4:12 ` Maxim Cournoyer
2019-03-29 4:34 ` bug#24450: [PATCH] " Maxim Cournoyer
2019-03-30 2:12 ` bug#24450: [PATCHv2] " T460s laptop
2019-03-31 14:40 ` bug#24450: [PATCH] " Maxim Cournoyer
2019-04-01 15:28 ` bug#24450: [PATCHv2] " Ludovic Courtès
2019-05-15 11:06 ` Ricardo Wurmus
2019-05-20 4:05 ` bug#24450: [PATCHv2] " Maxim Cournoyer
2019-05-20 15:05 ` Ludovic Courtès
2019-05-22 1:13 ` Maxim Cournoyer
2019-05-27 14:48 ` Ricardo Wurmus
2019-06-10 2:10 ` Maxim Cournoyer
2019-05-27 15:11 ` Ricardo Wurmus
2019-06-10 3:30 ` Maxim Cournoyer
2019-06-10 9:23 ` Ricardo Wurmus
2019-06-16 14:11 ` Maxim Cournoyer
2019-06-17 1:41 ` Ricardo Wurmus
2019-05-27 15:54 ` Ricardo Wurmus
2019-06-10 8:32 ` Maxim Cournoyer
2019-06-10 9:12 ` Ricardo Wurmus
2019-06-16 6:05 ` Maxim Cournoyer
2019-05-27 15:58 ` Ricardo Wurmus
2019-05-28 10:23 ` Ricardo Wurmus
2019-06-10 13:30 ` Maxim Cournoyer
2019-06-10 20:13 ` Ricardo Wurmus
2019-05-28 11:04 ` Ricardo Wurmus
2019-06-11 0:39 ` Maxim Cournoyer
2019-06-11 11:56 ` Ricardo Wurmus
2019-05-28 13:21 ` Ricardo Wurmus
2019-05-28 14:48 ` Ricardo Wurmus
2019-06-16 5:10 ` Maxim Cournoyer [this message]
2019-05-28 14:53 ` Ricardo Wurmus
2019-05-30 2:24 ` Maxim Cournoyer
2019-06-16 5:53 ` Maxim Cournoyer
2019-06-12 3:00 ` Maxim Cournoyer
2019-06-12 6:39 ` Ricardo Wurmus
2019-06-16 14:29 ` Maxim Cournoyer
2019-06-16 14:36 ` bug#24450: [PATCHv3] " Maxim Cournoyer
2019-07-02 1:54 ` Maxim Cournoyer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878su22jz7.fsf@gmail.com \
--to=maxim.cournoyer@gmail.com \
--cc=24450@debbugs.gnu.org \
--cc=ricardo.wurmus@mdc-berlin.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.