pmxbot IRC Log Viewer

[00:22:55] <StevenK> lifeless: Right, I've also been meaning to asking that -- we don't want to call a bare evaluate() ?

[00:31:00] <StevenK> lifeless: Ah ha, so making that change the original test now raises an IndexError, since require() will return []

[01:14:59] <lifeless> StevenK: StevenK yah ?

[01:20:42] <StevenK> lifeless: http://paste.openstack.org/show/494144/ is my current diff

[01:25:09] <lifeless> StevenK: line 35 is odd

[01:25:32] <lifeless> StevenK: see my comment earlier: I don't expect that any user can submit such a marker

[01:25:52] <lifeless> StevenK: I'd rather see something like

[01:26:00] <lifeless> pkg_resources.get_distribution("foo[extra]")

[01:26:05] <lifeless> pkg_resources.get_distribution("foo[extra,otherextra]")

[01:27:28] <lifeless> StevenK: the actual bug being thrown here - even just get_distribution("foo") will fail when the distribution has extras FWICT

[01:27:43] <lifeless> StevenK: the tests for the prior case handled *indirect* access

[01:28:01] <StevenK> lifeless: It will not fail

[01:28:11] <lifeless> StevenK: without this patch I mean

[01:28:33] <StevenK> lifeless: Without that patch it will not fail either

[01:28:54] <lifeless> StevenK: I must be misunderstanding how the failure occurs

[01:29:24] <StevenK> lifeless: I'm in an IBM office with Jamie and Nakato today, so hangouts can't a thing

[01:29:24] <lifeless> StevenK: AIUI pass_markers is being called on a requirement line from the distribution metadata that has an extra guard implemented via markers

[01:30:27] <lifeless> StevenK: there is no *entry* for the distribution in the requires_markers dict, so the get returns (), and we then try the evaluate with no parameters

[01:30:30] <lifeless> -> boom

[01:31:50] <StevenK> lifeless: Yes, but calling it on foo[extra] where it depends on bar;extra=='extra' will parse the metadata, and populate the object correctly

[01:32:08] <StevenK> The problem is calling it on bar;extra=='extra' only

[01:35:23] <lifeless> wow, tox -epy27 - 122 errors

[01:35:34] <StevenK> On setuptools? Yeah, don't do that

[01:35:50] <lifeless> StevenK: how are you testing a single thing ?

[01:36:51] <StevenK> lifeless: I'm not, I have an activated virtualenv with packaging six and mock installed and then run python setup.py test

[01:40:04] <lifeless> !PYTHONPATH=. py.test pkg_resources/tests/test_resources.py

[01:40:06] <lifeless> win

[01:41:54] <lifeless> StevenK: pkg_resources.get_distribution("foo['extra']") <- fails too

[01:42:11] <StevenK> foo['extra'] isn't valid?

[01:42:11] <lifeless> oh, thinko

[01:42:55] <lifeless> yeah

[01:43:00] <lifeless> now I get DistributionNotFound

[01:43:24] <StevenK> Right, since resolve() actually looks for it

[01:44:02] <lifeless> ok so - *why* are we evaluating this requirement with an inline marker ?

[01:44:06] <lifeless> I am confuse

[01:44:37] <lifeless> pip isn't calling get_distribution("foo;extra='extra'") is it ?

[01:44:57] <StevenK> It is!

[01:45:05] <lifeless> wat

[01:45:11] <StevenK> check_if_exsits() calls get_distribution

[01:45:42] <lifeless> but foo;extra='extra' isn't a valid marker for anything *other* that metadata files

[01:47:32] <lifeless> (because, an extra marker is only sane in a context where you can talk about looking for some specific extra)

[01:47:33] <StevenK> I think get_distribution() shouldn't be eval'ing markers

[01:47:49] <lifeless> StevenK: no, it should be

[01:48:14] <lifeless> StevenK: well, I htink it should be

[01:48:21] <lifeless> in the same way it evaluates version limits

[01:49:02] <StevenK> lifeless: So _prepare_file gets called for every requirement, which calls check_if_exists(), which calls into pkg_resources.get_distribution()

[01:49:15] <lifeless> StevenK: right, with you so far

[01:49:29] <StevenK> So foo[extra] -> bar;extra=='extra' works great

[01:49:44] <StevenK> But then it gets to bar;extra=='extra' and blows up

[01:49:44] <lifeless> StevenK: my specific point is that there should be -no way- to end up with str(self.req) -> foo;extra='extra'

[01:50:02] <lifeless> that str(self.req) is 100% broken if we're leaking that up

[01:51:17] <lifeless> StevenK: because of the special nature of extra in markers

[01:58:27] <lifeless> StevenK: remember that extra='something' in a marker means 'If the extra something has been requestsed in the distribution context *above* this one'

[01:59:01] <StevenK> Right, so we need to split out the marker in that case?

[01:59:03] <lifeless> StevenK: its entirely an internal detail used to reassemble the original dicts from setup()

[01:59:24] <lifeless> StevenK: so it should never ever leak out, or be accepted into, pkg_resources description of distributions

[02:00:06] <StevenK> Yes, I'm trying to sketch out in my head how that would work, and failing miserably

[02:01:27] <lifeless> StevenK: so lets focus on the specific issue

[02:01:39] <lifeless> StevenK: *how* is pip ending up with a str(req) that contains an extra marker

[02:03:10] <lifeless> StevenK: looks like self.req is a pkg_resources.Requirement.parse result

[02:04:46] <StevenK> It will be, yes

[02:05:54] <lifeless> actually no

[02:05:57] <lifeless> sometimes its a plain string

[02:06:14] <lifeless> check InstallRequirement.from_line

[02:09:22] <lifeless> StevenK: so since you have pip setup to break etc

[02:09:23] <StevenK> lifeless: req_set , line 339 or so

[02:09:45] <StevenK> discovered_reqs ends up including the extra marker

[02:09:47] <lifeless> StevenK: can you find out what self.req is at

[02:09:48] <lifeless> File "/tmp/pytest-14/test_extras_after_wheel0/workspace/venv/local/lib/python2.7/site-packages/pip-8.2.0.dev0-py2.7.egg/pip/req/req_install.py", line 996, in check_if_exists

[02:09:51] <lifeless> self.satisfied_by = pkg_resources.get_distribution(str(self.req))

[02:11:37] <StevenK> self.req would be bar; extra=='extra'

[02:11:48] <lifeless> StevenK: its an actual string with that value ?

[02:12:12] <StevenK> No, it's a pkg_resources.Requirement with that representation

[02:12:33] <lifeless> __repr__ or __str__ ?

[02:12:40] <StevenK> lifeless: But hold on, let me get my rpdb set up for you and I shall paste

[02:13:09] <lifeless> k

[02:17:10] <StevenK> (Pdb) p self.req

[02:17:10] <StevenK> <Requirement('simple==1.0; extra == "extra"')>

[02:17:20] <StevenK> (Pdb) p type(self.req)

[02:17:21] <StevenK> <class 'pip._vendor.packaging.requirements.Requirement'>

[02:22:49] <lifeless> StevenK: p str(self.req)

[02:26:31] <lifeless> StevenK: ok, so mmmm

[02:27:23] <lifeless> StevenK: I'm going to put a pastebin up with some thoughts

[02:28:34] <lifeless> https://etherpad.openstack.org/p/dist-extras

[02:56:24] <lifeless> StevenK: ^

[03:07:26] <lifeless> StevenK: I need to pop out, farewell get together for the HPers being WFR'd from the local office

[07:21:07] <lifeless> gjjj

[07:25:56] <lifeless> StevenK: back FWIW

[08:28:08] <StevenK> lifeless: I was travelling home, your write-up looks correct

[08:32:08] <lifeless> StevenK: after work for us both; lets pick it up monday.

[08:32:26] <lifeless> StevenK: dstufft might have had time to look at the etherpad too by then

Log file Viewer

Help | Karma | Search:

#pypa-dev logs for Friday the 15th of April, 2016