Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: dependencies --no-owns #274

Closed
wduminy opened this issue Mar 9, 2024 · 14 comments · Fixed by #348
Closed

Feature request: dependencies --no-owns #274

wduminy opened this issue Mar 9, 2024 · 14 comments · Fixed by #348

Comments

@wduminy
Copy link
Contributor

wduminy commented Mar 9, 2024

Thank for for creating this great utility.

A new option --no-owns on the dependencies command would help me to create a cleaner usage graph.

Looking at the graph below, applying the --no-owns option could create a view that shows only the uses links.
Perhaps also removing those nodes that do not participate in such a relation.

out

@duckki
Copy link

duckki commented Mar 29, 2024

This seems related to #265.

@muhuk
Copy link
Contributor

muhuk commented Nov 22, 2024

This would be quite useful in decipering the graph I get by running the tool on yer

image

Edges running over nodes are also not helping the situation.

cargo modules dependencies --no-externs --no-fns --no-traits --no-types --no-sysroot  | dot -Tsvg > mods.svg

@muhuk
Copy link
Contributor

muhuk commented Nov 22, 2024

I am specifically looking for circular dependencies like this one between layer, undo & session.

image


My mistake; undo does not use session, apparently it's the line from crate to the session module that decided to go under the box for some reason.

@regexident
Copy link
Owner

regexident commented Nov 24, 2024

Would any of you mind giving this PR's branch a try and report back on whether it addresses your needs?

Edit: Oops, had the wrong link. 🫣 Fixed.

@muhuk
Copy link
Contributor

muhuk commented Nov 25, 2024

No! But I'll give #348 a try later today.

@regexident
Copy link
Owner

This is what #348 produces for the smoke test project:

Without --no-owns

cargo run --release -- dependencies --manifest-path "tests/projects/smoke"  > ./graph-without-owns.dot

smoke-with-owns-edges

With --no-owns

cargo run --release -- dependencies --manifest-path "tests/projects/smoke" -
-no-owns > ./graph-without-owns.dot

smoke-without-owns-edges

@muhuk
Copy link
Contributor

muhuk commented Nov 25, 2024

Not a very useful output for my codebase. Notice that session -> undo and yer -> undo arrows are overlapping.

cargo modules dependencies --no-externs --no-fns --no-traits --no-types --no-sysroot --no-owns  | dot -Tsvg > mods.svg

image

@regexident
Copy link
Owner

regexident commented Nov 25, 2024

Given that finding an optimal minimally-crossing graph layout (which deals with edge-crossings, but feels related in nature) is known to be NP-complete on arbitrary graphs this issue feels somewhat out-of-scope for the tool itself. 😅

cargo-modules merely generates the .dot file, while allowing you to pick a different layout algorithm (--layout "…") if so desired. Have you by chance tried that with your graph?

If picking a different layout algorithm doesn't fix the edge-overlap issue, then setting splines=curved for the graph itself would be the next thing I'd try:

digraph {
    graph [
        // ...
        splines="curved"
    ]
    // ...
}

IIRC graph [ splines="curved" ] doesn't work with node [ shape="record" ] for some layout algorithms though, so you may have to disable the latter.

Another obvious caveat of this approach is that you have to manually make that change yourself, which may get in the way of one's workflow cycle. I've thought about exposing more graphviz settings (i.e. shape, splines, …) as CLI options but ended up not doing it due to scope-creep, as well as a fear of flooding the CLI with seldomly used options. But maybe there is a compromise that would work: #349

@muhuk
Copy link
Contributor

muhuk commented Nov 25, 2024

I appreciate this is a dot rendering issue. But I think it's totally possible to create such dependency (use) diagrams. C4 (plantuml) does it well for example. (source code for that diagram is here )

@wduminy
Copy link
Contributor Author

wduminy commented Nov 30, 2024

@muhuk Try this to see if it resolves the overlap issue you mention above.

cargo modules dependencies --no-externs --no-fns --no-traits --no-types --no-sysroot --no-owns  | dot -Gsplines=curved  -Tsvg > mods.svg 

@muhuk
Copy link
Contributor

muhuk commented Nov 30, 2024

@wduminy this is better, but the hierarchy in the architecture is still kind of hidden, because of the haphazard placement of nodes.

image

This time I can clearly see, for example, there are no depenceny cycles with undo. But if the # of modules double, and it will, I doubt looking at this graph would be quicker than cheking uses within the code.

@regexident
Copy link
Owner

@wduminy this is better, but the hierarchy in the architecture is still kind of hidden, because of the haphazard placement of nodes.

Sorting by the "hierarchy" of the architecture is difficult (at least without applying extensive network analysis to derive the most-likely hierarchy from the graph) since the graph isn't a DAG.

I generally find the circo layout to work quite well for this sort of dependency analysis as it makes the dependencies more apparent:

graph [ layout=circo ... ]:

circo

The fdp/sfdp layouts also tend to layout things with the central nodes near the center, so the hierarchy tends to go inside-out:

graph [ layout=fdp ... ]:
fdp

@regexident
Copy link
Owner

This time I can clearly see, for example, there are no depenceny cycles with undo. But if the # of modules double, and it will, I doubt looking at this graph would be quicker than cheking uses within the code.

Yeah, that's unfortunately an inherent problem of network analysis and a lot of research has gone into visual graph navigation over the years, with no clear winners though, imho.

The --focus-on and --max-depth options can help by being able to put the spotlight on a specific part of the graph, but they certainly are no silver bullet. And I actually haven't tested them against --no-owns yet, which filters out "owns" edges after all the other filters have been applied (which made it easier to get something working now, without having to deal with the fiddly edge-cases of structure-preserving graph filtering).

@muhuk
Copy link
Contributor

muhuk commented Dec 1, 2024

@regexident have you had the chance to look at the graph I shared here? Notice all the arrows go from left to right, lower-level modules (that mostly get incoming dependencies) are on the right, higher-level modules (that have more dependencies than incoming dependencies) are on the left.

Sorting by the "hierarchy" of the architecture is difficult (at least without applying extensive network analysis to derive the most-likely hierarchy from the graph) since the graph isn't a DAG.

But my use case is to figure out if there are circular dependencies. something::else is owned by something is trivial and obvious. I may be wrong but I see no point in having an owns graph. A recursive ls command would show the same info, if needed. But the real value, IMHO, is uses graphs. I want to be able to see couplings between modules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants