Enhance check_version
#183
Conversation
layday
force-pushed
the
enhance-check-version
branch
from
bfcebb0
to
7af11c6
Compare
layday
force-pushed
the
enhance-check-version
branch
from
39cc802
to
06a282e
Compare
|
I've made a few changes to return the dependency chain similar to how |
| @@ -49,49 +49,47 @@ class TypoWarning(Warning): | |||
| """ | |||
|
|
|||
|
|
|||
| class IncompleteCheckWarning(Warning): | |||
| def check_dependency(req_string, ancestral_req_strings=(), parent_extras=frozenset()): | |||
There was a problem hiding this comment.
I think it would be better to put effort in upstreaming this into pypa/packaging first, as proposed in pypa/packaging#317.
There was a problem hiding this comment.
The impression I got was that they were reluctant to add anything which manipulates distributions. distlib would probably be a good fit but everybody seems to be moving away from it.
There was a problem hiding this comment.
In my experience there's a long cycle with adding stuff to distlib... so if we want it soon we better add it here. We can always remove it if we do manage to upstream it.
There was a problem hiding this comment.
I am in no hurry to merge this as it only affects people using --no-isolation when trying to verify recursive extras (extras that depend on extras), which is mostly distro packagers, for whom this is only a nice to have. I would rather put the effort into merging somewhere this could live long term rather than here. Before merging it here, we should figure out what would it take to put it somewhere else and then make the call.
There was a problem hiding this comment.
I am in no hurry to merge this as it only affects people using
--no-isolation
As a (new) contributor to this project, this sentence on its own would be enough for me to abandon spending any more effort (on this PR).
There was a problem hiding this comment.
as it only affects people ... trying to verify recursive extras
check_version would not verify transitive dependencies at all, whether or not they came from extras.
layday
force-pushed
the
enhance-check-version
branch
5 times, most recently
from
b679c24
to
f084246
Compare
With this change, `Builder.check_dependencies` will verify that transitive dependencies are met, including those from extras, and will return a set of variable-length ancestral and unmet dependency tuples. Closes pypa#25.
layday
force-pushed
the
enhance-check-version
branch
from
f084246
to
10dff15
Compare
There was a problem hiding this comment.
LGTM, let's wait a few days to see if we can hear something from pypa/packaging#317.
|
While we are waiting, would anybody know why the pypy3 tests are failing? |
No, cold you investigate? |
|
It looks like it's flaky - it passed on Ubuntu this time. Appears to be coverage-related, possibly nedbat/coveragepy#1010, judging from this line: I don't know why it'd appear now, there might be some interaction with the |
layday
force-pushed
the
enhance-check-version
branch
from
10dff15
to
7eb12ed
Compare
|
I dropped the commit which enabled branch coverage and that seems to have fixed it (or perhaps not). We can add branch cov in a separate PR. |
|
Should we merge this now? |
|
I specifically asked to wait, please do not merge things like this without getting the ack from everybody that raised concerns. |
|
Well, I merely asked if we should merge it - that is, if it would've been time to merge it. It wasn't an invitation to merge it - sorry if I'd not made myself understood. |
|
The message was directed to Bernát, not you. |
|
Sorry, thought we moved past that. |
No description provided.