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

Fix NativeAOT not using the XAML compilation output #14966

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

MrJul
Copy link
Member

@MrJul MrJul commented Mar 13, 2024

What does the pull request do?

This PR fixes NativeAOT to work with the XAML compiler again.

What is the current behavior?

The wrong intermediate assembly is used by the IL Compiler, which doesn't contain the output from the XAML compiler. The resulting native application crashes.

What is the updated/expected behavior with this PR?

NativeAOT works.

How was the solution implemented (if it's not obvious)?

The problem was introduced by #13840, which doesn't swap the final intermediate assembly anymore (a good thing).
Instead, the IntermediateAssembly MSBuild item is changed to be the assembly output from the Avalonia XAML compiler.
However, for some reason, the NativeAOT target doesn't use IntermediateAssembly but hardcodes that to the original assembly.

This PR adds a new target that change the ILC input to the correct assembly.

Fixed issues

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.2.999-cibuild0046171-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 added the backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch label Mar 13, 2024
@hez2010
Copy link
Contributor

hez2010 commented Mar 14, 2024

Maybe we can instead wait for dotnet/runtime#99732 and get that PR backported to .NET 8 so that we don't need to do such a hack in Avalonia?

@MrJul
Copy link
Member Author

MrJul commented Mar 14, 2024

Maybe we can instead wait for dotnet/runtime#99732 and get that PR backported to .NET 8 so that we don't need to do such a hack in Avalonia?

If it gets backported quickly to .NET 8, then we can probably detect and tell people to use an up-to-date SDK. There's still .NET 7, but people caring about NativeAOT are probably using .NET 8 anyways due to all the improvements that release got (I've no numbers to back that up.)

If it doesn't, this workaround probably has to stay for a while.

@kekekeks
Copy link
Member

kekekeks commented Mar 14, 2024

I suspect that not swapping intermediate output will have problems with other IL patching tools such as:

  1. obfuscation software
  2. Fody
  3. ILRepack

I'm not sure that having hackfixes for particular use-cases is a good idea, we should consider reverting to the old behavior.

@TomEdwardsEnscape
Copy link
Contributor

TomEdwardsEnscape commented Mar 14, 2024

This is something which I noted as a potential problem when I opened #13840. Instead of reverting entirely, I suggest adding an extra step which copies the XAML-compiled assembly back on to the original file after we have generated it. This isn't amazing and shouldn't be necessary, but ensures that it's not possible to access the unprocessed assembly, and does not disrupt Avalonia's targets during incremental builds, nor reintroduce the original bug.

I want to say though, that if these other systems have the same problem as the AOT compile system then it is entirely their fault. Nobody should be reconstructing a hardcoded path to the intermediate output file, that is what the IntermediateAssembly item is for! However, if many of them do this then it may be more practical to sidestep the issue instead of convincing them all to fix their MSBuild scripts.

@MrJul
Copy link
Member Author

MrJul commented Mar 14, 2024

AFAIK ILRepack isn't distributed with any targets so it's up to the user to run the tool on the correct assembly (usually the one in the bin folder, not the one in obj.)

Fody correctly uses IntermediateAssembly. I've just double checked that Avalonia+Fody+NativeAOT works well with this PR.

Note that Fody overwrites the file in place; maybe we should do the same instead of swapping assemblies?

@TomEdwardsEnscape
Copy link
Contributor

Overwriting the original file leads to other complications elsewhere, because the file's timestamp is now later than it should be. This makes other targets (e.g. those injected by third party packages) skip execution because they incorrectly think they are up to date.

One option is to record the original timestamp, compile, restore the original timestamp, and then have a separate file elsewhere that tells you when the XAML compiler last ran. But...yuck. If no more observed problems come to light, let's avoid doing anything but applying this PR's workaround for this one case.

@maxkatz6 maxkatz6 added this pull request to the merge queue Mar 15, 2024
Merged via the queue into AvaloniaUI:master with commit 0aba3ed Mar 15, 2024
6 checks passed
@maxkatz6 maxkatz6 added backported-11.1.x and removed backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch labels Apr 20, 2024
@MrJul MrJul deleted the fix/compile-xaml-ilc branch April 24, 2024 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NativeAOT-compiled app fails to start with 11.1.0-beta1
6 participants