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

Add support for building CoreCLR for MacCatalyst/iOS simulator #109928

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

filipnavara
Copy link
Member

Re-open and rebase of #98127

Build instructions:

  • Build the runtime pack and tools: ./build.sh clr+clr.runtime+libs+packs -os [iossimulator/maccatalyst] -arch [x64/arm64] -cross -c Release
  • Run the sample app: ./dotnet.sh publish src/mono/sample/iOS/Program.csproj -c Release /p:TargetOS=maccatalyst /p:TargetArchitecture=arm64 /p:DeployAndRun=true /p:UseMonoRuntime=false /p:RunAOTCompilation=false /p:MonoForceInterpreter=false

Related work:

Notably, this doesn't include CI scripts to build this or the runtime packs. I am open to suggestions on how to better split this into more digestible/reviewable chunks.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 18, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Nov 18, 2024
src/coreclr/hosts/inc/coreclrhost.h Outdated Show resolved Hide resolved
src/coreclr/pal/src/CMakeLists.txt Outdated Show resolved Hide resolved
@@ -494,7 +494,7 @@ if new_level is -1, the nesting level will not be modified
--*/
int DBG_change_entrylevel(int new_level);

#ifdef __APPLE__
#ifdef TARGET_OSX
Copy link
Member

Choose a reason for hiding this comment

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

Is TARGET_OSX a subset of TARGET_APPLE?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is.

src/coreclr/pal/src/map/virtual.cpp Outdated Show resolved Hide resolved
src/native/corehost/hostpolicy/hostpolicy_context.cpp Outdated Show resolved Hide resolved
src/native/libs/System.Native/entrypoints.c Outdated Show resolved Hide resolved
@@ -228,6 +228,9 @@ endif(CLR_CMAKE_TARGET_WIN32)

# add the install targets
install_clr(TARGETS coreclr DESTINATIONS . sharedFramework COMPONENT runtime)
if(CLR_CMAKE_HOST_MACCATALYST OR CLR_CMAKE_HOST_IOS)
install_clr(TARGETS coreclr_static DESTINATIONS . sharedFramework COMPONENT runtime)
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to install this?

Copy link
Member Author

Choose a reason for hiding this comment

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

iOS generally restricts linking to static libraries or dynamic frameworks distributed with the app itself. The aim was to include statically built CoreCLR in the runtime pack.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if that means that the single file is the only deployment mechanism on iOS. If it is the case, then I am not sure what would be the scenario where developers would explicitly use the static version of coreclr.

Copy link
Member

Choose a reason for hiding this comment

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

Linking with a static library typically results in smaller apps, which is why we've always linked Mono statically by default.

Linking dynamically can make the build a little bit faster (from past experience in Xamarin, we never ported this to .NET when we migrated due to time constraints).

Copy link
Member

Choose a reason for hiding this comment

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

The single file is a host statically linked to all the native libraries including coreclr. The scenario when the developers would need static coreclr library would be when they want to use their own host. Thinking about it more, I guess distributing the static coreclr version actually makes sense to enable such scenarios.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that sounds like what we want.

Note that .dylib won't do (Apple doesn't allow them in iOS apps), each dynamic library has to be made into a .framework (which works).

FWIW Apple has recommended using no more than 6-8 .frameworks because otherwise it affects startup performance.

src/coreclr/hosts/inc/coreclrhost.h Outdated Show resolved Hide resolved
@@ -44,7 +44,7 @@ Module Name:

#endif // HOST_UNIX

#if defined(TARGET_OSX) && defined(HOST_ARM64) && !defined(HAVE_UNW_AARCH64_X19)
#if defined(TARGET_APPLE) && defined(HOST_ARM64) && !defined(HAVE_UNW_AARCH64_X19)
Copy link
Member

Choose a reason for hiding this comment

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

There are over 100 usages of __APPLE__ define to identify apple stuff in the PAL. I would prefer staying consistent with that (or change all the usages of the __APPLE__ to TARGET_APPLE)

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair enough, there's at least 147 usages of __APPLE__ in CoreCLR vs. 71 of TARGET_APPLE (in this PR). I can unify it on __APPLE__ if that's preferred.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, keeping uniformity is preferred. In some later PR, we can later change everything to TARGET_APPLE, but doing it now would unnecessarily grow the already large number of files changed in this PR and do that in places that are unrelated to the gist of this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. I'll update it.

(may take me a day or two to get to it; currently on sick leave)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

src/coreclr/pal/src/map/virtual.cpp Outdated Show resolved Hide resolved
src/coreclr/pal/src/map/virtual.cpp Outdated Show resolved Hide resolved
src/coreclr/pal/src/misc/dbgmsg.cpp Show resolved Hide resolved
src/native/corehost/apphost/static/CMakeLists.txt Outdated Show resolved Hide resolved
src/native/libs/System.Globalization.Native/CMakeLists.txt Outdated Show resolved Hide resolved
@@ -34,6 +34,7 @@
static const Entry s_sysNative[] =
{
DllImportEntry(SystemNative_FStat)
#if !defined(TARGET_APPLE) || defined(TARGET_OSX)
Copy link
Member

Choose a reason for hiding this comment

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

It seems that we tend to keep this list the same for all platforms and rather handle the missing functionality in the actual implementation of the methods. E.g. for WASI, many of the methods return EINVAL or NULL.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is preexisting implementation where it was not done that way. We just never compiled the entrypoints.c on mobile platforms.

Since this is used for corehost which is not particularly useful on mobile platforms maybe I can just drop this part of change and try to exclude corehost build.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. There are some additional changes I can revert once #110092 and #110023 gets in.

Copy link
Member Author

Choose a reason for hiding this comment

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

Rebased after #110092 merge now.

filipnavara and others added 10 commits November 30, 2024 17:47
…ld.sh clr+clr.nativeaotlibs+libs+packs -os [iossimulator/maccatalyst/ios] -arch arm64 -cross'

Build the singlefilehost even if we don't currently use it

Make coreclrhost.h compatible v C(not++)

Fix TargetOS::IsMacOS

WIP: AppleAppBuilder support for CoreCLR

Make the sample app working: dotnet publish src/mono/sample/iOS/Program.csproj -c Release /p:TargetOS=iossimulator /p:TargetArchitecture=arm64 /p:AppleAppBuilderRuntime=CoreCLR /p:DeployAndRun=true /p:UseMonoRuntime=false /p:RunAOTCompilation=false /p:MonoForceInterpreter=false

Cleanup

Update src/tasks/AppleAppBuilder/AppleAppBuilder.csproj

Co-authored-by: Adeel Mujahid <3840695+am11@users.noreply.github.com>

Use sys_icache_invalidate for all iOS-like platforms

Consolidate macOS-only entrypoints in a single #if block

Apply feedback

Fix jitinterface cross-build

Infer AppleAppBuilderRuntime from UseMonoRuntime and UseNativeAOTRuntime

Minor changes to test infrastructure

Add basic documentation

Add nearly complete support for building runtime tests

Fix building seh-unwind with older iOS SDK

Use enum for TargetRuntime in AppleAppBuilder

Fix LibraryBuilder build

Cleanup

Fix after rebase

Revert accidental change

Fix tvOS build

Drop CoreCLR tvOS Simulator support for now
Co-authored-by: Aaron Robinson <arobins@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Infrastructure-coreclr community-contribution Indicates that the PR has been added by a community member os-maccatalyst MacCatalyst OS
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

6 participants