-
Notifications
You must be signed in to change notification settings - Fork 29
Fix Apple and Android tools for m137 #32
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
base: main
Are you sure you want to change the base?
Conversation
522e9ab
to
0497c0c
Compare
@pblazej pushed fixes for the android prefixed build |
Whoops, forgot to mention that the VERSION file holds the commit that the CI uses by default. I've updated the commit to point at the |
Yup @davidliu it does the trick https://github.com/webrtc-sdk/webrtc-build/actions/runs/16111693894. Thanks a lot for taking a look! It should be ready for integration testing on Android then, right? |
I ran through some small testing with the build last week and seemed to work fine. Should be good to go. |
BTW @davidliu which platforms are actually used here (I mean linux and rasp ones)
I'll be probably doing another round of cleanup + final Apple script. |
5a083c8
to
47bd647
Compare
47bd647
to
b1b5fef
Compare
e720003
to
b0ea15f
Compare
32365ea
to
e43106a
Compare
@hiroshihorie I'm almost there with the build script, fixing the paths and misc stuff. Feel free to take a look, especially at global configs and build flags - is it 100% consistent with what you've used? Once we get the artifact, we can try automating the push to https://github.com/livekit/webrtc-xcframework, or at least use consistent release numbers (based on tags like @davidliu did here) with Android, still pushing to the above repo manually and copying the hash, etc. |
Looks like it builds for iOS 🎉 https://github.com/webrtc-sdk/webrtc-build/actions/runs/16170404165 so the rest is a matter of the script itself |
.github/workflows/build.yml
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do some cleanup in this file, mostly unused platforms, then remove "old" macOS/iOS stuff from run.py
I think the xcframework.zips seem to be nested in an outer dir, can we fix this ? |
I'm not sure what's going on 😅 I think double clicking the .zip file on mac automatically un-wraps the nested dir ?
|
I think it's due to https://github.com/actions/toolkit/blob/master/packages/artifact/docs/additional-information.md#is-my-artifact-compressed. We can probably skip zipping in bash and just upload with:
name: ${{ steps.build.outputs.framework_name }}.xcframework
path: build/_package/${{ matrix.name }}/${{ steps.build.outputs.framework_name }}.xcframework # here Feel free to test it, or I'll do that tomorrow morning. |
@hiroshihorie I think the action won't upload symlinks https://github.com/actions/upload-artifact#altering-compressions-level-speed-v-size so we need to unzip that "manually" (or automatically) before releasing anyway. |
This reverts commit 0cda6c6.
WEBRTC_VERSION=137.7151.00 | ||
WEBRTC_READABLE_VERSION=M137.7151@{#0} | ||
WEBRTC_COMMIT=blaze/m137-patching |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Waiting for the main branch to be merged, then we can override this and release 🤞
apple(_prefixed)
targets with xcframework stuff