-
-
Notifications
You must be signed in to change notification settings - Fork 632
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
Comments
Are you using the standard |
I am. |
Uncompressed the |
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:
|
To be clear, if my hunch is correct, the standard pants setup script needs to be fixed to use a higher setuptools floor. |
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. |
Yeah, looks like environment markers in install_requires, requires setuptools >=20.5: http://setuptools.readthedocs.io/en/latest/history.html#id276 |
Feel free to kill if you must. I'm AFK on vacation or I'd look into it.
…On Wed, May 30, 2018, 7:05 PM John Sirois ***@***.***> wrote:
Yeah, looks like environment markers in install_requires, requires
setuptools >=20.5:
http://setuptools.readthedocs.io/en/latest/history.html#id276
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#5882 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAfS_EVn9P5xJg8tReYbHPDZVmMbWUxpks5t3tFWgaJpZM4UTkgZ>
.
|
I'll try to retain and fix setup 1st since it'll need to be fixed someday anyhow. |
Working in pantsbuild/setup#23 |
Noting repro progress. I get a similar failure:
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. |
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. |
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. |
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? |
|
We're pretty close to being able to actually ship a pex: #4896 ... which would be my preference. |
My caveats above were nods to two possibilities:
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). |
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 |
Thanks @jsirois et al, I'll make sure that we are headed towards working with wheels internally. |
Noting a slight ammendment. We really only should force binary installation of |
Yea... I believe @kwlzn started working on integrating the new pex. |
#5906 tracks bumping our |
And #4956 tracks fixing the need to publish an sdist. |
Trying to upgrade from 1.7.0-rc0 to 1.8.0-dev1:
The text was updated successfully, but these errors were encountered: