setup_requires and install_requires don't play nicely together #209
Comments
|
Original comment by mordred (Bitbucket: mordred, GitHub: mordred): You'll notice that if you do "pip install ." instead of "python setup.py install" you get the behavior you expect, which is that the depend that setuptools temporarily installs locally for the purposes for fulfilling the setup_requires does not prevent the install_requires from firing properly. I kinda like 2 - except that it actually makes a behavioral change that could have knockon effects. Currently, other than setup_requires, in a pip installation, everything that gets listed in install_requires actually gets installed by pip and not by setuptools. This is important for situations where easy_install behaves poorly, like proxy-blocked places. 1 preseves the separation, but as you said means a double download. I wonder if easy_install could download the egg to a known location and then pass that info along to install_requires invocations so that install_requires could do the installation of the setup_requires downloaded thing. Then, if we wanted to get really clever, we could have that location be settable, so that in a pip-driven install, pip could request of setuptools that it puts downoaded setup_requires artifacts in $DIR, and then pip could consume them. @dstufft does that sound extra crazy? |
|
Original comment by dstufft (Bitbucket: dstufft, GitHub: dstufft): Well 2 won't actually work because Pip can't install from a That being said not deleting the |
|
Original comment by tsmithe (Bitbucket: tsmithe, GitHub: tsmithe): I'd just like to record my experiences here, as I reported to distutils-sig: "I've got an extension that I build with distutils. It requires numpy both to build and to run, so I have numpy in both setup_requires and install_requires. Yet setup.py builds numpy twice -- once for the build stage, and then again on installation. This seems inefficient to me -- why not just build it once?" This seems to be a slightly different problem to the egg issue discussed by Marc, though Donald's proposed caching solution would seem to fix both. |
|
Original comment by msabramo (Bitbucket: msabramo, GitHub: msabramo): @dstufft's cache idea sounds nice to me. But it sounds more like a nicety to keep the current directory looking clean (which would be nice!). I was wondering if an even smaller change to fix the problem at hand is to simply make the egg part of the |
|
Original comment by msabramo (Bitbucket: msabramo, GitHub: msabramo): This is extremely simplistic and thus most probably wrong when considering a library with the history and complexity of --- a/setuptools/dist.py
+++ b/setuptools/dist.py
@@ -3,6 +3,7 @@ __all__ = ['Distribution']
import re
import os
import sys
+import tempfile
import warnings
import distutils.log
import distutils.core
@@ -326,7 +327,7 @@ class Distribution(_Distribution):
links = opts['find_links'][1].split() + links
opts['find_links'] = ('setup', links)
cmd = easy_install(
- dist, args=["x"], install_dir=os.curdir, exclude_scripts=True,
+ dist, args=["x"], install_dir=tempfile.gettempdir(), exclude_scripts=True,
always_copy=False, build_directory=None, editable=False,
upgrade=False, multi_version=True, no_report=True, user=False
)It seems to work for this particular case that I tested: |
|
Original comment by dpursehouse (Bitbucket: dpursehouse, GitHub: dpursehouse): I'm experiencing a similar problem; I'm not sure if it's exactly the same one as described in this issue. I'm installing jenkins-job-builder in a virtualenv, from a requirements.txt file that also specifies pbr: The PIP_INDEX_URL environment variable specifies a server on an internal network, and there is no access to the public internet (i.e. can't get to pypi.python.org). During the installation, pbr is successfully installed in the virtualenv from the internal package server, but then when jjb is downloaded it tries to install pbr again, but from the external index server. |
|
Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco): Not yet. It's been a busy week, and I'm currently concerned about the failing tests in the tip with the new zipimport. Once that's settled, released, and stable, I'll revisit this issue. Feel free to ping periodically if you don't see any activity. |
|
Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco): @msabramo Why does your patch work? I don't understand why installing the package to a temporary directory doesn't trigger the install_requires directive to be satisfied for pbr. Is it because '.' is on the sys.path, and thus setuptools will consider any eggs on sys.path as installed? |
|
Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco): It seems unwise to me to change setuptools to use a global temp directory as a cache simply to address this issue. One concern is the possibility that an untrusted agent could write to that directory in advance, fulfilling the requirement with untrusted code. Another option that's been proposed is to have the eggs created in a specially-named directory, such as "./.egg-cache". That also might address the issue, and would avoid the additional security risk (see #80). There are additional issues too that I would like to see solved, such as the ability for setup_requires requirements to be more strict than system-installed packages (#141). So the proposed patch doesn't address the issue directly and probably has unwanted consequences. I'd prefer a solution that addresses the issue directly by carefully keeping setup_requires and install_requires packages distinct, but I'd also accept a PR that bypasses the issue by fixing #80. |
|
Original comment by msabramo (Bitbucket: msabramo, GitHub: msabramo): See https://bitbucket.org/pypa/setuptools/pull-request/84/put-setup_requires-eggs-in/diff This implements @jaraco's idea using a specially named directory in the current directory ( |
|
Original comment by syllog1sm (Bitbucket: syllog1sm, GitHub: syllog1sm): As a workaround, I've created a separate package to install the headers: https://github.com/syllog1sm/headers_workaround I noticed that Issue #80, which seems to refer to this as well, is Closed: Resolved, but this still seems to be a problem for me. I think this problem probably affects most extension modules, since it's very common to require numpy headers. |
|
Original comment by bignose (Bitbucket: bignose, GitHub: bignose): This is biting us with the ‘python-daemon’ distribution. As of version 2.0, the distribution needs ‘docutils’ available for the ‘setup.py egg_info’ command. According to the Setuptools documentation for the That's not what happens though: |
|
Original comment by bignose (Bitbucket: bignose, GitHub: bignose): It turns out the ‘python-daemon’ setup failure was due to a circular import. The ‘docutils’ library was needed for ‘egg_info’, but was also imported before ‘setup()’ even had an opportunity to act. No dependency declaration could help there. So that was not an instance of the behaviour in this bug report. Sorry for the noise! |
ghost
added
- Use setuptools.setup by default if available for import. - The objective for that complicated import/cython is commented. - Also have more specific variable names, and have their usage be clarified. - Remove Cython from install_requires as it already is declared under setup_requires, and that it conflicts with install_requires due to issues in setuptools. - pypa/setuptools#209 - pypa/setuptools#391 - This commit goes back to breaking installation in environments without Cython, but should be rectified in the next commit.
- Use setuptools.setup by default if available for import. - The objective for that complicated import/cython is commented. - Also have more specific variable names, and have their usage be clarified. - Remove Cython from install_requires as it already is declared under setup_requires, and that it conflicts with install_requires due to issues in setuptools. - pypa/setuptools#209 - pypa/setuptools#391 - This commit goes back to breaking installation in environments without Cython, but should be rectified in the next commit.
#5998) * Change check for Cython to attempt fallback to setuptools on supported platforms. This attempts to solve Issue #5984 without causing more problems for Issue #5558. * Correct argument to extras_require. * Refactor the declaration and usage of Cython - Use setuptools.setup by default if available for import. - The objective for that complicated import/cython is commented. - Also have more specific variable names, and have their usage be clarified. - Remove Cython from install_requires as it already is declared under setup_requires, and that it conflicts with install_requires due to issues in setuptools. - pypa/setuptools#209 - pypa/setuptools#391 - This commit goes back to breaking installation in environments without Cython, but should be rectified in the next commit. * Actually fix the specific usage of Cython - In order for setup.py to make use of the Cython installed during by the setup_requires to be usable, it must be imported after it is installed by that step, thus it cannot be done at the top level and this can be achieved by importing it when it's actually called. A build_ext factory function is created for this. - Still allow whatever Cython already present on the current environment be attempted to be used. This may end up being unnecessary as if it is always declared in setup_requires (which isn't currently the case due to the complicated/documented block), it _should_ pull in the exact/supported version of Cython for usage. This should be investigated so that this complicated logic may be avoided. * Make distutils happy by not using factory function - As it turns out, calling setup.py build_ext will result in a code path within setuptools that will invoke distutils in a way that forces an issubclass check, which ultimately results in an exception due to that function not being a class. - To fix this, we go back to constructing a base class except also using the "new" style class (to make Python 2 happy) so that the __new__ method will actually be called so that the logic to select the Cython version of build_ext will be invoked. - Also use the super class protocol to invoke parent methods. * Declare version bounds directly on setup_requires - This allows us to fully remove the brittle version checks - Also this allows us to directly declare setup_requires if Cython is definitely required, as this would allow the correct version to be installed directly by setuptools during the execution of the setup step. - No need to check for failed Cython imports due to the presence of the setup_requires declaration. * Add comment explaining the initialisation routine for KivyBuildExt. Details of how setuptools deals with both cmdclass and when setup_requires dependencies come in to scope are both relevant. * Bring comment in to line with earlier changes. The cython checks are significantly simpler now, and the rationale is also slightly different.
Originally reported by: msabramo (Bitbucket: msabramo, GitHub: msabramo)
What's the latest thinking on how to deal with a package that has
setup_requires=['foo']andinstall_requires=['foo']?My observation is that this causes setuptools to download a foo.egg file to the current directory, install_requires then decides it doesn't need to install the package, and then the software craps out when it tries to import it because it can't import from the egg :-(
For example, I work on a project that uses pbr. The
setup.pylooks like this:Now starting with a clean virtualenv and no .egg files in the current directory, here's what I see (with pip 1.5.5 and setuptools 3.4.4):
OK, fine. pbr wasn't available in the virtualenv so setuptools downloaded a .egg so that it could use it to do setupy things.
Not so fine is that because the .egg file was already present because it was downloaded in the
setup_requiresphase, the setuptoolsinstall_requireslogic decided that it didn't have to install pbr in the virtualenv (I guess at this point the .egg file is on the PYTHONPATH so it can be imported?). This is not good because code outside of setuptools doesn't know how to do anything with that .egg:So this is not good.
Ideas:
@jaraco: What do you think?
The text was updated successfully, but these errors were encountered: