Let pip deal with whl filename

[Antelox]

I tried to create a whl from an alpine linux docker image of the latest Unicorn release and the whl build works fine but due to that if statement removed in this PR, during pip install it returns the following error:

ERROR: unicorn-2.0.1.post1-py2.py3-none-manylinux1_x86_64.whl is not a supported wheel on this platform.

This because the if forces to set manylinux1 tag in the filename and that matters for pip during the installation. In fact, doing a simple rename of the whl file by replacing manylinux1 with any, pip installs the whl without any problems. Of course rather renaming like I did, the proper way to do this is through auditwheel repair, which in this case actually repairs and creates a new whl file indeed with appropriate tags.
Doing this you will generate a whl file that will be compatible for both tags platform, hence when installing from alpine for instance the end user will not end up compiling the whl every time since the whl file pushed to pypi will be detected as fine for the given platform.

• whl file created by the current setup.py: unicorn-2.0.1.post1-py2.py3-none-manylinux1_x86_64.whl
• whl file created by the current setup.py after the auditwheel repair: unicorn-2.0.1.post1-py2.py3-none-manylinux1_x86_64.musllinux_1_2_x86_64.whl
• whl file created by the setup.py modified in this PR: unicorn-2.0.1.post1-py2.py3-none-any.whl

The auditwheel repair is not required if this PR gets accepted but if you think that this change will break builds for other platforms then I guess this can be added into your Github Actions during whl creation before pushing to pypi index, just in case.
Of course more tests can be performed using different docker images but this change should be fine enough.

[wtdcode]

The manylinux1 hack is known outdated but I also don't think dropping manylinux is a good choice for pip.

[Antelox]

I see but what I meant is whl filename should not be touched. Rather it should be used proper platform/docker to build a whl for maximum compatibility and during wheel creation pip will set proper filename/tag.

In addition to this, the support for manylinux1 has ended on January 1st, 2022.

Removing that hack you would still be able to build whl with manylinux1 tag if you use the relative docker image and at same time you will offer the possibility to build whl in other platforms too without having to specify the --plat-name argument.

https://github.com/pypa/manylinux

[wtdcode]

I see but what I meant is whl filename should not be touched. Rather it should be used proper platform/docker to build a whl for maximum compatibility and during wheel creation pip will set proper filename/tag.

In addition to this, the support for manylinux1 has ended on January 1st, 2022.

Removing that hack you would still be able to build whl with manylinux1 tag if you use the relative docker image and at same time you will offer the possibility to build whl in other platforms too without having to specify the --plat-name argument.

https://github.com/pypa/manylinux

We are actually using manylinux14 here:

unicorn/.github/workflows/PyPI-publishing.yml

Line 41 in 7b8c63d

name: 'manylinux2014_x86_64'

I mean we could upgrade manylinux1 to something else but none here seems not appropriate.

[Antelox]

I think I didn't explain myself well.

Main issue here is you are not considering there is also musllinux images so you are forcing putting manylinux1 during building even when wheel will not be created on manylinux platforms. In your case it's hidden behavior because you forcing putting that tag and building wheel on that platform but if you try to build on musl based platforms and install the wheel, you will get an error as reported on my first message.
Also, that none is something pip evaluates and put, it seems. Via auditwheel it's always possible to add more tags according to the platform, if I'm not wrong.

Anyway feel free to reject but I'm pretty sure this code has to go, things have changed over these 7 years (I do see that code was added back then).

[wtdcode]

Main issue here is you are not considering there is also musllinux images so you are forcing putting manylinux1 during building even when wheel will not be created on manylinux platforms. In your case it's hidden behavior because you forcing putting that tag and building wheel on that platform but if you try to build on musl based platforms and install the wheel, you will get an error as reported on my first message. Also, that none is something pip evaluates and put, it seems. Via auditwheel it's always possible to add more tags according to the platform, if I'm not wrong.

I got your point now. It's not good to force manylinux indeed, though we never actually build Unicorn on Alphine.

Anyway feel free to reject but I'm pretty sure this code has to go, things have changed over these 7 years (I do see that code was added back then).

Yes, I agree with you. How could I reproduce your problem? I could try to figure the correct way for this.

[Antelox]

$ docker run -it --rm python:3-alpine sh
# apk update
# apk add gcc make cmake pkgconfig linux-headers git musl-dev
# git clone https://github.com/unicorn-engine/unicorn.git
# cd unicorn
/unicorn # pip wheel --src . bindings/python/
Processing ./bindings/python
  Preparing metadata (setup.py) ... done
Building wheels for collected packages: unicorn
  Building wheel for unicorn (setup.py) ... done
  Created wheel for unicorn: filename=unicorn-2.0.1.post1-py2.py3-none-manylinux1_x86_64.whl size=16331760 sha256=54faceb9cf61faeb5efbe10a9ed8b30d62fe78090109c115da1c52ed75f2f08f
  Stored in directory: /tmp/pip-ephem-wheel-cache-pae3egvh/wheels/a9/88/60/db7262f0825f7584dceea3e6a1a6b964d613cafd16f5dc1980
Successfully built unicorn

[notice] A new release of pip available: 22.3.1 -> 23.0.1
[notice] To update, run: pip install --upgrade pip
/unicorn # pip install unicorn-2.0.1.post1-py2.py3-none-manylinux1_x86_64.whl 
ERROR: unicorn-2.0.1.post1-py2.py3-none-manylinux1_x86_64.whl is not a supported wheel on this platform.

[notice] A new release of pip available: 22.3.1 -> 23.0.1
[notice] To update, run: pip install --upgrade pip

If you then try to install auditwheel, it will repair the wheel indeed.

/unicorn # pip install auditwheel
/unicorn # apk add patchelf
/unicorn # auditwheel repair unicorn-2.0.1.post1-py2.py3-none-manylinux1_x86_64.whl 
INFO:auditwheel.main_repair:Repairing unicorn-2.0.1.post1-py2.py3-none-manylinux1_x86_64.whl
INFO:auditwheel.wheeltools:Previous filename tags: manylinux1_x86_64
INFO:auditwheel.wheeltools:New filename tags: manylinux1_x86_64, musllinux_1_2_x86_64
INFO:auditwheel.wheeltools:Previous WHEEL info tags: py2-none-manylinux1_x86_64, py3-none-manylinux1_x86_64
INFO:auditwheel.wheeltools:New WHEEL info tags: py2-none-manylinux1_x86_64, py3-none-manylinux1_x86_64, py2-none-musllinux_1_2_x86_64, py3-none-musllinux_1_2_x86_64
INFO:auditwheel.main_repair:
Fixed-up wheel written to /unicorn/wheelhouse/unicorn-2.0.1.post1-py2.py3-none-manylinux1_x86_64.musllinux_1_2_x86_64.whl
/unicorn # pip install wheelhouse/unicorn-2.0.1.post1-py2.py3-none-manylinux1_x86_64.musllinux_1_2_x86_64.whl 
Processing ./wheelhouse/unicorn-2.0.1.post1-py2.py3-none-manylinux1_x86_64.musllinux_1_2_x86_64.whl
Installing collected packages: unicorn
Successfully installed unicorn-2.0.1.post1
WARNING: Running pip as the 'root' user can result in broken permissions and conflicting behaviour with the system package manager. It is recommended to use a virtual environment instead: https://pip.pypa.io/warnings/venv

[notice] A new release of pip available: 22.3.1 -> 23.0.1
[notice] To update, run: pip install --upgrade pip

But in theory this last step would not be needed with the code from this PR.
Also, in theory, if you build from ubuntu as you doing now using the code from this PR, wheel should be fine on musl-based platforms too. I will make a try regarding this last thing...

[Antelox]

I can also confirm that the whl generated from an ubuntu docker using the code from this PR got installed successfully on the ubuntu docker itself as well as on an alpine docker, without any auditwheel steps...

[Antelox]

Any updates?

[wtdcode]

Any updates?

I once tested this approach and got some problems I remember... I will retry asap.

[wtdcode]

I read through the docs.

Seems that we should have something like unicorn-2.0.2-py2.py3-none-manylinux2014_x86_64.whl. What surprises me is that, musllinux is not included in manylinux?

[wtdcode]

Regarding PEP656, I think the correct way to do this is to add a brand new wheel (i.e. build wheel on alphine containers instead of manylinux) because much of our code (or qemu) might be not. I will add a new CI artifact for that.

[Antelox]

I read through the docs.

Seems that we should have something like unicorn-2.0.2-py2.py3-none-manylinux2014_x86_64.whl. What surprises me is that, musllinux is not included in manylinux?

manylinux and musllinux are 2 separate platform tags, that is why. In any case the final whl filename is something that pip/auditwheel will take care of. What matter is the docker image you use to run the pip wheel command.

Regarding PEP656, I think the correct way to do this is to add a brand new wheel (i.e. build wheel on alphine containers instead of manylinux) because much of our code (or qemu) might be not. I will add a new CI artifact for that.

Yeah so basically to control this and to produce proper wheel you should use a glibc-based docker image as well as a musl libc-based docker image. On each of them you should be able to produce a wheel package that is compatible with relative distros.
For manylinux there are official docker images (https://github.com/pypa/manylinux), for musllinux there isn't any yet but you can use python:3-alpine perhaps. On each of them if you run auditwheel repair it will eventually "repair" the wheel produced by pip wheel command and will properly set the filename.