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

feat: Add macOS to build matrix #102

Merged
merged 8 commits into from
Sep 6, 2024
Merged

Conversation

Saadnajmi
Copy link
Contributor

@Saadnajmi Saadnajmi commented Sep 6, 2024

Add macOS to the build matrix for dawn, and add the libraries to the xcframework. I can't get dawn building locally (most likely to my using Xcode 16 Beta). However, if this PR lands (and CI passes), then we'll have prebuilt macOS binaries available :).

@wcandillon
Copy link
Owner

indeed the renaming from iOS to apple is needed now.

@Saadnajmi Saadnajmi changed the title Add macOS to build matrix feat: Add macOS to build matrix Sep 6, 2024
@Saadnajmi Saadnajmi marked this pull request as ready for review September 6, 2024 16:02
@Saadnajmi
Copy link
Contributor Author

Marking this as ready for review with a few caveats. I can't build dawn locally, most likely because I am using the Xcode 16 beta. I'll try with Xcode 15 later. Because of this, I'm not entirely sure the PR is accurate but comparing with #88 I think it looks mostly correct? And if this works on your machine (and Ci) then we also get prebuilt binaries from the CI artifacts 😀

});

libs.forEach((lib) => {
console.log(`Building fat binary for macOS: ${lib}`);
Copy link
Owner

Choose a reason for hiding this comment

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

This is not needed right?

"xcodebuild -create-xcframework " +
`-library ${projectRoot}/libs/apple/${lib}_macos.a ` +
`-library ${projectRoot}/libs/apple/arm64_macosx/${lib}.a ` +
` -output ${projectRoot}/libs/apple/${lib}_macos.xcframework `,
Copy link
Owner

Choose a reason for hiding this comment

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

this probably should be (no fatlib for macos because there is no macos simulator):

    // macOS
    $(
      "xcodebuild -create-xcframework " +
        `-library ${projectRoot}/libs/apple/x86_64_macosx/${lib}.a ` +
        `-library ${projectRoot}/libs/apple/arm64_macosx/${lib}.a ` +
        ` -output ${projectRoot}/libs/apple/${lib}_macos.xcframework `,
    );

@@ -8,7 +8,7 @@ index 046a6af10d..5a63ac3d6d 100644

- add_library(${output_target} SHARED ${all_objects})
-
+ if(${CMAKE_SYSTEM_NAME} STREQUAL "iOS" OR ${CMAKE_SYSTEM_NAME} STREQUAL "visionOS")
+ if(${CMAKE_SYSTEM_NAME} STREQUAL "iOS" OR ${CMAKE_SYSTEM_NAME} STREQUAL "visionOS" OR ${CMAKE_SYSTEM_NAME} STREQUAL "Darwin")
Copy link
Owner

Choose a reason for hiding this comment

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

🙌🏼

@wcandillon
Copy link
Owner

looking good. Due to the way podspecs work at the moment, I suggest that we build UNIVERSAL or MAC_CATALYST_ARM64. If we want to ship the arm64 to the x86 separately, we would need to taker a completely different approach.

@wcandillon wcandillon self-requested a review September 6, 2024 19:48
@wcandillon wcandillon merged commit 13bdf17 into wcandillon:main Sep 6, 2024
1 of 5 checks passed
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