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

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

Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
<Nullable>enable</Nullable>
<EnableDefaultItems>true</EnableDefaultItems>
<IsRoslynComponent>true</IsRoslynComponent>
<PublishTrimmed>false</PublishTrimmed>
Copy link
Member

Choose a reason for hiding this comment

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

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

<PublishTrimmed>true</PublishTrimmed>
to only enable trimming for executables?

Copy link
Member Author

Choose a reason for hiding this comment

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

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

Copy link
Member

@kotlarmilos kotlarmilos Sep 5, 2023

Choose a reason for hiding this comment

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

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.

<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.

Copy link
Member

Choose a reason for hiding this comment

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

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).

Copy link
Member

@kotlarmilos kotlarmilos Sep 5, 2023

Choose a reason for hiding this comment

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

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

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

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

</PropertyGroup>

<ItemGroup>
Expand Down