Skip to content

Commit

Permalink
Fix non-NativeAOT builds with PublishAot=true in the project file (#1…
Browse files Browse the repository at this point in the history
…8727)

The gist is:
- If you enable `<PublishAot>true</PublishAot>` then the NativeAOT build targets get included.
- Unless `NativeCompilationDuringPublish` is disabled, the NativeAOT targets are chained through `BeforeTargets` attribute to some publish targets (computing the resolved publish paths). Xamarin runs those publish targets even for non-publish builds, so it brings the whole ILC compilation along. That's undesirable.
- The `RunILLink` property is set unconditionally by the NativeAOT build integration. If we don't fix it then ILLink never runs, and neither do all the custom steps to generate registrars and `main()`.

Update: Apparently, we still need to fix runtime pack resolution for iOS-like platforms with `PublishAotUsingRuntimePack=true`.

---------

Co-authored-by: Filip Navara <[email protected]>
  • Loading branch information
filipnavara authored Aug 16, 2023
1 parent c9032f2 commit b4975e2
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 6 deletions.
7 changes: 4 additions & 3 deletions dotnet/targets/Xamarin.Shared.Sdk.props
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,10 @@
</PropertyGroup>

<!-- Various options when using NativeAOT -->
<PropertyGroup Condition="'$(PublishAot)' == 'true'">
<!-- This turns off some NativeAOT logic we don't want nor need -->
<NativeCompilationDuringPublish>false</NativeCompilationDuringPublish>
</PropertyGroup>
<PropertyGroup Condition="'$(PublishAot)' == 'true' And '$(_IsPublishing)' == 'true'">
<!-- Disable our own assembly IL stripping logic, because ILC does that already -->
<EnableAssemblyILStripping>false</EnableAssemblyILStripping>
Expand All @@ -154,8 +158,5 @@

<!-- We must find the BCL libraries using the runtime pack instead of using the built-in NativeAOT BCL -->
<PublishAotUsingRuntimePack>true</PublishAotUsingRuntimePack>

<!-- This turns off some NativeAOT logic we don't want nor need -->
<NativeCompilationDuringPublish>false</NativeCompilationDuringPublish>
</PropertyGroup>
</Project>
29 changes: 26 additions & 3 deletions dotnet/targets/Xamarin.Shared.Sdk.targets
Original file line number Diff line number Diff line change
Expand Up @@ -464,13 +464,36 @@
<IlcCompileDependsOn>Compile;_ComputeLinkerArguments;_ComputeManagedAssemblyToLink;SetupOSSpecificProps;PrepareForILLink;_XamarinComputeIlcCompileInputs</IlcCompileDependsOn>
</PropertyGroup>

<!--
Workaround for SDK issue where setting PublishAot=true during build cannot resolve correct
runtime pack and IL compiler pack.
Setting PublishAotUsingRuntimePack=true unconditionally results in the runtime pack label to
be updated to "NativeAOT" instead of "Mono" and trying to build against NativeAOT runtime
pack instead of the Mono one.
Setting PublishAotUsingRuntimePack=false causes the NuGet resolution to check for known
ILCompiler packs and these don't exist for iOS-like platforms.
The workaround ensure that PublishAotUsingRuntimePack is false when SDK updates the runtime
pack labels, and thus the "Mono" label is used. We then set it to true right before
ProcessFrameworkReferences target to skip downloading the target ILCompiler pack.
-->
<Target Name="_WorkaroundAotRuntimePackResolution" BeforeTargets="ProcessFrameworkReferences" Condition="'$(PublishAot)' == 'true' And '$(_UseNativeAot)' != 'true'">
<PropertyGroup>
<PublishAotUsingRuntimePack>true</PublishAotUsingRuntimePack>
</PropertyGroup>
</Target>

<Target Name="_ComputeLinkerFeatures">
<PropertyGroup Condition="'$(PublishAot)' == 'true'">
<!-- Yep, we want to run ILLink as well, because we need our custom steps to run (NativeAOT sets this to false, so set it back to true) -->
<RunILLink>true</RunILLink>
</PropertyGroup>

<PropertyGroup Condition="'$(_UseNativeAot)' == 'true'">
<!-- The one and only registrar is 'managed-static' when using NativeAOT -->
<Registrar Condition="'$(Registrar)' == ''">managed-static</Registrar>

<!-- Yep, we want to run ILLink as well, because we need our custom steps to run (NativeAOT sets this to false, so set it back to true) -->
<RunILLink>true</RunILLink>
</PropertyGroup>

<PropertyGroup>
Expand Down
17 changes: 17 additions & 0 deletions tests/dotnet/UnitTests/ProjectTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1345,6 +1345,23 @@ public void BuildAndExecuteAppWithWinExeOutputType (ApplePlatform platform, stri
Assert.AreEqual ($"WinExe is not a valid output type for macOS", errors [0].Message, "Error message");
}

[Test]
[TestCase (ApplePlatform.iOS, "iossimulator-x64")]
[TestCase (ApplePlatform.MacOSX, "osx-arm64")]
[TestCase (ApplePlatform.MacCatalyst, "maccatalyst-x64")]
public void PublishAotDuringBuild (ApplePlatform platform, string runtimeIdentifiers)
{
var project = "MySimpleApp";
Configuration.IgnoreIfIgnoredPlatform (platform);
Configuration.AssertRuntimeIdentifiersAvailable (platform, runtimeIdentifiers);

var project_path = GetProjectPath (project, runtimeIdentifiers: runtimeIdentifiers, platform: platform, out var appPath);
Clean (project_path);
var properties = GetDefaultProperties (runtimeIdentifiers);
properties ["PublishAot"] = "true";
DotNet.AssertBuild (project_path, properties);
}

void AssertThatDylibExistsAndIsReidentified (string appPath, string dylibRelPath)
{
var dylibPath = Path.Join (appPath, "Contents", "MonoBundle", dylibRelPath);
Expand Down

6 comments on commit b4975e2

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

💻 [CI Build] Windows Integration Tests passed 💻

All Windows Integration Tests passed.

Pipeline on Agent
Hash: b4975e2493b51c329dfc56fead1350dcf18ff9e9 [CI build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

💻 [CI Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: [CI build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

💻 [CI Build] Tests on macOS M1 - Mac Ventura (13.0) passed 💻

All tests on macOS M1 - Mac Ventura (13.0) passed.

Pipeline on Agent
Hash: [CI build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)
  • iOS (no change detected)
  • tvOS (no change detected)
  • watchOS (no change detected)
  • macOS (no change detected)
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)
.NET (No breaking changes)
Legacy Xamarin (stable) vs .NET

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: b4975e2493b51c329dfc56fead1350dcf18ff9e9 [CI build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

📚 [CI Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent
Hash: [CI build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

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

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: simulator tests.

🎉 All 232 tests passed 🎉

Tests counts

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 6 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 38 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: b4975e2493b51c329dfc56fead1350dcf18ff9e9 [CI build]

Please sign in to comment.