-
Notifications
You must be signed in to change notification settings - Fork 137
Add universal build for macos (needed for obs-studio) #855
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
WalkthroughThis pull request extends the build system to support universal Apple Darwin targets. The GitHub Actions workflow is updated with a new matrix entry for 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/libmoq.yml:
- Around line 28-29: Replace the invalid Rust target "universal-apple-darwin"
with the two concrete targets "x86_64-apple-darwin" and "aarch64-apple-darwin"
wherever the workflow lists targets (search for the string "target:
universal-apple-darwin"); update any matrix or list used by the
dtolnay/rust-toolchain action so it installs both x86_64-apple-darwin and
aarch64-apple-darwin instead of the universal name, ensuring the build produces
a universal binary by building both architectures.
In `@rs/libmoq/build.sh`:
- Around line 119-128: The workflow currently attempts to rustup-install
"universal-apple-darwin" which is invalid; update the libmoq.yml job that
prepares the Rust targets so it installs both x86_64-apple-darwin and
aarch64-apple-darwin (the actual Rust targets) before invoking build.sh, since
build.sh (which checks TARGET and uses lipo to combine LIB_X86 and LIB_ARM64
when TARGET=="universal-apple-darwin") expects both architecture-specific
targets to be present; ensure the job runs rustup target add x86_64-apple-darwin
aarch64-apple-darwin on macos-latest prior to building.
| - target: universal-apple-darwin | ||
| os: macos-latest |
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.
Rust target installation will fail for universal-apple-darwin.
universal-apple-darwin is not a valid Rust target triple. The dtolnay/rust-toolchain action at line 39 will either fail or have no effect. The build script actually needs both x86_64-apple-darwin and aarch64-apple-darwin targets installed to compile the universal binary.
🐛 Proposed fix: Install both actual targets for universal builds
- name: Install Rust
uses: dtolnay/rust-toolchain@stable
with:
- targets: ${{ matrix.target }}
+ targets: ${{ matrix.target == 'universal-apple-darwin' && 'x86_64-apple-darwin,aarch64-apple-darwin' || matrix.target }}Also applies to: 36-39
🤖 Prompt for AI Agents
In @.github/workflows/libmoq.yml around lines 28 - 29, Replace the invalid Rust
target "universal-apple-darwin" with the two concrete targets
"x86_64-apple-darwin" and "aarch64-apple-darwin" wherever the workflow lists
targets (search for the string "target: universal-apple-darwin"); update any
matrix or list used by the dtolnay/rust-toolchain action so it installs both
x86_64-apple-darwin and aarch64-apple-darwin instead of the universal name,
ensuring the build produces a universal binary by building both architectures.
| # Copy header | ||
| cp "$INCLUDE_SOURCE" "$PACKAGE_DIR/include/" | ||
|
|
||
| # Copy static library | ||
| case "$TARGET" in | ||
| *-windows-*) | ||
| cp "$TARGET_DIR/moq.lib" "$PACKAGE_DIR/lib/" | ||
| ;; | ||
| *) | ||
| # Unix-like (macOS, Linux, etc.) | ||
| cp "$TARGET_DIR/libmoq.a" "$PACKAGE_DIR/lib/" | ||
| ;; | ||
| esac | ||
| if [[ "$TARGET" == "universal-apple-darwin" ]]; then | ||
| echo "Creating universal binary..." | ||
| lipo -create "$LIB_X86" "$LIB_ARM64" -output "$PACKAGE_DIR/lib/$LIB_FILE" | ||
| else | ||
| cp "$LIB_SOURCE" "$PACKAGE_DIR/lib/" | ||
| fi |
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.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the build.sh file
find . -name "build.sh" -path "*/libmoq/*" | head -5Repository: moq-dev/moq
Length of output: 75
🏁 Script executed:
# Search for the build.sh file
fd "build.sh" rs/libmoqRepository: moq-dev/moq
Length of output: 73
🏁 Script executed:
# Look for workflow files that might reference libmoq
fd -e "yml" -e "yaml" | grep -i "workflow\|action" | head -20Repository: moq-dev/moq
Length of output: 37
🏁 Script executed:
# Examine the build.sh file around lines 119-128
cat -n rs/libmoq/build.sh | sed -n '100,135p'Repository: moq-dev/moq
Length of output: 1272
🏁 Script executed:
# Check entire build.sh to understand variable definitions
wc -l rs/libmoq/build.shRepository: moq-dev/moq
Length of output: 77
🏁 Script executed:
# Search for workflow/CI files
fd -e "yml" -e "yaml" .githubRepository: moq-dev/moq
Length of output: 37
🏁 Script executed:
# Read the beginning of build.sh to understand setup and variable definitions
cat -n rs/libmoq/build.sh | head -100Repository: moq-dev/moq
Length of output: 3948
🏁 Script executed:
# Search for where LIB_X86 and LIB_ARM64 are defined
rg "LIB_X86|LIB_ARM64" rs/libmoq/Repository: moq-dev/moq
Length of output: 338
🏁 Script executed:
# Look for GitHub workflow files
find .github -type f \( -name "*.yml" -o -name "*.yaml" \) 2>/dev/nullRepository: moq-dev/moq
Length of output: 257
🏁 Script executed:
# Examine the libmoq.yml workflow file
cat .github/workflows/libmoq.ymlRepository: moq-dev/moq
Length of output: 2621
Workflow target configuration needs fixing for universal binary builds.
The packaging logic using lipo is correct. However, the libmoq.yml workflow incorrectly tries to install universal-apple-darwin as a Rust target (line 34), which doesn't exist. For the universal build job to succeed on macos-latest (ARM runner), the workflow should instead install both x86_64-apple-darwin and aarch64-apple-darwin targets before running build.sh. The build script itself correctly handles the "universal-apple-darwin" target by explicitly building both architectures and combining them with lipo, but this requires both underlying targets to be available.
🤖 Prompt for AI Agents
In `@rs/libmoq/build.sh` around lines 119 - 128, The workflow currently attempts
to rustup-install "universal-apple-darwin" which is invalid; update the
libmoq.yml job that prepares the Rust targets so it installs both
x86_64-apple-darwin and aarch64-apple-darwin (the actual Rust targets) before
invoking build.sh, since build.sh (which checks TARGET and uses lipo to combine
LIB_X86 and LIB_ARM64 when TARGET=="universal-apple-darwin") expects both
architecture-specific targets to be present; ensure the job runs rustup target
add x86_64-apple-darwin aarch64-apple-darwin on macos-latest prior to building.
In order to build the moqbs fork properly for macos, we need a unified build for macos that support both architectures.