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

SOABI support for Python 2.X and PyPy #3075

Merged
merged 6 commits into from Oct 14, 2015
Merged

Conversation

natefoo
Copy link
Member

@natefoo natefoo commented Sep 3, 2015

Additionally, fix the version portion of the Python tag on wheels built with PyPy that use the Python API. It will now be the Python major version concatenated with the PyPy major and minor versions.

Fixes #2671, #2882.

xrefs:

impl = get_abbr_impl()
if not soabi and impl in ('cp', 'pp'):
d = 'd' if hasattr(sys, 'pydebug') and sys.pydebug else ''
m = 'm' if impl == 'cp' else ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this inaccurate? This doesn't tell us if Python was compiled with --pymalloc or not does it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe using something like sysconfig.get_config_var('WITH_PYMALLOC') would let us inspect it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, that bit came from Nick's comments here. sysconfig.get_config_var('WITH_PYMALLOC') does indeed seem to work, updating shortly.

@natefoo
Copy link
Member Author

natefoo commented Sep 4, 2015

I verified the ABI flags with all subsets of dmu on Pythons 2.6 and 2.7, and dm on 3.2, 3.3, 3.4, and 3.5.

@dstufft
Copy link
Member

dstufft commented Sep 11, 2015

Hmm, I wonder what the effect of merging this would be. If we merge it in pip install wheels made by a wheel without this patch applied? What about the inverse, if we merge it in wheel, will pip without this patch install those wheels?

@qwcode
Copy link
Contributor

qwcode commented Sep 11, 2015

ditto what @dstufft said, and also would be nice to add tests for this.

the duplication of this pep425tags module in pip and wheel is pretty odd.

maybe move it to https://github.com/pypa/packaging , which pip vendors, and then wheel could depend on packaging.

@natefoo
Copy link
Member Author

natefoo commented Sep 15, 2015

@dstufft wheels built with an unmodified wheel can be installed with the modified pip. Wheel will generate e.g. SQLAlchemy-1.0.4-cp27-none-linux_x86_64.whl, pip will prefer e.g. SQLAlchemy-1.0.4-cp27-cp27mu-linux_x86_64.whl but ABI none will be second in the list of supported tags.

Default wheels built with a modified wheel cannot be installed with the unmodified pip since on 2.x, unmodified pip will not match any wheel with an ABI tag other than none. Other than defaulting wheel to produce none ABI wheels (and doing that seems counterproductive), I am not sure how to prevent this.

@qwcode Agree regarding the duplication, and there will be more when the enhanced platform detection for Linux lands. The two pep425tags versions are not exactly the same but near enough that they could be recombined. I'll work on adding some tests.

@dstufft
Copy link
Member

dstufft commented Sep 15, 2015

@natefoo That's fine, it just means we should probably make sure that pip is released prior to wheel.

@dholth
Copy link
Member

dholth commented Sep 17, 2015

pep425tags was sortof supposed to be a thing vendored into wheel and into pip, but it has diverged. https://bitbucket.org/dholth/pep425tags

@natefoo
Copy link
Member Author

natefoo commented Sep 17, 2015

I have changed the way wide unicode detection works because it was doing the wrong thing on Python >= 3.3 (in the unlikely event that the SOABI config var is unset).

I also added some unit tests for the manual ABI flag detection.

@anthrotype
Copy link

👍

@dstufft
Copy link
Member

dstufft commented Oct 11, 2015

Can you merge in develop into this or rebase it please?

Additionally, fix the version portion of the Python tag on wheels built
with PyPy that use the Python API. It will now be the Python major
version concatenated with the PyPy major and minor versions.

Fixes pypa#2671, pypa#2882.
- Catch IOError wherever sysconfig is used (pypa#1074)
- Reapply pypa#2915

Also, be sure to normalize the SOABI
Pythons with PEP 393 Flexible String Representation (so >= 3.3).
Granted, on these Pythons, the SOABI config var should always be set,
but the manual SOABI code path should still try to do the right thing.
@natefoo
Copy link
Member Author

natefoo commented Oct 13, 2015

@dstufft Ok, I rebased it.

@dstufft
Copy link
Member

dstufft commented Oct 13, 2015

Hmm, I'm not sure why that test is failing. It doesn't seem related to this PR, but it's passing on develops last build on Travis it appears.

Another thing I thought of though while reviewing this again. It's currently not handling the case where 2.x has unicode disabled. Right now this will just be treated as a narrow unicode build. I wonder if this is a case that we should care about or not? It's not handled by PEP3149 because that's specific to Python 3.x and 3.x doesn't allow disabling unicode.

The other thing I noticed, it's going to treat a missing config variable the same as not having a feature enabled. Is that the right behavior? Will it return False typically and only None if something is wrong? If it's only None if something is wrong, should we log a warning incase someone gets something that is not compatible?

@natefoo
Copy link
Member Author

natefoo commented Oct 13, 2015

I'm getting the same error if I run the tests locally against develop.

Unfortunately there's no flag for "no unicode", so adding one would require a standards update, correct?. FWIW I have never encountered a no-unicode Python in the wild. Perhaps to avoid the standards issue we could drop the ABI tag back to "none" for no-unicode builds?

For the config variable issue, I could warn and also fall back to the "old" logic for detecting the flags, as laid out here and as implemented in the earlier commits in this PR.

@dstufft
Copy link
Member

dstufft commented Oct 13, 2015

It would probably require a standards update yes, going back to none might be reasonable in that case (it's certainly no worse then where we are today anyways). Warning and falling back is probably reasonable there too.

@natefoo
Copy link
Member Author

natefoo commented Oct 13, 2015

Okay, about done with the changes. Two things:

  1. I can't find any way to detect a --with-pymalloc build other than checking the WITH_PYMALLOC config var, so the fallback code will just assume it is, as Nick suggested (as long as the interpreter is CPython). Hopefully the warning RuntimeWarning: Config variable 'WITH_PYMALLOC' is unset, Python ABI tag may be incorrect will be enough guidance here.
  2. In trying to figure out how to detect a no-unicode build, I realized I actually don't know of a way to create a no-unicode Python 2 build. Is this possible? --disable-unicode does not work, and configuring without --enable-unicode* simply creates a UCS-2 build.

@dstufft
Copy link
Member

dstufft commented Oct 13, 2015

Hmm, I thought --disable-unicode was the way to do it, maybe I'm wrong (or maybe it broke and nobody noticed).

@natefoo
Copy link
Member Author

natefoo commented Oct 13, 2015

Ahh, looks like it works for 2.7.10. 2.6.9's ./configure fails on: checking what type to use for unicode... configure: error: invalid value for --enable-unicode. Use either ucs2 or ucs4 (lowercase).

@dstufft
Copy link
Member

dstufft commented Oct 13, 2015

Awesome.

Assuming pymalloc with a warning is probably fine. Ideally this case shouldn't get hit.

Sent from my iPhone

On Oct 13, 2015, at 2:16 PM, Nate Coraor notifications@github.com wrote:

Ahh, looks like works for 2.7.10. 2.6.9's ./configure fails on: checking what type to use for unicode... configure: error: invalid value for --enable-unicode. Use either ucs2 or ucs4 (lowercase).


Reply to this email directly or view it on GitHub.

   are unavailable, but issue a warning if this is used.
2. Explicitly handle the case where the unicode detection finds wide
   unicode but this is a 3.3+ build (necessary due to #1)
3. Fix tests broken due to pypa#2.
@natefoo
Copy link
Member Author

natefoo commented Oct 13, 2015

Ok, I pushed a new change that creates a fallback if config vars are unavailable, and forces a none tag if sys.maxunicode does not exist.

For the record, a --disable-unicode install of 2.7.10 is very broken - make install fails and even once you work around that, standard modules don't import due to all the missing attributes under sys. Maybe it worked better with earlier Pythons. But get_abi_tag() will nonetheless do the right thing when sys.maxunicode does not exist.

@dstufft
Copy link
Member

dstufft commented Oct 13, 2015

Ha. Probably it would have been fine. I've never had a Unicode-less build, I just knew it was an option. Thanks for making those changes. I'll give it another review when I get home.

Sent from my iPhone

On Oct 13, 2015, at 4:02 PM, Nate Coraor notifications@github.com wrote:

Ok, I pushed a new change that creates a fallback if config vars are unavailable, and forces a none tag if sys.maxunicode does not exist.

For the record, a --disable-unicode install of 2.7.10 is very broken - make install fails and even once you work around that, standard modules don't import due to all the missing attributes under sys. Maybe it worked better with earlier Pythons. But get_abi_tag() will nonetheless do the right thing when sys.maxunicode does not exist.


Reply to this email directly or view it on GitHub.

dstufft added a commit that referenced this pull request Oct 14, 2015
SOABI support for Python 2.X and PyPy
@dstufft dstufft merged commit 461b7aa into pypa:develop Oct 14, 2015
@dstufft
Copy link
Member

dstufft commented Oct 14, 2015

Ok merged. Wheel should wait to pull this change in until pip 8 is released.

@dstufft
Copy link
Member

dstufft commented Oct 14, 2015

Or unless we release it in another version I guess.

@natefoo
Copy link
Member Author

natefoo commented Oct 14, 2015

Thanks @dstufft!

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants