[build] Use SharedFramework.Sdk to create workload packs #22660
Conversation
The workload packaging logic has been updated to use more of the [Microsoft.DotNet.SharedFramework.Sdk][0] MSBuild SDK included in the dotnet/arcade repo. This SDK allows us to easily exclude the symbols files that we've been shipping directly in all of our workload packs, and instead move them into `*.symbols.nupkg` packages that can be archived and uploaded to a symbol server separately. The targets updates in `dotnet/package/common.csproj` lean heavily on the targets and items defined in [sharedfx.targets][1]. [0]: https://github.com/dotnet/arcade/tree/21f46f494265f11e124c47ee868fac199768a35b/src/Microsoft.DotNet.SharedFramework.Sdk [1]: https://github.com/dotnet/arcade/blob/2b1931c05dd6ceb89120131a8b39eaa81735179a/src/Microsoft.DotNet.SharedFramework.Sdk/targets/sharedfx.targets#L398-L429
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@rolfbjarne I'm working on some packaging changes here to separate out symbols from the workload sdk packs, tagging you in for some early review whenever you have the chance. One thing I was unsure about is how https://github.com/dotnet/macios/blob/main/tests/test-libraries/nugets/FrameworksInRuntimesNativeDirectory/FrameworksInRuntimesNativeDirectory.csproj#L32 are used and if they need to be generating these FrameworkList and RuntimeList files? I can update them to do so in this newer way if needed, or maybe we can remove that bit from the projects? |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Testing end to end in VS with the following, expecting the VS build to fail but symbol check should be green: |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
These projects are used to create NuGets with a specific structure for tests. Any implementation that doesn't change the contents of those NuGets would work just fine. AFAIK any RuntimeList.xml files are ignored (and looking at some I have locally for these nugets it looks like they're wrong anyway), so those can be skipped if need be. |
dotnet/package/common.csproj
Outdated
| <FrameworkListFileClass Include="@(NativeRuntimeAsset->'%(Filename)%(Extension)')" Profile="$(_PlatformName)" /> | ||
| </ItemGroup> | ||
| <ItemGroup Condition=" '$(PlatformPackageType)' == 'ToolPack' " > | ||
| <_ToolsFilesToPackage Include="$(_packagePath)**" Exclude="$(_packagePath)**\*.pdb" /> |
There was a problem hiding this comment.
Will this remove the *.pdb files we ship with the mlaunch app (which we include in the iOS and tvOS SDK packs in ./tools/lib/mlaunch/mlaunch.app)?
There was a problem hiding this comment.
Yes, these changes move those .pdb files into a symbols.nupkg file, and they will no longer be distributed in the main nupkg file. Do we need these in the SDK pack that is going to be installed on disk? Are there any other cases that we should explicitly include?
| <ItemGroup Condition=" '$(PlatformPackageType)' == 'TargetingPack' or '$(PlatformPackageType)' == 'RuntimePack' " > | ||
| <_FxAssembliesToPackage Include="$(_packagePath)$(_PackTargetPath)/*.dll" /> | ||
| <FilesToPackage Include="@(_FxAssembliesToPackage)" TargetPath="$(_PackTargetPath)" /> | ||
| <FilesToPackage Include="@(_FxAssembliesToPackage->'%(RelativeDir)%(Filename).pdb')" TargetPath="$(_PackTargetPath)" IsSymbolFile="true" Condition=" '$(PlatformPackageType)' != 'RuntimePack' " /> |
There was a problem hiding this comment.
I'm a bit confused as to whether the runtime packs will still have the pdbs (further above a comment says "Always include symbols in runtime packs ..."), but I remembered one important detail: the pdb files are necessary during the build (and I don't think the build fetches symbols from *.symbols.nupkg), because the AOT compiler uses them to generate native debug information (dSYM bundles) - and because the native debug information bundles contain this information, the pdb files are not actually included in the app bundle.
There was a problem hiding this comment.
The runtime packs will continue to distribute the same .pdbs that they include today. This line is meant to catch any .pdbs that we would try to put into a ref pack and instead move them into only the symbols.nupkg file. Though, I'm not sure if we would ever really want or need to do that, so this line could also probably be removed.
✅ [CI Build #7d79676] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [CI Build #7d79676] Build passed (Build packages) ✅Pipeline on Agent |
|
🔥 Failed to compare API and create generator diff 🔥 Error: 'make' failed for the hash f650284. Pipeline on Agent |
✅ [CI Build #7d79676] Build passed (Build macOS tests) ✅Pipeline on Agent |
💻 [CI Build #7d79676] Tests on macOS X64 - Mac Sonoma (14) passed 💻✅ All tests on macOS X64 - Mac Sonoma (14) passed. Pipeline on Agent |
💻 [CI Build #7d79676] Tests on macOS M1 - Mac Monterey (12) passed 💻✅ All tests on macOS M1 - Mac Monterey (12) passed. Pipeline on Agent |
💻 [CI Build #7d79676] Tests on macOS arm64 - Mac Sequoia (15) passed 💻✅ All tests on macOS arm64 - Mac Sequoia (15) passed. Pipeline on Agent |
💻 [CI Build #7d79676] Tests on macOS M1 - Mac Ventura (13) passed 💻✅ All tests on macOS M1 - Mac Ventura (13) passed. Pipeline on Agent |
Additional context: dotnet/android@dc08972
The workload packaging logic has been updated to use more of the
Microsoft.DotNet.SharedFramework.Sdk MSBuild SDK included in the
dotnet/arcade repo.
This SDK allows us to easily exclude the symbols files that we've
been shipping directly in all of our workload packs, and instead move
them into
*.symbols.nupkgpackages that can be archived anduploaded to a symbol server separately.
Symbols are filtered out of regular nupkg files by setting the
%(FilesToPackage.IsSymbolFile) metadata to "true"
The targets updates in
dotnet/package/common.csprojlean heavily onthe targets and items defined in sharedfx.targets.