Fix for KeyNotFoundException when a package dependency does not appear in the list of libraries taken from the lock file #26
Conversation
…r in the list of libraries taken from the lock file
| // Not all dependencies are necessarily included in the list of libraries | ||
| // for the current target framework and runtime identifier. Add these to libraryNodes | ||
| // so we can still record these dependencies. | ||
| foreach (var dependency in libraries.SelectMany(library => library.Dependencies)) |
There was a problem hiding this comment.
Iterating over all the libraries, and it's dependencies here has some perf penalties. I mean we have just exited a foreach loop where we just iterated over all libraries. Can we somehow combine this with the above loop?
There was a problem hiding this comment.
Yeah, absolutely. I thought it looked cleaner as a separate step, but I can move it up into the first loop if you prefer that.
| // so we can still record these dependencies. | ||
| foreach (var dependency in libraries.SelectMany(library => library.Dependencies)) | ||
| { | ||
| libraryNodes.TryAdd(dependency.Id, new PackageReferenceNode(dependency.Id, "")); |
There was a problem hiding this comment.
Can we get the version of the dependency? Add empty string as version is not optimal
There was a problem hiding this comment.
So, that one's a bit tricky. All we have here is a PackageDependency object. This does not have a single version, but a version range (i.e. "at least 2.2.0").
That's not a problem by itself, but it does raise the question of how to handle cases where A and B both depend on C, but they have different version ranges.
You'd have to go through all of the libraries that appear only as dependencies, combine their version ranges, and only then add them to libraryNodes. It's an extra step and it would mean not being able to do everything in one iteration as you requested in the other comment. It's a bit more complicated and it doesn't really add anything useful for the user. "Which libraries depend on this node and with what version ranges" can already be answered by looking at the "Reverse Depends" window, after all.
|
Hi! @TomTondeur thanks for this PR. I'd like to discuss some of the changes before merging this. Minimal repro would be nice, but I understand if it's not possible. If not, maybe it's possible to share the |
…roper' nodes overriding the nodes generated by dependencies
|
Thank you for you contribution @TomTondeur! Your changes have been merged. |
|
Thanks for getting the fix into master, @bjorkstromm ! |
Hi, I recently discovered this tool and it seems really helpful, especially for the kind of projects I have to deal with at work which have very big dependency trees that are constantly being updated.
Unfortunately, when trying this tool on some of those projects, I've been getting a KeyNotFoundException on line 307. It seems that -for some reason- some of the dependencies of my packages are not listed in lock file, except as dependencies. I haven't been able to find an explanation for why that is, I only really have surgace-level knowledge of project.assets.json contents. I'm getting this with various packages (eg: Nerdbank.GitVersioning, NETStandard.Library) but have been unable to put together a minimal repro.
Regardless, my project files build and run just fine at the moment, so I assume this must be valid and should be a situation this tool ought to handle.