Misc graphs enhancements (branch yem/graphs)

From: "Yann E. MORIN" <[email protected]>

Variables should be prefixed with BR_ when they are not user-facing.

As a side effect, the new variable is prettier than the previous one. :-)

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

From: "Yann E. MORIN" <[email protected]>

Currently, we are using a crude, ad-hoc parsing of argv[]. This is a limiting factor to adding new options.

Use argparse instead, and introduce a single argument for now:
--package, -p PACKAGE

In the (near) future, we'll be able to add more option arguments, such as depth-limiting for big graphs.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
 package/pkg-generic.mk        |  2 +-
 support/scripts/graph-depends | 23 +++++++++++++----------
 2 files changed, 14 insertions(+), 11 deletions(-)

package/pkg-generic.mk +1 -1

index b3a4c17..080ee56 100644
@@ -495,7 +495,7 @@ $(1)-show-depends:
 $(1)-graph-depends:
                        @$(INSTALL) -d $(O)/graphs
                        @cd "$(CONFIG_DIR)"; \
-                       $(TOPDIR)/support/scripts/graph-depends $(1) \
+                       $(TOPDIR)/support/scripts/graph-depends -p $(1) \
                        |dot -T$(BR_GRAPH_OUT) -o $(O)/graphs/$$(@).$(BR_GRAPH_OUT)
 
 $(1)-dirclean:         $$($(2)_TARGET_DIRCLEAN)

support/scripts/graph-depends +5 -4

index ac24086..fc3cadd 100755
@@ -1,13 +1,13 @@
 #!/usr/bin/python
 
 # Usage (the graphviz package must be installed in your distribution)
-#  ./support/scripts/graph-depends [package-name] > test.dot
+#  ./support/scripts/graph-depends [-p package-name] > test.dot
 #  dot -Tpdf test.dot -o test.pdf
 #
 # With no arguments, graph-depends will draw a complete graph of
-# dependencies for the current configuration. With an argument,
-# graph-depends will draw a graph of dependencies for the given
-# package name.
+# dependencies for the current configuration.
+# If '-p <package-name>' is specified, graph-depends will draw a graph
+# of dependencies for the given package name.
 #
 # Limitations
 #

@@ -21,6 +21,7 @@
 
 import sys
 import subprocess
+import argparse
 
 # In FULL_MODE, we draw the full dependency graph for all selected
 # packages

support/scripts/graph-depends +2 -6

     mode = FULL_MODE
-elif len(sys.argv) == 2:
-    mode = PKG_MODE
-    rootpkg  = sys.argv[1]
 else:
-    print "Usage: graph-depends [package-name]"
-    sys.exit(1)
+    mode = PKG_MODE
+    rootpkg = args.package
 
 allpkgs = []
 
--

1.8.3.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Quoted 5 lines

From: "Yann E. MORIN" <[email protected]>

Currently, we are using a crude, ad-hoc parsing of argv[]. This is a limiting factor to adding new options.

Quoted 2 lines

Use argparse instead, and introduce a single argument for now:
--package, -p PACKAGE

Is an option really needed for package? I mean it could be the remaining argument(s); or maybe it does not fit in developments you have in mind ;).

Quoted 6 lines

support/scripts/graph-depends +0 -0

index ac24086..fc3cadd 100755
@@ -31,14 +32,16 @@ PKG_MODE  = 2
 

support/scripts/graph-depends PKG_MODE = 2 · line 36

Quoted 5 lines

support/scripts/graph-depends +2 -1

 mode = 0
 
-if len(sys.argv) == 1:
+parser = argparse.ArgumentParser(description="Graph pacakges dependencies")
+parser.add_argument("--package", '-p', metavar="PACKAGE",

Any reason to mixed double and single quotes?

support/scripts/graph-depends PKG_MODE = 2 · line 40

Quoted 4 lines

support/scripts/graph-depends +4 -0

+                    help="Graph the dependencies of PACKAGE")
+args = parser.parse_args()
+
+if args.package == None:

In Python, None is a singleton, and it is recommended to use "is" or "is not" for testing them [1].

From: "Yann E. MORIN" <[email protected]>

Currently, the complete dependency chain of a package is used to generate the dependency graph. When this dependency chain is long, the generated graph becomes almost unreadable.

However, it is often sufficient to get the first few levels of dependency of a package.

Add a new variable BR2_GRAPH_DEPTH, that the user can set to limit the depth of the dependency list.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
 Makefile                      |  3 ++-
 package/pkg-generic.mk        |  2 +-
 support/scripts/graph-depends | 49 ++++++++++++++++++++++++++++++++-----------
 3 files changed, 40 insertions(+), 14 deletions(-)

Makefile +2 -1

index b0eec7f..467e21c 100644
@@ -150,6 +150,7 @@ endif
 # Need that early, before we scan packages
 # Avoids doing the $(or...) everytime
 BR_GRAPH_OUT := $(or $(BR2_GRAPH_OUT),pdf)
+BR_GRAPH_DEPTH := $(or $(BR2_GRAPH_DEPTH),0)
 
 BUILD_DIR:=$(BASE_DIR)/build
 BINARIES_DIR:=$(BASE_DIR)/images

@@ -672,7 +673,7 @@ graph-build: $(O)/build/build-time.log
 graph-depends:
        @$(INSTALL) -d $(O)/graphs
        @cd "$(CONFIG_DIR)"; \
-       $(TOPDIR)/support/scripts/graph-depends \
+       $(TOPDIR)/support/scripts/graph-depends -d $(BR_GRAPH_DEPTH) \
        |dot -T$(BR_GRAPH_OUT) -o $(O)/graphs/$(@).$(BR_GRAPH_OUT)
 
 else # ifeq ($(BR2_HAVE_DOT_CONFIG),y)

package/pkg-generic.mk +1 -1

index 080ee56..c162902 100644
@@ -495,7 +495,7 @@ $(1)-show-depends:
 $(1)-graph-depends:
                        @$(INSTALL) -d $(O)/graphs
                        @cd "$(CONFIG_DIR)"; \
-                       $(TOPDIR)/support/scripts/graph-depends -p $(1) \
+                       $(TOPDIR)/support/scripts/graph-depends -p $(1) -d $(BR_GRAPH_DEPTH) \
                        |dot -T$(BR_GRAPH_OUT) -o $(O)/graphs/$$(@).$(BR_GRAPH_OUT)
 
 $(1)-dirclean:         $$($(2)_TARGET_DIRCLEAN)

support/scripts/graph-depends +2 -0

index fc3cadd..ebf511b 100755
@@ -8,6 +8,8 @@
 # dependencies for the current configuration.
 # If '-p <package-name>' is specified, graph-depends will draw a graph
 # of dependencies for the given package name.
+# If '-d <depth>' is specified, graph-depends will limit the depth of
+# the dependency graph to 'depth' levels.
 #
 # Limitations
 #

support/scripts/graph-depends +31 -13

+    max_depth = int(args.depth)
+
 allpkgs = []
 
 # Execute the "make show-targets" command to get the list of the main

@@ -193,6 +201,7 @@ if mode == FULL_MODE:
     deps = get_all_depends(filtered_targets)
     if deps != None:
         dependencies += deps
+    rootpkg = 'all'
 
 # In pkg mode, start directly with get_all_depends() on the requested
 # package

@@ -201,26 +210,42 @@ elif mode == PKG_MODE:
 
 dependencies = remove_redundant_deps(dependencies)
 
-# Start printing the graph data
-print "digraph G {"
-
-# First, the dependencies. Usage of set allows to remove duplicated
-# dependencies in the graph
-for dep in set(dependencies):
-    print "%s -> %s" % (pkg_node_name(dep[0]), pkg_node_name(dep[1]))
+# Make the dependencies a dictionnary { 'pkg':[dep1, dep2, ...] }
+dict_deps = {}
+for dep in dependencies:
+    if not dict_deps.has_key(dep[0]):
+        dict_deps[dep[0]] = []
+    dict_deps[dep[0]].append(dep[1])
 
-# Then, the node attributes: color, style and label.
-for pkg in allpkgs:
+# Print the attributes of a node: label and fill-color
+def print_attrs(pkg):
     if pkg == 'all':
         print "all [label = \"ALL\"]"
         print "all [color=lightblue,style=filled]"
-        continue
-
+        return
     print "%s [label = \"%s\"]" % (pkg_node_name(pkg), pkg)
-
     if mode == PKG_MODE and pkg == rootpkg:
         print "%s [color=lightblue,style=filled]" % pkg_node_name(rootpkg)
     else:
         print "%s [color=grey,style=filled]" % pkg_node_name(pkg)
 
+# Print the dependency graph of a package
+def print_pkg_deps(depth, pkg):
+    if pkg in done_deps:
+        return
+    done_deps.append(pkg)
+    print_attrs(pkg)
+    if not dict_deps.has_key(pkg):
+        return
+    if max_depth == 0 or depth < max_depth:
+        for d in dict_deps[pkg]:
+            print "%s -> %s" % (pkg_node_name(pkg), pkg_node_name(d))
+            print_pkg_deps(depth+1, d)
+
+# Start printing the graph data
+print "digraph G {"
+
+done_deps = []
+print_pkg_deps(0, rootpkg)
+
 print "}"
--

1.8.3.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Same comments as for patch 2/6 ;)

From: "Yann E. MORIN" <[email protected]>

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
 docs/manual/common-usage.txt | 2 ++
 1 file changed, 2 insertions(+)

docs/manual/common-usage.txt +2 -1

index d9de598..c9d5eb9 100644
@@ -96,6 +96,8 @@ to +make+ or set in the environment:
   build-time graphs
 * +BR2_GRAPH_OUT+ to set the filetype of generated graphs, either +pdf+ (the
   default), or +png+.
+* +BR2_GRAPH_DEPTH+ (an integer) to limit the depth of the dependency graph.
+  The default, +0+, is to not limit the depth.
 
 An example that uses config files located in the toplevel directory and
 in your $HOME:
--

1.8.3.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

From: "Yann E. MORIN" <[email protected]>

Al packages depend on 'toolchain'. Currently, 'graph-depends' graphs this dependency. The resulting graph is thus cluttered with less-than-useful information.

Instead, do not graph the 'toolchain' dependency for any package, save for the fake 'all' package. The graph is now a bit more readable.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
 support/scripts/graph-depends | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Are you sure this works? I applied your patch series, and I still see all packages having a dependency on the toolchain. The PDF result is
http://free-electrons.com/~thomas/pub/bug-toolchain-graph-depends.pdf.

From: "Yann E. MORIN" <[email protected]>

Al packages depend on 'toolchain'. Currently, 'graph-depends' graphs this

dependency. The resulting graph is thus cluttered with less-than-useful information.

Instead, do not graph the 'toolchain' dependency for any package, save for the fake 'all' package. The graph is now a bit more readable.

Signed-off-by "Yann E. MORIN" <[email protected]>

Cc Thomas Petazzoni <[email protected]>

---
 support/scripts/graph-depends | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

support/scripts/graph-depends +5 -3

index ebf511b..4f532ba 100755
@@ -162,13 +162,16 @@ def has_redundant_deps(deps, pkg):
 def remove_redundant_deps(deps):
     newdeps = []
     for dep in deps:
-        if dep[0] != "all":
+        if dep[0] == "all" and dep[1] == "toolchain":
+            newdeps.append(dep)
+            continue
+        if dep[0] != "all" and dep[1] != "toolchain":
             newdeps.append(dep)
             continue
         if not has_redundant_deps(deps, dep[1]):
             newdeps.append(dep)
             continue
-        sys.stderr.write("Removing redundant dep all -> %s\n" % dep[1])
+        sys.stderr.write("Removing redundant dep %s -> %s\n" % (dep[0],dep[1]))
     return newdeps
 
 TARGET_EXCEPTIONS = [
--

1.8.3.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Yann E. MORIN (6):
Makefile: rename non-user-facing variable support/graph-depends: use argparse to parse argv[] support/graph-depends: add option to limit the depth of the graph
manual: document BR2_GRAPH_DEPTH
support/graph-depends: don't show toolchain dependency for all packages
graphs: make graphs with lots of packages nicer to look at

Quoted 8 lines

Yann E. MORIN (6):
Makefile: rename non-user-facing variable support/graph-depends: use argparse to parse argv[] support/graph-depends: add option to limit the depth of the graph
manual: document BR2_GRAPH_DEPTH
support/graph-depends: don't show toolchain dependency for all packages
graphs: make graphs with lots of packages nicer to look at

Applied, thanks. In the last patch, I've removed excessive spaces in the expression:

	max( ( ( n_pkgs + 10 ) / 48 , 2) )

If you have other fixes to do (such as things suggested by Samuel), please send followup patches as appropriate.