| Message ID | 20230122234309.2111129-2-me@stevenhay.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] support/scripts/graph-depends cleanup done_deps global | expand |
Steve, All, On 2023-01-23 00:43 +0100, Steve Hay via buildroot spake thusly: > Signed-off-by: Steve Hay <me@stevenhay.com> Please write a commit log that explains the change (don't describe it. explain it): as you could see from our discussion on IRC, the title of your commit is not enough to understand what you are trying to achieve. Maybe explain that it tries to represent both forward and backward dependencies of a single package, with a little schema (here for pkg D): $ make pkg-d-graph-both-depends pkg A -. .-> pkg E \ / pkg B ----> pkg D ----> pkg F / \ pkg C -' '-> pkg G Also, please insert this in the current package rules: $ make help [...] Package-specific: <pkg> - Build and install <pkg> and all its dependencies <pkg>-source - Only download the source files for <pkg> [...] <pkg>-show-depends - List packages on which <pkg> depends <pkg>-show-rdepends - List packages which have <pkg> as a dependency <pkg>-show-recursive-depends - Recursively list packages on which <pkg> depends <pkg>-show-recursive-rdepends - Recursively list packages which have <pkg> as a dependency <pkg>-graph-depends - Generate a graph of <pkg>'s dependencies <pkg>-graph-rdepends - Generate a graph of <pkg>'s reverse dependencies <pkg>-graph-both-depends - Generate a graph of both <pkg>'s forward and reverse dependencies <pkg>-dirclean - Remove <pkg> build directory [...] > --- > support/scripts/graph-depends | 55 +++++++++++++++++++++++++---------- > 1 file changed, 39 insertions(+), 16 deletions(-) > > diff --git a/support/scripts/graph-depends b/support/scripts/graph-depends > index 3e3373950f..91aaf5d867 100755 > --- a/support/scripts/graph-depends > +++ b/support/scripts/graph-depends [--SNIP--] > @@ -246,6 +251,8 @@ def parse_args(): > help="Graph the dependencies of PACKAGE") > parser.add_argument("--depth", '-d', metavar="DEPTH", dest="depth", type=int, default=0, > help="Limit the dependency graph to DEPTH levels; 0 means no limit.") > + parser.add_argument("--rdepth", metavar="RDEPTH", dest="rdepth", type=int, default=0, > + help="Limit the dependency graph to DEPTH levels; 0 means no limit.") Do we really need to have two different options for the direct and reverse depths? If so, then be sure to modify the <pkg>-graph-rdepends accordingly (in package/pkg-generic). [--SNIP--] > @@ -330,21 +337,37 @@ def main(): > logging.error("Error: incorrect color list '%s'" % args.colors) > sys.exit(1) > > - deps, rdeps, dict_types, dict_versions = brpkgutil.get_dependency_tree() > - dict_deps = deps if args.direct else rdeps > - > - check_circular_deps(dict_deps) > - if check_only: > - sys.exit(0) > - > - dict_deps = remove_extra_deps(dict_deps, rootpkg, args.transitive, arrow_dir) > > # Start printing the graph data > if draw_graph: > outfile.write("digraph G {\n") > > - print_pkg_deps(outfile, dict_deps, dict_types, dict_versions, stop_list, exclude_list, > - arrow_dir, draw_graph, 0, args.depth, rootpkg, colors) > + > + deps, rdeps, dict_types, dict_versions = brpkgutil.get_dependency_tree() > + > + # forward > + if args.direct: > + dict_deps = deps > + direct = True > + #arrow_dir = "forward" # hack > + check_circular_deps(dict_deps) > + if check_only: > + sys.exit(0) I am not very fond of this: if one were to call: $ graph-depends --direct --reverse --check-only then one would expect to have both checks be run, but here only the direct one will be. I think we need to change check_circular_deps() to return False (no circular dependencies) or True (circular dependencies) (or it can raise an exception that we catch) and store that for later concumption. circ_deps = [] if args.direct: if check_circular_deps(...): circ_deps.append('direct') if not args.checkonly: dict_deps = remove_extra_deps(...) print_pkg_deps(...) if args.reverse: if check_circular_deps(...): circ_deps.append('reverse') if not args.checkonly: dict_deps = remove_extra_deps(...) print_pkg_deps(...) if circ_deps: print(f"Bummer: circular {' and '.join(circ_deps)} dependencies detected") os.exit(1) Something along those lines... Regards, Yann E. MORIN. > + dict_deps = remove_extra_deps(dict_deps, rootpkg, args.transitive, direct) > + print_pkg_deps(outfile, dict_deps, dict_types, dict_versions, stop_list, exclude_list, > + direct, draw_graph, 0, args.depth, rootpkg, colors) > + > + # reverse > + if args.reverse: > + dict_deps = rdeps > + direct = False > + #arrow_dir = "back" # hack > + check_circular_deps(dict_deps) > + if check_only: > + sys.exit(0) > + dict_deps = remove_extra_deps(dict_deps, rootpkg, args.transitive, direct) > + print_pkg_deps(outfile, dict_deps, dict_types, dict_versions, stop_list, exclude_list, > + direct, draw_graph, 0, args.depth, rootpkg, colors) > > if draw_graph: > outfile.write("}\n") > -- > 2.30.2 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/mailman/listinfo/buildroot
Yann, All, On 1/23/23 2:20 AM, Yann E. MORIN wrote: > Do we really need to have two different options for the direct and > reverse depths? > > If so, then be sure to modify the <pkg>-graph-rdepends accordingly (in > package/pkg-generic). I think it is nice to have the "control knob" available for both, but the current API to this script uses default values of 0 (no limit) in all cases--so no change to <pkg>-graph-rdepends is needed. I've added the ability to specify depth to the makefile. > I am not very fond of this: if one were to call: > $ graph-depends --direct --reverse --check-only > > then one would expect to have both checks be run, but here only the > direct one will be. > > I think we need to change check_circular_deps() to return False (no > circular dependencies) or True (circular dependencies) (or it can raise > an exception that we catch) and store that for later concumption. I agree that it should check both and have updated the script so that it does, but I'd rather the enhancements you're talking about be handled with a separate patch. Revised patch forthcoming. Steve
diff --git a/support/scripts/graph-depends b/support/scripts/graph-depends index 3e3373950f..91aaf5d867 100755 --- a/support/scripts/graph-depends +++ b/support/scripts/graph-depends @@ -159,11 +159,11 @@ def check_circular_deps(deps): # This functions trims down the dependency list of all packages. # It applies in sequence all the dependency-elimination methods. -def remove_extra_deps(deps, rootpkg, transitive, arrow_dir): +def remove_extra_deps(deps, rootpkg, transitive, direct): # For the direct dependencies, find and eliminate mandatory # deps, and add them to the root package. Don't do it for a # reverse graph, because mandatory deps are only direct deps. - if arrow_dir == "forward": + if direct: for pkg in list(deps.keys()): if not pkg == rootpkg: for d in get_mandatory_deps(pkg, deps): @@ -197,14 +197,16 @@ def print_attrs(outfile, pkg, pkg_type, pkg_version, depth, colors): outfile.write("%s [color=%s,style=filled]\n" % (name, color)) + # Print the dependency graph of a package def print_pkg_deps(outfile, dict_deps, dict_types, dict_versions, stop_list, exclude_list, - arrow_dir, draw_graph, depth, max_depth, pkg, colors, done_deps=None): + direct, draw_graph, depth, max_depth, pkg, colors, done_deps=None): if done_deps is None: done_deps = [] if pkg in done_deps: return done_deps.append(pkg) + if draw_graph: print_attrs(outfile, pkg, dict_types[pkg], dict_versions[pkg], depth, colors) elif depth != 0: @@ -231,9 +233,12 @@ def print_pkg_deps(outfile, dict_deps, dict_types, dict_versions, stop_list, exc break if add: if draw_graph: - outfile.write("%s -> %s [dir=%s]\n" % (pkg_node_name(pkg), pkg_node_name(d), arrow_dir)) + if direct: + outfile.write("%s -> %s [dir=%s]\n" % (pkg_node_name(pkg), pkg_node_name(d), "forward")) + else: + outfile.write("%s -> %s [dir=%s]\n" % (pkg_node_name(d), pkg_node_name(pkg), "forward")) print_pkg_deps(outfile, dict_deps, dict_types, dict_versions, stop_list, exclude_list, - arrow_dir, draw_graph, depth + 1, max_depth, d, colors, done_deps) + direct, draw_graph, depth + 1, max_depth, d, colors, done_deps) def parse_args(): @@ -246,6 +251,8 @@ def parse_args(): help="Graph the dependencies of PACKAGE") parser.add_argument("--depth", '-d', metavar="DEPTH", dest="depth", type=int, default=0, help="Limit the dependency graph to DEPTH levels; 0 means no limit.") + parser.add_argument("--rdepth", metavar="RDEPTH", dest="rdepth", type=int, default=0, + help="Limit the dependency graph to DEPTH levels; 0 means no limit.") parser.add_argument("--stop-on", "-s", metavar="PACKAGE", dest="stop_list", action="append", help="Do not graph past this package (can be given multiple times)." + " Can be a package name or a glob, " + @@ -267,7 +274,7 @@ def parse_args(): help="Draw (do not draw) transitive dependencies") parser.add_argument("--direct", dest="direct", action='store_true', default=True, help="Draw direct dependencies (the default)") - parser.add_argument("--reverse", dest="direct", action='store_false', + parser.add_argument("--reverse", dest="reverse", action='store_true', default=False, help="Draw reverse dependencies") parser.add_argument("--quiet", '-q', dest="quiet", action='store_true', help="Quiet") @@ -330,21 +337,37 @@ def main(): logging.error("Error: incorrect color list '%s'" % args.colors) sys.exit(1) - deps, rdeps, dict_types, dict_versions = brpkgutil.get_dependency_tree() - dict_deps = deps if args.direct else rdeps - - check_circular_deps(dict_deps) - if check_only: - sys.exit(0) - - dict_deps = remove_extra_deps(dict_deps, rootpkg, args.transitive, arrow_dir) # Start printing the graph data if draw_graph: outfile.write("digraph G {\n") - print_pkg_deps(outfile, dict_deps, dict_types, dict_versions, stop_list, exclude_list, - arrow_dir, draw_graph, 0, args.depth, rootpkg, colors) + + deps, rdeps, dict_types, dict_versions = brpkgutil.get_dependency_tree() + + # forward + if args.direct: + dict_deps = deps + direct = True + #arrow_dir = "forward" # hack + check_circular_deps(dict_deps) + if check_only: + sys.exit(0) + dict_deps = remove_extra_deps(dict_deps, rootpkg, args.transitive, direct) + print_pkg_deps(outfile, dict_deps, dict_types, dict_versions, stop_list, exclude_list, + direct, draw_graph, 0, args.depth, rootpkg, colors) + + # reverse + if args.reverse: + dict_deps = rdeps + direct = False + #arrow_dir = "back" # hack + check_circular_deps(dict_deps) + if check_only: + sys.exit(0) + dict_deps = remove_extra_deps(dict_deps, rootpkg, args.transitive, direct) + print_pkg_deps(outfile, dict_deps, dict_types, dict_versions, stop_list, exclude_list, + direct, draw_graph, 0, args.depth, rootpkg, colors) if draw_graph: outfile.write("}\n")
Signed-off-by: Steve Hay <me@stevenhay.com> --- support/scripts/graph-depends | 55 +++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 16 deletions(-)