crater

crater copied to clipboard

Hi,

It often happens that a crate brakes causing many downstream build failures. It would be nice to know in the ui if a crate build-fail on its own code or in a dependency. It may be possible to find this info in the logs, but if not we are also testing all the dependencies, so if any of the dependencies are build-fail then this crate is build-dependencies-fail.

Just my unrequested 2cents.

I would expand this to building the crates in an order such that we'd know ahead of time that crate B doesn't need to be looked at since it's guaranteed to fail as it depends on crate A. This could potentially lower runtimes when lots of crates fail... though I guess in the happy case it doesn't help much.

What would be involved in doing this? Is there code that constructs a dependency tree for all the crates? (If so where?) Then I guess we weed need to do a topological sort on that tree to make a run order. Then check whether any of a crates dependencies did not build and mark it as skipped. We could combine with #100 and mark blacklisted crates as skipped as well. But I don't know the code so I'm just guessing. With some good guidance I myte eavan give this a try.

This would be currently difficult, I imagine, but I think we'd use cargo metadata during lockfile generation to discover dependencies, then use that to build up the graph, and use it during report generation (during the run itself seems not hugely useful). Given that, it might be good enough to run cargo metadata before we generate the final report, perhaps in parallel with uploading the logs.

In time we'll want to build a dependency graph (even if not for this issue) because it'll let us get some parallelism wins (we can work our way up the dependency tree in parallel if there are no common dependencies).

Roughly noting down some initial thoughts on steps:

1. we'll want to use cargo metadata (https://github.com/rust-lang/cargo/blob/master/src/cargo/ops/cargo_output_metadata.rs) as @Mark-Simulacrum says, either by calling it as a command or depending on Cargo as a library (probably the former)
2. we build up a acyclic digraph (e.g. https://docs.rs/petgraph/0.4.11/petgraph/graph/type.DiGraph.html or steal whatever cargo uses) during the prepare phase and serialize it to disk (petgraphs implement serialize)
3. we add another subcommand that will generate a .dot file indicating the full crater dependency tree along with appropriate colors for passed/failed for each node. petgraph appears to support .dot natively so this may be just a matter of associating nodes with their pass/fail status and throwing the whole thing at petgraph
4. we use the in-memory graph representation to build crates in a consistent order
5. if a dependency fails, mark all dependees as skipped

Once we have this stuff in place, things like the blacklist become much easier. There are some subtleties to be aware of though, like keeping track of enabled feature flags as they are part of what defines 'a dependency', but that's all in cargo metadata as well so it should be manageable.

If this sounds interesting @Eh2406 then I can write up some more detailed instructions for steps 1 and 2 - I'd like to do it in bite size chunks so we can get each step merged independently and benefit from whatever has been built so far, but you're more than welcome to tackle it all if you feel like it!

I can try to work on it and even if I can't it will be documented for someone else to pick up. :-)

That's a fair point! I've put writing down instructions on my todo list, may take a couple of weeks to get to it.

Just for reference, the graph thing is implemented in #193, the only thing left to do is parsing the cargo dependency graph and adding edges between the crates in the existing graph.

One specific use case that I would like to avoid breaking:

• Sometimes we have a bug fix or other change and we want to evaluate the impact
• If we evaluate this as a deny-by-default lint, then we can get a good measure of the total number of impacted crates by taking advantage of lint capping:
  • If a crate X contains a violation, we get an error when building it
  • But other crates that depend on X will build X with lints capped, and hence they only get a warning
    • This way, if those other crates themselves have a failure, we get an error there too
• Now we know the total number of affected crates.

If we just make a hard-error, and there is some root dependency, we don't know whether things that rely on it contain errors or not.

Similar concerns apply to lint improvements: many crates use #[deny] locally. If we make changes to a lint, this can cause them to fail to build. This mostly doesn't matter because of lint capping. But if we started using that to skip building their dependencies, then we get "no data" for things that depend on them.