[iOS][tvOS] Fix building runtime tests on iOS-like platforms #91542

simonrozsival · 2023-09-04T11:37:23Z

ghost · 2023-09-04T11:37:36Z

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

Work in progress.

Author:	simonrozsival
Assignees:	simonrozsival
Labels:	`NO-REVIEW`, `area-Infrastructure-coreclr`
Milestone:	-

simonrozsival · 2023-09-04T11:37:48Z

/azp run runtime-ioslike

azure-pipelines · 2023-09-04T11:37:56Z

Azure Pipelines successfully started running 1 pipeline(s).

vitek-karas · 2023-09-04T14:19:27Z

@sbomer please review this (once it's ready).

vitek-karas · 2023-09-04T14:22:01Z

src/tests/Common/XUnitWrapperGenerator/XUnitWrapperGenerator.csproj

@@ -5,6 +5,7 @@
    <Nullable>enable</Nullable>
    <EnableDefaultItems>true</EnableDefaultItems>
    <IsRoslynComponent>true</IsRoslynComponent>
+    <PublishTrimmed>false</PublishTrimmed>


Given that PublishTrimmed behavior is really only defined for applications, would it instead make sense to modify

runtime/src/tests/Directory.Build.props

Line 210 in 9eb0266

<PublishTrimmed>true</PublishTrimmed>

to only enable trimming for executables?

That seems like a better way of solving the problem. I'm not very familiar with this part of the build process though so I'm not sure what the implications of that would be. cc @mdh1418

The property PublishTrimmed set by default for the runtime tests may improve AOT compilation time by excluding unnecessary assemblies. However, according to the comment below it is not intended behavior.

PublishTrimmed is expected to be set on per-project basis, not globally for entire subtrees.

If we want to set the PublishTrimmed on per-project basis and avoid additional complexity in the MergedRunner , we can remove these props without side effects. I don't expect significant performance changes as AOT Interp mode is used, which compiles the corelib only.

runtime/src/tests/Directory.Build.props

Lines 208 to 212 in 9eb0266

<PropertyGroup Condition="'$(TargetOS)' == 'ios' Or '$(TargetOS)' == 'tvos'">

<EnableAggressiveTrimming>true</EnableAggressiveTrimming>

<PublishTrimmed>true</PublishTrimmed>

<SkipTestUtilitiesReference>true</SkipTestUtilitiesReference>

</PropertyGroup>

Other option is to set these props for executables only, as @vitek-karas suggested.

Note: The XUnitWrapperGenerator project may become obsolete soon, as Xunit wrappers are not used for the Mono runtime tests targeting apple mobile platforms due to parallel AOT compilation on Helix. We plan to do the same for the Native AOT runtime tests.

PublishTrimmed is expected to be set on per-project basis, not globally for entire subtrees.

That comment was meant in the context of the SDK - as in "The scenario SDK should focus on is when the property is set on per-project basis". Runtime repo is all kinds of special, so I would not necessarily apply it here.

That said - it should be set only for applications - it makes little sense for libraries.
For libraries you should instead set IsTrimmable=true but it has the same limitations (won't work on netstandard2.0 and you will get a similar error there as well).

@simonrozsival You may change the property group condition below to include Condition="'$(TargetsAppleMobile)' == 'true' and '$([System.String]::Copy($(OutputType)).ToLower())' == 'exe'".

runtime/src/tests/Directory.Build.props

Line 208 in 9eb0266

<PropertyGroup Condition="'$(TargetOS)' == 'ios' Or '$(TargetOS)' == 'tvos'">

I am not sure if there is a cleaner way for OutputType=Exe.

simonrozsival · 2023-09-05T12:45:12Z

@sbomer even with the change in this PR, the iOS tests build still fail slightly further down the line with an error that I think was caused by the changes in #91233:

Common/mergedrunner.targets#L18
src/tests/Common/mergedrunner.targets(18,5): error : This project has an assembly name identical to another project, if this CoreCLRTestLibrary, you should reference $(TestLibraryProjectPath) instead of constructing the path yourself: /Users/runner/work/1/s/src/tools/illink/src/ILLink.Tasks/ILLink.Tasks.csproj

I'm not sure how to fix that, do you have any suggestions?

simonrozsival · 2023-09-05T13:45:54Z

/azp run runtime-ioslike

azure-pipelines · 2023-09-05T13:46:06Z

Azure Pipelines successfully started running 1 pipeline(s).

kotlarmilos · 2023-09-05T16:08:57Z

@simonrozsival You fixed the CI! Let's resolve the comment above and it should be ready for review.

sbomer · 2023-09-05T16:56:48Z

@simonrozsival I think that check is supposed to validate that multiple merged test runners don't produce the same output assemblies. ILLink.Tasks isn't an output assembly - it's only referenced for the purposes of build ordering. I would try adjusting this check to only include ProjectReferences that have '%(ProjectReference.ReferenceOutputAssembly) != 'false'`:

runtime/src/tests/Common/mergedrunner.targets

Line 10 in ce3b14d

    
           <_ProjectAssemblyReferences Include="@(ProjectReference)" Condition="'%(ProjectReference.OutputItemType)' == ''" />

cc @ViktorHofer @jkoritzinsky

simonrozsival · 2023-09-06T07:38:12Z

/azp run runtime-ioslike

azure-pipelines · 2023-09-06T07:38:22Z

Azure Pipelines successfully started running 1 pipeline(s).

src/tests/Directory.Build.props

ViktorHofer · 2023-09-06T09:30:00Z

Thanks for fixing.

simonrozsival · 2023-09-06T11:25:01Z

@sbomer thanks for the suggestion!

The runtime tests now build correctly and the test failures all seem unrelated.

akoeplinger · 2024-01-15T16:55:34Z

/backport to release/8.0-staging

github-actions · 2024-01-15T16:55:45Z

Started backporting to release/8.0-staging: https://github.com/dotnet/runtime/actions/runs/7531913098

Do not trim XUnitWrapperGenerator

db1d160

simonrozsival added the NO-REVIEW Experimental/testing PR, do NOT review it label Sep 4, 2023

dotnet-issue-labeler bot added the area-Infrastructure-coreclr label Sep 4, 2023

ghost assigned simonrozsival Sep 4, 2023

vitek-karas requested a review from sbomer September 4, 2023 14:19

vitek-karas reviewed Sep 4, 2023

View reviewed changes

Avoid including ILLink

158327f

build-analysis bot mentioned this pull request Sep 5, 2023

Microsoft.NET.HostModel.Tests failing with "No space left on device" #91039

Closed

simonrozsival added 2 commits September 6, 2023 09:10

Publis trimmed only for exes

407c13a

Fix duplicates check in merged test runner

9a40f2e

kotlarmilos reviewed Sep 6, 2023

View reviewed changes

src/tests/Directory.Build.props Show resolved Hide resolved

src/tests/Directory.Build.props Show resolved Hide resolved

simonrozsival changed the title ~~[WIP] Do not trim XUnitWrapperGenerator~~ [WIP] Fix building runtime tests on iOS-like platforms Sep 6, 2023

ViktorHofer approved these changes Sep 6, 2023

View reviewed changes

simonrozsival changed the title ~~[WIP] Fix building runtime tests on iOS-like platforms~~ [iOS][tvOS] Fix building runtime tests on iOS-like platforms Sep 6, 2023

simonrozsival marked this pull request as ready for review September 6, 2023 09:50

kotlarmilos mentioned this pull request Sep 6, 2023

Baseservices exceptions unhandled runtime test is failing on apple mobile platforms #91669

Closed

kotlarmilos approved these changes Sep 6, 2023

View reviewed changes

simonrozsival removed the NO-REVIEW Experimental/testing PR, do NOT review it label Sep 6, 2023

simonrozsival merged commit 54d9fa8 into dotnet:main Sep 7, 2023

simonrozsival deleted the disable-trimming-for-xunit-wrapper-generator branch September 7, 2023 07:46

ghost locked as resolved and limited conversation to collaborators Oct 7, 2023

snakefoot mentioned this pull request Oct 29, 2023

Build error NETSDK1124 with NET8 RC2 when multiple TargetFrameworks #94406

Closed

akoeplinger mentioned this pull request Jan 15, 2024

[8.0] runtime-extra-platforms: ios and tvos - Trimming assemblies requires .NET Core 3.0 or higher #96793

Closed

github-actions bot unlocked this conversation Jan 15, 2024

github-actions bot locked as resolved and limited conversation to collaborators Jan 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[iOS][tvOS] Fix building runtime tests on iOS-like platforms #91542

[iOS][tvOS] Fix building runtime tests on iOS-like platforms #91542

simonrozsival commented Sep 4, 2023 •

edited

Loading

ghost commented Sep 4, 2023

simonrozsival commented Sep 4, 2023

azure-pipelines bot commented Sep 4, 2023

vitek-karas commented Sep 4, 2023

vitek-karas Sep 4, 2023

simonrozsival Sep 4, 2023

kotlarmilos Sep 5, 2023 •

edited

Loading

vitek-karas Sep 5, 2023

kotlarmilos Sep 5, 2023 •

edited

Loading

simonrozsival commented Sep 5, 2023 •

edited

Loading

simonrozsival commented Sep 5, 2023

azure-pipelines bot commented Sep 5, 2023

kotlarmilos commented Sep 5, 2023

sbomer commented Sep 5, 2023

simonrozsival commented Sep 6, 2023

azure-pipelines bot commented Sep 6, 2023

ViktorHofer commented Sep 6, 2023

simonrozsival commented Sep 6, 2023

akoeplinger commented Jan 15, 2024

github-actions bot commented Jan 15, 2024

	<PropertyGroup Condition="'$(TargetOS)' == 'ios' Or '$(TargetOS)' == 'tvos'">
	<EnableAggressiveTrimming>true</EnableAggressiveTrimming>
	<PublishTrimmed>true</PublishTrimmed>
	<SkipTestUtilitiesReference>true</SkipTestUtilitiesReference>
	</PropertyGroup>

[iOS][tvOS] Fix building runtime tests on iOS-like platforms #91542

[iOS][tvOS] Fix building runtime tests on iOS-like platforms #91542

Conversation

simonrozsival commented Sep 4, 2023 • edited Loading

ghost commented Sep 4, 2023

simonrozsival commented Sep 4, 2023

azure-pipelines bot commented Sep 4, 2023

vitek-karas commented Sep 4, 2023

vitek-karas Sep 4, 2023

Choose a reason for hiding this comment

simonrozsival Sep 4, 2023

Choose a reason for hiding this comment

kotlarmilos Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

vitek-karas Sep 5, 2023

Choose a reason for hiding this comment

kotlarmilos Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

simonrozsival commented Sep 5, 2023 • edited Loading

simonrozsival commented Sep 5, 2023

azure-pipelines bot commented Sep 5, 2023

kotlarmilos commented Sep 5, 2023

sbomer commented Sep 5, 2023

simonrozsival commented Sep 6, 2023

azure-pipelines bot commented Sep 6, 2023

ViktorHofer commented Sep 6, 2023

simonrozsival commented Sep 6, 2023

akoeplinger commented Jan 15, 2024

github-actions bot commented Jan 15, 2024

simonrozsival commented Sep 4, 2023 •

edited

Loading

kotlarmilos Sep 5, 2023 •

edited

Loading

kotlarmilos Sep 5, 2023 •

edited

Loading

simonrozsival commented Sep 5, 2023 •

edited

Loading