Re: [PATCH 3/7] import cran: description->package: Also return package dependencies.

[Ricardo Wurmus]

Ludovic Courtès <address@hidden> writes:

> Ricardo Wurmus <address@hidden> skribis:
>
>> Ricardo Wurmus <address@hidden> writes:
>>
>>> Ludovic Courtès <address@hidden> writes:
>>>
>>>> Ricardo Wurmus <address@hidden> skribis:
>>>>
>>>>> * guix/import/cran.scm (description->package): Return package
>>>>>   dependencies in addition to generated package expression.
>>>>
>>>> What would you think of making it return a SRFI-41 stream of packages
>>>> instead?  Or maybe two values: the package asked for, and the stream
>>>> containing its dependencies.  That would hide the package lookup
>>>> machinery.
>>>
>>> That’s a very good idea!  I’ll play around with this and submit a new
>>> patch.
>>
>> I’ve been playing with this for a while and although it looks prettier
>> to a user it has some disadvantages to use streams here.  The first is
>> that it becomes more difficult to avoid duplicate work.  The second is
>> that we can no longer easily generate an order of package expressions
>> from leaf nodes to the root.
>>
>> Consider this case where “plotly” should be imported.  The new inputs
>> look like this:
>>
>>  plotly  : (ggplot2 digest)
>>  ggplot2 : (digest mass)
>>  digest  : ()
>>  mass    : ()
>>
>> The generated stream may look like this:
>>
>>   (plotly ggplot2 digest mass digest)
>>
>> This happens because we only know about the next set of inputs upon
>> evaluation of the stream elements.  Of course there are ways to fix
>> this, but they involve extending or wrapping “cran->guix-package” to
>> make sure that we keep track of top-level dependencies as we traverse
>> the list of dependencies recursively.
>>
>> Maybe I’m doing this wrong?
>>
>>
>> I rewrote description->package to essentially do this:
>>
>>     (stream-cons the-package-expression
>>      (stream-concat
>>       (let ((unpackaged (unpackaged-dependencies
>>                          propagate guix-name)))
>>         (stream-map (cran->guix-package name repository)
>>                     (list->stream unpackaged)))))
>>
>> I.e. the first element of the stream is the imported package expression;
>> the tail is a concatenation of streams of packages with their
>> dependencies just like this one.  Here’s the stream before
>> concatenation:
>>
>>   (plotly (ggplot2 (digest ()) (mass ()))
>>           (digest ()))
>>
>> To render this in the correct order we’d have to turn this tree inside
>> out.  Maybe this isn’t so well-suited to a representation as a stream
>> after all.  To turn it inside out we have to evaluate the whole tree
>> anyway.
>>
>> What do you think?  Is it worth expressing this as a stream?  If so, do
>> you have any recommendations on how to approach this?
>>
>> Or would it be okay to let “description->package” return two values: a
>> package expression and a list of unpackaged package names, which can
>> then be processed by a separate procedure?  (That’s what I submitted
>> earlier.)
>
> I think you should go ahead with what you proposed if the other option
> sounds like a headache.  Sorry for blocking you for this long!

No problem.  I would really like to use streams, so it’s worth the
wait.  Can’t avoid the headache :)

> In theory it would be possible to do something like:
>
>   (define (description->package repo meta)
>     (stream-cons `(package …)
>                  (stream-unfold (lambda (state)
>                                   (description->package
>                                    repo
>                                    (first-dependency state)))
>                                 (lambda (state)
>                                   (done? state))
>                                 (lambda (state)
>                                   (next-dependency state))
>                                 (make-state propagate (setq)))))
>
> … where the state is roughly a pair containing the list of next
> dependencies and the set of already visited dependencies (to avoid
> duplicates).

That’s a good hint.  “stream-unfold” makes my head spin, to be honest.

> But again, this is probably easier said than done, so no problem with
> keeping the initial option.

I’ll read the documentation for “stream-unfold” again and try once more.
If I really cannot make it I’ll go ahead with the initial
implementation and submit a new patch set.

~~ Ricardo