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

Add flag to prevent multiple wheel repairs #281

Closed
ofek opened this issue Jan 19, 2021 · 11 comments · Fixed by #289
Closed

Add flag to prevent multiple wheel repairs #281

ofek opened this issue Jan 19, 2021 · 11 comments · Fixed by #289

Comments

@ofek
Copy link
Sponsor Contributor

ofek commented Jan 19, 2021

if out_wheel is not None:
analyzed_tag = analyze_wheel_abi(out_wheel).overall_tag
if reqd_tag < get_priority_by_name(analyzed_tag):
logger.info(('Wheel is eligible for a higher priority tag. '
'You requested %s but I have found this wheel is '
'eligible for %s.'),
args.PLAT, analyzed_tag)
out_wheel = repair_wheel(args.WHEEL_FILE,
abi=analyzed_tag,
lib_sdir=args.LIB_SDIR,
out_dir=args.WHEEL_DIR,
update_tags=args.UPDATE_TAGS,
patcher=patcher)

from CI logs:

+ auditwheel repair wheelhouse/coincurve-14.0.0-cp36-cp36m-linux_i686.whl --plat manylinux2014_i686 -w out
INFO:auditwheel.main_repair:Repairing coincurve-14.0.0-cp36-cp36m-linux_i686.whl
INFO:auditwheel.wheeltools:Previous filename tags: linux_i686
INFO:auditwheel.wheeltools:New filename tags: manylinux2014_i686
INFO:auditwheel.wheeltools:Previous WHEEL info tags: cp36-cp36m-linux_i686
INFO:auditwheel.wheeltools:New WHEEL info tags: cp36-cp36m-manylinux2014_i686
INFO:auditwheel.main_repair:Wheel is eligible for a higher priority tag. You requested manylinux2014_i686 but I have found this wheel is eligible for manylinux2010_i686.
INFO:auditwheel.wheeltools:Previous filename tags: linux_i686
INFO:auditwheel.wheeltools:New filename tags: manylinux2010_i686
INFO:auditwheel.wheeltools:Previous WHEEL info tags: cp36-cp36m-linux_i686
INFO:auditwheel.wheeltools:New WHEEL info tags: cp36-cp36m-manylinux2010_i686
INFO:auditwheel.main_repair:
Fixed-up wheel written to /out/coincurve-14.0.0-cp36-cp36m-manylinux2010_i686.whl
+ cp out/coincurve-14.0.0-cp36-cp36m-manylinux2010_i686.whl out/coincurve-14.0.0-cp36-cp36m-manylinux2014_i686.whl /io/dist

Ideally, only one wheel would be produced: the one with the requested platform or the one with the higher priority tag.

If one doesn't add an additional step to delete one then it's just wasted space on PyPI currently.

@lkollar
Copy link
Contributor

lkollar commented Feb 9, 2021

If I understand correctly the problem here is that you requested manylinux2014 but auditwheel generated manylinux2010 tag? The wheel itself is manylinux2010 compliant, which means it will work on both manylinux2010 and manylinux2014 platforms and uploading the 2010 version should be sufficient. What is the advantage in producing a 2014 variant as well?

@ofek
Copy link
Sponsor Contributor Author

ofek commented Feb 9, 2021

@lkollar It produces the 2014 one I requested and unrequested 2010

@lkollar
Copy link
Contributor

lkollar commented Feb 9, 2021

Are you referring to the cp line in the CI output which copies the 2010 version to 2014? Where is that coming from?

@ofek
Copy link
Sponsor Contributor Author

ofek commented Feb 9, 2021

Part of my build script:

https://github.com/ofek/coincurve/blob/64e25d98caee83271158a0fc520058f9ac2d7394/.github/scripts/build-linux-wheels.sh#L32-L37

in this case it's showing that 2 wheels are produced, not 1

@lkollar
Copy link
Contributor

lkollar commented Feb 9, 2021

That makes sense, thanks. I agree, there is not much point in leaving the higher tagged wheel around, we could just delete it.

@mayeut
Copy link
Member

mayeut commented Feb 14, 2021

Keeping the highest / lowest priority tags should not be left as an auditwheel choice.
While I agree that for most, the highest priority tag will be what should be uploaded to PyPI, I can understand if some project wants only the one it requested for some reason (one of them being to clearly state that there's no support on outdated systems).

As for the multiple wheels being produced, it's been discussed in multiple places except here (or couldn't find it):

The comment in the manylinux issue states that files packaged in manylinux1 & manylinux2010 wheels could be different (and that's true if one uses a rebuilt ncurses 5.x on manylinux2010). However, with PEP 600 superseding previous PEPs, it shouldn't be the case anymore (and we have to drop ncurses from manylinux1 whitelist here).

Having both wheels on PyPI has only one advantage which is to help tracking progress of manylinux policies/images. I put something together not long ago for this: https://mayeut.github.io/manylinux-timeline/

Getting stats is certainly not a good reason to waste space on PyPI (or rely on packagers to throw one wheel or the other).
The wheel specification allows for multiple platform tags in a single wheel and I think that's the way it should be handled here. It gives the intent on requested compatibility and at the same time allows to get a larger audience using also the highest policy tag found by auditwheel.

e.g. pybase64-1.1.5-cp35-cp35m-manylinux1_x86_64.manylinux_2_24_x86_64.whl where the requested policy is manylinux_2_24_x86_64 and highest policy found by auditwheel is manylinux1_x86_64 .
It's not only a matter of renaming the file. The WHEEL file inside the wheel also needs updating.

This would require a new flag to enforce the highest priority tag that can be used to let projects decide how they want to handle this.

@lkollar
Copy link
Contributor

lkollar commented Feb 15, 2021

Thanks for looking into this @mayeut. So if I understand correctly, you propose introducing a flag to produce a single file with combined wheel tags? E.g. something like --single-wheel.

@mayeut
Copy link
Member

mayeut commented Feb 17, 2021

@lkollar,

I was thinking of always producing a single file and introducing a new flag like --highest-plat-allowed or something like that.
Some examples assuming the wheel is compatible with all policies:
auditwheel repair --plat manylinux2014_x86_64 ... produces wheel-...-manylinux1_x86_64.manylinux2014_x86_64.whl
auditwheel repair --plat manylinux2014_x86_64 --highest-plat-allowed manylinux2010_x86_64 ... produces wheel-...-manylinux2010_x86_64.manylinux2014_x86_64.whl
auditwheel repair --plat manylinux2014_x86_64 --highest-plat-allowed manylinux2014_x86_64 ... produces wheel-...-manylinux2014_x86_64.whl
auditwheel repair --plat manylinux2014_x86_64 --highest-plat-allowed use-plat ... produces wheel-...-manylinux2014_x86_64.whl

This might seem a bit too much in which case, always produce a single wheel but introduce a --only-plat flag.
auditwheel repair --plat manylinux2014_x86_64 ... produces wheel-...-manylinux1_x86_64 .manylinux2014_x86_64.whl
auditwheel repair --plat manylinux2014_x86_64 --only-plat ... produces wheel-...-manylinux2014_x86_64.whl

Maybe the second option is enough (at least it covers the same cases as the 2 wheels produced today but only generates one) ?

@mayeut
Copy link
Member

mayeut commented Feb 21, 2021

@lkollar, I tried something in #289 (which builds on top of #287, #288). If you can have a look at those 3 PRs, I'm very interested in your feedback (there's no hurry though).

@lkollar
Copy link
Contributor

lkollar commented Feb 21, 2021

Thanks @mayeut, I'll have a look in the next couple of days.

@mayeut
Copy link
Member

mayeut commented Mar 6, 2021

This is now fixed in master but will require a new auditwheel release.

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