Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Update manylinux2010 policy with i686 symbol versions #141

Merged
merged 1 commit into from
Oct 2, 2019

Conversation

mayeut
Copy link
Member

@mayeut mayeut commented Feb 16, 2019

Symbol versions for manylinux2010 were probably extracted on a x86_64 CentOS 6 using the scripts/calculate_symbol_versions.py
These are only a subset of allowed symbol versions. This commit updates symbol_versions to be the union of symbol versions found on x86_64 and i686 images.

This also raises the question, should symbol policies be split by platform ?
If the answer's yes, I can work on that in this PR.

@ehashman
Copy link
Member

This doesn't sound right to me, why would ABI versions differ between architectures on the same image with same GCC version?

@mayeut
Copy link
Member Author

mayeut commented Feb 16, 2019

I guess, and this is only a guess, that other (older) symbol versions existed on i386 but the compatibility (thus symbol version) were dropped when x86_64 was introduced (on that platform only, since it was new, there was no need to maintain those older versions)

@mayeut
Copy link
Member Author

mayeut commented Feb 16, 2019

PS, all the older symbols are part of manylinux1 policy.

@mayeut
Copy link
Member Author

mayeut commented Feb 16, 2019

Here's a debug log using pypa/manylinux#182 PR for i686 image:

DEBUG:auditwheel.policy.versioned_symbols:Package requires GLIBC_2.10, incompatible with policy manylinux1_i686 which requires {'GLIBC_2.2', 'GLIBC_2.2.5', 'GLIBC_2.3.4', 'GLIBC_2.2.4', 'GLIBC_2.3.2', 'GLIBC_2.1.1', 'GLIBC_2.0', 'GLIBC_2.2.2', 'GLIBC_2.2.3', 'GLIBC_2.3.3', 'GLIBC_2.5', 'GLIBC_2.1', 'GLIBC_2.1.2', 'GLIBC_2.2.1', 'GLIBC_2.3', 'GLIBC_2.2.6', 'GLIBC_2.1.3', 'GLIBC_2.4'}
DEBUG:auditwheel.policy.versioned_symbols:Package requires GLIBC_2.1.3, incompatible with policy manylinux2010_i686 which requires {'GLIBC_2.2.5', 'GLIBC_2.10', 'GLIBC_2.11', 'GLIBC_2.3.4', 'GLIBC_2.7', 'GLIBC_2.3.2', 'GLIBC_2.5', 'GLIBC_2.3.3', 'GLIBC_2.6', 'GLIBC_2.3', 'GLIBC_2.8', 'GLIBC_2.12', 'GLIBC_2.2.6', 'GLIBC_2.9', 'GLIBC_2.4'}
DEBUG:auditwheel.policy.versioned_symbols:Package requires GLIBC_2.0, incompatible with policy manylinux2010_i686 which requires {'GLIBC_2.2.5', 'GLIBC_2.10', 'GLIBC_2.11', 'GLIBC_2.3.4', 'GLIBC_2.7', 'GLIBC_2.3.2', 'GLIBC_2.5', 'GLIBC_2.3.3', 'GLIBC_2.6', 'GLIBC_2.3', 'GLIBC_2.8', 'GLIBC_2.12', 'GLIBC_2.2.6', 'GLIBC_2.9', 'GLIBC_2.4'}
auditwheel: error: cannot repair "/tmp/built_wheel/spam-0.1.0-cp27-cp27m-linux_i686.whl" to "manylinux2010_i686" ABI because of the presence of too-recent versioned symbols. You'll need to compile the wheel on an older toolchain.

@ehashman
Copy link
Member

2 comments:

  • I see the addition of the older symbols here but my comment was WRT the new GCC_* symbols introduced in this PR
  • Can you share the test wheel/package so I can play around with it? The debug log is helpful but I'd like to dig in.

It's curious we haven't gotten any reports of this previously, seems like a bit of an edge case...

@mayeut
Copy link
Member Author

mayeut commented Feb 16, 2019

You're right, I did not check on those newer symbols. I will have a look at gcc change log to check why there are newer symbol versions on i686.

These logs come from tests on a custom cibuildwheel, on an updated branch of PR 182.
For the updated branch of PR 182: I used this, https://github.com/mayeut/manylinux/tree/manylinux2010
Custom cibuildwheel is there: https://github.com/mayeut/cibuildwheel/tree/manylinux2010

To get started, build the docker images (both):

PLATFORM="x86_64" TRAVIS_COMMIT=latest ./build.sh
PLATFORM="i686" TRAVIS_COMMIT=latest ./build.sh

Then start tests on cibuildwheel:

CIBW_PLATFORM=linux python ./bin/run_test.py test/01_basic

PS, I do not need those newer symbols for tests to pass. These were from the scripts/calculate_symbol_versions.py run on i686 image

@mayeut
Copy link
Member Author

mayeut commented Feb 16, 2019

I found out why there are different symbols for GCC:
https://github.com/gcc-mirror/gcc/blob/master/libgcc/config/i386/libgcc-glibc.ver

You can look for %ifdef __x86_64__ around line 100

@veblush
Copy link
Contributor

veblush commented Sep 26, 2019

Any progress or blocking issue? I need this PR too.

@lkollar
Copy link
Contributor

lkollar commented Sep 27, 2019

I think this is fine, but PEP 571 declared GCC_4.3.0 as highest version so if we really need the higher version we should update the PEP as well. We can also split this PR or land @veblush's PR. What do you think @mayeut?

@veblush
Copy link
Contributor

veblush commented Sep 27, 2019

I agree with updating GLIBC and GCC in separate CLs and updating GLIBC is more important because almost everything built on 32-bit CentOS 6 are not compliant with manylinux2010 because of GLIBC. But I think we need to revisit GCC issue because GCC_4.3.0 should change to at least GCC_4.4.0 in PEP 571, I think.

From the libgcc, they put symbols introduced with GCC 4.3 in GCC_4.4.0 only for 32 bits. As a result, __extenddftf2 function, as an example, is in GCC_4.3.0 for x86_64 but in `GCC_4.4.0' for i686.

You can see this in the comment.

# 128 bit long double support was introduced with GCC 4.3.0 to 64bit
# and with GCC 4.4.0 to 32bit.  These lines make the symbols to get
# a @@GCC_4.3.0 or @@GCC_4.4.0 attached.

I think GCC_4.3.0 in PEP 571 should be updated to GCC_4.4.0 to address 32-bit applications linked with these functions.

GCC_4.4.0 {
  __addtf3
  __copysigntf3
  __divtc3
  __divtf3
  __eqtf2
  __extenddftf2
  __extendsftf2
  __fabstf2
  __fixtfdi
  __fixtfsi
  __fixunstfdi
  __fixunstfsi
  __floatditf
  __floatsitf
  __floatunditf
  __floatunsitf
  __getf2
  __gttf2
  __letf2
  __lttf2
  __multc3
  __multf3
  __negtf2
  __netf2
  __powitf2
  __subtf3
  __trunctfdf2
  __trunctfsf2
  __trunctfxf2
  __unordtf2
}

@mayeut
Copy link
Member Author

mayeut commented Sep 27, 2019

@lkollar,
I think that, if we want to follow strictly the PEP, #194 can be merged first and release a new version to address this need.

Once python/peps#1180 gets merged we could merge this PR (I will rebase after #194 gets merged in). Another thing that comes to mind is that auditwheel should probably define policies per architecture but that's another matter. I'm sure that #192 has the same issue as original manylinux2010 policy (since I extracted the symbols from CentOS 7 amd64 image) and might be in even worse shape given that PEP 599 extends the number of supported architectures.

@mayeut mayeut force-pushed the update-manylinux2010-policy branch from 920da86 to db2245f Compare September 28, 2019 14:29
@mayeut
Copy link
Member Author

mayeut commented Sep 28, 2019

Rebased on top of #194

@mayeut mayeut mentioned this pull request Sep 28, 2019
@codecov
Copy link

codecov bot commented Sep 28, 2019

Codecov Report

Merging #141 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #141   +/-   ##
=======================================
  Coverage   87.43%   87.43%           
=======================================
  Files          19       19           
  Lines         963      963           
  Branches      210      210           
=======================================
  Hits          842      842           
  Misses         84       84           
  Partials       37       37

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3191806...970a066. Read the comment docs.

@mayeut mayeut force-pushed the update-manylinux2010-policy branch 2 times, most recently from d4a90f1 to 8b7c92b Compare September 30, 2019 23:20
Symbol versions for manylinux2010 were probably extracted on a x86_64 CentOS 6 using the scripts/calculate_symbol_versions.py

These are only a subset of allowed symbol versions. This commit updates symbol_versions to be the union of symbol versions found on x86_64 and i686 images.
@mayeut mayeut force-pushed the update-manylinux2010-policy branch from 8b7c92b to 970a066 Compare October 1, 2019 19:17
@veblush
Copy link
Contributor

veblush commented Oct 1, 2019

python/peps#1180 was merged now so it seems fine to get this merged, too. Can we get a new release auditwheel on this change?

@auvipy auvipy merged commit 5ddd128 into pypa:master Oct 2, 2019
@mayeut mayeut deleted the update-manylinux2010-policy branch October 2, 2019 21:29
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants