Change the Normalization algorithm #51
Conversation
This adds a test for a bug we observed in how we rewrite the Normalized graph. The old algorithm had bugs and was more complex. The current algorithm builds up the final graph by starting at the roots with the normalized versions of anything reachable
|
cc @non |
|
@ianoc review? |
|
lgtm |
|
@johnynek This change appears to be causing some problems in my repository -- when I run I'm trying to come up with a small test case to track down the problem. I'll keep you posted. |
|
Okay. That's strange because we explicitly find all the root dependencies and put in all their transitive dependencies, but maybe the root dependency checking is in correct. (we tried this on 1 repo internally and worked as expected). |
|
cc @non |
|
@kevingessner OK, thanks, feel free to pass along a minimized test case would be great, but also feel free to pass along a complete file if that ends up being too much work. |
|
@johnynek It looks like roots that have no dependencies aren't added to the normalized graph. For instance com.maxmind.geoip -- if you add that as the only entry in dependencies.yaml, nothing is generated. |
|
cc @non ^ |
|
yep.... that makes sense. Sorry. We can add a test for this case, and make sure to explicitly addNode, not just addEdge. Or you can take it! :) |
| val dependencies = dependenciesUv.map { uv => | ||
| MavenCoordinate(uv, canonicals(uv)) | ||
| } | ||
| // add the edges from versionedH -> deps |
There was a problem hiding this comment.
here is the bug. We need to addNode(versionedH) no matter what. cc @non @kevingessner.
Would be nice to add a test to repro, then fix. Unfortunately I'm about to get on a plane and won't have time.
There was a problem hiding this comment.
Here's a patch that makes the test fail: https://gist.github.com/kevingessner/ce8981aecb64dd2885d19e4a35fa366a
There was a problem hiding this comment.
I can fix this, I have a repro test now.
This adds a test for a bug we observed in how we
rewrite the Normalized graph. The old algorithm
had bugs and was more complex. The current algorithm
builds up the final graph by starting at the roots
with the normalized versions of anything reachable.