-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Declarative setuptools dependencies. #2603
Conversation
<http://docs.python.org/2/install/index.html#distutils-configuration-files>`_ to | ||
manage the fulfillment. | ||
|
||
For example, to have the dependency located at an alternate index, add this: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add to which file? I would just mention setup.cfg here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This prose was just moved. Its not setup.cfg specific - e.g. that can go into ~/.pydistutils.cfg. The link given in the prose lists the files it can be put into.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have rearranged things more to try and make the section better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be clear, these docs were already there (I wrote them 2 years ago I think?). this PR just moved them.
I have not reviewed the code, but I am violently in support of the idea. |
|
||
def setup_requires(self): | ||
try: | ||
setup_requires = self._cfg.get('metadata', 'requires-setup') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why reverse the name? why not just "setup-requires" as the key? pretty confusing to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @qwcode that setup-requires
is a better name for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As per IRC, I followed the d2to1 convention (d2to1 defined requires-dist as being the install_requires line to setup()). I don't like it and once we've got consensus on what it should be I will happily change it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to repeat what I said in IRC, I think:
"requires-setup" --> "setup-requires"
"requires-dist" --> "install-requires"
"requires-setup" is confusing cause it reverses the causality (i.e. setup requires these things, not the reverse)... 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
this is quite surprising. this would have pip supporting a setup.cfg version of "install_requires" as well couldn't this have been implemented differently so that egg_info could be run? |
We discussed on IRC - egg_info can't be run without creating a temporary execution environment and then running it. I felt that that was very wasteful of peoples time, and since we've got this whole declarative metadata language in PEP-426, which has support in pip since 6.0 and so on - we should be evolving towards that. Note that since this is opt-in there is no compatibility pressure on folk to adopt it (which is good, since it lets authors choose their timeline). |
There are a few open questions I think here:
These two open questions are somewhat tied together, if we're not doing (2), then we have to install the For me personally, I'm on board with having pip read I worry that we're going to get into a situation where people only list their I also worry with (2) that it's a change in behavior that I'm not sure people will expect, It's certainly not the thing I would expect. That's not that I think one method or another is more "correct", just that it goes against my expectation of what would happen. I also worry that it will make it more likely that people will forget to include things in |
I'm not really following this discussion, but with regard to @dstufft's (2) it does sound like a concern. My issue is that it seems as if something in Cases where setup.py needs something like numpy for the build are relatively benign, as numpy is likely needed for runtime as well. But a setup.py that needs a build tool such as cython at build time, which ends up installing cython into the target environment, is probably more of an issue as other packages may well detect if cython is present as part of setup, and if it is then regenerate the C files, but if not then use pregenerated ones. So there's a change in behaviour depending on the order things are installed in (even over an extended period). I don't know of any actual issues that would be caused, but the behaviour seems fragile. |
@dstufft yes, those are the two questions :). I agree that we need buy-in from setuptools. The point of this PR was to be minimal - to demonstrate that it is feasible, in a clean way, to support a declarative setup-requires, and to learn - e.g. we learnt that install_requires + extra_requires also needed to be declarative [or incur a high implementation and user experience cost]. We have basically four choices vis-a-vis install-requires detection that I can think of. a) put it in setup.cfg For this PR I did (a) because I think its the best overall. Lets break it down (enumerated for easier discussion). Firstly, (a) is very straightforward to interact with and reason about: we don't need any new commands or UI to manage the setup dependencies - that is, if package A setup-requires B>1.5 and package C setup-requires B>2 and D install-requires B==2.0, we can use our existing UI and conflict logic rather than having a new UI to address the setup-only bits. This new UI and conflict logic will apply to all of the b, c and d implementation strategies. Secondly, (a) is less work both to code up, and to execute for users than any of b, c or d: we only need one subprocess invocation (install) rather than 3 (one with setup neutered to get setup_requires, one to get egg_info actually built, install), and we only need to install dependencies that are common to setup and runtime once. Caching per pip-call (b) or per target environment (c,d) would mitigate the overhead of installation a little, but not entirely. Thirdly, (a) is useful for Linux distros, where being able to determine the dependencies ahead of time without building any packages is essential to the isolated build process used to build RPMs and DEBs. b/c/d all depend on building and running the build time dependencies, so it won't be any better for those members of the ecosystem, whereas something introspectable can at least in principle be better. Four, (a) is different to existing setup.py's in a couple of ways. Firstly, this won't handle existing setup_requires using projects. They woud need to opt-in. Secondly, it duplicates data already passed to setup(), and unless we teach setuptools to read setup.cfg, we'll end up with folk that do opt-in to the declarative format that this PR enables, having to also maintain their dependencies in setup() [otherwise they won't get exported correctly to wheels and eggs]. Thus, (a) is not as attractive as b/c/d. OTOH if we do modify setuptools, then there is no friction, its just a simple step-forward for users that opt-in. Five, (a) isn't turing complete, and there may well be turing-complete setup_requires calculations out there that can't be expressed by PEP-426 environment markers etc. Since we need to figure those things out anyway, that doesn't concern me: those projects would result in 'I can't migrate' bugs, and we can fix those. Six, on the question of downsides to installing setup-requires in the target environment. The example of cython is a common one ;). One downside folk are concerned about is production systems where not having compilers like cython present is important. However, for that - we have a better answer than the old --no-install one (which is deprecated), which is 'pip wheel' + install from wheels. That will also neatly solve the I've-just-got-something-locally case: if you're installing from source, expect source dependencies present. If you're not then you should be installing binaries :). Seven, one of the things that frustrated me in the past with setuptools PYTHONPATH hackery for setup_requires is that debugging setup_requires becomes that little bit harder. Getting a Pythons shell with the same dependencies that will be loaded by setup_requires is convoluted. (no, pdb in the top of setup.py doesn't get you that). (a) or (c) would make this substantially easier. |
I like the idea of |
FYI bdist_wheel reads from requires.txt inside .egg-info; as long as setup.py egg_info keeps writing to requires.txt bdist_wheel will work. It will also read install requirements from its own setup.cfg [metadata] section but this is mostly deprecated. It doesn't touch setup_requires and it doesn't need to b/c there is no setup phase in wheel. |
There is evidence from users in the discussion on #2478 that b/c/d which double-install things like numpy is considered undesirable. |
Oh, and another note to include is that PEP-426 is part of a larger initiative to get us to a declarative metadata nirvana, and I think (a) is very much aligned with that, being entirely declarative. |
b58703f
to
493911e
Compare
@rbtcollins that is a very good point. Having this information in a declarative form is clearly in line with PEP 426. However, PEP 426 doesn't say anything about what you do with the information. The issue I have is that by installing the install-requires dependencies into the target location, you're not actually doing what the user requested, which is to install packages A, B, C and any runtime dependencies they may declare. Build and install time dependencies are needed by pip to do the requested install, but the user didn't ask for them, and it's bad form to dump stuff the user didn't ask for into the target location. At least that's the concern I have here. It may be that it's a completely separate problem that already exists. I don't have much experience with the existing setup_requires machinery, so it may well be that we already install setup_requires dependencies into the target. If so, then I've misunderstood the issue and I apologise. Maybe it should be raised as a separate issue in that case, though. |
I was originally onboard with the simplicity and straightforwardness of this approach. Indeed, in isolation, I think it's the right thing to do. However, it is a fairly big behavior change that for people who are not related to the people packaging the software to introduce installation of setup_requires. With putting them into setup.cfg, it's opt-in for packagers and we can socialize it over time - but a packager opting in won't change behavior for consumers. So it seems to me we should go with (b) - which is the smallest amount of change and solves the biggest systemic problem right now. And then we should circle back around once that's solid and try to sell folks more broadly on (a) |
Actually, I'm confused. Why can't we put the setup dependencies in setup.cfg (i.e.(a)) and yet still sandbox the packages that are being installed just to support the setup/build step? I'm very much in favour of moving to a system where dependencies are specified declaratively. It seems to me that it should be possible to do this without changing how the process of acquiring those dependencies and using them to allow the build to proceed works. But as I say, I'm no expert so if there are good reasons why that's impossible, it would be good if someone could explain them. |
What I had in mind with the wheel-caching (inspired from @dstufft idea, if I'm not mistaken)
|
@pfmoore that's basically what (b) is, put the setup-requires into That does mean possibly building the same thing twice, but that's where we are currently, except now we have control over the process. In particular that could mean that we can make more improvements in additional PRs down the road to reduce the number of times we need to build something if it appears in both |
Ah, OK thanks. |
Well that decision influences the way this is implemented even if we install into a temporary location, because it controls whether or not we have to execute As I said above, I'm OK with that part as long as setuptools implements it too (/cc @jaraco), so I'd be interested to hear any reservations you (or anyone else) has about it.
As implemented, if you declare any of your dependencies inside of
Yea, I actually saw it before I posted :) I just had already typed it all up and didn't feel like editing it. |
I'm basically in agreement with everything @dstufft said. As regards installation of setup requirements:
So, in summary:
|
I'm not against "install_requires" existing in setup.cfg (or other things), but just don't think it should happen as an implementation side-effect like this. This is a pretty complicated PR for a niche issue that I imagine many people are not following that might have an opinion or questions. I don't see how we can conclude people are generally ok with it based on discussion in this PR, or the "setup_requires for dev environments" thread on distutils-sig I imagine if we opened a general "let's do static metadata in setup.cfg" discussion on pypa-dev or distutils-sig, people would have questions related to:
|
what about the concern @rbtcollins raised about needing a UI change to manage conflict overrides for setup-requires. In the plan where setup-requires are installed into the install environment, you get that for free using the current UI. So what to do? don't support overrides for setup-requires? |
Conflicts with what? Install it with |
That leads to one set of temporary installs setup_requires. Which yes will work today but doesn't do anything to get us out of the performance hell. Which when I look around at the various forums and folk whinging about setup_requires are ~50% of the headache folk have. As a short term strategy, sure, but given that most setup_requires are also runtime requires, I still thing we need to be more aggressive if we wish to get to a better situation. I appreciate the conceptual clarity of 'setup_requires don't propogate during install', and am now heading down that path, but we can write the most beautiful and careful code and still end up with something folk don't like or want. Doing a bounce via wheels will mitigate the performance headache somewhat, which is what I'm currently hacking on. (before anything else I need to fix wheel thing-with-setup-requires-in-setup.cfg to work, and that will form the basis for making install have an implicit wheel invocation before it operates). |
conflicts within the setup-requires dep chain. see #2603 (comment) he said: "Whats not easy is allowing the equivalent of the current 'foo bar baz==x' UI where when foo and bar have bad dependency interactions you can override it from the top level. So if foo's setup_requires have mutual bad interactions, how do you fix it?" |
this was his first point in the big 7-point analysis #2603 (comment) |
How are conflicts within the dep chain not already a problem with |
I mean fundamentally installing into a temp location is basically what setuptools is doing now, egg vs flat shouldn't matter because either way you can only have 1 version loaded into memory. |
You do realize that when you are installing the setup_requires, each of On Fri, Mar 27, 2015, at 06:42 AM, Donald Stufft wrote:
Links: |
Which transitive requirements? The outcome should essentially be the same as if you did |
I think they are, but since pip would now be handling it, I think the idea is that they are worthy of the same "resolution ui" that install_requires has, and since the a) plan gave it for free, it was mentioned as an advantage to that plan. |
where I'm at right now
|
It may have been missed as it was a small part of another comment I made, but isn't it true that installing setup_requires stuff into the installation location raises a question of what actually is the installation location? For The same questions apply to To my mind, this makes "temporary location which we add to sys.path" the only viable option, as it's the only one that actually works in all cases. |
I haven't had a chance to fully absorb this issue, but I have read through the history and I have some comments and concerns. First, I concur that installing setup_requires into the installation location is a bad idea. There is an explicit contract made by setuptools that such will not happen (ref). Second, this contract means that each package can declare setup_requires dependencies for build/test/install/other commands, but they won't be present and thus won't conflict with other packages. As a result, it's possible to use mutually-exclusive versions to install different packages. Package A might require builder-helper-1.0 and Package B might require the incompatible builder-helper-2.0, and they can currently be built into the same environment and as part of the same install invocation. If requirements are installed into the target environment, that would either fail (due to conflict) or require uninstall/install steps to ensure a compatible version is present. Third, in packages that I maintain that use setup_requires for tests and doc build support, others have suggested that setup_requires should be conditionally required, which is much harder to do in a declarative system. Currently, setup_requires serves several use cases (implementing commands, adding build-time dependencies, etc). I've considered splitting out setup_requires into separate declarations such as build_requires, docs_require, commands_require, etc). A perhaps better approach would be for each of the requirements to have tags indicating at which scope and purpose they're relevant, such as "requires=pbr[command:build], requests, pytest-runner[comand:test], pytest[tests], cython[build], my-utils[setup, build, runtime]". However, what we have now is "setup_requires", which is just a first-order approximation for anything that's required to run setup.py (and thus anything else that relies on that machinery). Fourth, I do see there are some problems with setup_requires dependencies in general. In particular, there's the issue that they aren't present in the metadata (ticketed, but yet unsolved due to historical artifacts and upgrade issues. Additionally, it's been requested that setup_requires supersede installed packages, which is similar but less obtrusive than the issue raised in my second point above. But perhaps the most serious shortcoming is the unticketed behavior that setup_requires aren't re-used between runs. It could be that behavior is by design, but in practice, that leads to a lot of redundant download/install steps for setup_requires requirements. Here's a place where adding support in setuptools to specify where these requirements are to be stored could enable pip to re-use a cache instead of getting a separate one for each package. Regardless, I recommend whatever is done should be done in concert with setuptools instead of in spite of it. Whatever solution is drafted should consider the whole of the ecosystem, including the setuptools implementation and rollout. As always, I invite tickets (for discussion) and pull requests. |
@jaraco thanks for the link; that certainly documents the setuptools contract today; one of the things touched on was the point that this new thing needn't have identical semantics. However, that said - Donald has ruled vis-a-vis this in the pip context, and setup_requires won't be installed into the target environment. I am interested in doing this work in setuptools too. I started out with a thread on distutils-sig, followed up with discussion on #pypa-dev and am following up with changes to pip ... because pip has to change fairly deeply to enable this whereas setuptools merely needs to merge in metadata from setup.cfg into its existing keyword arguments - I expect that to be much more shallow. E.g. AFAICT I am doing this in concert with setuptools. If you have further places I should be engaging in discussion, let me know and I will do so. Do you have any concerns about the proposal as it stands: for clarity the key bits are:
Within PIP, today, I'm not doing a ReqSet per setup-requires case: the code really isn't structured to do that easily. And - I'd like to suggest that as far as pip is concerned, handling the completely isolated approach of setuptools for setup-requires isn't pragmatically needed - when I looked through the setup-requires on pypi there are only 9 '<' clauses in total and 20 == clauses, out of 1800. |
This will need to be rebased (sorry!). |
(I'll likely reopen this when its top of the stack) |
This adds support per the discussion we had on distutils-sig for
declarative requirements in setup.cfg.
Supported are requires-setup (the problem to be solved). requires-dist
(defined by d2to1, and needed as a consequences of supporting setup
requirements, because we can't run egg_info during the
pre-installation phase to detect requires. Similarly extras are
supported, for the same reason.
Neither setup-requires nor extras are defined by d2to1, so we may want
to bikeshed a little over the syntax there, but the basic structure works.