[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] gnu: refresh: Add --list-upstream-closure option.

From: Eric Bavier
Subject: Re: [PATCH] gnu: refresh: Add --list-upstream-closure option.
Date: Wed, 16 Jul 2014 16:45:45 -0500
User-agent: mu4e; emacs 23.3.1

Ludovic Courtès writes:

>> For the sake of brevity and human consumption, the option doesn't print
>> *all* upstream packages, just the "top-level" upstream packages,
>> i.e. those whose inputs encompass all other upstream packages.
> This looks very useful!

I'm glad.

>> I'm not sure that the option name or all the terminology I used is
>> appropriate, so any comments or suggestions are welcome.
> The term “upstream” in the context of this command makes me think of
> package maintainers which are considered “upstream” compared to the
> distro.
> What about “dependent” instead?

Yes, I like "dependent" better.

>> +  (define (package-direct-inputs package)
>> +    (append (package-native-inputs package)
>> +            (package-inputs package)
>> +            (package-propagated-inputs package)))
>> +
>> +  (let ((inverse-package-dependency-graph
> ‘inverse-dag’ or ‘dag’ is enough for a local var.  If needed a comment
> can clarify what it is.


>> +         (fold-packages
>> +          (lambda (package forest)
>> +            (for-each
>> +             (lambda (d)
>> +               ;; Insert a tree edge from each of package's inputs to 
>> package.
>> +               (hash-set! forest d
>> +                          (cons package
>> +                                (hash-ref forest d '()))))
>> +             (map cadr (package-direct-inputs package)))
>> +            forest)
>> +          (make-hash-table))))
> Could you use a vhash here instead of the hash table?

Yes, that shouldn't be a problem.  Could I ask why the request?

>> +    (map package-full-name
>> +         (fold-forest-leaves
>> +          cons '()
>> +          (lambda (node)
>> +            (hash-ref inverse-package-dependency-graph node '()))
>> +          packages))))
> The function is documented as returning “package specifications” so I’d
> remove ‘map’ from here and move it to the call site.

I'm fine with moving the mapping to the call site.  I used "package
specification" here because that term is used elsewhere when talking
about a string like "zlib-1.2.7".  I'll change the documentation to be
more clear.

> What about ‘fold-tree’ instead, and change ‘next’ to ‘children’?
>> +(define (fold-forest-leaves proc init next roots)
>> +  "Like fold-forest, but call (PROC NODE RESULT) only for leaf nodes."
> ‘fold-tree-leaves’.

I realize now that the structure isn't necessarily a forest, so yes,
"fold-tree" would be more appropriate.

> Also could you write a few tests for these two?

Sure thing.

> I believe ‘upstream-packages’ (renamed to ‘dependent-packages’?) could
> be moved to (guix packages) and have a few tests as well.  That’d be
> great.

Because of the use of 'fold-packages' from (gnu packages), putting
dependent-packages into (guix packages) would cause a circular
dependency.  I think I'd also want to make the interface more general if
it were to go somewhere more widely available.

I'll post an updated patch soon.

Eric Bavier

Please avoid sending me Word or PowerPoint attachments.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]