Skip to content
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

Upgrade to 1.8.0-dev1 fails on install_requires #5882

Closed
sivy opened this issue May 30, 2018 · 24 comments
Closed

Upgrade to 1.8.0-dev1 fails on install_requires #5882

sivy opened this issue May 30, 2018 · 24 comments
Assignees
Labels

Comments

@sivy
Copy link

sivy commented May 30, 2018

Trying to upgrade from 1.7.0-rc0 to 1.8.0-dev1:

sivy$ ./pants test src/python/***/tests
New python executable in /Users/sivy/.cache/pants/setup/bootstrap-Darwin-x86_64/pants.9ak2zb/install/bin/python2.7
Also creating executable in /Users/sivy/.cache/pants/setup/bootstrap-Darwin-x86_64/pants.9ak2zb/install/bin/python
Installing pip, wheel...done.
Collecting setuptools==5.4.1
 Downloading https://nexus.***.net/repository/pypi-all/packages/b1/ba/02218786fe9d5beeb2cfdaf0e423219d132845774d703f6e8a708dd5e642/setuptools-5.4.1-py2.py3-none-any.whl (528kB)
   100% |████████████████████████████████| 532kB 1.0MB/s
Installing collected packages: setuptools
Successfully installed setuptools-5.4.1
Collecting pantsbuild.pants==1.8.0-dev1
 Downloading https://nexus.***.net/repository/pypi-all/packages/6f/09/d6a9240b11dbca065b7361fbc4cbff7404d7497b3badc75b4f3a35c3dfdc/pantsbuild.pants-1.8.0.dev1.tar.gz (3.3MB)
   100% |████████████████████████████████| 3.3MB 1.4MB/s
   Complete output from command python setup.py egg_info:
   error in pantsbuild.pants setup command: 'install_requires' must be a string or list of strings containing valid project/version requirement specifiers

   ----------------------------------------
Command "python setup.py egg_info" failed with error code 1 in /private/var/folders/1g/wn5xl4m546v5dxps7jmjbm045zkmqp/T/pip-build-l_1sLT/pantsbuild.pants/
./pants: line 102: /Users/sivy/.cache/pants/setup/bootstrap-Darwin-x86_64/1.8.0-dev1/bin/python: No such file or directory
sivy$
@stuhood
Copy link
Member

stuhood commented May 30, 2018

Are you using the standard pants script from https://www.pantsbuild.org/install.html ?

@sivy
Copy link
Author

sivy commented May 30, 2018

I am.

@sivy
Copy link
Author

sivy commented May 30, 2018

Uncompressed the tar.gz and setup.py does look ok... install_requires is a list of strings. 🤔

@jsirois
Copy link
Contributor

jsirois commented May 30, 2018

Perhaps setuptools being used is too old? We just added an environment marker to subprocess32 and I think environment markers in requirements have not always been around. Cracking open the tarball from pypi and looking at setup.py:

# DO NOT EDIT THIS FILE -- AUTOGENERATED BY PANTS
# Target: PythonLibrary(BuildFileAddress(src/python/pants/BUILD, pants-packaged))

from setuptools import setup

setup(**
{   'classifiers': [   'Intended Audience :: Developers',
                       'License :: OSI Approved :: Apache Software License',
                       'Operating System :: MacOS :: MacOS X',
                       'Operating System :: POSIX :: Linux',
                       'Programming Language :: Python',
                       'Topic :: Software Development :: Build Tools'],
    'description': 'A scalable build tool for large, complex, heterogeneous repos.',    'entry_points': {   'console_scripts': [   'pants = pants.bin.pants_loader:main']},
    'install_requires': [   'packaging==16.8',
                            'twitter.common.collections<0.4,>=0.3.1',
                            'setproctitle==1.1.10',
                            'six<2,>=1.9.0',
                            'ansicolors==1.0.2',
                            'subprocess32==3.2.7; python_version < "3"',
                            'contextlib2==0.5.5',
                            'psutil==4.3.0',
                            'fasteners==0.14.1',
                            'twitter.common.dirutil<0.4,>=0.3.1',
                            'pathspec==0.5.0',
                            'scandir==1.2',
                            'setuptools==30.0.0',
                            'requests[security]<2.19,>=2.5.0',
                            'pyopenssl==17.3.0',
                            'pystache==0.5.3',
                            'faulthandler==2.6',
                            'cffi==1.11.1',
                            'pex==1.3.2',
                            'docutils<0.13,>=0.12',
                            'Markdown==2.1.1',
                            'Pygments==1.4',
                            'twitter.common.confluence<0.4,>=0.3.1',
                            'wheel==0.29.0',
                            'futures==3.0.5',
                            'pywatchman==1.4.1'],
    'license': 'Apache License, Version 2.0',

@jsirois
Copy link
Contributor

jsirois commented May 30, 2018

To be clear, if my hunch is correct, the standard pants setup script needs to be fixed to use a higher setuptools floor.

@jsirois
Copy link
Contributor

jsirois commented May 30, 2018

Noting that one fix is to kill the environment marker in our requirements.txt. IIRC this was added pre-emptively by Benjy for consumers of pants as a python3 library. Since pants does not support running under python3 today, I can't imagine it can be used as a python3 library today, except by luck use of just the right subset of imports that happens to be python3 compatible.

@jsirois
Copy link
Contributor

jsirois commented May 30, 2018

Yeah, looks like environment markers in install_requires, requires setuptools >=20.5: http://setuptools.readthedocs.io/en/latest/history.html#id276

@benjyw
Copy link
Contributor

benjyw commented May 30, 2018 via email

@jsirois
Copy link
Contributor

jsirois commented May 31, 2018

I'll try to retain and fix setup 1st since it'll need to be fixed someday anyhow.

@jsirois
Copy link
Contributor

jsirois commented May 31, 2018

Working in pantsbuild/setup#23

@jsirois
Copy link
Contributor

jsirois commented May 31, 2018

Noting repro progress. I get a similar failure:

$ ./pants
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   122  100   122    0     0    389      0 --:--:-- --:--:-- --:--:--   389
100   181  100   181    0     0    232      0 --:--:-- --:--:-- --:--:--   232
  0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0
100 2576k  100 2576k    0     0   725k      0  0:00:03  0:00:03 --:--:-- 1359k
New python executable in /home/jsirois/.cache/pants/setup/bootstrap-Linux-x86_64/pants.ciXYmT/install/bin/python2.7
Also creating executable in /home/jsirois/.cache/pants/setup/bootstrap-Linux-x86_64/pants.ciXYmT/install/bin/python
Installing pip, wheel...done.
Collecting setuptools==5.4.1
  Downloading https://files.pythonhosted.org/packages/b1/ba/02218786fe9d5beeb2cfdaf0e423219d132845774d703f6e8a708dd5e642/setuptools-5.4.1-py2.py3-none-any.whl (528kB)
    100% |████████████████████████████████| 532kB 1.5MB/s 
Installing collected packages: setuptools
Successfully installed setuptools-5.4.1
You are using pip version 9.0.3, however version 10.0.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
Collecting pantsbuild.pants==1.8.0.dev1
  Downloading https://files.pythonhosted.org/packages/6f/09/d6a9240b11dbca065b7361fbc4cbff7404d7497b3badc75b4f3a35c3dfdc/pantsbuild.pants-1.8.0.dev1.tar.gz (3.3MB)
    100% |████████████████████████████████| 3.3MB 438kB/s 
    Complete output from command python setup.py egg_info:
    error in pantsbuild.pants setup command: 'install_requires' must be a string or list of strings containing valid project/version requirement specifiers
    
    ----------------------------------------
Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-build-t9kNqm/pantsbuild.pants/
You are using pip version 9.0.3, however version 10.0.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
./pants: line 102: /home/jsirois/.cache/pants/setup/bootstrap-Linux-x86_64/1.8.0.dev1/bin/python: No such file or directory

IFF I forbid pip using binary packages:

$ git diff
diff --git a/pants b/pants
index 53b9e32..92dbd55 100755
--- a/pants
+++ b/pants
@@ -90,7 +90,7 @@ function bootstrap_pants {
         "${staging_dir}/install/bin/pip" install \
           "${SETUPTOOLS_REQUIREMENT}" && \
       "${staging_dir}/install/bin/python" \
-        "${staging_dir}/install/bin/pip" install \
+        "${staging_dir}/install/bin/pip" install --no-binary :all: \
           "${pants_requirement}" && \
       ln -s "${staging_dir}/install" "${staging_dir}/${pants_version}" && \
       mv "${staging_dir}/${pants_version}" "${PANTS_BOOTSTRAP}/${pants_version}"

@sivy - your example failure shows a custom pypi repo and it seems to show that repo does not contain the binaries (wheels) we publish. My edit above simulated that scenario.

Without that edit, aka using the pants published wheels, setup proceeds just fine (ie: its specifically old setuptools bdist code that chokes on enviornment markers).

So a short-term workaround is to use the wheels we publish. I understand there could be a number of reasons you don't want to do this or can't however.

@jsirois
Copy link
Contributor

jsirois commented May 31, 2018

Noting that upgrading setuptools then does fix the above repro:

$ git diff
diff --git a/pants b/pants
index 53b9e32..76fc848 100755
--- a/pants
+++ b/pants
@@ -28,7 +28,7 @@ VENV_VERSION=${VENV_VERSION:-15.2.0}
 #   https://github.com/pantsbuild/pants/issues/3948
 #   https://github.com/pantsbuild/setup/issues/14
 #   https://github.com/pantsbuild/setup/issues/19
-SETUPTOOLS_REQUIREMENT="setuptools==5.4.1"
+SETUPTOOLS_REQUIREMENT="setuptools==20.6.8"
 
 VENV_PACKAGE=virtualenv-${VENV_VERSION}
 VENV_TARBALL=${VENV_PACKAGE}.tar.gz
@@ -90,7 +90,7 @@ function bootstrap_pants {
         "${staging_dir}/install/bin/pip" install \
           "${SETUPTOOLS_REQUIREMENT}" && \
       "${staging_dir}/install/bin/python" \
-        "${staging_dir}/install/bin/pip" install \
+        "${staging_dir}/install/bin/pip" install --no-binary :all: \
           "${pants_requirement}" && \
       ln -s "${staging_dir}/install" "${staging_dir}/${pants_version}" && \

I'm going to table this though for a pause and a bit of thought. There are 3 issues listed in the setup script re pinning setuptools lo to 5.4.1 so a simple bump could break things for certain use cases. I think the setup script may need to have a table of pants version ranges that map to setuptools versions to try or some such, but that's a bit of complexity I'd like to think through before committing to.

@sivy
Copy link
Author

sivy commented May 31, 2018

FYI: I've resolved this in my particular use case by switching back to 1.7.0rc0; it's meeting our needs currently. Prioritize this as necessary.

@benjyw
Copy link
Contributor

benjyw commented May 31, 2018

Perhaps the runner script should be tied to the version of Pants? Or at least to the minor version? I.e., when upgrading Pants past a minor version you must upgrade the setup script? We can create a meta-script to make that easy. It seem painful to have to support many versions of pants in a single script.

In practice, I have had to manually edit the script from time to time after upgrading Pants, usually to deal with setuptools issues, so maybe it's time to formalize that?

@stuhood
Copy link
Member

stuhood commented May 31, 2018

pip install --only-binary might be a good way to force the whl usage...

@stuhood
Copy link
Member

stuhood commented May 31, 2018

Perhaps the runner script should be tied to the version of Pants?

We're pretty close to being able to actually ship a pex: #4896 ... which would be my preference.

@jsirois
Copy link
Contributor

jsirois commented May 31, 2018

My caveats above were nods to two possibilities:

  1. The user is attempting to use pants on - say - BSD, which we technically support
  2. The user is under a policy of building from sources (don't trust binaries)

Although I too would love to force binary usage, it seems to me we can't and so this issue needs to be fixed.

@stuhood
Copy link
Member

stuhood commented May 31, 2018

Although I too would love to force binary usage, it seems to me we can't and so this issue needs to be fixed.

Only publishing binaries for some platforms, while allowing other platforms to check out a git tag to build from source seems reasonable.

A distinction here is whether a source tarball stored on PyPI is your source distribution mechanism, or something else. If we were to start releasing pexes, we could also release a complete source tarball (ie, including our bootstrap scripts, etc). git archive --format=zip -o ${file}.zip HEAD is 5 MB currently.

@jsirois
Copy link
Contributor

jsirois commented Jun 1, 2018

I think that's right. Since we don't have a bdist builder for rust, we no longer support python as the standard build pipeline, aka we definitely only support wheel consumption where we build the wheels with a proper embedded native engine resource built OOB. This recalls the fact we only publish a source distribution today as a hack around lack of support for manylinux in pex (now fixed) which interferes with plugin installation. I'll look at fixing that. IE, pulling publishing of an sdist from our release process. @sivy what this all means is the fact you're getting away with setting up via pants sdists today is by luck. Starting back around last June we shouldn't have been publishing sdists to pypi any longer for pantsbuild.pants, we only were forced to due to a technicality that I'm about to fix. So fair warning.

@sivy
Copy link
Author

sivy commented Jun 1, 2018

Thanks @jsirois et al, I'll make sure that we are headed towards working with wheels internally.

@jsirois
Copy link
Contributor

jsirois commented Jun 1, 2018

Noting a slight ammendment. We really only should force binary installation of pantsbuild.pants and then only prefer binaries for the transitive dependency closure underneath it, at least until such time as we pull support for pip, aka pull publishing via pypi, in favor of pex-only publishing.

@stuhood
Copy link
Member

stuhood commented Jun 1, 2018

lack of support for manylinux in pex (now fixed)

Yea... I believe @kwlzn started working on integrating the new pex.

@jsirois
Copy link
Contributor

jsirois commented Jun 4, 2018

#5906 tracks bumping our pex dependency to pick up manylinux support.

@jsirois
Copy link
Contributor

jsirois commented Jun 4, 2018

And #4956 tracks fixing the need to publish an sdist.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants