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

Include musllinux builds #1054

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Include musllinux builds #1054

wants to merge 1 commit into from

Conversation

TECH7Fox
Copy link

Removes the filter to include musllinux builds for Alpine, which is very popular for containers, and more. In my case I need it specifically for Home Assistant. Tested it and the CI runs fine and includes the additional musllinux wheels.

Was there a reason to originally exclude it?

@TECH7Fox
Copy link
Author

@jlaine could you take a look at this? Would be very useful since we can't deploy to HA without this.

Copy link

codecov bot commented Mar 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (cbd1039) to head (3628e25).

Additional details and impacted files
@@            Coverage Diff            @@
##              main     #1054   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           30        30           
  Lines         5846      5846           
=========================================
  Hits          5846      5846           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@jlaine
Copy link
Collaborator

jlaine commented Mar 11, 2024

While I'm not opposed to this as such, this is not a trivial effort. The first step is going to be to make sure there are musllinux wheels for all binary dependencies:

It almost certainly involves producing binary builds of opus and vpx for musllinux:

https://github.com/aiortc/aiortc-codecs

The build system also needs to be updated to fetch the right version of these builds and put them in their own directory.

Finally we need some sort of smoke test to check the wheels are actually functional, or at the very least do not have missing symbols. Currently tests on the wheels are disabled because they took too long to run on all platforms. See #1059

@jlaine jlaine added invalid This doesn't seem right changes requested Some changes are required before the PR can be merged. and removed invalid This doesn't seem right labels Mar 11, 2024
@jlaine
Copy link
Collaborator

jlaine commented Mar 11, 2024

The linting error will go away if you rebase on top of main, which should allow you to view the failures with the wheels.

@TECH7Fox
Copy link
Author

Thank you for the message. For now I got aiortc successfully building in home-assistant/wheels-custom-integrations#684

So we can use it in HA. But this would still be helpful for others.

I will try to get this working when I have some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changes requested Some changes are required before the PR can be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants