From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Bavier Subject: Re: [PATCH] gnu: refresh: Add --list-upstream-closure option. Date: Wed, 16 Jul 2014 16:45:45 -0500 Message-ID: <877g3d3rli.fsf@gmail.com> References: <877g3fp5bc.fsf@member.fsf.org> <8738e2l3ws.fsf@gnu.org> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:34248) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X7X0w-0007jV-BU for guix-devel@gnu.org; Wed, 16 Jul 2014 17:45:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X7X0q-0003rd-L2 for guix-devel@gnu.org; Wed, 16 Jul 2014 17:45:26 -0400 In-reply-to: <8738e2l3ws.fsf@gnu.org> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org To: =?utf-8?Q?Ludovic_Court=C3=A8s?= Cc: guix-devel@gnu.org 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. OK >> + (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. > > WDYT? 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. See http://www.gnu.org/philosophy/no-word-attachments.html