Log file Viewer
#pypa-dev logs for Tuesday the 24th of March, 2015
[01:20:48] <lifeless> dstufft: the logger stuff - I am confused, I preserved the logic exactly AFAICt
[01:21:10] <dstufft> I'd say just let it use the HTTP cache, for any properly setup server all HTTP access will be cached for 10 minutes for the index pages, and a long ass time for the fiels themselves
[01:21:36] <dstufft> lifeless: I sort out went around in circles there as I better understood what the changes were
[01:22:09] <dstufft> lifeless: I think right now I'd just do what I said in the last comment, add the new line and modify things so that the skip method returns just a boolean and move the skip reason to just be hardcoded into that string
[01:26:17] <dstufft> ok, then I'd go back to my second to last comment :), it might not be techincally possible for the skip_reason to ever be None if satisfied_by is not None, but explicitly declaring it to None before the conditonal I feel like will cause people to get confused less since it *looks* like it might be a bug right now
[01:26:47] <dstufft> you can omit the part about if skip_reason is not None and req_to_install.satisfied_by: if you think it's better to just be if req_to_install.satisfied_by: I'm OK either way on that
[01:28:36] <lifeless> we can't move the skip_skip_installed in the guard since satisfied_by isn't set.
[01:31:24] <dstufft> the thing that feels like a bug to me, is that it's theortically possible for the logger call that uses skip_reason to run without the skip_reason being declared
[01:33:59] <lifeless> so my concern is this: we haven't considered what should be output if its satisfied and we haven't run the check_skip_installed code
[01:34:21] <lifeless> the reason we haven't considered that is because the code isn't setup to ever encounter that
[01:34:53] <lifeless> but - your code base, and I fundamentally have no long term self-interest here (other than the easier it is for you to maintain, the better it can be maintained :))
[01:37:08] <dstufft> https://bpaste.net/show/e8bc65d86bc1 I think would achieve a similar and is more obvious to me about the particular thing that confused me.
[01:37:11] <lifeless> one advantage of an assert is that if someone adds code to set satisfied_by earlier
[01:38:44] <dstufft> both makes sense to me, because both states are things we assume won't happen yea?
[01:41:35] <dstufft> cool, so then i'd add both assert statements, and drop the changes to PackageFinder and rely on the HTTP cache (for now anyways, I'll probably go in and change the return values and add a lru cache to the call unless you do it first, but if not it's ok)
[01:47:35] <dstufft> lifeless: fwiw, you've made some solid improvements to one of the crappier parts of pip's code base, So thanks for that ;)
[01:50:21] <lifeless> https://github.com/pypa/pip/pull/2587/files#diff-abf8962da0a79184dd2c4bf311fc9b57R348
[01:53:18] <lifeless> I haven't tested but I suspect just dropping that catch (or making it log an error straight up) would be reasonable
[01:57:09] <dstufft> I think the reason that exists, is so if you have something installed that isn't available on PyPI (you installed it from an sdist or something), which satisifies your current requirements
[01:58:19] <lifeless> 14:07 < lifeless> but it will cause a silent failure for the no-install case, because we will be missing one or
[01:59:02] <dstufft> In the current code, th eonly time that'll be re-raised is if we're *not* skipping the install, but if we're skipping because whatever is installed already satisifies the requirement we don't want to fail just because the thing isn't available on PyPI
[02:00:08] <dstufft> pip install ./thingthatisntonpypi-1.0.tar.gz && pip install thingthatisnotonpypi
[02:00:52] <dstufft> pip install ./thingthatisntonpypi-1.0.tar.gz && pip install --upgrade thingthatisnotonpypi
[02:02:10] <dstufft> I *think* that with the current code, that won't fail (it'll just silently not find anything on PyPI), but if you did something like pip install -U thingthatisnotonpypi>=2.0, it would fail then
[02:08:14] <dstufft> I think we want to preserve that property (assuming my thoughts on why that is like that are correct)
[02:10:07] <dstufft> the thing that is installed gets a version, we just can't sanely locate new versions for it, but we can determine if what is already installed matches the given specifier
[02:36:40] <dstufft> lifeless: are all the changes in https://github.com/pypa/pip/pull/2587 ready to go? I did a quick skim and it looks good to me still, tests are passing so unless you plan on adding more to it i'm going to merge
[02:40:04] <dstufft> gonna watch some TV, if you need anything just ping me and I should hear it (unless I fell asleep -.-)
[07:07:08] <lifeless> PYTHONPATH=. pip install tests/data/packages/SetupRequires -f tests/data/packages
[14:40:06] <sigmavirus24> qwcode: I believe it was discussed on distutils-sig and deemed not a terrible diea
[14:41:52] <dstufft> qwcode: the general idea of doing it for setup-requires was discussed, I'm still on the fence about it but leaning towards "It's probably OK and will solve some problems people have at the expense of creating some other problelms", so people using it will need to make a decision about trade offs
[14:42:14] <dstufft> as far as pulling install_requires from setup.cfg as well, that wasn't fully discussed, lifeless and myself discussed it here in IRC some
[14:43:15] <dstufft> the underlying problem is you have to do one of two things, either read install_requires from setup.cfg and not execute setup.py egg_info or you have to install the setup_requires stuff to a temporary location, add that temporary location to the path and then execute the setup.py egg_info
[14:43:34] <dstufft> because we need to collect all of the dependencies before we actually install anything into the target environment
[14:44:19] <qwcode> I would guess the rationale for doing this w/o a PEP is that this is in the terrain of "project implementation", i.e. not interfering with the "internal metadata" format?
[14:44:20] <dstufft> I believe lifeless picked the "read install_requires from setup.cfg" to implement and was going to leave it like that for discussion to see how people felt about that, or if they think it should go the other way
[14:47:45] <qwcode> it's just surprising to me that the d2 effort went on for years trying to get stuff into setup.cfg (officially with PEPs)... and for the last year, it's been about PEP426... and now this could just shuffled in like this?
[14:50:04] <qwcode> I guess for me, it's about being more clear on what's the terrain of PEP426, and what's the terrain of the tools that implement it... this feels ad hoc to me
[14:51:31] <dstufft> PEP426 metadata is all about what exists inside of a distribution could be an sdist or a Wheel
[14:51:38] <ionelmc> dstufft: just to understand this correctly, the PR basically solves the problem of people being too lazy to do lazy imports in setup.py yes?
[14:52:14] <dstufft> but there are cases where you're *not* installing from a distribution, you could be installing from a directory, or installing from a VCS repo or what have you
[14:52:39] <dstufft> PEP 426 doesn't handle those cases at all, because there's no distribution so there's no PEP 426 metadata
[14:52:47] <qwcode> but this creates a whole new way of declaring "install_requires" using setup.cfg.... that's really my concern.... we're creating a big fork in how projects will be configured
[14:53:01] <sigmavirus24> ionelmc: setup_requires only uses easy_install for installation because setuptools handles it, which means that configuring alternative indices is problematic (if I understood the changes correct)
[14:53:52] <lifeless> ionelmc: lazy imports are not always possible - try making version number look up properly lazy - its freakishly unreliable
[14:54:24] <lifeless> ionelmc: secondly, there's a bunch of features easy_install doesn't have, such as using pip's download cache, https validation, additional indices etc
[14:54:43] <dstufft> qwcode: the install_requires thing is just a decision, the change is primarily focused on setup_requires, afaik lifeless just picked to also include install_requires because it was easier than installing into a temp location. We can decide that we don't want to support install_requires from setup.cfg while still keeping the important part of this change (that setup_requires comes from setup.cfg), it'll just be more effort to first install the
[14:55:02] <lifeless> ionelmc: thirdly distros like Debian, Redhat etc really really don't like packages that go to the internet at build time
[14:55:42] <lifeless> ionelmc: so they generally run builds without internet connectivity, and when setup_requires is triggered, that fails, then they add the new requirements
[14:57:21] <dstufft> 1) Because they don't get installed until after the setup() function starts to execute, you have to do things to delay imports that rely on them, this is annoying (and sometimes hard) 2) setup_requires are handled by setuptools not by pip, so you get two different things touching the network (with two different sets of configuration) which leads to confusion and problems (setuptools doesn't support a configurable certificate bundle I believe,
[14:57:53] <ionelmc> would the PR make existing usage of setup_requires use pip's installer/cache/etc ?
[14:58:48] <dstufft> this solves both of those problems for projects which opt in to the solution by having pip locate the setup_requires from a setup.cfg (and then either execute setup.py egg_info once you install those toa temp location, or also look up install_requires from there, depending on what way is decided)
[14:59:27] <sigmavirus24> ionelmc: "would the PR make existing usage of setup_requires use pip's installer/cache/etc ?" that's not plausible
[14:59:51] <sigmavirus24> As we've already explained, setuptools uses easy_install for anything in setup_requires.
[15:00:00] <dstufft> It's possible to do that, but it requires monkeypatching setuptools being run inside of a subprocess from outside the subprocess
[15:01:57] <qwcode> let's at least call them "setup-requires" and "install-requires" (not "requires-setup" and "requires-dist")
[15:02:23] <sigmavirus24> qwcode: I have 0 opinions on the color of that yak's hair so we should do whatever people agree makes the most sense
[15:02:46] <dstufft> qwcode: so ignoring the install_requires bit for right now (because that was just an implementation choice, not a key part of the thing being solved), the idea here is that package authors can choose to use setup.cfg to declare their setup_requires which means that (unless they include some backwards compat code inside of setup.py) their thing can only be installed by pip (possibly easy_install too if a patch is added there) but not by
[15:04:18] <dstufft> what you get from that is you don't have to do tricks to delay imports, and pip controls the installation of the setup-requires
[15:04:20] <lifeless> qwcode: e.g. if the reviewer consensus is that nicer keys matter, fine. Or compat on the d2to1 key but nicer on the new one, etc.
[15:05:26] <lifeless> qwcode: why do we care? ecosystem impact - folk have used d2to1 (e.g. all the pbr users out there are basically using chunks of d2to1)
[15:06:02] <dstufft> lifeless: otoh pbr users don't use requires-dist do they? They're using requirements.txt yea?
[15:06:07] <lifeless> qwcode: essentially I've just been conservative in the choice, and am totally happy to do $whatever as far as that is concerned
[15:06:10] <qwcode> for me "requires-setup" is boggling cause it reverses the causality... and "requires-dist" just creates something that you have to mentally associate with "install_requires", and worse, it almost looks like it might be from PEP426... and we don't want that confusion
[15:07:44] <dstufft> ionelmc: the key thing is that you can stick your setup_requires inside of a setup.cfg and pip will ensure they are installed before your setup.py is executed
[15:08:14] <dstufft> it also currently forces you to put your install_requires in setup.cfg if you're putting your setup_requires in setup.cfg
[15:08:49] <dstufft> qwcode doesn't seem to be a fan of the not using egg_info to get the install_requires and instead reading them from setup.cfg
[15:09:23] <dstufft> ionelmc: it _only_ changes if the package author opts in by adding their setup_requires to their setup.cfg
[15:09:49] <dstufft> any package that uses setup_requires in their setup.py which doesn't specify them in setup.cfg will still behave as it does today
[15:10:18] <lifeless> the key thing is this. Right now there is *no* way to avoid easy_install. There is /nothing/ that can be done.
[15:10:24] <qwcode> thinking about the fork that will get created (i.e. supporting install_requires in setup.cfg), and when that will be a pain down the road...
[15:10:49] <dstufft> setup.py egg_info is the command pip runs to get the dependencies from a sdist (right now anyways)
[15:11:11] <dstufft> lifeless: two ways to specify install_requires, both inside of setup.py and inside of setup.cfg
[15:11:15] <lifeless> dstufft: btw we can probably avoid egg_info in sdists too and isolate that to just VCS checkouts
[15:11:31] <ionelmc> does the PR use egg_info to get the setup_requires from setup.cfg? or does it read it directly?
[15:11:39] <lifeless> dstufft: right, but if this is followed up with a patch to setuptools to allow it to read them from setup.cfg too...
[15:12:01] <lifeless> dstufft: (or even if this PR becomes 'depend on a new setuptools that does that...')
[15:12:10] <qwcode> lifeless, not pip vs setuptools fork, just that some projects using install_requires from setup, others using from setup.cfg....
[15:12:25] <sigmavirus24> ionelmc: "ok, cause sigmavirus24 confused me, he said that doesn't change" what did I say didn't change?
[15:15:00] <ionelmc> dstufft: what's this `dist requires`? what's the difference to the setup requires?
[15:15:06] <dstufft> I personally feel pretty OK about the part of this change where it reads setup_requires from setup.cfg. I'm less sure about the part where it reads install_requires from setup.cfg too
[15:16:20] <dstufft> ionelmc: requires-dist == install_requires, it's runtime dependencies via setup.py invocation dependencies
[15:16:41] <ionelmc> dstufft: does the PR use egg_info to get the setup_requires from setup.cfg? or does it read it directly?
[15:17:01] <qwcode> lifeless, this installs "requires-setup" into the install environment... doesn't setup_requires currently just drop dists in the cwd?
[15:17:09] <lifeless> ionelmc: because you can't create egg_info if your setup_requires are not present.
[15:17:15] <dstufft> you can't use egg_info to get the setup_requires, because you can't invoke setup.py egg_info without first installing setup_requires
[15:17:38] <ionelmc> should it use a different name for it tho? it creates confusion (you'd expect different behavior for setup_requires from setup.py)
[15:18:05] <lifeless> qwcode: it does. My reasoning for that was as follows: this is new, we can choose to be better, and folk that need no-pollution-of-runtime-etc should be using the build-wheels-and-install-them-path anyway
[15:18:22] <ionelmc> like, as ridiculous as it sounds, `pip_requires` - cause only pip supports this thing
[15:18:46] <dstufft> qwcode: How do you feel about the concept of reading setup_requires from setup.cfg? (ignoring all the other parts of this for right now)
[15:18:55] <ionelmc> as there's no clear standard for this, maybe it should be marked as pip-specific
[15:20:45] <qwcode> dstufft, ok I guess. it's certainly easier than 3 executions. I'd really like to hear jaraco get in on this, since we'd ideally like setuptools to adopt as well
[15:22:34] <qwcode> dstufft, and feeling more ok with install-requires support too I think. but I'd really want to be clear for everyone in the docs how this is not PEP-free cowboy changes.... i.e. how it would all be ok with PEP426
[15:23:26] <dstufft> 1) when invoking easy_install, this could work the same way that pip works. easy_install reads from setup.cfg before invoking the setup.py and people can do non-lazy imports at the module scope of setup.py
[15:23:49] <dstufft> 2) When invoking ``setup.py <anything>`` without the setup_requires already installed, this can't work unless you do lazy imports
[15:28:38] <qwcode> it's a possible slippery slope to support more things in this new "metadata" stanza...and then it starts getting confused with PEP426's metadata file... I understand the difference, but still that concerns me a bit
[15:29:55] <lifeless> dstufft: pushed a few changes to the PR, if you're reading it. docs and a little code style, nothing conceptually different.
[20:07:46] <lifeless> dstufft: I probably relatedly need to implement the topological sort thing for deps, which is what I was going to ask you yesterday, but I realised I could hack around it with the simple fix for ordering.
[20:46:04] <lifeless> dstufft: I think I'll need to implement that as a dep for this PR, because I've thought of a case I can make this break.
[20:47:09] <lifeless> if we switc the order we process deps, then that would work but a two-level dep would still break
[20:47:40] <dstufft> unless the PR does the does the "install into temp location" thing which would work then too, but a topo sort is a good thing anyways
[20:49:25] <dstufft> lifeless: yea i see it, didn't get a chance to read it other than a skim real quick
[20:50:52] <lifeless> dstufft: and start a targeted thread on the d2to1 compat question on the list.
[20:51:43] <dstufft> jaraco is often in here, though not atm, I pinged him on the issue, he's also on pypa-dev
[20:51:45] <lifeless> dstufft: [in case its not obvious I'm entirely happy to write up the needed setuptools patch]
[22:50:30] <lifeless> I'll wrap this topological sort up after dropping C and Lynne off at the airport
[22:50:46] <lifeless> I'm doing it on top of my setup_requires thing because that makes testing it a lot easier :)
[22:51:54] <lifeless> thoughts on moving this forward solicted though, since the code is written now ... :)