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

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 27907 in the body.
You can then email your comments to 27907 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Roel Janssen <roel <at> gnu.org>
To: guix-patches <guix-patches <at> gnu.org>
Subject: [PATCH] graph:  Provide access to the package record in the emit
Date: Tue, 01 Aug 2017 16:40:05 +0200

[0001-graph-Provide-access-to-the-package-record-in-the-em.patch (text/x-diff, attachment)]

[Message part 2 (text/plain, inline)]

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

Message #8 received at 27907 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Roel Janssen <roel <at> gnu.org>
Cc: 27907 <at> debbugs.gnu.org
Subject: Re: [bug#27907] [PATCH] graph: Provide access to the package record
 in the emit
Date: Tue, 01 Aug 2017 21:25:49 +0200

Roel Janssen <roel <at> 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’.

Message #13 received at 27907-done <at> debbugs.gnu.org (full text, mbox):

From: Roel Janssen <roel <at> gnu.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27907-done <at> debbugs.gnu.org
Subject: Re: [bug#27907] [PATCH] graph: Provide access to the package record
 in the emit
Date: Wed, 02 Aug 2017 00:47:53 +0200

Ludovic Courtès writes:

> Roel Janssen <roel <at> 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

Message #16 received at 27907 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Roel Janssen <roel <at> gnu.org>
Cc: 27907 <at> debbugs.gnu.org
Subject: Re: [bug#27907] [PATCH] graph: Provide access to the package record
 in the emit
Date: Fri, 25 Aug 2017 00:26:23 +0200

Hi!

ludo <at> gnu.org (Ludovic Courtès) skribis:

> Roel Janssen <roel <at> 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’.

Message #19 received at 27907 <at> debbugs.gnu.org (full text, mbox):

From: Roel Janssen <roel <at> gnu.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27907 <at> debbugs.gnu.org
Subject: Re: [bug#27907] [PATCH] graph: Provide access to the package record
 in the emit
Date: Fri, 25 Aug 2017 11:00:51 +0200

Ludovic Courtès writes:

> Hi!
>
> ludo <at> gnu.org (Ludovic Courtès) skribis:
>
>> Roel Janssen <roel <at> 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

Message #22 received at 27907 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Roel Janssen <roel <at> gnu.org>
Cc: 27907 <at> debbugs.gnu.org
Subject: Re: [bug#27907] [PATCH] graph: Provide access to the package record
 in the emit
Date: Fri, 25 Aug 2017 16:50:52 +0200

Roel Janssen <roel <at> 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’.

Message #25 received at 27907 <at> debbugs.gnu.org (full text, mbox):

From: Roel Janssen <roel <at> gnu.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 27907 <at> debbugs.gnu.org
Subject: Re: [bug#27907] [PATCH] graph: Provide access to the package record
 in the emit
Date: Fri, 25 Aug 2017 18:19:36 +0200

Ludovic Courtès writes:

> Roel Janssen <roel <at> 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

Message #28 received at 27907 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Roel Janssen <roel <at> gnu.org>
Cc: 27907 <at> debbugs.gnu.org
Subject: Re: [bug#27907] [PATCH] graph: Provide access to the package record
 in the emit
Date: Sat, 26 Aug 2017 10:02:53 +0200

Roel Janssen <roel <at> gnu.org> skribis:

> Ludovic Courtès writes:
>
>> Roel Janssen <roel <at> 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’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.