man-group / pkglib

Company-centric Python packaging and testing library
39 stars 22 forks source link

pyinstall shouldn't crash if we've already backed out a dependency. #30

Closed Wilfred closed 10 years ago

Wilfred commented 10 years ago

I think this occurs for the following reason:

Consider a graph of the form:

  D   # B and C both depend on D
 / \
B   C # A depends on B and C
 \ /
  A   # We want A

If we back out B then we back out D too. Backing out C then throws a KeyError, since self.best on our Resolver no longer contains D.

In any case, I've seen the KeyError, this is a fix, and I believe the change is legitimate.

@eeaston any feedback or insight would be warmly welcomed :)

eeaston commented 10 years ago

Hi Wilfred, yes, this looks like a bug in the backout code. That particular path was one of the most recent set of changes and I need to find a good way to test all these cases other than by catching issues in the wild. This fix is fine, merging.