Re: [Buildroot] [PATCH 2/2] support/scripts/graph-depends allow for forward and reverse depends on same graph

["Yann E. MORIN" <yann.morin.1998@free.fr>]

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 E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot