Open sdboyer opened 8 years ago
@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.
This issue was moved to golang/dep#444
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 coreos/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:
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:
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 realresult
returned from a solve run still has access to the finished solver state, so it can interrogate itssolver.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.