Report over- and under-constrained projects from the Result

[fabulous-gopher]

From @sdboyer on June 29, 2016 14:11

In Masterminds/glide#485, we discussed the importance of informing the user when they might want to take some corrective actions in their manifest. The original impetus came from rkt/rkt#2735, where they were discussing their desire to name all dependencies, transitive and not, in their manifest - and so, wanting something to compare between lock and manifest.

I laid out my view on how that approach might be harmful. @lucab seemed content with my suggestion in Masterminds/glide#485 that, rather than generating warnings when there is anything in the lock that's not in the manifest, we instead focus on just two cases:

• Deps that enter the depgraph unconstrained, or with only a "preference" from e.g. godep, a la Glide does not respect glide.lock for transitive dependencies Masterminds/glide#479, JSON marshalers for manifest and lock #16 - so that the user can add a constraint to their manifest.
• Constraints expressed in a [root] manifest on transitive dependencies, where another project with that direct dependency also expresses a constraint - so that the user can maybe drop that constraint from their manifest.

The former warning notifies the user when there's a dep flapping in the wind unconstrained, over which they might want to assert some control. The latter deals with the opposite - the user has expressed a constraint that may be redundant with what's already in the depgraph, and could consider dropping it.

There's differences in these cases. The former tends to arise from an oversight, laziness, or a new dep falling in the cracks with tooling. Corrective action is usually called for, so the warning is usually useful. Pretty straightforward.

With the latter, however, it's impossible for the solver to disambiguate between:

• The user quite intentionally providing a further-narrowing constraint than constraints expressed by actual deps, perhaps because they've done some testing and KNOW that there's some versions they want to see excluded
• The user unintentionally leaving a constraint around (maybe because tooling generated it, or because it was a good idea at one time...) that further narrows a constraint expressed in the depgraph. This might happen if:
  • Some overly-cautious tooling generated it
  • The constraint was the result of a conscious analysis at one time, but those issues have gone away
  • The constraint was put in b/c a transitive dep WAS unconstrained, but a newer version of its parent dep later added a constraint

All of this is even further complicated by the fact that, depending on which versions of deps ultimately get selected, the relationship of the root constraint to the depgraph's constraint (or lack thereof) may be different.

Given that the solver can't reasonably differentiate between these sorts of ambiguities, it suggests that wording around this class of "warning" should be comparably couched.

I don't think implementing this should require changes in the solving algorithm. Instead, we can probably do it with some extra methods on Result. The real result returned from a solve run still has access to the finished solver state, so it can interrogate its solver.selection property to find and report cases that fall into either of these scenarios. This is an especially big benefit, given that emitting warnings from within the solver requires a refactor to take some kind of injected logger, separate from the trace logger.

Copied from original issue: sdboyer/gps#46

[fabulous-gopher]

From @sdboyer on June 29, 2016 14:19

@davecheney and @kardianos - wanted to give you guys a quick heads-up about this discussion. Not to troll, chest-beat, start a fight, reopen old wounds, or anything like that - so please please, do feel free to ignore if this just seems annoying.

Just, I know that the basic topic of "should the manifest have direct and transitive deps, or just direct deps?" is an area we've disagreed on in the past. This issue deals a little more concretely with that, so I thought y'all might find it interesting.