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!: Better Catalyst build support #1313

Merged
merged 5 commits into from
Aug 30, 2024
Merged

feat!: Better Catalyst build support #1313

merged 5 commits into from
Aug 30, 2024

Conversation

dpogue
Copy link
Member

@dpogue dpogue commented Apr 16, 2023

Platforms affected

iOS, macOS

Motivation and Context

With the deprecation of cordova-osx and the availability of Catalyst to deploy iOS apps on macOS as their own app bundles, we should support targeting Catalyst builds.

Closes #420.
Closes #677.
Closes #1274.
Closes #1417.

Description

After the work in #1310 caused the build output folder to be named based on the platform, I realized this actually solved one of the remaining questions around where to put Catalyst output files... now they end up in build/Debug-maccatalyst or build/Release-maccatalyst

We detect if we are building for Catalyst (as opposed to iOS Device vs iOS Simulator) by the --target=mac option.

Testing

cordova build ios --target=mac works
cordova run ios --target=mac works

Checklist

  • I've run the tests to see all new and existing tests pass
  • I added automated test coverage as appropriate for this change
  • I've updated the documentation if necessary

@codecov-commenter
Copy link

codecov-commenter commented Apr 16, 2023

Codecov Report

Attention: Patch coverage is 69.76744% with 13 lines in your changes missing coverage. Please review.

Project coverage is 81.56%. Comparing base (b6ae567) to head (1372d4f).

Files with missing lines Patch % Lines
lib/build.js 56.25% 7 Missing ⚠️
lib/run.js 77.77% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1313      +/-   ##
==========================================
+ Coverage   80.22%   81.56%   +1.34%     
==========================================
  Files          16       16              
  Lines        1871     1888      +17     
==========================================
+ Hits         1501     1540      +39     
+ Misses        370      348      -22     

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

@dpogue dpogue added this to the 7.1.0 milestone Jun 6, 2023
@dpogue dpogue changed the title VERY WIP: Better Catalyst build support WIP: Better Catalyst build support Mar 29, 2024
@dpogue dpogue modified the milestones: 7.1.0, 8.0.0 Mar 29, 2024
@dpogue dpogue changed the title WIP: Better Catalyst build support feat!: Better Catalyst build support Aug 20, 2024
dpogue and others added 3 commits August 28, 2024 19:17
`cordova build ios --device --target=mac ...`

Output is to `build/Debug-maccatalyst`
Xcode warns that only one can be enabled at a time, so we need to pick
one, and Catalyst seems like the better option for a proper macOS app
experience.
If neither `--device` nor `--emulator` are specified for the build
command, it will check for a connected device and assume `--device` if
one is found. However, it was also checking for the availability of the
ios-deploy tool which is used to deploy to a connected device.

If we're just building, we don't need to check for a deploy tool. The
run command already has this check to ensure that ios-deploy is
available before actually trying to deploy.

Closes apacheGH-420.
Closes apacheGH-677.
Copy link
Member

@erisu erisu left a comment

Choose a reason for hiding this comment

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

LGTM

Tested mobilespec on iOS and Catalyst builds.

Geolocation plugin might need additional investigation for Catalyst.

@dpogue
Copy link
Member Author

dpogue commented Aug 30, 2024

Geolocation plugin might need additional investigation for Catalyst.

Looks like location access needs a capability to be turned on in the entitlements file for macOS/Catalyst. Once you do that, it works as expected. We can update the plugin to inject that.

@dpogue dpogue merged commit f81013b into apache:master Aug 30, 2024
10 checks passed
@dpogue dpogue deleted the catalyst branch August 30, 2024 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants