Build wheels for Linux and Windows on Github actions #38
Conversation
only building 64 bit on linux since nasm causes an error currently ``` [ 24%] Building ASM_NASM object simd/CMakeFiles/simd.dir/i386/jchuff-sse2.asm.o /project/libjpeg-turbo/simd/i386/jchuff-sse2.asm:472: error: invalid operand type make[2]: *** [simd/CMakeFiles/simd.dir/i386/jchuff-sse2.asm.o] Error 1 make[1]: *** [simd/CMakeFiles/simd.dir/all] Error 2 ```
|
This looks really great! This project is barely maintained :( I meant to take it over, but later the project where I used this dropped the dependency. I agree proper automation would really help maintenance. I'll try to review this and we'll see if we can get it merged... |
| with: | ||
| submodules: 'true' | ||
| - name: Build wheels | ||
| uses: pypa/[email protected] |
There was a problem hiding this comment.
this looks reasonable: seems to be a maintained and active project.
| - uses: actions/setup-python@v4 | ||
| with: | ||
| python-version: '3.8' | ||
| - uses: ilammy/msvc-dev-cmd@v1 |
There was a problem hiding this comment.
this one I will need to dig into a little bit, to see how legit it is...
Is it recommended by the CIBW folks?
There was a problem hiding this comment.
It looks legit to me, although no mention of it in the CIBW docs that I can see.
I don't know much about Windows; is this needed so that get-nasm.py script works?
There was a problem hiding this comment.
It’s necessary to setup the environment for nmake.
| shell: cmd | ||
| run: | | ||
| python get-nasm.py ${{ matrix.arch }} | ||
| set path=%path%;%CD%\nasm-2.14.02 |
There was a problem hiding this comment.
Maybe this 2.14.02 string should be an argument to the get-nasm.py script? Easier to maintain (?)
There was a problem hiding this comment.
No strong opinion on this. If you want I can change it.
|
@jbaiter this looks pretty good to me! I can merge it (after changing the repo name) But I don't think I can access/change the "secrets" (inside "Settings") part of this project in order to enable PyPI upload. |
|
Are all the .c/.h files apart from the epeg ones really necessary? |
I created a workflow to build wheel files for Linux and Windows automatically on Github actions.
To support multiple platforms easily, I included the
libjpeg-turborepo as a submodule and build it so it can be linked.Because of that the
jpegtran_build.pyfile changed slightly to find the necessary objects.It should be easy to add MacOS builds as well, but I have zero experience with MacOS. So maybe somebody else can add it later. Also building it on 32bit linux platforms causes issues with the assembler so I didn't include it for now. Building turbojpeg without NASM worked, so I don't know if the issue is with NASM or turbojpeg.
For testing I changed the name of the package to
jpegtran-cffi-wheels, so I can do real testing. You can see the results here https://pypi.org/project/jpegtran-cffi-wheels/. If you would like to merge it you should change it back first.Also it requires the
pypi_passwordto be set in the repository secrets config.I also fixed some tiny issues with the readme file for pypi.