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

Switch to OS-specific architecture lists #131

Closed
wants to merge 5 commits into from

Conversation

colindean
Copy link
Contributor

Another stab at fixing #119

@colindean
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor Author

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

@colindean
Copy link
Contributor Author

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
Copy link
Contributor

Closing in favor of #134

@jordemort jordemort closed this Jan 22, 2023
@colindean colindean deleted the patch-1 branch January 22, 2023 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants