Option to remove external nodes?

[ejmount]

Option to remove external nodes?

[SebastianHambura]

I've tried running cargo modules generate graph --types --uses --traits --layout dot --no-externs | dot -Tpng -O and cargo modules generate graph --types --uses --traits --layout dot | dot -Tpng -O and i both cases I get the same (wrong) result as @ejmount

In case it helps, on my setup I have :

• cargo-modules v0.9.0
• rustc 1.67.1
• cargo 1.67.1

[Tehforsch]

I had the same problem. Looking at the code, external nodes should be filtered by should_retain_moduledef.
However, when I looked at it, the function was only called for internal items anyways.
So I checked the call site and it turns out that the only items which are removed from the graph are those that get put on the stack in Filter::filter.
That stack is populated by a breadth-first traversal of the owner_only_graph, which is constructed by removing all the Relationship::Uses edges on the original graph. That means that the BF traversal never gets to any of the external nodes and therefore they are also never removed from the graph and show up in the final result.

For my purposes, I could just replace the

let owner_only_graph = Self::owner_only_graph(&graph);
let mut traversal = Bfs::new(&owner_only_graph, root_idx);
while let Some(node_idx) = traversal.next(&owner_only_graph) {
    ...
}

with

let mut traversal = Bfs::new(&graph, root_idx);
while let Some(node_idx) = traversal.next(&graph) {
    ...
}

and get the result I wanted. Clearly that isn't an actual solution though, since it makes lots of smoke tests fail:

failures:
    focus_on::glob_path::smoke
    focus_on::self_path::smoke
    focus_on::simple_path::smoke
    focus_on::use_tree::smoke
    max_depth::depth_0::smoke
    max_depth::depth_1::smoke
    max_depth::depth_2::smoke
    selection::no_externs::smoke
    selection::no_modules::smoke
    selection::no_traits::smoke
    selection::no_types::smoke

Just thought I'd write it down in case it helps anyone.

[regexident]

Thanks for looking into this @Tehforsch! 🙇‍♂️

@ejmount, @visd0m, @SebastianHambura, @lucasMontenegro would you mind giving #258 a spin (e.g. cargo run --release -- dependencies --manifest-path <MANIFEST_PATH> …) and confirming whether or not this changes fixes your issues?🙏

[regexident]

@lucasMontenegro thanks everybody, stay tuned for v0.13.5 then!

[regexident]

Hi Ewan,

would you by chance be able to provide a minimal(!) reproducible main.rs file along with the actual, as well as expected output? I'm not exactly sure what you mean, so some real code would help greatly. 🙂

[ejmount]

Given a file like this, with appropriate Cargo.toml

struct AThing;

pub mod alpha {
    use delta::ATrait;
    pub mod beta {
        use crate::AThing;
        use chrono::DateTime;

        pub struct AnotherThing {
            dt: chrono::Duration,
        }

        pub mod gamma {

            use rand::CryptoRng;
        }
    }
    pub mod delta {
        use super::beta::AnotherThing;
        use rand::Rng;

        pub trait ATrait {}


    }

}

And running cargo modules generate graph --with-types --with-uses --with-traits --layout dot | dot -Tpng -O, I get this:

When what I actually want is something like this:

In particular I want to keep the uses links in between modules in the current crate, while removing the ones pointing to external items. Removing the --with-uses parameter removes the external items, but also removes the links inside the crate.

[regexident]

@ejmount Hm, when I run the same command I get exactly what you "actually want":

I've added a corresponding snapshot test here: #175

Are you sure you didn't accidentally also add --with-externs? Nodes with "external" should only ever get generated if --with-externs was provided.

[visd0m]

I have the same issue, don't know If I can help somehow.

[lucasMontenegro]

Hello again. I'm having a similar issue—or perhaps the same one.

The command I'm executing is:

cargo run --profile release -- dependencies --manifest-path ../veloren/Cargo.toml -p veloren-common-state --lib --no-externs

I think at least part of the problem is due to dependencies created after the resolution of a macro. For example, in the output of that command is this line:

    "veloren_common_state::plugin::Plugin::execute_prepared" -> "tracing_core::callsite::DefaultCallsite" [label="uses", color="#7f7f7f", style="dashed"] [constraint=false]; // "uses" edge

Which doesn't make sense since Veloren does not directly depend on tracing_core. When we look in Plugin::execute_prepared there is a call to tracing::warn. Which I suppose does create a dependency to tracing_core::callsite::DefaultCallsite, see this link.

[lucasMontenegro]

In my output I also have the lines:

    "veloren_common_state::special_areas" -> "approx" [label="uses", color="#7f7f7f", style="dashed"] [constraint=false]; // "uses" edge
    "veloren_common_state::state" -> "approx" [label="uses", color="#7f7f7f", style="dashed"] [constraint=false]; // "uses" edge

But veloren-common-state does not mention approx anywhere. vek, on the other hand, has this line: pub extern crate approx;. Within veloren-common-state the only files that include use vek::*; happen to be src/state.rs and src/special_areas.rs.

There is no other edge mentioning approx so I don't know if it's being used at all.

[lucasMontenegro]

Thank you @regexident, the graph no longer contains nodes labeled "external".

Original:

Fixed:

without-externs.txt
without-externs-fixed.txt