Python packaging Github Workflow

[pydsigner]

Attempt at addressing #599. Getting some errors on each platform, I might benefit from some of your insight as more experienced with Go and building this package in particular.

[pydsigner]

@tdewolff any thoughts on what could be wrong here? I'm operating from a Windows machine locally, so I'm fighting way upstream to execute manual builds locally.

[pydsigner]

It might be worth considering a switch from having a Python extension stuck into the CGo compile process to using CFFI on a raw CGo library?

[pydsigner]

Need to figure out exactly why the .so is being excluded from the windows wheels and fix it, and then massage a few parts of the artifact upload/download process. After that, we'll have ARM64 + 86_64 Linux and 86_64 Windows wheels!

[pydsigner]

Tested a cross-compiled Windows wheel— it works!

[pydsigner]

@tdewolff the way the workflow is set up it will generate wheels and an sdist for any change on master, any PR to master, and any new tag. If the trigger is a release tag, the distributions will be uploaded to the Github release associated with the tag (or a release will be created). PyPI publishing will also take place, but this requires a PyPI API token to be added to the Github secrets for this repository as PYPI_API_TOKEN.

[pydsigner]

It's also possible to set PyPI up without the API token now, using trusted publishing: https://blog.pypi.org/posts/2023-04-20-introducing-trusted-publishers/. This involves a small tweak to the workflow yaml file, which I can make if you'd prefer that approach.

[tdewolff]

Thanks a lot for the hard work, very much appreciated! The PR looks good and ready to merge as far as I can see. Just a few questions:

• What's the benefit of using CFFI over the original compilation?
• Would using Docker resolve the MacOS case? Would this replace QEMU?
• Wouldn't it be sufficient to only run the GitHub Action on new releases (new tags) and not for all changes to master?

I've added the PYPI_API_TOKEN to the repo secrets.

[codecov-commenter]

Codecov Report

Merging #600 (426399a) into master (76935f3) will not change coverage.
Report is 15 commits behind head on master.
The diff coverage is n/a.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@           Coverage Diff           @@
##           master     #600   +/-   ##
=======================================
  Coverage   83.70%   83.70%           
=======================================
  Files          25       25           
  Lines        6749     6749           
=======================================
  Hits         5649     5649           
  Misses       1023     1023           
  Partials       77       77           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[pydsigner]

• What's the benefit of using CFFI over the original compilation?

The advantage is that now the Go toolchain does not have to be Python aware. The double bridge from Go to C to Python was proving a difficult hill to surmount— you would have to be able to get a pkgconfig file for Windows Python, for example. With CFFI, Go only has to make it to C, and then Python reaches out from the other side. This ostensibly costs a bit of performance, but there's not really any logic moved into Python, and the multiple conversion steps for strings going in and out was happening anyways. Anyone really concerned about speed will probably use the minify.file() entry point, which should clock about the same speed as native Go, though I haven't tested to see that myself.

• Would using Docker resolve the MacOS case? Would this replace QEMU?

We're only using QEMU for the Linux ARM build, there's actually a native macOS runner available from Github. The issue I ran into is that cgo, at least running on macOS, does not support macOS as a build target at all.

$ go build -buildmode=c-shared -o src/minify/_minify.so
-buildmode=c-shared not supported on darwin/arm64 CGO_ENABLED=1

and likewise

$ go build -buildmode=c-shared -o src/minify/_minify.so
-buildmode=c-shared not supported on darwin/amd64 CGO_ENABLED=1

An xx-go Docker setup would be the next thing to try, but I ran out of energy and time for a platform that brings me less personal value (I'm developing on Windows for Linux targets).

• Wouldn't it be sufficient to only run the GitHub Action on new releases (new tags) and not for all changes to master?

I think I may have been thinking too much in Python-only terms; since the Go packages are only updated on release, there's probably not much value in re-running. Longer-term it might be nice to have CI for the bindings though, and automatically test the wheels that are produced for each potential upstream change.

[pydsigner]

I've made a change according to your 3rd question and also restored the ability to build/install the Python bindings without manually compiling the cgo extension. This should ensure existing downstream consumers of the Python package don't experience unexpected breakage.

[tdewolff]

Excellent, thank you very much! Merged the PR!