-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[nativeaot][tests] Add smoke unit runtime tests for Native AOT on iOS platforms #87260
[nativeaot][tests] Add smoke unit runtime tests for Native AOT on iOS platforms #87260
Conversation
Tagging subscribers to this area: @hoyosjs Issue DetailsWork in progress. This PR aims to include a subset of smoke runtime tests for Native AOT on iOS. Currently, it includes some temporary workarounds that will be resolved once the CI reports green. DescriptionTBD
|
Azure Pipelines successfully started running 2 pipeline(s). |
eng/pipelines/extra-platforms/runtime-extra-platforms-ioslike.yml
Outdated
Show resolved
Hide resolved
@@ -48,7 +48,6 @@ - (void)viewDidLoad { | |||
[button addTarget:self action:@selector(buttonClicked:) forControlEvents:UIControlEventTouchUpInside]; | |||
[button setFrame:CGRectMake(50, 300, 200, 50)]; | |||
[button setTitle:@"Click me (wire me up)" forState:UIControlStateNormal]; | |||
[button setExclusiveTouch:YES]; |
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.
Why are we removing this?
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.
setExclusiveTouch
property is not supported on tvOS. It doesn't change tests functionality and we wanted to avoid #ifdef
macros if possible.
src/tests/build.proj
Outdated
<_LinkerFlagsToDrop Include="@(NativeFramework->'-framework %(Identity)')" /> | ||
<LinkerArg Remove="@(_LinkerFlagsToDrop)" /> | ||
<ExtraAppLinkerArgs Include="@(LinkerArg)" /> | ||
<GlobalizationkNativeLibs Include="$(MicrosoftNetCoreAppRuntimePackDir)runtimes/$(TargetOS)-$(TargetArchitecture)/native/icudt.dat" /> |
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.
Why is this needed? I don't think we are doing this explicitly for mono. The file should be picked up by the build.
Additional nit: GlobalizationkNativeLibs
is misspelled.
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.
I think Mono and Native AOT use different publish paths. During the Mono publish the icudt.dat
file ends up in the publish directory. After the Native AOT publish, it has to be added explicitly to the publish directory to be picked up by the AppleAppBuilder.
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.
I see, if I am not mistaken that should be handled by the common ComputeResolvedFilesToPublishList
target, so if we need to explicitly include that file, it would probably be better to do something like:
<ResolvedFileToPublish Include="$(MicrosoftNetCoreAppRuntimePackDir)runtimes/$(TargetOS)-$(TargetArchitecture)/native/icudt.dat">
This way copying to the publish folder would be done by the build (so the manual Copy bellow will not be needed).
The right place to include the file explicitly is probably somewhere in AppleBuild.targets
as we would want to have the same behaviour for the HelloiOS sample as well.
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.
The native libs get populated in ResolveRuntimePackAssets
and copied to the publish directory in _CopyFilesMarkedCopyLocal
.
During the build, the ResolveRuntimePackAssets
is skipped due to the condition below.
<Target Name="ResolveRuntimePackAssets" DependsOnTargets="ResolveFrameworkReferences"
Condition="'@(RuntimePack)' != ''">
I assume that the RuntimePack is null since the CI builds don't use the runtime packs. If icudt.dat
is explicitly referenced in ResolvedFileToPublish
item, I think it happens after the _CopyFilesMarkedCopyLocal
target and the file doesn't end up in the publish directory.
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.
Thanks for checking that out and my apologies as I was not aware separate builds are used in this case, so setting up what I suggested would not work.
For mono we also collect all the native dependencies:
Line 319 in 4d5d2dc
<RuntimePackNativeLibs Include="$(MicrosoftNetCoreAppRuntimePackDir)/**/*.dll;$(MicrosoftNetCoreAppRuntimePackDir)/native/**/*.a;$(MicrosoftNetCoreAppRuntimePackDir)/native/**/*.dylib" /> |
/azp run runtime-ioslikesimulator |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
LGTM! Thanks! Great job 🚀
eng/liveBuilds.targets
Outdated
@@ -128,7 +128,7 @@ | |||
<MonoIncludeFiles Include="$(MonoArtifactsPath)\include\**\*.*" /> | |||
</ItemGroup> | |||
|
|||
<Error Condition="'@(RuntimeFiles)' == ''" Text="The '$(RuntimeFlavor)' subset must be built before building this project." /> | |||
<Error Condition="'@(RuntimeFiles)' == '' and !('$(RuntimeFlavor)' == 'CoreCLR' and '$(TargetsAppleMobile)' == 'true')" Text="The '$(RuntimeFlavor)' subset must be built before building this project." /> |
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.
this sounds wrong to me, I think I hit the same issue when adding the runtime packs which is why I added the BuildNativeAOTRuntimePack
that makes the RuntimeFiles pick up NativeAOT assets instead of CoreCLR on L80.
You could set that property too, though I wonder if longer-term we should introduce RuntimeFlavor=NativeAOT
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.
Thanks, updated!
/azp run runtime-ioslikesimulator |
No commit pushedDate could be found for PR 87260 in repo dotnet/runtime |
/azp run runtime-ioslikesimulator |
No commit pushedDate could be found for PR 87260 in repo dotnet/runtime |
/azp run runtime-ioslikesimulator |
Azure Pipelines successfully started running 1 pipeline(s). |
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.
Looks good!
The Native AOT tests are passing and the failures shouldn't be related. Thank you all for valuable feedback and assistance! |
Description
This PR aims to add smoke unit runtime tests for Native AOT on ioslike platforms. The
src/tests
pipeline is modified to build tests for the native AOT ioslike platforms. This is achieved by using the ILC publish targets and adjusting the ILC path to use the target runtime packs for the compilation. The build job template is updated to support hostedOs and hostedArch properties used as a build target platform due to ILC limitations to be built for devices. Additionally, a new target for creating an app bundle is added.This PR also introduces
runtime-ioslike-mono
andruntime-ioslike-coreclr
azp runs to balance the load on devices. The tests are enabled on ios/iossimulator and tvos/tvossimulator platforms. The maccatalyst platform is temporarily disabled, awaiting a new preview version of SDK to be used in the runtime.Tests that fail on a device are disabled due to dynamic code generation, globalization issues, or invalid handling of the SIGSEGV error.