Re: [PATCH] ui: 'package->recutils' serializes the source field.

[Mathieu Lirzin <mthl@gnu.org>]

Alex Kost <alezost@gmail.com> writes:

> David Craven (2016-08-05 17:58 +0300) wrote:
>
>> * guix/ui.scm (package->recutils): Format origin.
>> ---
>>  guix/ui.scm | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>>
>> diff --git a/guix/ui.scm b/guix/ui.scm
>> index 4d1b65c..e232548 100644
>> --- a/guix/ui.scm
>> +++ b/guix/ui.scm
>> @@ -26,6 +26,7 @@
>>  (define-module (guix ui)
>>    #:use-module (guix utils)
>>    #:use-module (guix store)
>> +  #:use-module (guix base32)
>>    #:use-module (guix config)
>>    #:use-module (guix packages)
>>    #:use-module (guix profiles)
>> @@ -33,6 +34,7 @@
>>    #:use-module (guix combinators)
>>    #:use-module (guix build-system)
>>    #:use-module (guix serialization)
>> +  #:use-module (guix git-download)
>>    #:use-module ((guix build utils) #:select (mkdir-p))
>>    #:use-module ((guix licenses) #:select (license? license-name))
>>    #:use-module ((guix build syscalls) #:select (terminal-columns))
>> @@ -878,6 +880,22 @@ WIDTH columns."
>>    ;; Note: Don't i18n field names so that people can post-process it.
>>    (format port "name: ~a~%" (package-name p))
>>    (format port "version: ~a~%" (package-version p))
>> +
>> +  (let ((uri (origin-uri (package-source p))))
>
> This will fail for some packages (for example, for 'gcc-toolchain')
> because the source value can be #f, and (origin-uri #f) errors.
>
>> +    (cond
>> +      ((git-reference? uri)
>> +        (begin
>
> 'begin' is not needed here.
>
>> +          (format port "source-git-url: ~a~%"
>> +                       (git-reference-url uri))
>> +          (format port "source-git-commit: ~a~%"
>> +                       (git-reference-commit uri))
>> +          (format port "source-git-recursive: ~a~%"
>> +                       (git-reference-recursive? uri))))
>> +      (#t
>
> I think using 'else' here would be more Schemey than '#t'.
>
>> +        (format port "source-uri: ~a~%" uri))))
>> +
>> +  (format port "source-hash: ~a~%" (bytevector->base32-string
>> +                                      (origin-sha256 (package-source p))))
>>    (format port "outputs: ~a~%" (string-join (package-outputs p)))
>>    (format port "systems: ~a~%"
>>            (string-join (package-transitive-supported-systems p)))
>
> After all I would write it like this:
>
>   (let ((source (package-source p)))
>     (when (origin? source)
>       (let ((uri (origin-uri source)))
>         (cond
>          ((git-reference? uri)
>           (format port "source-git-url: ~a~%"
>                   (git-reference-url uri))
>           (format port "source-git-commit: ~a~%"
>                   (git-reference-commit uri))
>           (format port "source-git-recursive: ~a~%"
>                   (git-reference-recursive? uri)))
>          (else
>           (format port "source-uri: ~a~%" uri))))
>       (format port "source-hash: ~a~%"
>               (bytevector->base32-string (origin-sha256 source)))))
>
> However, I'm not sure whether all these source things are needed to be
> in the output, I would wait for other opinions.
>
> (I agree with you though and I would like to add the same info for Emacs
> interface :-))
>
> Also note that packages may have multiple sources.  For example, for
> 'sudo' package, the output will be:
>
>   source-uri: (https://www.sudo.ws/sudo/dist/sudo-1.8.17p1.tar.gz ftp://ftp.sudo.ws/pub/sudo/OLD/sudo-1.8.17p1.tar.gz)

I don't have an opinion on what should be displayed or not (at least not
yet).  However I think there is an issue with FSDG compliance here, when
upstream includes non-free stuff for the same reason that 'guix build
--source PACKAGE' is required by FSDG to provide a cleaned version of the
tarball.

"source-uris" should point to the same thing that 'guix build -S
PACKAGE' provides however in our current infrastructure this is not
possible.

For the same reason I think Emacs interface should not display the
origin URL of those packages.  I Don't know how to achieve this, maybe
displaying only the URL when there is no snippet part would work even if
it will remove more package source URLs than needed.

WDYT?

-- 
Mathieu Lirzin