Skip to content

Apple: XCFramework build and distribution tooling - #5259

Merged
sauwming merged 9 commits into
masterfrom
apple-xcframework-distribution
Sep 18, 2026
Merged

sauwming merged 9 commits into
masterfrom
apple-xcframework-distribution

Conversation

@sauwming

@sauwming sauwming commented Sep 15, 2026

Copy link
Copy Markdown
Member

Adds build/apple/, which builds PJSIP as a binary PJSIP.xcframework for iOS, the iOS simulator and macOS, along with the SwiftPM and CocoaPods manifests that distribute it. Nothing here changes the library, and an ordinary source build is unaffected.

Motivation: CocoaPods Trunk becomes permanently read-only on 2 December 2026, so a binary distribution needs a route that does not depend on it.

How it builds

With CMake, out of tree. The working tree is untouched except pjlib/include/pj/config_site.h, which is backed up and restored. Each slice is configured and built once with both of its architectures in the same pass, so three configures cover the three slices.

The configuration is written down once, as CMake options in the build script, leaving config_site.h to carry only the four settings CMake has no option for. The TLS backend shows why that is better than the autotools route this originally used: autotools has no --with-ssl=apple at all and the choice has to be forced through config_site.h against the darwin probe, where CMake takes PJLIB_WITH_SSL=apple directly.

CMAKE_BUILD_TYPE is Release, so -O3 -DNDEBUG. Assertions compile out; PJ_ASSERT_RETURN still returns its error code.

What the artifact contains

  • TLS from Apple's Network framework, not OpenSSL.
  • No DTLS-SRTP, which follows: transport_srtp_dtls.c is implemented against OpenSSL only. SDES-SRTP is unaffected, but there is no WebRTC interoperability.
  • Video with H.264 through VideoToolbox as the only codec, AVFoundation capture, and Metal plus OpenGL ES rendering.
  • Opus built from a checksum-pinned 1.6.1 release and bundled. AMR, G.729 and G.722.1 excluded for patent and copyleft reasons.
  • Echo cancellation from CoreAudio. Neither the Speex nor the WebRTC canceller is built — Speex deliberately, and WebRTC because the CMake build has no WebRTC on Apple (webrtc_aec3 does not compile on macOS, where PlatformThreadId is undefined, and the NEON flags break the x86_64 half of a simulator build).
Slice Architectures Minimum OS Size
ios-arm64 arm64 iOS 15.0 4.9 MB
ios-arm64_x86_64-simulator arm64, x86_64 iOS 15.0 9.7 MB
macos-arm64_x86_64 arm64, x86_64 macOS 11.0 10 MB

Two parts worth reviewing closely

The shipped headers are an ABI contract. Public PJSIP headers contain inline code and layout-sensitive structures, so a consumer compiling against different macro values than the binary was built with gets silent memory corruption rather than a link error. The distribution therefore freezes the build's configuration into the headers it ships — around 585 macros per slice: everything pjproject declares overridable with the #ifndef/#define idiom, everything used as an array dimension, and every -D the build puts on the compile line.

Pinning a value must never change one, and several idioms make that easy to get wrong: a guard block that also defines siblings loses them once it stops firing, a bare defined(X) branch flips the moment X exists at all, and values derived from the build's own -D flags resolve differently if the probe does not pass them. The build compares the fully preprocessed macro set before and after freezing and fails if anything moved. That check caught five real cases, including PJMEDIA_AUDIO_DEV_HAS_COREAUDIO — the freeze would otherwise have silently disabled the audio backend.

Non-API symbols are hidden. Every defined symbol outside the public pj API is demoted to a local symbol. Without it the archive exports symbols from libsrtp, libyuv and Opus, which are exactly what a WebRTC-based SDK bundles, so an application linking both would fail on duplicate definitions. Exports are narrowed to the C API, the pj namespace including its vtables and typeinfo, and std:: template instantiations. The deny list is computed from the merged object rather than written down, so a library added later cannot leak by being forgotten.

Both are documented at length in build/apple/README.md.

Verification

build/apple/tests/verify-xcframework.sh checks a built framework the way a consumer would, in three tiers:

  1. Per slice, compiled with no -D flags at all — itself the test that the build's macros were frozen into the headers. Asserts the configuration, compiles the Clang module and public headers, checks that a consumer -D cannot move the ABI, links the archive beside an object defining the same third-party symbols a WebRTC SDK would, and confirms nothing outside the pj API is exported.
  2. A SwiftPM consumer that builds and runs, using the linker settings copied verbatim from the shipped manifest.
  3. A real iOS app installed on a booted simulator: opens UDP and TLS transports (the check that the Apple backend can open a listener rather than merely link), starts pjsua, and enumerates audio and video devices.

All three pass, with tier 3 reporting the same device counts the autotools-built artifact did. The suite was also checked negatively: corrupting a frozen macro fails tier 1 with the exact assertion, and removing a framework from Package.swift.in fails the link on all three slices.

Also included

Two CMake fixes that are not strictly part of the tooling but that a CMake-built distribution needs. Kept here rather than split out, by maintainer preference:

  • iOS OpenGL ES renderer. PJMEDIA_WITH_VIDEODEV_OPENGL was probed with find_package(OpenGL COMPONENTS GLES2), which never succeeds on iOS — the ES implementation there is OpenGLES.framework. The option was forced off and the renderer silently dropped, although ios_opengl_dev.m is in the source list and the autotools build ships it.
  • PJ_IOS_SAMPLE_HEADERS. The iOS sample staging copies five generated headers into the source tree, and CMake puts the source include directory ahead of the binary one, so a later build with different options silently compiles against whatever configured the tree last. The new option lets a build opt out; the distribution build refuses to start if those headers are present at all. Note the hazard is not new — an autotools build writes those same paths by design — so this is an escape hatch rather than a cure.

Known gaps

  • No WebRTC echo canceller, unlike an autotools build, for the reasons above. CoreAudio's own canceller is used instead.
  • PJMEDIA_HAS_VPX_CODEC and PJMEDIA_HAS_OPENH264_CODEC are set() in CMake but absent from config_auto.h.cm, so those settings never reach the headers. Pre-existing, not Apple-specific.
  • find_package(Pj) fails on any host where FFMPEG is enabled: FindFFMPEG.cmake includes Pj/GetMacroValue, but only the Find modules are installed, not the Pj/ helpers two of them include. Pre-existing.
  • Everything here has only been run on Apple Silicon with Xcode 26.2. The test script derives architecture and deployment target from the artifact, so the Intel path should be correct, but it is untested.
  • No CI runs any of this yet.

Adds build/apple/, which builds PJSIP as a binary PJSIP.xcframework for
iOS, the iOS simulator and macOS, plus the SwiftPM and CocoaPods manifests
that distribute it. Nothing here changes the library or affects an ordinary
source build.

TLS comes from Apple's Network framework rather than OpenSSL, so the
artifact links no OpenSSL. DTLS-SRTP is consequently unavailable, as
transport_srtp_dtls.c is implemented against OpenSSL only. Video is enabled
with H.264 through VideoToolbox as the only video codec. Opus is built from
a checksum-pinned release and bundled; AMR and bcg729 are excluded for
patent and copyleft reasons respectively.

Two parts are less obvious than they look, and are documented at length in
build/apple/README.md:

- The shipped headers are an ABI contract. Public PJSIP headers contain
  inline code and layout-sensitive structures, so the distribution freezes
  the build's configuration into the headers it ships: config_site.h per
  slice, PJ_AUTOCONF injected into pj/config.h, and every -DPJ* macro the
  build passes on the command line appended per slice, read back from the
  build's own CFLAGS. Several public headers branch on those macros.

- Every symbol outside the public pj API is demoted to a local symbol
  before the final archive is written. Without it the archive exports
  around 1,200 third-party symbols from libsrtp, libyuv, the WebRTC AEC and
  Opus, which are exactly what a WebRTC based SDK bundles, so linking both
  would fail on duplicate definitions.

build/apple/tests/verify-xcframework.sh checks a built framework the way a
consumer would use it, in three tiers: per-slice header, symbol and link
checks compiled with no -D flags at all; a SwiftPM consumer that builds and
runs using the shipped manifest's linker settings; and a real iOS app
installed on a booted simulator that opens UDP and TLS transports, starts
pjsua and enumerates audio and video devices.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sauwming sauwming self-assigned this Sep 15, 2026
@sauwming sauwming added this to the release-2.18 milestone Sep 15, 2026
@sauwming
sauwming requested a balanced review from Copilot September 16, 2026 00:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

ABI freezing, symbol hiding, configuration cleanup, codec selection, deployment metadata, and teardown verification contain correctness gaps.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds tooling to build, package, distribute, and verify PJSIP as an Apple XCFramework.

Changes:

  • Builds iOS, simulator, and macOS binary slices with pinned dependencies.
  • Adds SwiftPM and CocoaPods distribution manifests.
  • Adds three-tier consumer verification and documentation.
File summaries
File Description
.gitignore Ignores generated distribution output.
build/apple/build-xcframework.sh Builds and packages the XCFramework.
build/apple/config_site.h Defines distribution configuration.
build/apple/module.modulemap Defines the Clang module.
build/apple/Package.swift.in Provides the SwiftPM manifest template.
build/apple/PJSIP.podspec.in Provides the CocoaPods specification.
build/apple/PJSIPUmbrella.h Exposes the framework’s C headers.
build/apple/PrivacyInfo.xcprivacy Declares privacy API usage.
build/apple/README.md Documents building, verification, and release.
build/apple/spm/Sources/PJSIPLinkerSettings/include/PJSIPLinkerSettings.h Declares the linker-settings shim module.
build/apple/spm/Sources/PJSIPLinkerSettings/shim.c Implements the linker-settings shim.
build/apple/tests/tier1/config_assertions.c Checks shipped configuration macros.
build/apple/tests/tier1/symbol_clash.c Tests third-party symbol collisions.
build/apple/tests/tier1/use_api.m Compiles the public module and API.
build/apple/tests/tier2/Package.swift.template Defines the SwiftPM test consumer.
build/apple/tests/tier2/Sources/App/main.swift Exercises the framework from Swift.
build/apple/tests/tier3/Info.plist Configures the simulator test app.
build/apple/tests/tier3/main.m Exercises transports and media devices.
build/apple/tests/verify-xcframework.sh Runs the three verification tiers.
Review details
  • Files reviewed: 18/19 changed files
  • Comments generated: 7
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread build/apple/build-xcframework.sh Outdated
Comment thread build/apple/build-xcframework.sh Outdated
Comment thread build/apple/build-xcframework.sh Outdated
Comment thread build/apple/build-xcframework.sh
Comment thread build/apple/build-xcframework.sh Outdated
Comment thread build/apple/build-xcframework.sh Outdated
Comment thread build/apple/tests/tier3/main.m
Seven issues raised in review, all verified against a rebuilt artifact.

ABI freezing was incomplete. Only macros the build passed on the command
line were frozen into the shipped headers, leaving header defaults that a
consumer could still override from its own command line: PJSIP_MAX_MODULE,
PJSIP_MAX_URL_SIZE and around sixty more size public structure arrays, so
overriding one compiles against a different layout with no link error. The
set is now found by scanning the shipped headers for macros used as array
dimensions, plus any macro those values refer to, and pinned to the values
the binary was built with. Tier 1 now compiles with deliberately conflicting
-D values and fails if any of them takes effect.

Symbol hiding exempted every Itanium-mangled C++ symbol, which left 63
global-namespace pjsua2 helper classes exported and would have leaked a
bundled C++ library wholesale. The exemption is now limited to the pj
namespace, including its vtables and typeinfo, and to std:: template
instantiations, which are weak and meant to be shared. The verifier repeated
the same blanket rule and so could not detect this; it now shares the
narrowed one.

G.722.1 was documented as excluded but --disable-g7221-codec was never
passed, so libg7221codec was linked into every slice. Its wrapper is off by
default, which is what made the omission easy to miss.

NO_OPUS produced an artifact without Opus while the generated manifests
still advertised it and the verifier required it. It no longer generates
manifests, and is documented as an iteration aid rather than a build mode.

The SwiftPM platform version truncated the minor component, so
IOS_DEPLOYMENT_TARGET=15.1 advertised .v15. The exact value is now emitted
using the string form.

config_site.h was left installed when the checkout had none of its own, so a
subsequent ordinary source build silently inherited the distribution
configuration. The original presence is now tracked and the file removed
when it was absent; a backup left by an interrupted run is no longer
restored over the current one.

Tier 3 printed its success marker before pjsua_destroy(), and simctl does
not surface an exit code, so a teardown failure still passed. Teardown now
runs and is checked first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sauwming

Copy link
Copy Markdown
Member Author

All seven findings addressed in 55f72c7, verified against a full rebuild.

ABI freezing was incomplete. Correct, and this was the most substantive one. Only command-line macros were frozen; header defaults such as PJSIP_MAX_MODULE and PJSIP_MAX_URL_SIZE remained overridable, and they size arrays inside public structures. The set is now derived by scanning the shipped headers for macros used as array dimensions — 66 of them — plus the transitive closure over macros those values refer to, which pulled in PJ_ICE_COMP_BITS (reached via PJ_ICE_MAX_COMP = (1<<PJ_ICE_COMP_BITS)), exactly the gap that would have let an override still change the result. 101 macros are now pinned per slice. Tier 1 compiles with -DPJSIP_MAX_MODULE=9999 and three others and fails if any takes effect; that check fails against the pre-fix artifact and passes against the new one.

The __Z exemption was too broad. Correct. 63 global-namespace pjsua2 helper classes (PendingOnDtmfDigitCallback, DevAudioMedia, CodecFmtpUtil, call_param) were exported, and the structural point about a future bundled C++ library stands. The exemption is now limited to the pj namespace including vtables and typeinfo, and to std:: template instantiations, which are weak and meant to be shared. Non-pj/std exported C++ symbols: 63 → 0. The verifier shared the blanket rule, as you noted, and now shares the narrowed one.

G.722.1. Correct — --disable-g7221-codec was never passed and libg7221codec (160K) was linked into every slice. Now passed; the library is no longer built at all.

NO_OPUS. Correct. It now skips manifest generation entirely rather than emitting metadata that contradicts the artifact, and is documented as an iteration aid, not a release mode.

Deployment target minor version. Correct. Now emitted as .iOS("15.0") using the string form, preserving the exact value.

config_site.h left installed. Correct, and the sibling case was real too — a backup from an interrupted run would have been restored over the current file. Original presence is now tracked; both branches unit-tested.

Tier 3 marker before teardown. Correct. pjsua_destroy() now runs and is checked before the marker is printed.

Two notes where I did not follow the suggestion exactly. std:: instantiations are kept exported deliberately rather than hidden — they are weak symbols intended to coalesce across the image, and hiding them risks exception-typeinfo mismatches across the boundary. And the ABI freeze pins macros rather than emitting a full resolved configuration dump: the array-dimension scan is evidence-based and bounded, where a dump would freeze thousands of macros including many that are not ABI-visible.

All three verification tiers pass on the rebuilt artifact, with no deployment-target warnings.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

ABI freezing can remain incomplete, and generated manifests can advertise unsupported slices or deployment targets.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

build/apple/build-xcframework.sh:59

  • The documented simulator floor is not enforced. Setting IOS_DEPLOYMENT_TARGET below 14.0 lets Clang record 14.0 in simulator objects while the generated SwiftPM and CocoaPods manifests advertise the lower value, producing an artifact whose metadata overstates compatibility. Reject values below 14.0 before building.
    build/apple/build-xcframework.sh:577
  • Partial builds still emit manifests that advertise both platforms. For example, the documented SLICES=macos workflow reaches this branch and generates a Package.swift and podspec declaring iOS support, although the XCFramework has no iOS slice. Suppress release manifests for partial builds, as for NO_OPUS, or render platform declarations from the slices actually present.

build/apple/build-xcframework.sh:326

  • This scan is not sufficient to freeze all public layouts: it only discovers macros used as a direct [MACRO] dimension and misses conditional members. For example, PJMEDIA_HAS_RTCP_XR adds fields to public pjmedia_rtcp_session (pjmedia/include/pjmedia/rtcp.h:274-285) but is not supplied by this config or CFLAGS, so a consumer can override it and compile a different structure. Collect conditional layout switches too, or freeze all applicable object-like PJ configuration macros, and add a negative override test.
    find "$hdr" \( -name '*.h' -o -name '*.hpp' \) -print0 \
        | xargs -0 grep -hoE '\[[A-Z][A-Z0-9_]{3,}\]' 2>/dev/null \
        | tr -d '[]' | sort -u \
  • Files reviewed: 19/20 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread build/apple/build-xcframework.sh Outdated
Second round of review findings on the XCFramework tooling.

The layout freeze missed conditional structure members. Scanning only for
macros used as array dimensions could not see switches like
PJMEDIA_HAS_RTCP_XR, which adds two fields to the public
pjmedia_rtcp_session, so a consumer could still define it and compile a
different structure. The scan now also collects every macro pjproject itself
declares overridable with the #ifndef/#define idiom, which is precisely the
set a consumer might try to set. That takes the pinned set from 101 to 635
macros per slice.

Macros whose body invokes a function-like macro are excluded, because
pre-defining such a name skips the #ifndef block that also defines its
helper and leaves the body calling something undefined. Two qualify, and
neither decides a layout: PJNATH_STUN_SOFTWARE_NAME builds a string, and
PJSIP_MAX_TIMER_COUNT reads runtime configuration through pjsip_cfg().

A failure to preprocess the staged headers returned success and produced an
empty macro set. Because the command-line macros were collected separately
the non-empty check still passed, so the build could package headers with no
layout macros pinned at all. It is now fatal.

IOS_DEPLOYMENT_TARGET below 14.0 is rejected. Clang records 14.0 in
simulator objects regardless, so a lower value only made the generated
manifests advertise compatibility the artifact does not have.

A partial SLICES build no longer generates manifests. They declare both
platforms and one checksum for the whole artifact, so a subset build was
emitting a Package.swift and podspec claiming slices that were not present.

Tier 1 now also asserts that PJMEDIA_HAS_RTCP_XR cannot be overridden,
covering the conditional-member case rather than only array dimensions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sauwming

Copy link
Copy Markdown
Member Author

Second round addressed in 5e99e95, verified against a full rebuild.

Preprocessing failure was not fatal. Correct, and this was the important one — it is the same silent-no-op shape as the missing-marker bug from the last round. abi_macros() returned success on failure, and because cflags_macros() still produced entries the non-empty check downstream passed, so the build could have shipped headers with no layout macros pinned at all. Now fatal.

The array-dimension scan missed conditional members. Correct, and PJMEDIA_HAS_RTCP_XR was exactly the right example — I confirmed it adds xr_enabled and xr_session to pjmedia_rtcp_session and was not in the frozen set. The scan now also collects every macro pjproject declares overridable with the #ifndef/#define idiom, which is precisely the set a consumer might try to set and is a superset of the array dimensions. 101 → 635 pinned macros per slice. Tier 1 now asserts PJMEDIA_HAS_RTCP_XR cannot be overridden.

Worth recording that the first attempt at this broke the headers, and only the test suite caught it. Pinning PJNATH_STUN_SOFTWARE_NAME skips the #ifndef block that also defines its helper PJNATH_MAKE_SW_NAME2, so the pinned body called a macro that no longer existed and all three tiers failed to compile. Macros whose body invokes a function-like macro are now excluded. Exactly two qualify and neither decides a layout: that one builds a string, and PJSIP_MAX_TIMER_COUNT reads runtime config through pjsip_cfg().

Simulator floor not enforced. Correct. IOS_DEPLOYMENT_TARGET below 14.0 is now rejected before the build starts, rather than producing manifests that overstate compatibility.

Partial builds emitted full manifests. Correct — SLICES=macos generated a Package.swift and podspec declaring iOS support with no iOS slice present. Partial builds now suppress manifests the same way NO_OPUS does, and the closing summary no longer points at files it did not write. Verified by running SLICES=macos: one slice, no manifests.

All three tiers pass on the rebuilt full artifact, no deployment-target warnings.

@sauwming
sauwming requested a review from nanangizz September 16, 2026 03:38
sauwming and others added 4 commits September 17, 2026 12:00
CMake puts the source include directory ahead of the binary one, so a
generated header sitting in the source tree shadows the one the current
build generated. PJ_IOS_SAMPLE_LIBS copies five of them there for the Xcode
samples, which means a later build with different options silently compiles
against whatever configured the tree last. Configuring with
PJMEDIA_WITH_G7221_CODEC=OFF produced a correct config_auto.h in the build
directory and still failed, because an earlier build's copy said the codec
was enabled.

The hazard is not new -- the autotools build writes those same five paths by
design, so any tree that has run ./aconfigure poisons a CMake build the same
way -- but the copy step reaches it without autotools.

PJ_IOS_SAMPLE_HEADERS, on by default so the samples keep working, lets a
build that must not be influenced by the state of the tree opt out.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The distribution was built through configure-iphone and make, which meant
distcleaning and reconfiguring the working tree once per architecture, five
times for a full run. CMake builds out of tree and takes both architectures
of a slice in one pass, so the tree is no longer touched at all except for
config_site.h, and three configures replace five.

It also describes the distribution better. Every setting except DTLS-SRTP,
the iLBC backend and two quality tunables now has a CMake option, so the
configuration is written down once in the build script instead of being
split between configure flags and config_site.h. The TLS backend is the
clearest case: autotools has no --with-ssl=apple at all and the choice had
to be forced through config_site.h against the darwin probe, where CMake
takes PJLIB_WITH_SSL=apple directly.

The artifact loses the WebRTC echo canceller, which the CMake build does not
offer on Apple: webrtc_aec3 does not compile on macOS, where PlatformThreadId
is undefined, and the NEON flags break the x86_64 half of a simulator build.
CoreAudio has its own canceller, and the distribution already disabled the
Speex one on those grounds, so this follows the same reasoning. Tier 1 now
asserts the headers say so rather than ignoring it.

Slices are correspondingly smaller: 4.9, 9.7 and 10 MB against 6.5, 13 and
14 before. CMAKE_BUILD_TYPE is Release, so the build is -O3 -DNDEBUG;
assertions compile out while PJ_ASSERT_RETURN still returns its error code.

Two things needed care. Prelinking is inherently per architecture, because
ld -r takes one -arch and would otherwise thin a fat archive, so each slice
is prelinked once per architecture and recombined -- the saving is in
configuring and compiling once, not in the linking. And an iOS build searches
only inside its sysroot, so Opus needs CMAKE_FIND_ROOT_PATH as well as
CMAKE_PREFIX_PATH or it is silently dropped.

The script refuses to start when the generated headers are present in the
source tree, since CMake puts the source include directory first and they
would shadow the ones the build generates.

Verified: all three slices build, and all three verification tiers pass,
including the iOS app on a simulator reporting the same device counts as the
autotools artifact did.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PJMEDIA_WITH_VIDEODEV_OPENGL was probed with find_package(OpenGL COMPONENTS
GLES2), which never succeeds on iOS: the ES implementation there is
OpenGLES.framework, not the GL and GLES2 libraries FindOpenGL looks for. The
option was therefore forced off and the renderer silently dropped, although
ios_opengl_dev.m is in the source list and the autotools build ships it.

iOS now looks for the framework instead and links it, leaving the existing
probe for every other platform.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sauwming sauwming changed the title Apple: add XCFramework build and distribution tooling Apple: XCFramework build and distribution tooling Sep 17, 2026
@sauwming
sauwming requested a balanced review from Copilot September 17, 2026 05:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Exported OpenGLES handling, universal-slice architecture metadata, and several documented build modes currently produce incorrect behavior.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 21/22 changed files
  • Comments generated: 7
  • Review effort level: Balanced

Comment thread build/apple/build-xcframework.sh Outdated
Comment thread build/apple/build-xcframework.sh Outdated
Comment thread build/apple/build-xcframework.sh Outdated
Comment thread pjmedia/CMakeLists.txt Outdated
Comment thread build/apple/build-xcframework.sh
Comment thread build/apple/tests/tier3/Info.plist
Comment thread pjmedia/CMakeLists.txt Outdated
The fat slices named a single architecture. CMake configures once per
slice, so pj_detect_arch() sees only the first of CMAKE_OSX_ARCHITECTURES
and the m_auto.h it generates gives the whole slice that architecture's
PJ_M_NAME -- an x86_64 half reported arm64 to its own sources and to
consumers. Rather than configure once per architecture, the machine name
and its PJ_M_* macro now dispatch on the compiler's own architecture
macro, applied to the build tree before compiling so the library and the
headers staged from it cannot disagree. The two other machine values are
checked to be common to the slice: floating point, and the endianness
that Darwin already derives from __BIG_ENDIAN__.

Replace the bare find_library() for OpenGLES with a Find module that
publishes OpenGLES::OpenGLES, and select it over OpenGL in the installed
package config. find_dependency(OpenGL) could not succeed on iOS, which
is why the branch exists at all, so find_package(Pj) failed there; the
imported target also keeps the build machine's SDK path out of the
exported link interface. The framework is now gated on the feature
option as well, so PJMEDIA_WITH_VIDEODEV_OPENGL=OFF no longer links it.

In the build script: NO_OPUS left slice_libs() returning the result of
its own [ -n "$prefix" ] test, which set -e turned into an abort, so the
documented iteration mode could not complete; the archive list is read
into an array, so a path containing spaces survives; and the two
renderers the artifact promises join the options checked against
CMakeCache.txt, Metal for every slice and OpenGL ES for the iOS ones.

The tier 3 bundle took its MinimumOSVersion from a fixed plist while the
compiler target came from the artifact, so a 14.x artifact would fail to
install on a matching simulator because of the harness; it is now set
from the slice's own minimum.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The build can fail under non-Make CMake generators, interrupted runs can overwrite user configuration backups, and deployment-target probe failures are masked.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

build/apple/build-xcframework.sh:703

  • The macro-freezing step later reads flags.make, which only Makefile generators create. CMake honors CMAKE_GENERATOR, so an environment configured for Ninja or Xcode will build successfully here and then abort in cflags_macros(), despite the script documenting only a CMake requirement. Force the Unix Makefiles generator for this build (or make macro extraction generator-independent).
    build/apple/tests/verify-xcframework.sh:105
  • A failed deployment-target probe silently becomes 15.0, which is not artifact-derived and is wrong for the default macOS 11 slice. The verifier can therefore compile against a newer target and report success without checking the artifact's actual minimum. Treat extraction/probing failure as a verification failure instead of inventing a version.
  • Files reviewed: 23/24 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread build/apple/build-xcframework.sh Outdated
A backup left by an interrupted run is the checkout's only copy of
config_site.h, not something to delete: after a kill that the EXIT trap
cannot catch, the tree holds this script's config and the backup holds
the user's. The old code removed it and backed up the injected file,
losing the original. Nothing can tell that apart from a backup that has
since gone stale, so refuse and let a human reconcile it.

Force the Unix Makefiles generator for every cmake call here. The freeze
step reads the build's own flags.make, which only the Makefile
generators write, so CMAKE_GENERATOR=Ninja in the environment built
fine and then aborted at the freeze -- and for Opus failed outright,
before the main build even started.

The deployment-target probe never worked. vtool prints the object's path
before the load command, the temp directory it extracts into is named
pjsip-minos.XXXXXX, and /minos/ matched that path line first, so awk
printed its empty second field. The 15.0 default then hid it, and the
macOS slice was being verified against macos15.0 rather than the 11.0 it
actually declares. Match on the field, and treat a probe failure as a
verification failure rather than inventing a version every caller then
compiles against.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@sauwming
sauwming merged commit be91f8d into master Sep 18, 2026
56 checks passed
@sauwming
sauwming deleted the apple-xcframework-distribution branch September 18, 2026 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants