[bug#27907] [PATCH] graph: Provide access to the package record in the emit

[bug#27907] [PATCH] graph: Provide access to the package record in the emit

[Roel Janssen]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-graph-Provide-access-to-the-package-record-in-the-em.patch --]
[-- Type: text/x-diff, Size: 2856 bytes --]

From 0243051f4a650f729e20645606d45d7138f2aa8c Mon Sep 17 00:00:00 2001
From: Roel Janssen <roel@gnu.org>
Date: Tue, 1 Aug 2017 16:30:02 +0200
Subject: [PATCH] graph:  Provide access to the package record in the emit
 functions.

* guix/graph.scm (export-graph): Pass the node to the emit functions, instead
  of the node's label.
---
 guix/graph.scm | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/guix/graph.scm b/guix/graph.scm
index d7fd5f3..0219787 100644
--- a/guix/graph.scm
+++ b/guix/graph.scm
@@ -22,6 +22,7 @@
   #:use-module (guix monads)
   #:use-module (guix records)
   #:use-module (guix sets)
+  #:use-module (guix packages)
   #:use-module (rnrs io ports)
   #:use-module (srfi srfi-1)
   #:use-module (srfi srfi-9)
@@ -170,9 +171,9 @@ typically returned by 'node-edges' or 'node-back-edges'."
           name))
 (define (emit-epilogue port)
   (display "\n}\n" port))
-(define (emit-node id label port)
+(define (emit-node id node port)
   (format port "  \"~a\" [label = \"~a\", shape = box, fontname = Helvetica];~%"
-          id label))
+          id (package-full-name node)))
 (define (emit-edge id1 id2 port)
   (format port "  \"~a\" -> \"~a\" [color = ~a];~%"
           id1 id2 (pop-color id1)))
@@ -213,11 +214,11 @@ var nodes = {},
   (format port "</script><script type=\"text/javascript\" src=\"~a\"></script></body></html>"
           (search-path %load-path "graph.js")))
 
-(define (emit-d3js-node id label port)
+(define (emit-d3js-node id node port)
   (format port "\
 nodes[\"~a\"] = {\"id\": \"~a\", \"label\": \"~a\", \"index\": nodeArray.length};
 nodeArray.push(nodes[\"~a\"]);~%"
-          id id label id))
+          id id (package-full-name node) id))
 
 (define (emit-d3js-edge id1 id2 port)
   (format port "links.push({\"source\": \"~a\", \"target\": \"~a\"});~%"
@@ -241,9 +242,9 @@ nodeArray.push(nodes[\"~a\"]);~%"
 (define (emit-cypher-epilogue port)
   (format port ""))
 
-(define (emit-cypher-node id label port)
+(define (emit-cypher-node id node port)
   (format port "MERGE (p:Package { id: ~s }) SET p.name = ~s;~%"
-          id label ))
+          id (package-name node)))
 
 (define (emit-cypher-edge id1 id2 port)
   (format port "MERGE (a:Package { id: ~s });~%" id1)
@@ -296,7 +297,7 @@ true, draw reverse arrows."
                                         (ids          (mapm %store-monad
                                                             node-identifier
                                                             dependencies)))
-                     (emit-node id (node-label head) port)
+                     (emit-node id head port)
                      (for-each (lambda (dependency dependency-id)
                                  (if reverse-edges?
                                      (emit-edge dependency-id id port)
-- 
2.7.4


[-- Attachment #2: Type: text/plain, Size: 599 bytes --]


Dear Guix,

I would like to expand the Cypher back-end and in the long run add a
SPARQL graph back-end to GNU Guix.  For this, I will need to have access
to the package records inside the emit-* functions.

This patch makes this change by essentially changing the "label"
parameter of the emit-* functions passed as "(node-label head)" into a
"node" parameter, passed as "head".

The rest of the patch adapts the current emit-* functions to this
change.

I tested the Graphviz, D3js, and Cypher back-ends, and all seem to work
as before.

Is it OK to apply this change?

Kind regards,
Roel Janssen

[bug#27907] [PATCH] graph: Provide access to the package record in the emit

[Ludovic Courtès]

Roel Janssen <roel@gnu.org> skribis:

> I would like to expand the Cypher back-end and in the long run add a
> SPARQL graph back-end to GNU Guix.  For this, I will need to have access
> to the package records inside the emit-* functions.
>
> This patch makes this change by essentially changing the "label"
> parameter of the emit-* functions passed as "(node-label head)" into a
> "node" parameter, passed as "head".
>
> The rest of the patch adapts the current emit-* functions to this
> change.
>
> I tested the Graphviz, D3js, and Cypher back-ends, and all seem to work
> as before.
>
> Is it OK to apply this change?

Sure, looks good to me!

Ludo’.

bug#27907: [PATCH] graph: Provide access to the package record in the emit

[Roel Janssen]

Ludovic Courtès writes:

> Roel Janssen <roel@gnu.org> skribis:
>
>> I would like to expand the Cypher back-end and in the long run add a
>> SPARQL graph back-end to GNU Guix.  For this, I will need to have access
>> to the package records inside the emit-* functions.
>>
>> This patch makes this change by essentially changing the "label"
>> parameter of the emit-* functions passed as "(node-label head)" into a
>> "node" parameter, passed as "head".
>>
>> The rest of the patch adapts the current emit-* functions to this
>> change.
>>
>> I tested the Graphviz, D3js, and Cypher back-ends, and all seem to work
>> as before.
>>
>> Is it OK to apply this change?
>
> Sure, looks good to me!
>
> Ludo’.

Thanks! I pushed this patch.

Kind regards,
Roel Janssen

[bug#27907] [PATCH] graph: Provide access to the package record in the emit

[Ludovic Courtès]

Hi!

ludo@gnu.org (Ludovic Courtès) skribis:

> Roel Janssen <roel@gnu.org> skribis:
>
>> I would like to expand the Cypher back-end and in the long run add a
>> SPARQL graph back-end to GNU Guix.  For this, I will need to have access
>> to the package records inside the emit-* functions.
>>
>> This patch makes this change by essentially changing the "label"
>> parameter of the emit-* functions passed as "(node-label head)" into a
>> "node" parameter, passed as "head".
>>
>> The rest of the patch adapts the current emit-* functions to this
>> change.
>>
>> I tested the Graphviz, D3js, and Cypher back-ends, and all seem to work
>> as before.
>>
>> Is it OK to apply this change?
>
> Sure, looks good to me!

Actually no!  :-)

The problem was that it broke all non-package-related “node types” (like
“guix graph -t references”), and it had the problem that it ignores the
‘label’ procedure in <node-type>.  And “make check” failed.

So I reverted it in 5e60bef9802e448924f889d34d95a249b008652c.  We need
to rethink about it.

Cheers,
Ludo’.

[bug#27907] [PATCH] graph: Provide access to the package record in the emit

[Roel Janssen]

Ludovic Courtès writes:

> Hi!
>
> ludo@gnu.org (Ludovic Courtès) skribis:
>
>> Roel Janssen <roel@gnu.org> skribis:
>>
>>> I would like to expand the Cypher back-end and in the long run add a
>>> SPARQL graph back-end to GNU Guix.  For this, I will need to have access
>>> to the package records inside the emit-* functions.
>>>
>>> This patch makes this change by essentially changing the "label"
>>> parameter of the emit-* functions passed as "(node-label head)" into a
>>> "node" parameter, passed as "head".
>>>
>>> The rest of the patch adapts the current emit-* functions to this
>>> change.
>>>
>>> I tested the Graphviz, D3js, and Cypher back-ends, and all seem to work
>>> as before.
>>>
>>> Is it OK to apply this change?
>>
>> Sure, looks good to me!
>
> Actually no!  :-)
>
> The problem was that it broke all non-package-related “node types” (like
> “guix graph -t references”), and it had the problem that it ignores the
> ‘label’ procedure in <node-type>.  And “make check” failed.
>
> So I reverted it in 5e60bef9802e448924f889d34d95a249b008652c.  We need
> to rethink about it.
>
> Cheers,
> Ludo’.

Oops!  I am sorry about this.  Would it not break if we include a check for whether
the node type is a package or not.  Then, non-package node types are
handled the “old way” and packages are handled the “new way”.

I think we cannot have a generic way of exposing the specifics of a node
type, so if we need to expose more information for the other node types,
we have to add a type-specific implementation.

If this sounds like a good idea I'll write a new patch.  And while I'm
at it, what set of commands fully cover the graph code for all node
types?  Just all variants in 'guix graph --type=X'?

Thanks!

Kind regards,
Roel Janssen

[bug#27907] [PATCH] graph: Provide access to the package record in the emit

[Ludovic Courtès]

Roel Janssen <roel@gnu.org> skribis:

> Oops!  I am sorry about this.  Would it not break if we include a check for whether
> the node type is a package or not.  Then, non-package node types are
> handled the “old way” and packages are handled the “new way”.
>
> I think we cannot have a generic way of exposing the specifics of a node
> type, so if we need to expose more information for the other node types,
> we have to add a type-specific implementation.

Actually, we might need to discuss the specifics of why you wanted to do
it in the first place.  :-)

It is to pass extra rendering info to the backends?  (It would be
helpful for instance to adjust the node color or size depending on
certain parameters such as its size or number of dependents.)

> If this sounds like a good idea I'll write a new patch.  And while I'm
> at it, what set of commands fully cover the graph code for all node
> types?  Just all variants in 'guix graph --type=X'?

“make check TESTS=tests/graph.scm” covers all the node types I think.

Ludo’.

[bug#27907] [PATCH] graph: Provide access to the package record in the emit

[Roel Janssen]

Ludovic Courtès writes:

> Roel Janssen <roel@gnu.org> skribis:
>
>> Oops!  I am sorry about this.  Would it not break if we include a check for whether
>> the node type is a package or not.  Then, non-package node types are
>> handled the “old way” and packages are handled the “new way”.
>>
>> I think we cannot have a generic way of exposing the specifics of a node
>> type, so if we need to expose more information for the other node types,
>> we have to add a type-specific implementation.
>
> Actually, we might need to discuss the specifics of why you wanted to do
> it in the first place.  :-)
>
> It is to pass extra rendering info to the backends?  (It would be
> helpful for instance to adjust the node color or size depending on
> certain parameters such as its size or number of dependents.)

Not necessarily rendering information, even though it could improve the
displayment of packages like you say.  I'd like to export more
information to a graph database, so that the packages can be searched,
explored and linked to in a graph that also contains stuff like how
programs were run and what files that run produced.

So, this is essentially an interoperability thing for communicating with
other systems.

>
>> If this sounds like a good idea I'll write a new patch.  And while I'm
>> at it, what set of commands fully cover the graph code for all node
>> types?  Just all variants in 'guix graph --type=X'?
>
> “make check TESTS=tests/graph.scm” covers all the node types I think.
>
> Ludo’.

Then I will run that command before I propose a new patch.

Kind regards,
Roel Janssen

[bug#27907] [PATCH] graph: Provide access to the package record in the emit

[Ludovic Courtès]

Roel Janssen <roel@gnu.org> skribis:

> Ludovic Courtès writes:
>
>> Roel Janssen <roel@gnu.org> skribis:
>>
>>> Oops!  I am sorry about this.  Would it not break if we include a check for whether
>>> the node type is a package or not.  Then, non-package node types are
>>> handled the “old way” and packages are handled the “new way”.
>>>
>>> I think we cannot have a generic way of exposing the specifics of a node
>>> type, so if we need to expose more information for the other node types,
>>> we have to add a type-specific implementation.
>>
>> Actually, we might need to discuss the specifics of why you wanted to do
>> it in the first place.  :-)
>>
>> It is to pass extra rendering info to the backends?  (It would be
>> helpful for instance to adjust the node color or size depending on
>> certain parameters such as its size or number of dependents.)
>
> Not necessarily rendering information, even though it could improve the
> displayment of packages like you say.  I'd like to export more
> information to a graph database, so that the packages can be searched,
> explored and linked to in a graph that also contains stuff like how
> programs were run and what files that run produced.
>
> So, this is essentially an interoperability thing for communicating with
> other systems.

So I think an option would be to pass an extra property alist to the
‘emit-node’ and ‘emit-edge’ procedures of the backend.  The node type
would produce that alist and it would be up to the backend to make sense
of it.  Something along these lines.  WDYT?

Ludo’.