Re: [PATCH 2/2] import: Add stackage importer and updater.

[Ludovic Courtès]

Federico Beffa <address@hidden> skribis:

> On Wed, Feb 8, 2017 at 4:30 PM, Ludovic Courtès <address@hidden> wrote:
>> Hello Federico,
>>
>> Federico Beffa <address@hidden> skribis:
>
>> [...]
>
>>> +    (if name-version
>>> +        (hackage->guix-package name-version
>>> +                               #:include-test-dependencies?
>>> +                               include-test-dependencies?)
>>> +        (begin
>>> +          (format (current-error-port)
>>> +                  "guix import stackage: package not found: ~a~%" 
>>> package-name)
>>> +          #f))))
>>
>> The ‘if’ is unnecessary here: (guix scripts import) produces an error
>> message when an importer returns something other than a ‘package’ sexp.
>
> I've implemented all suggested changes apart from this one for two reasons:
>
> * If the package is not included in an LTS release then the procedure
> generating name-version returns #f. If I remove the 'if' then
> hackage->guix-package will be called with a boolean instead of a
> string as the first parameter and that will throw an exception.
>
> * Here we can give a more accurate error message: we can say that the
> package was not found in the LTS release, while, by returning, the
> error would be "failed to download cabal file" which is somewhat
> misleading.

Good points!

In that case it’s better to either use ‘warning’ from (guix ui), but
that may not be appropriate here because this module is not supposed to
contain UI code, or to use:

  (raise (condition
            (&message
             (message (format #f (_ "~a: Stackage package not found")
                                 package-name)))))

That way, error messages are always reported in a consistent manner.

Ludo’.