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

WinUI3 app compiled as AOT publishes managed pdb instead of native #108087

Closed
jevansaks opened this issue Sep 20, 2024 · 12 comments
Closed

WinUI3 app compiled as AOT publishes managed pdb instead of native #108087

jevansaks opened this issue Sep 20, 2024 · 12 comments
Labels
area-NativeAOT-coreclr tracking-external-issue The issue is caused by external problem (e.g. OS) - nothing we can do to fix it directly

Comments

@jevansaks
Copy link

Description

When generating app packages for a WinUI3 app, the msix contains the AOT'd exe but the symbol package contains the managed pdb instead of the native pdb.

Reproduction Steps

  1. Clone this repo: https://github.com/jevansaks/WinUI3AppAOT
  2. Go into WinUI3SingleProjMSIX subdir
  3. msbuild /p:Platform=x64 /p:Configuration=Release /p:GenerateAppxPackageOnBuild=true

Expected behavior

msixsym file should contain the native PDB

Actual behavior

Result: bin\x64\Release\net8.0-windows10.0.22621.0\win-x64\AppPackages\WinUI3SingleProjMSIX_1.0.4.0_x64_Test\WinUI3SingleProjMSIX_1.0.4.0_x64.msixsym contains only the managed pdb, not the native pdb.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

ComputeLinkedFilesToPublish target fixes this up for the native binary, but the pdb is left untouched. I recommend updating this target to rewrite the _DebugSymbolsIntermediatePath and DebugSymbolsProjectOutputGroupOutput to do the same so that GetPackagingOutputs sees native pdb.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Sep 20, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Sep 20, 2024
@jevansaks jevansaks changed the title WinUI3 app compiled as AOT publishes managed pdb instead of natiev WinUI3 app compiled as AOT publishes managed pdb instead of native Sep 20, 2024
@jevansaks
Copy link
Author

CC @MichalStrehovsky

@jkotas jkotas added area-NativeAOT-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Sep 21, 2024
@Sergio0694
Copy link
Contributor

We should also verify whether this is an actual issue with WinUI 3, or whether it affects UWP .NET 9 as well. Or in general, any packaged MSIX app using Native AOT. We're working on extracting all that tooling out of WinAppSDK and into a standalone single-project MSIX package (that both WinUI 3 and UWP .NET 9 will take a dependency on), so if the issue is caused by something in the MSIX tooling, there's a chance this will affect anyone using that with Native AOT, not just WinUI 3 apps.

I can help put together a UWP .NET 9 repro mirroring the one Jevan shared and report back 🙂

@MichalStrehovsky
Copy link
Member

MichalStrehovsky commented Sep 23, 2024

I'm having trouble with the repro - first complaint indicated I need to run restore, so I added /restore to the command line, but that is still failing with:

  C:\Program Files\dotnet\sdk\9.0.100-rc.1.24452.12\Sdks\Microsoft.NET.Sdk\targets\Microsoft.PackageDependencyResolutio
n.targets(266,5): error NETSDK1060: Error reading assets file: Error loading lock file 'D:\Temp\WinUI3AppAOT\WinUI3Sing
leProjMSIX\obj\project.assets.json' : Could not load file or assembly 'System.Text.Json, Version=8.0.0.4, Culture=neutr
al, PublicKeyToken=cc7b13ffcd2ddd51' or one of its dependencies. The system cannot find the file specified. [D:\Temp\Wi
nUI3AppAOT\WinUI3SingleProjMSIX\WinUI3SingleProjMSIX.csproj]

Before I spend more time trying to get a repro, can you try with .NET 9? I believe this was fixed in .NET 9 (#98342).

@jevansaks
Copy link
Author

For the repro, my pubxml files didn't push because of the default .gitignore. I just fixed that.

For building, I did:

nuget restore .\WinUI3SingleProjMSIX.csproj
msbuild /p:Platform=x64 /p:Configuration=Release /p:GenerateAppxPackageOnBuild=true

I also retargeted to .net9 (just change line 6 in the csproj from net8 to net9) and it still repro'd.

WinAppSDK's target __GetPublishItems is ultimately calling VS's DebugSymbolsProjectOutputGroup which just does:

  <Target
      Name="DebugSymbolsProjectOutputGroup"
      Returns="@(DebugSymbolsProjectOutputGroupOutput)"
      DependsOnTargets="$(DebugSymbolsProjectOutputGroupDependsOn)"/>

The PR you linked fixes it so that ComputeResolvedFilesToPublishList doesn't have any PDBs in it, but the MSIX packaging targets are still looking to collect pdbs (in target GetPackagingOutputs) and they get those by calling DebugSymbolsProjectOutputGroup.

The same modification made in that PR should be done to _DebugSymbolsProjectOutputGroupOutput, but after removing the pdb from _DebugSymbolsProjectOutputGroupOutput, we need to add back the AOT pdb so that the packaged symbols are correct.

@MichalStrehovsky
Copy link
Member

The same modification made in that PR should be done to _DebugSymbolsProjectOutputGroupOutput, but after removing the pdb from _DebugSymbolsProjectOutputGroupOutput, we need to add back the AOT pdb so that the packaged symbols are correct.

Hmm, but _DebugSymbolsProjectOutputGroupOutput seems to be defined by the WinAppSdk, not .NET SDK. I don't think I can mess with that.

Digging into this more, I tried an experiment:

  1. Take your repro but replace PublishAot with PublishTrimmed.
  2. Build same as usual and check the PDB in the MSIX package.

The PDB in the package is the correct one (matches the size of the one under obj\x64\Release\net8.0-windows10.0.22621.0\win-x64\linked\ instead of the one under obj\x64\Release\net8.0-windows10.0.22621.0\win-x64).

But looking at _DebugSymbolsProjectOutputGroupOutput it has the same problem - it points to the PDB before trimming. It looks like WinAppSDK does a bunch of postprocessing to decide what to package. This seems to happen around <!-- Remove inputs to CreateReadyToRunImages/ILLink targets --> in Microsoft.Build.Msix.Packaging.targets.

For whatever reason, this logic is explicitly excluded for PublishAot and only kicks in for PublishTrimmed and PublishReadyToRun. I think we need someone from WinAppSdk to explain the expected behavior.

@MichalStrehovsky MichalStrehovsky added the tracking-external-issue The issue is caused by external problem (e.g. OS) - nothing we can do to fix it directly label Sep 27, 2024
@jevansaks
Copy link
Author

So yes, WinAppSDK is defining the target that collects the PDBs. But it's mostly just a copy of the old AppxPackage targets. They're calling the target DebugSymbolsProjectOutputGroup which is defined in C:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Current\Bin\amd64\Microsoft.Common.CurrentVersion.targets, and then that ultimately pulls from items that are defined in that same file in VS:

  <Target
      Name="DebugSymbolsProjectOutputGroup"
      Returns="@(DebugSymbolsProjectOutputGroupOutput)"
      DependsOnTargets="$(DebugSymbolsProjectOutputGroupDependsOn)"/>

Returns DebugSymbolsProjectOutputGroupOutput, which comes from:

  <ItemGroup Condition="'$(_DebugSymbolsProduced)' != 'false' and '$(OutputType)' != 'winmdobj'">
    <DebugSymbolsProjectOutputGroupOutput Include="@(_DebugSymbolsIntermediatePath->'%(FullPath)')">
      <FinalOutputPath>@(_DebugSymbolsOutputPath->'%(FullPath)')</FinalOutputPath>
      <TargetPath>@(_DebugSymbolsIntermediatePath->'%(Filename)%(Extension)')</TargetPath>
    </DebugSymbolsProjectOutputGroupOutput>
  </ItemGroup>

And _DebugSymbolsIntermediatePath comes also from that same file:

    <ItemGroup Condition="'$(_DebugSymbolsProduced)' == 'true' and '$(PdbFile)' != ''">
        <_DebugSymbolsIntermediatePathTemporary Include="$(PdbFile)"/>
        <!-- Add any missing .pdb extension, as the compiler does -->
        <_DebugSymbolsIntermediatePath Include="@(_DebugSymbolsIntermediatePathTemporary->'%(RootDir)%(Directory)%(Filename).pdb')"/>
    </ItemGroup>

Are you suggesting that MSIX packaging should be getting the pdbs from a different Target/item group?

@MichalStrehovsky
Copy link
Member

Are you suggesting that MSIX packaging should be getting the pdbs from a different Target/item group?

DebugSymbolsProjectOutputGroup gets the symbols that are result of build, not result of publish.

The paths from there are wrong for publish: for example, when doing a publish with PublishAot we're interested in native symbols; for PublishTrimmed we're interested in symbols for the trimmed assembly, not the untrimmed thing that build produced.

Microsoft.Build.Msix.Packaging.targets has extra code to compensate for this in PublishTrimmed - MSIX packaging works correctly in a PublishTrimmed app (the <!-- Remove inputs to CreateReadyToRunImages/ILLink targets --> part I quoted above). I think PublishAot also needs some special logic in the MSIX packaging, similar to the targets stuff around the <!-- Remove inputs to CreateReadyToRunImages/ILLink targets --> (that part specifically excludes PublishAot right now and I don't know if there's similar code for PublishAot somewhere else in MSIX targets).

@Sergio0694
Copy link
Contributor

I think I can confirm the issue is also affecting UWP on .NET 9 (as expected, since we use the same exact MSIX tooling as WinAppSDK anyway). Publishing a package for a test app, I see what I assume is the correct .pdb in the \native folder (it's ~19.5 MB in this case), but the .msixsym file is just 7 KB (?). That certainly doesn't seem right.

@Scottj1s FYI if the fix is in the MSIX tooling, this is one more good reason to split that out of WinAppSDK. Once we do that and fix this, then both UWP on .NET 9 and WinAppSDK will work correctly here. And if WinAppSDK had just a package reference on the MSIX tooling, then anyone would be able to get the fix at any time without being tied to any WinAppSDK version 😄

@MichalStrehovsky
Copy link
Member

I'm closing this as this doesn't seem to be actionable in this repo. We can reopen if/when we find out some fix is needed here and know what the fix should be.

@MichalStrehovsky MichalStrehovsky closed this as not planned Won't fix, can't repro, duplicate, stale Oct 9, 2024
@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Oct 9, 2024
@jevansaks
Copy link
Author

@MichalStrehovsky what is the "correct" way to pick up the accumulated .pdbs for packaging that would handle regular case, Trimming, ReadyToRun, and AOT? Is there a different target and Item Group that the MSIX targets should be using?

@jlaanstra
Copy link

jlaanstra commented Oct 9, 2024

There is actually a difference here between ILLink and LinkNative.

For ILLink, the pdb generated by the linker is included in _LinkedResolvedFileToPublishCandidate here

<_LinkedResolvedFileToPublishCandidate Include="@(_PDBToLink->'$(IntermediateLinkDir)%(Filename)%(Extension)')" />
which then get included in ResolvedFileToPublish here
<ResolvedFileToPublish Include="@(_LinkedResolvedFileToPublish)" />

For LinkNative, the pdb generated for the native exe doesn't seem to be included in ResolvedFileToPublish. It is only removing the pdb produced by the build from the item group here

<_DebugSymbolsIntermediatePath Remove="@(_DebugSymbolsIntermediatePath)" />

@MichalStrehovsky it probably would be good to make sure the native pdb gets added to the ResolvedFileToPublish item group, similar to what ILLink does?

@MichalStrehovsky
Copy link
Member

There is actually a difference here between ILLink and LinkNative.

This difference is a leftover from many years ago. We'd like to make it work same as ILLink. Tracked at dotnet/sdk#40488.

@MichalStrehovsky what is the "correct" way to pick up the accumulated .pdbs for packaging that would handle regular case, Trimming, ReadyToRun, and AOT? Is there a different target and Item Group that the MSIX targets should be using?

There isn't one that I know of, but I didn't know about the ways MSIX target does it for R2R/Trimming either so it doesn't say much. I don't know how much MSIX packaging just relies on implementation details of R2R/Trimming right now and how much of it is standard. The fact that MSIX packaging needs to special case PublishTrimmed and PublishReadyToRun makes it feel more like "it's relying on implementation details" than "we have a standardized way to know what symbol files are part of publish".

@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-NativeAOT-coreclr tracking-external-issue The issue is caused by external problem (e.g. OS) - nothing we can do to fix it directly
Projects
Archived in project
Development

No branches or pull requests

5 participants