Add bst graph command

[ion232]

Closes#1915

• Adds a new command to bst called 'graph' which reimplements the same API and functionality as contrib/bst-graph.
• The implementation details are within _stream.py as another function called 'graph'.
• Removes contrib/bst-graph.

EDIT: I've added another commit that changes the functionality to makes two separate graphs for buildtime and runtime, as this is clearer imo.

[jjardon]

@ion232 is there anything else pending to mark this a non-draft?

[gtristan]

I'm not really in favor of having bst-graph at all.

I think we should either:

• Enhance bst show with features like --depth or such, allowing some scripting to construct the graph and display it however it wants
• Open up some very limited Stream / App APIs from the buildstream module and make a limited set of APIs public
  • Allowing at least to load the build graph once and have access to the Element and Source data model, eliminating the need to load the build graph multiple times (as would be the case with bst show scripting) and allowing users to do whatever introspective operations they might want

Either of these approaches don't add a first class graphing feature but allow users to do more powerful things without requiring special buildstream features to do so... specifically this should be interesting if we add APIs for Source objects to report information about their URLs etc, so that we can do SBoMs in a way that is stable and does not abuse internal APIs (like fdsdk's collect_manifest thing does).

EDIT: It looks like this script already gets the work done with a single bst show, so that has already been thought out pretty well.

I find the output to be particularly horrible to look at for sufficiently big graphs (I've finally looked at one today, after all these years), I suspect that whatever useful things one might want to achieve by looking at this, could be better achieved with some scripting (e.g. perhaps a combination of bst show commands with bst artifact list-contents could get you as far as finding out "Why does this file I don't want end up in my image ?" and this would be more reliable than looking at this huge graph IMHO).

Side note, the script does not work with junctions, it seems to be a simple quoting error that shouldn't be hard to fix..

That said I think we made the right choice keeping this in contrib/. This avoids scope creep and avoids the addition of dependencies which are unrelated to the core mission and APIs - separate projects can do nice things with our APIs and use more elaborate dependencies, and ask us to open up some APIs as needed.

[ion232]

@ion232 is there anything else pending to mark this a non-draft?

Not really. I was possibly going add some other features but didn't get around to it. I was waiting for feedback.

[harrysarson]

Thanks for this script! I hit a small issue when using it on a complex buildstream project containing junctions. The generated graphviz ends up containing colons in the wrong place making it invalid. See small patch below that fixes the script base on a recommendation I found here https://graphviz.readthedocs.io/en/stable/manual.html#node-ports-compass

diff --git a/contrib/bst-graph b/contrib/bst-graph
index 3fe93e1ff..5c05c6b4b 100755
--- a/contrib/bst-graph+++ b/contrib/bst-graph@@ -29,6 +29,7 @@ installed.
import argparse
import subprocess
import re
+import urllib.parse
from graphviz import Digraph
from ruamel.yaml import YAML
@@ -55,6 +56,10 @@ def parse_args():
return parser.parse_args()
+def escape(s):+ return urllib.parse.quote_plus(s.encode())++
def parse_graph(lines):
'''Return nodes and edges of the parsed grpah.
@@ -80,12 +85,13 @@ def parse_graph(lines):
build_dep = parser.load(build_dep)
runtime_dep = parser.load(runtime_dep)
- nodes.add(name)- [build_deps.add((name, dep)) for dep in build_dep if dep]- [runtime_deps.add((name, dep)) for dep in runtime_dep if dep]+ safe_name = escape(name)- return nodes, build_deps, runtime_deps+ nodes.add((safe_name, name))+ [build_deps.add((safe_name, escape(dep))) for dep in build_dep if dep]+ [runtime_deps.add((safe_name, escape(dep))) for dep in runtime_dep if dep]+ return nodes, build_deps, runtime_deps
def generate_graph(nodes, build_deps, runtime_deps):
'''Generate graph from given nodes and edges.
@@ -99,8 +105,8 @@ def generate_graph(nodes, build_deps, runtime_deps):
A graphviz.Digraph object
'''
graph = Digraph()
- for node in nodes:- graph.node(node)+ for tag, name in nodes:+ graph.node(tag, label=name)
for source, target in build_deps:
graph.edge(source, target, label='build-dep')
for source, target in runtime_deps:

[abderrahim]

@harrysarson This PR is about integrating the bst-graph script into buildstream proper. For fixes to the existing contrib/bst-graph script, please open a PR. Thanks.

[ion232]

Closes#1915

• Adds a new command to bst called 'graph' which reimplements the same API and functionality as contrib/bst-graph.
• The implementation details are within _stream.py as another function called 'graph'.
• Removes contrib/bst-graph.

EDIT: I've added another commit that changes the functionality to makes two separate graphs for buildtime and runtime, as this is clearer imo.

[jjardon]

@ion232 is there anything else pending to mark this a non-draft?

[gtristan]

I'm not really in favor of having bst-graph at all.

I think we should either:

• Enhance bst show with features like --depth or such, allowing some scripting to construct the graph and display it however it wants
• Open up some very limited Stream / App APIs from the buildstream module and make a limited set of APIs public
  • Allowing at least to load the build graph once and have access to the Element and Source data model, eliminating the need to load the build graph multiple times (as would be the case with bst show scripting) and allowing users to do whatever introspective operations they might want

Either of these approaches don't add a first class graphing feature but allow users to do more powerful things without requiring special buildstream features to do so... specifically this should be interesting if we add APIs for Source objects to report information about their URLs etc, so that we can do SBoMs in a way that is stable and does not abuse internal APIs (like fdsdk's collect_manifest thing does).

EDIT: It looks like this script already gets the work done with a single bst show, so that has already been thought out pretty well.

I find the output to be particularly horrible to look at for sufficiently big graphs (I've finally looked at one today, after all these years), I suspect that whatever useful things one might want to achieve by looking at this, could be better achieved with some scripting (e.g. perhaps a combination of bst show commands with bst artifact list-contents could get you as far as finding out "Why does this file I don't want end up in my image ?" and this would be more reliable than looking at this huge graph IMHO).

Side note, the script does not work with junctions, it seems to be a simple quoting error that shouldn't be hard to fix..

That said I think we made the right choice keeping this in contrib/. This avoids scope creep and avoids the addition of dependencies which are unrelated to the core mission and APIs - separate projects can do nice things with our APIs and use more elaborate dependencies, and ask us to open up some APIs as needed.

[ion232]

@ion232 is there anything else pending to mark this a non-draft?

Not really. I was possibly going add some other features but didn't get around to it. I was waiting for feedback.

[harrysarson]

Thanks for this script! I hit a small issue when using it on a complex buildstream project containing junctions. The generated graphviz ends up containing colons in the wrong place making it invalid. See small patch below that fixes the script base on a recommendation I found here https://graphviz.readthedocs.io/en/stable/manual.html#node-ports-compass

diff --git a/contrib/bst-graph b/contrib/bst-graph
index 3fe93e1ff..5c05c6b4b 100755
--- a/contrib/bst-graph+++ b/contrib/bst-graph@@ -29,6 +29,7 @@ installed.
import argparse
import subprocess
import re
+import urllib.parse
from graphviz import Digraph
from ruamel.yaml import YAML
@@ -55,6 +56,10 @@ def parse_args():
return parser.parse_args()
+def escape(s):+ return urllib.parse.quote_plus(s.encode())++
def parse_graph(lines):
'''Return nodes and edges of the parsed grpah.
@@ -80,12 +85,13 @@ def parse_graph(lines):
build_dep = parser.load(build_dep)
runtime_dep = parser.load(runtime_dep)
- nodes.add(name)- [build_deps.add((name, dep)) for dep in build_dep if dep]- [runtime_deps.add((name, dep)) for dep in runtime_dep if dep]+ safe_name = escape(name)- return nodes, build_deps, runtime_deps+ nodes.add((safe_name, name))+ [build_deps.add((safe_name, escape(dep))) for dep in build_dep if dep]+ [runtime_deps.add((safe_name, escape(dep))) for dep in runtime_dep if dep]+ return nodes, build_deps, runtime_deps
def generate_graph(nodes, build_deps, runtime_deps):
'''Generate graph from given nodes and edges.
@@ -99,8 +105,8 @@ def generate_graph(nodes, build_deps, runtime_deps):
A graphviz.Digraph object
'''
graph = Digraph()
- for node in nodes:- graph.node(node)+ for tag, name in nodes:+ graph.node(tag, label=name)
for source, target in build_deps:
graph.edge(source, target, label='build-dep')
for source, target in runtime_deps:

[abderrahim]

@harrysarson This PR is about integrating the bst-graph script into buildstream proper. For fixes to the existing contrib/bst-graph script, please open a PR. Thanks.

[ion232]

Closes#1915

• Adds a new command to bst called 'graph' which reimplements the same API and functionality as contrib/bst-graph.
• The implementation details are within _stream.py as another function called 'graph'.
• Removes contrib/bst-graph.

EDIT: I've added another commit that changes the functionality to makes two separate graphs for buildtime and runtime, as this is clearer imo.

[jjardon]

@ion232 is there anything else pending to mark this a non-draft?

[gtristan]

I'm not really in favor of having bst-graph at all.

I think we should either:

• Enhance bst show with features like --depth or such, allowing some scripting to construct the graph and display it however it wants
• Open up some very limited Stream / App APIs from the buildstream module and make a limited set of APIs public
  • Allowing at least to load the build graph once and have access to the Element and Source data model, eliminating the need to load the build graph multiple times (as would be the case with bst show scripting) and allowing users to do whatever introspective operations they might want

Either of these approaches don't add a first class graphing feature but allow users to do more powerful things without requiring special buildstream features to do so... specifically this should be interesting if we add APIs for Source objects to report information about their URLs etc, so that we can do SBoMs in a way that is stable and does not abuse internal APIs (like fdsdk's collect_manifest thing does).

EDIT: It looks like this script already gets the work done with a single bst show, so that has already been thought out pretty well.

I find the output to be particularly horrible to look at for sufficiently big graphs (I've finally looked at one today, after all these years), I suspect that whatever useful things one might want to achieve by looking at this, could be better achieved with some scripting (e.g. perhaps a combination of bst show commands with bst artifact list-contents could get you as far as finding out "Why does this file I don't want end up in my image ?" and this would be more reliable than looking at this huge graph IMHO).

Side note, the script does not work with junctions, it seems to be a simple quoting error that shouldn't be hard to fix..

That said I think we made the right choice keeping this in contrib/. This avoids scope creep and avoids the addition of dependencies which are unrelated to the core mission and APIs - separate projects can do nice things with our APIs and use more elaborate dependencies, and ask us to open up some APIs as needed.

[ion232]

@ion232 is there anything else pending to mark this a non-draft?

Not really. I was possibly going add some other features but didn't get around to it. I was waiting for feedback.

[harrysarson]

Thanks for this script! I hit a small issue when using it on a complex buildstream project containing junctions. The generated graphviz ends up containing colons in the wrong place making it invalid. See small patch below that fixes the script base on a recommendation I found here https://graphviz.readthedocs.io/en/stable/manual.html#node-ports-compass

diff --git a/contrib/bst-graph b/contrib/bst-graph
index 3fe93e1ff..5c05c6b4b 100755
--- a/contrib/bst-graph+++ b/contrib/bst-graph@@ -29,6 +29,7 @@ installed.
import argparse
import subprocess
import re
+import urllib.parse
from graphviz import Digraph
from ruamel.yaml import YAML
@@ -55,6 +56,10 @@ def parse_args():
return parser.parse_args()
+def escape(s):+ return urllib.parse.quote_plus(s.encode())++
def parse_graph(lines):
'''Return nodes and edges of the parsed grpah.
@@ -80,12 +85,13 @@ def parse_graph(lines):
build_dep = parser.load(build_dep)
runtime_dep = parser.load(runtime_dep)
- nodes.add(name)- [build_deps.add((name, dep)) for dep in build_dep if dep]- [runtime_deps.add((name, dep)) for dep in runtime_dep if dep]+ safe_name = escape(name)- return nodes, build_deps, runtime_deps+ nodes.add((safe_name, name))+ [build_deps.add((safe_name, escape(dep))) for dep in build_dep if dep]+ [runtime_deps.add((safe_name, escape(dep))) for dep in runtime_dep if dep]+ return nodes, build_deps, runtime_deps
def generate_graph(nodes, build_deps, runtime_deps):
'''Generate graph from given nodes and edges.
@@ -99,8 +105,8 @@ def generate_graph(nodes, build_deps, runtime_deps):
A graphviz.Digraph object
'''
graph = Digraph()
- for node in nodes:- graph.node(node)+ for tag, name in nodes:+ graph.node(tag, label=name)
for source, target in build_deps:
graph.edge(source, target, label='build-dep')
for source, target in runtime_deps:

[abderrahim]

@harrysarson This PR is about integrating the bst-graph script into buildstream proper. For fixes to the existing contrib/bst-graph script, please open a PR. Thanks.

[ion232]

Closes#1915

• Adds a new command to bst called 'graph' which reimplements the same API and functionality as contrib/bst-graph.
• The implementation details are within _stream.py as another function called 'graph'.
• Removes contrib/bst-graph.

EDIT: I've added another commit that changes the functionality to makes two separate graphs for buildtime and runtime, as this is clearer imo.

[jjardon]

@ion232 is there anything else pending to mark this a non-draft?

[gtristan]

I'm not really in favor of having bst-graph at all.

I think we should either:

• Enhance bst show with features like --depth or such, allowing some scripting to construct the graph and display it however it wants
• Open up some very limited Stream / App APIs from the buildstream module and make a limited set of APIs public
  • Allowing at least to load the build graph once and have access to the Element and Source data model, eliminating the need to load the build graph multiple times (as would be the case with bst show scripting) and allowing users to do whatever introspective operations they might want

Either of these approaches don't add a first class graphing feature but allow users to do more powerful things without requiring special buildstream features to do so... specifically this should be interesting if we add APIs for Source objects to report information about their URLs etc, so that we can do SBoMs in a way that is stable and does not abuse internal APIs (like fdsdk's collect_manifest thing does).

EDIT: It looks like this script already gets the work done with a single bst show, so that has already been thought out pretty well.

I find the output to be particularly horrible to look at for sufficiently big graphs (I've finally looked at one today, after all these years), I suspect that whatever useful things one might want to achieve by looking at this, could be better achieved with some scripting (e.g. perhaps a combination of bst show commands with bst artifact list-contents could get you as far as finding out "Why does this file I don't want end up in my image ?" and this would be more reliable than looking at this huge graph IMHO).

Side note, the script does not work with junctions, it seems to be a simple quoting error that shouldn't be hard to fix..

That said I think we made the right choice keeping this in contrib/. This avoids scope creep and avoids the addition of dependencies which are unrelated to the core mission and APIs - separate projects can do nice things with our APIs and use more elaborate dependencies, and ask us to open up some APIs as needed.

[ion232]

@ion232 is there anything else pending to mark this a non-draft?

Not really. I was possibly going add some other features but didn't get around to it. I was waiting for feedback.

[harrysarson]

Thanks for this script! I hit a small issue when using it on a complex buildstream project containing junctions. The generated graphviz ends up containing colons in the wrong place making it invalid. See small patch below that fixes the script base on a recommendation I found here https://graphviz.readthedocs.io/en/stable/manual.html#node-ports-compass

diff --git a/contrib/bst-graph b/contrib/bst-graph
index 3fe93e1ff..5c05c6b4b 100755
--- a/contrib/bst-graph+++ b/contrib/bst-graph@@ -29,6 +29,7 @@ installed.
import argparse
import subprocess
import re
+import urllib.parse
from graphviz import Digraph
from ruamel.yaml import YAML
@@ -55,6 +56,10 @@ def parse_args():
return parser.parse_args()
+def escape(s):+ return urllib.parse.quote_plus(s.encode())++
def parse_graph(lines):
'''Return nodes and edges of the parsed grpah.
@@ -80,12 +85,13 @@ def parse_graph(lines):
build_dep = parser.load(build_dep)
runtime_dep = parser.load(runtime_dep)
- nodes.add(name)- [build_deps.add((name, dep)) for dep in build_dep if dep]- [runtime_deps.add((name, dep)) for dep in runtime_dep if dep]+ safe_name = escape(name)- return nodes, build_deps, runtime_deps+ nodes.add((safe_name, name))+ [build_deps.add((safe_name, escape(dep))) for dep in build_dep if dep]+ [runtime_deps.add((safe_name, escape(dep))) for dep in runtime_dep if dep]+ return nodes, build_deps, runtime_deps
def generate_graph(nodes, build_deps, runtime_deps):
'''Generate graph from given nodes and edges.
@@ -99,8 +105,8 @@ def generate_graph(nodes, build_deps, runtime_deps):
A graphviz.Digraph object
'''
graph = Digraph()
- for node in nodes:- graph.node(node)+ for tag, name in nodes:+ graph.node(tag, label=name)
for source, target in build_deps:
graph.edge(source, target, label='build-dep')
for source, target in runtime_deps:

[abderrahim]

@harrysarson This PR is about integrating the bst-graph script into buildstream proper. For fixes to the existing contrib/bst-graph script, please open a PR. Thanks.

[ion232]

Closes#1915

• Adds a new command to bst called 'graph' which reimplements the same API and functionality as contrib/bst-graph.
• The implementation details are within _stream.py as another function called 'graph'.
• Removes contrib/bst-graph.

EDIT: I've added another commit that changes the functionality to makes two separate graphs for buildtime and runtime, as this is clearer imo.

[jjardon]

@ion232 is there anything else pending to mark this a non-draft?

[gtristan]

I'm not really in favor of having bst-graph at all.

I think we should either:

• Enhance bst show with features like --depth or such, allowing some scripting to construct the graph and display it however it wants
• Open up some very limited Stream / App APIs from the buildstream module and make a limited set of APIs public
  • Allowing at least to load the build graph once and have access to the Element and Source data model, eliminating the need to load the build graph multiple times (as would be the case with bst show scripting) and allowing users to do whatever introspective operations they might want

Either of these approaches don't add a first class graphing feature but allow users to do more powerful things without requiring special buildstream features to do so... specifically this should be interesting if we add APIs for Source objects to report information about their URLs etc, so that we can do SBoMs in a way that is stable and does not abuse internal APIs (like fdsdk's collect_manifest thing does).

EDIT: It looks like this script already gets the work done with a single bst show, so that has already been thought out pretty well.

I find the output to be particularly horrible to look at for sufficiently big graphs (I've finally looked at one today, after all these years), I suspect that whatever useful things one might want to achieve by looking at this, could be better achieved with some scripting (e.g. perhaps a combination of bst show commands with bst artifact list-contents could get you as far as finding out "Why does this file I don't want end up in my image ?" and this would be more reliable than looking at this huge graph IMHO).

Side note, the script does not work with junctions, it seems to be a simple quoting error that shouldn't be hard to fix..

That said I think we made the right choice keeping this in contrib/. This avoids scope creep and avoids the addition of dependencies which are unrelated to the core mission and APIs - separate projects can do nice things with our APIs and use more elaborate dependencies, and ask us to open up some APIs as needed.

[ion232]

@ion232 is there anything else pending to mark this a non-draft?

Not really. I was possibly going add some other features but didn't get around to it. I was waiting for feedback.

[harrysarson]

Thanks for this script! I hit a small issue when using it on a complex buildstream project containing junctions. The generated graphviz ends up containing colons in the wrong place making it invalid. See small patch below that fixes the script base on a recommendation I found here https://graphviz.readthedocs.io/en/stable/manual.html#node-ports-compass

diff --git a/contrib/bst-graph b/contrib/bst-graph
index 3fe93e1ff..5c05c6b4b 100755
--- a/contrib/bst-graph+++ b/contrib/bst-graph@@ -29,6 +29,7 @@ installed.
import argparse
import subprocess
import re
+import urllib.parse
from graphviz import Digraph
from ruamel.yaml import YAML
@@ -55,6 +56,10 @@ def parse_args():
return parser.parse_args()
+def escape(s):+ return urllib.parse.quote_plus(s.encode())++
def parse_graph(lines):
'''Return nodes and edges of the parsed grpah.
@@ -80,12 +85,13 @@ def parse_graph(lines):
build_dep = parser.load(build_dep)
runtime_dep = parser.load(runtime_dep)
- nodes.add(name)- [build_deps.add((name, dep)) for dep in build_dep if dep]- [runtime_deps.add((name, dep)) for dep in runtime_dep if dep]+ safe_name = escape(name)- return nodes, build_deps, runtime_deps+ nodes.add((safe_name, name))+ [build_deps.add((safe_name, escape(dep))) for dep in build_dep if dep]+ [runtime_deps.add((safe_name, escape(dep))) for dep in runtime_dep if dep]+ return nodes, build_deps, runtime_deps
def generate_graph(nodes, build_deps, runtime_deps):
'''Generate graph from given nodes and edges.
@@ -99,8 +105,8 @@ def generate_graph(nodes, build_deps, runtime_deps):
A graphviz.Digraph object
'''
graph = Digraph()
- for node in nodes:- graph.node(node)+ for tag, name in nodes:+ graph.node(tag, label=name)
for source, target in build_deps:
graph.edge(source, target, label='build-dep')
for source, target in runtime_deps:

[abderrahim]

@harrysarson This PR is about integrating the bst-graph script into buildstream proper. For fixes to the existing contrib/bst-graph script, please open a PR. Thanks.

[ion232]

Closes#1915

• Adds a new command to bst called 'graph' which reimplements the same API and functionality as contrib/bst-graph.
• The implementation details are within _stream.py as another function called 'graph'.
• Removes contrib/bst-graph.

EDIT: I've added another commit that changes the functionality to makes two separate graphs for buildtime and runtime, as this is clearer imo.

[jjardon]

@ion232 is there anything else pending to mark this a non-draft?

[gtristan]

I'm not really in favor of having bst-graph at all.

I think we should either:

• Enhance bst show with features like --depth or such, allowing some scripting to construct the graph and display it however it wants
• Open up some very limited Stream / App APIs from the buildstream module and make a limited set of APIs public
  • Allowing at least to load the build graph once and have access to the Element and Source data model, eliminating the need to load the build graph multiple times (as would be the case with bst show scripting) and allowing users to do whatever introspective operations they might want

Either of these approaches don't add a first class graphing feature but allow users to do more powerful things without requiring special buildstream features to do so... specifically this should be interesting if we add APIs for Source objects to report information about their URLs etc, so that we can do SBoMs in a way that is stable and does not abuse internal APIs (like fdsdk's collect_manifest thing does).

EDIT: It looks like this script already gets the work done with a single bst show, so that has already been thought out pretty well.

I find the output to be particularly horrible to look at for sufficiently big graphs (I've finally looked at one today, after all these years), I suspect that whatever useful things one might want to achieve by looking at this, could be better achieved with some scripting (e.g. perhaps a combination of bst show commands with bst artifact list-contents could get you as far as finding out "Why does this file I don't want end up in my image ?" and this would be more reliable than looking at this huge graph IMHO).

Side note, the script does not work with junctions, it seems to be a simple quoting error that shouldn't be hard to fix..

That said I think we made the right choice keeping this in contrib/. This avoids scope creep and avoids the addition of dependencies which are unrelated to the core mission and APIs - separate projects can do nice things with our APIs and use more elaborate dependencies, and ask us to open up some APIs as needed.

[ion232]

@ion232 is there anything else pending to mark this a non-draft?

Not really. I was possibly going add some other features but didn't get around to it. I was waiting for feedback.

[harrysarson]

Thanks for this script! I hit a small issue when using it on a complex buildstream project containing junctions. The generated graphviz ends up containing colons in the wrong place making it invalid. See small patch below that fixes the script base on a recommendation I found here https://graphviz.readthedocs.io/en/stable/manual.html#node-ports-compass

diff --git a/contrib/bst-graph b/contrib/bst-graph
index 3fe93e1ff..5c05c6b4b 100755
--- a/contrib/bst-graph+++ b/contrib/bst-graph@@ -29,6 +29,7 @@ installed.
import argparse
import subprocess
import re
+import urllib.parse
from graphviz import Digraph
from ruamel.yaml import YAML
@@ -55,6 +56,10 @@ def parse_args():
return parser.parse_args()
+def escape(s):+ return urllib.parse.quote_plus(s.encode())++
def parse_graph(lines):
'''Return nodes and edges of the parsed grpah.
@@ -80,12 +85,13 @@ def parse_graph(lines):
build_dep = parser.load(build_dep)
runtime_dep = parser.load(runtime_dep)
- nodes.add(name)- [build_deps.add((name, dep)) for dep in build_dep if dep]- [runtime_deps.add((name, dep)) for dep in runtime_dep if dep]+ safe_name = escape(name)- return nodes, build_deps, runtime_deps+ nodes.add((safe_name, name))+ [build_deps.add((safe_name, escape(dep))) for dep in build_dep if dep]+ [runtime_deps.add((safe_name, escape(dep))) for dep in runtime_dep if dep]+ return nodes, build_deps, runtime_deps
def generate_graph(nodes, build_deps, runtime_deps):
'''Generate graph from given nodes and edges.
@@ -99,8 +105,8 @@ def generate_graph(nodes, build_deps, runtime_deps):
A graphviz.Digraph object
'''
graph = Digraph()
- for node in nodes:- graph.node(node)+ for tag, name in nodes:+ graph.node(tag, label=name)
for source, target in build_deps:
graph.edge(source, target, label='build-dep')
for source, target in runtime_deps:

[abderrahim]

@harrysarson This PR is about integrating the bst-graph script into buildstream proper. For fixes to the existing contrib/bst-graph script, please open a PR. Thanks.

[ion232]

Closes#1915

• Adds a new command to bst called 'graph' which reimplements the same API and functionality as contrib/bst-graph.
• The implementation details are within _stream.py as another function called 'graph'.
• Removes contrib/bst-graph.

EDIT: I've added another commit that changes the functionality to makes two separate graphs for buildtime and runtime, as this is clearer imo.

[jjardon]

@ion232 is there anything else pending to mark this a non-draft?

[gtristan]

I'm not really in favor of having bst-graph at all.

I think we should either:

• Enhance bst show with features like --depth or such, allowing some scripting to construct the graph and display it however it wants
• Open up some very limited Stream / App APIs from the buildstream module and make a limited set of APIs public
  • Allowing at least to load the build graph once and have access to the Element and Source data model, eliminating the need to load the build graph multiple times (as would be the case with bst show scripting) and allowing users to do whatever introspective operations they might want

Either of these approaches don't add a first class graphing feature but allow users to do more powerful things without requiring special buildstream features to do so... specifically this should be interesting if we add APIs for Source objects to report information about their URLs etc, so that we can do SBoMs in a way that is stable and does not abuse internal APIs (like fdsdk's collect_manifest thing does).

EDIT: It looks like this script already gets the work done with a single bst show, so that has already been thought out pretty well.

I find the output to be particularly horrible to look at for sufficiently big graphs (I've finally looked at one today, after all these years), I suspect that whatever useful things one might want to achieve by looking at this, could be better achieved with some scripting (e.g. perhaps a combination of bst show commands with bst artifact list-contents could get you as far as finding out "Why does this file I don't want end up in my image ?" and this would be more reliable than looking at this huge graph IMHO).

Side note, the script does not work with junctions, it seems to be a simple quoting error that shouldn't be hard to fix..

That said I think we made the right choice keeping this in contrib/. This avoids scope creep and avoids the addition of dependencies which are unrelated to the core mission and APIs - separate projects can do nice things with our APIs and use more elaborate dependencies, and ask us to open up some APIs as needed.

[ion232]

@ion232 is there anything else pending to mark this a non-draft?

Not really. I was possibly going add some other features but didn't get around to it. I was waiting for feedback.

[harrysarson]

Thanks for this script! I hit a small issue when using it on a complex buildstream project containing junctions. The generated graphviz ends up containing colons in the wrong place making it invalid. See small patch below that fixes the script base on a recommendation I found here https://graphviz.readthedocs.io/en/stable/manual.html#node-ports-compass

diff --git a/contrib/bst-graph b/contrib/bst-graph
index 3fe93e1ff..5c05c6b4b 100755
--- a/contrib/bst-graph+++ b/contrib/bst-graph@@ -29,6 +29,7 @@ installed.
import argparse
import subprocess
import re
+import urllib.parse
from graphviz import Digraph
from ruamel.yaml import YAML
@@ -55,6 +56,10 @@ def parse_args():
return parser.parse_args()
+def escape(s):+ return urllib.parse.quote_plus(s.encode())++
def parse_graph(lines):
'''Return nodes and edges of the parsed grpah.
@@ -80,12 +85,13 @@ def parse_graph(lines):
build_dep = parser.load(build_dep)
runtime_dep = parser.load(runtime_dep)
- nodes.add(name)- [build_deps.add((name, dep)) for dep in build_dep if dep]- [runtime_deps.add((name, dep)) for dep in runtime_dep if dep]+ safe_name = escape(name)- return nodes, build_deps, runtime_deps+ nodes.add((safe_name, name))+ [build_deps.add((safe_name, escape(dep))) for dep in build_dep if dep]+ [runtime_deps.add((safe_name, escape(dep))) for dep in runtime_dep if dep]+ return nodes, build_deps, runtime_deps
def generate_graph(nodes, build_deps, runtime_deps):
'''Generate graph from given nodes and edges.
@@ -99,8 +105,8 @@ def generate_graph(nodes, build_deps, runtime_deps):
A graphviz.Digraph object
'''
graph = Digraph()
- for node in nodes:- graph.node(node)+ for tag, name in nodes:+ graph.node(tag, label=name)
for source, target in build_deps:
graph.edge(source, target, label='build-dep')
for source, target in runtime_deps:

[abderrahim]

@harrysarson This PR is about integrating the bst-graph script into buildstream proper. For fixes to the existing contrib/bst-graph script, please open a PR. Thanks.

[ion232]

Closes#1915

• Adds a new command to bst called 'graph' which reimplements the same API and functionality as contrib/bst-graph.
• The implementation details are within _stream.py as another function called 'graph'.
• Removes contrib/bst-graph.

EDIT: I've added another commit that changes the functionality to makes two separate graphs for buildtime and runtime, as this is clearer imo.

[jjardon]

@ion232 is there anything else pending to mark this a non-draft?

[gtristan]

I'm not really in favor of having bst-graph at all.

I think we should either:

• Enhance bst show with features like --depth or such, allowing some scripting to construct the graph and display it however it wants
• Open up some very limited Stream / App APIs from the buildstream module and make a limited set of APIs public
  • Allowing at least to load the build graph once and have access to the Element and Source data model, eliminating the need to load the build graph multiple times (as would be the case with bst show scripting) and allowing users to do whatever introspective operations they might want

Either of these approaches don't add a first class graphing feature but allow users to do more powerful things without requiring special buildstream features to do so... specifically this should be interesting if we add APIs for Source objects to report information about their URLs etc, so that we can do SBoMs in a way that is stable and does not abuse internal APIs (like fdsdk's collect_manifest thing does).

EDIT: It looks like this script already gets the work done with a single bst show, so that has already been thought out pretty well.

I find the output to be particularly horrible to look at for sufficiently big graphs (I've finally looked at one today, after all these years), I suspect that whatever useful things one might want to achieve by looking at this, could be better achieved with some scripting (e.g. perhaps a combination of bst show commands with bst artifact list-contents could get you as far as finding out "Why does this file I don't want end up in my image ?" and this would be more reliable than looking at this huge graph IMHO).

Side note, the script does not work with junctions, it seems to be a simple quoting error that shouldn't be hard to fix..

That said I think we made the right choice keeping this in contrib/. This avoids scope creep and avoids the addition of dependencies which are unrelated to the core mission and APIs - separate projects can do nice things with our APIs and use more elaborate dependencies, and ask us to open up some APIs as needed.

[ion232]

@ion232 is there anything else pending to mark this a non-draft?

Not really. I was possibly going add some other features but didn't get around to it. I was waiting for feedback.

[harrysarson]

Thanks for this script! I hit a small issue when using it on a complex buildstream project containing junctions. The generated graphviz ends up containing colons in the wrong place making it invalid. See small patch below that fixes the script base on a recommendation I found here https://graphviz.readthedocs.io/en/stable/manual.html#node-ports-compass

diff --git a/contrib/bst-graph b/contrib/bst-graph
index 3fe93e1ff..5c05c6b4b 100755
--- a/contrib/bst-graph+++ b/contrib/bst-graph@@ -29,6 +29,7 @@ installed.
import argparse
import subprocess
import re
+import urllib.parse
from graphviz import Digraph
from ruamel.yaml import YAML
@@ -55,6 +56,10 @@ def parse_args():
return parser.parse_args()
+def escape(s):+ return urllib.parse.quote_plus(s.encode())++
def parse_graph(lines):
'''Return nodes and edges of the parsed grpah.
@@ -80,12 +85,13 @@ def parse_graph(lines):
build_dep = parser.load(build_dep)
runtime_dep = parser.load(runtime_dep)
- nodes.add(name)- [build_deps.add((name, dep)) for dep in build_dep if dep]- [runtime_deps.add((name, dep)) for dep in runtime_dep if dep]+ safe_name = escape(name)- return nodes, build_deps, runtime_deps+ nodes.add((safe_name, name))+ [build_deps.add((safe_name, escape(dep))) for dep in build_dep if dep]+ [runtime_deps.add((safe_name, escape(dep))) for dep in runtime_dep if dep]+ return nodes, build_deps, runtime_deps
def generate_graph(nodes, build_deps, runtime_deps):
'''Generate graph from given nodes and edges.
@@ -99,8 +105,8 @@ def generate_graph(nodes, build_deps, runtime_deps):
A graphviz.Digraph object
'''
graph = Digraph()
- for node in nodes:- graph.node(node)+ for tag, name in nodes:+ graph.node(tag, label=name)
for source, target in build_deps:
graph.edge(source, target, label='build-dep')
for source, target in runtime_deps:

[abderrahim]

@harrysarson This PR is about integrating the bst-graph script into buildstream proper. For fixes to the existing contrib/bst-graph script, please open a PR. Thanks.