caketop / python-starlark-go

🐍 Python bindings for starlark-go 🐍
https://python-starlark-go.readthedocs.io/
Apache License 2.0
23 stars 8 forks source link

Switch to OS-specific architecture lists #131

Closed colindean closed 1 year ago

colindean commented 1 year ago

Another stab at fixing #119

colindean commented 1 year ago

The failing test will be fixed in #133 but I think the matrix exclusion of '*' isn't working as expected, so don't merge this yet.

jordemort commented 1 year ago

This doesn't seem quite right; if we're going to build universal2 wheels, then I don't think we also want to build separate x86_64 and arm64 wheels. Our existing universal2 wheels appear to be x86_64-only. This doesn't appear to change how they're built; i.e. I suspect this is still building a broken universal2.

You can grab the artifacts out of one of the test runs, i.e. at the bottom of https://github.com/caketop/python-starlark-go/actions/runs/3970286648 and try installing some wheels locally and see if they work.

I'm using https://github.com/asottile/setuptools-golang to build the Go part and I have a sneaking suspicion it's not universal2-aware - we might have to drop universal2 entirely and go with arch-specific wheels instead. I think that's probably fine. I'd be curious to know if the arm64 wheels this is building for macOS are any good, though, or if the Go bits are still coming out x86_64 there. We might have to inject some Go environment variables to lock down the target arch.

colindean commented 1 year ago

I'm out of time for this for day but I think the state after 7144cefacae8b99a59f75ba4857374998e07dd3e is the correct one but needs some work to exclude things from the matrix correctly.

colindean commented 1 year ago

setuptools-golang's author said "No" but indicated that lipo might help here.

we might have to drop universal2 entirely and go with arch-specific wheels instead

I'm leaning toward this.

jordemort commented 1 year ago

Closing in favor of https://github.com/caketop/python-starlark-go/pull/134