-
Notifications
You must be signed in to change notification settings - Fork 369
Create a new Microsoft.DotNet.SharedFramework.Sdk for creating shared frameworks #5714
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
Conversation
Start working on a new SharedFramework SDK tooling story to unify between the dotnet/runtime, dotnet/windowsdesktop, and dotnet/aspnetcore shared framework authoring stories. After ReadyToRun, filter the itemgroup down to the files to bundle in the shared framework. Produce deps.json file for the shared framework. Various fixes found through local testing. Add support for resource assemblies. Fix multi-rid build. Fix typo Produce runtimeconfig.json. Copy over the CreateFrameworkListFile task to enable writing a FrameworkList.xml or RuntimeList.xml for their respective shared framework packs. Require the consumer to provide a package overrides file since all known customers already have their own mechanism to supply one. Implement templated platform manifest generation. Remove unused static. Re-enable publishing R2R symbols. Use GetFilesToPackage classifications for AppHost packs as well. Ensure that we don't include our generated files in the platform manifest check. Clean up error handling in GeneratePlatformManifestEntriesFromTemplate. Implement harvesting platform manifest support so shared frameworks that can use it don't need to use the templating approach. Implement dumping the completed shared framework to disk. Add back acquiring NuGet since we're going to need that when producing packages. Implement creating a compressed archive of the shared framework based on the on-disk layout. Output archive to packages. Implement packing a shared framework using the .NET SDKs packing infra. Package symbol files in the symbol package. Implement conditionally creating a symbols archive. Copy over targets from the old shared framework SDK and enable opt-in generation of .msi installers. Additional cleanup and validation TBD. Update deb, rpm, pkg paths to depend on the correct variables. Update cross-arch msi builds to work in the new system. Convert installer targets to be a recursive setup to enable building multiple types of packages from one build. Wire up installer generation to the build. Set GenerateInstallers to true to generate installers for the platform. Add ability to pass scripts to the macOS pkgbuild invocation. Fix shared framework name. Update creating the VS insertion packages. Batch up the installer build project calls instead of serially calling MSBuild. Fix RPM targets enough to get to the rpmbuild invocation. Make some progress on producing deb packages with dotnet-deb-tool to validate the targets flow. Fix typo. Also, building .deb packages works! (WSL->Windows drives apparently can have issues) Move the underlying scripts of dotnet-deb-tool into the shared framework SDK. Include RPM template files in the SDK. Remove the package testing files. Fix packing. Fix installer versioning and naming. Change many of the private installer properties to be prefixed with an underscore to make it easier to understand which properties are private. Fix light command package drop location. Enable adding dependencies for debs and rpms created with the built-in infra. Allow a different name for installers vs the archive. Make Wix exe bundles buildable. Fix product band version wix variable. Implement MacOS bundle proj. Various fixes to get to the point of building packages in dotnet/windowsdesktop. Fix building installers for targeting packs. Fix VS insertion package generation. Fix wix acquisition Filter symbol files out of the implementation package. Fix binplacing of resource files when publishing to disk. Fix Wix installer branding. Remove outdated todo Enable servicing and rid-based skipping in the same (for servicing) or similar (for RIDs) mechanism as the old shared framework sdk. Avoid re-evaluating when calling into the PublishSharedFrameworkToDisk target to avoid parallel copy issues and reduce the amount of work MSBuild has to do. Hook in package label versioning with the Arcade versioning story. Remove completed TODO Pass through RepoRoot so Arcade doesn't try to find it from the package cache. Various fixes around templated manifests and symbol file discovery/packaging. Fix issues found when packaging the .NET runtime shared framework. Use MSBuild to automatically determine the relative path for symbols instead of requiring it to be manually updated. Implement changes needed to support the shared host installer. Enable features for building the hostfxr and host installers. Update branding. Preliminary fixes for the bundle installer. Detect perfmap files as symbol files. Make sure we publish to disk before building our deb/rpms. Enhance build invocation caching to also cache R2R and auxiliary file generation so each build only runs each target once. This removes instabiility with attempts to ready-to-run the shared framework simultaneously by enabling the MSBuild scheduler to only execute the targets once. Fix typos in macos bundle creation. Actually write the distribution file to disk. Pass brand name instead of shared framework friendly name.
5bc7da4
to
72181d3
Compare
…Installer and bundle generation docs still TBD.
cc: @NikolaMilosavljevic for review |
src/Microsoft.DotNet.SharedFramework.Sdk/Microsoft.DotNet.SharedFramework.Sdk.csproj
Outdated
Show resolved
Hide resolved
or have an easily calculatable assembly version and file version. | ||
- All native files have an easily calculatable file version. | ||
|
||
To use, set `UseTemplatedPlatformManifes`t to true and define a set of `PlatformManifestFileEntry` items |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible for a hybrid where we only list the items which are unique and let common items be harvested? If we did this and validated it it, that might eliminate the need for explicit selection of modes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you mean here. Do you mean that we harvest the files from the targeting pack and then template the rest? Or that we harvest from whatever platform we're on and use the template as a baseline for the missing files?
I'm slightly concerned about both of those options. I prefer the design of either being fully implicit (harvest the runtime packs) or fully explicit (templating). I fear allowing a mix of the two might cause some of the data to be generated inconsistently depending on which platform you build the ref pack on (in particular with file versions).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically I'm thinking how we might allow harvesting without needing every single runtime available while minimizing the checked in template (which presumably needs to define versions and some way to calculate them). I'm not worried about inconsistency since anything with the template must be an accurate representation of the actual content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So there's a process to defining versions for files:
- By default, if a file is in the ref pack, we assume that the runtime pack will have the same version.
- The developer can specify a fallback assembly or file version if any items don't have versions and need to have assembly or file versions.
- The developer can specify versions for specific files if they aren't present in the ref pack.
The other reason to avoid harvesting is that harvesting more than just the current target platform is not compatible with source-build to my knowledge. And since we don't need file versions or assembly versions for native files on non-Windows, harvesting on non-Windows only isn't worth it IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
only about 1/3 through this. but need to take a break
/// %(ReferencedByDefault): Empty (default) or "true"/"false". Indicates whether this file | ||
/// should be referenced by default when the SDK uses this framework. | ||
/// </summary> | ||
public ITaskItem[] FileClassifications { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it more natural to express this as metatdata on the Files
or using a separate item that needs to be mapped to those files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's more natural to specify this in a separate set of items. The FilesToPackage
item group can be generated from Reference
, PackageReference
, ProjectReference
, or any other mechanism to reference assemblies that are not part of a framework dependency. As a result, there's no good way to modify those items after resolution and before this target. We could use a <JoinItems />
task to join the lists together into a single list of items, but I don't think that gives much in addition to just passing the two lists into CreateFrameworkListFile
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, Reference and Project reference should preserve metadata on the original item, but PackageReference does not. I'm mainly thinking about how we minimize the duplication in checked in projects. The more times we require devs to duplicate file lists, the more chance there is that folks get it wrong or out of sync.
src/Microsoft.DotNet.SharedFramework.Sdk/src/CreateFrameworkListFile.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.SharedFramework.Sdk/src/CreateLightCommandPackageDrop.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.SharedFramework.Sdk/src/CreateLightCommandPackageDrop.cs
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.SharedFramework.Sdk/src/ExecWithRetries.cs
Outdated
Show resolved
Hide resolved
I don't think I can spend the time to do a full review, but a couple things:
|
I echo Davis' comment. I have cloned the source repo to do some diffing locally. I think a better approach would have been to first commit the copy of the old SDK, and then send a new PR with actual changes. |
I have some rudimentary tooling at https://devdiv.visualstudio.com/Personal/_git/dagood?path=%2FVstsTools%2FCompareBuildOutputs%2FProgram.cs that might be able to help with nupkg and WiX compares--it has some fiddling to remove some "unstable" strings to make diffs a bit cleaner. (And it runs in parallel--I had a powershell script earlier but it was agonizingly slow.) Note that the source code has been in flux depending on what I was comparing at the time, so it might not be directly useful. |
The sharefx.props/targets files are completely new and encompass archive creation as well as . Any tasks with the same name as in the old SDK are identical. The installer.targets file is a stripped down version of the old SDK's installer.targets file. The Windows path of bundle.targets is pulled from the old SDK's installer.targets. The macOS path of bundle.targets is completely new. The wix.targets and the acquisition folder are copied from the old SDK with some minor changes around which properties are public and which are "private". The installer targets workflow has changed to fan out for installer builds and call into the
I've validated the NuGet package metadata, NuGet file list and location in the package, and archive layout. I've also checked the RPM/Deb metadata and file lists to ensure they match the packages currently produced in CI. I've also manually installed a few of the Wix MSIs to validate their install/uninstall experience and their entries in the Add/Remove Programs view. I've started the exe bundle installers to validate that they are correctly branded. I've validated that all inputs to
The original approach I took with this SDK was to start from scratch until I realized I needed to also copy over all of the installer generation, so the old SDK kind of came over piecemeal. |
The sharedfx.targets are designed to do the following steps:
|
…only projects can also generate archives.
Add PackageTargetOS property for the dotnet-runtime-deps packages to hook in to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a solid idea how dotnet/aspnetcore would use this other than to read all the docs, add the packages, and iterate. Are there any repos where you've got branches using the new stuff❔
We also have many more urgent infrastructure issues than changing how we create shared Fx and targeting pack assets. It'll be a few months before we have resources available to really think about this.
|
||
## Build Skip Support for Unsupported Platforms and Servicing | ||
|
||
This SDK also supports automatically skipping builds on unsupported platforms or in servicing releases. If a project with a list of provided RIDs in `RuntimeIdentifiers` is built with the `RuntimeIdentifier` property set to a RID that is not in the `RuntimeIdentifiers` list, the build will be skipped. This enables cleanly skipping optional packs, installers, or bundles that only exist on specific platforms. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If aspnetcore was going to use this, the "or in servicing releases" bit must be something we could override. We Zip both our shared Fx contributions and our targeting packs. The shared Fx Zips are produced unconditionally. And, we've had multiple 3.1.x releases that had to include updated targeting pack content.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is implemented this way--that targeting pack vs. sharedfx behavior is universal. I think what I landed with in dotnet/runtime etc. is pretty nice if I may say so myself 😄:
<!--
Servicing build settings for Setup/Installer packages. Instructions:
* To enable a package build for the current patch release, set PatchVersion to match the current
patch version of that package. ("major.minor.patch".) This is normally the same as
PatchVersion above, but not always. Notably, NETStandard has its own patch version.
* When the PatchVersion property above is incremented at the beginning of the next servicing
release, all packages listed below automatically stop building because the property no longer
matches the metadata. (Do not delete the items!)
If the PatchVersion below is never changed from '0', the package will build in the 'master'
branch, and during a forked RTM release ("X.Y.0"). It will stop building for "X.Y.1" unless
manually enabled by updating the metadata.
-->
<ItemGroup>
<!-- Targeting packs are only patched in extreme cases. -->
<ProjectServicingConfiguration Include="Microsoft.NETCore.App.Ref" PatchVersion="0" />
</ItemGroup>
</Target> | ||
|
||
<!-- | ||
The Microsoft build's per-package servicing policy conflicts with the source-build restrictions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, what's "the Microsoft build"❔
If it matters, different repos have different incremental servicing policies e.g. dotnet/runtime services many packages individually while dotnet/aspnetcore only disables the targeting packs in most (wish it were all
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Microsoft build is the one that Microsoft builds--the one that ends up on dot.net, NuGet.org, blob storage (dailies), etc.
Non-Microsoft builds would be the ones that Red Hat and Fedora do for their distros, based on source-build. And any other builds from source that we might not even know about.
Servicing differences are a known, significant issue. I wrote up a summary doc on the problem at https://github.com/dotnet/source-build/tree/release/3.1/Documentation/planning/nongranular-servicing-readiness with a proposal to add tests to validate that the Microsoft servicing behavior won't wreck the source-built product. (Not implemented.)
<Exec Command="tar -C '$(_OutputPathRoot)' -czf $(PackageOutputPath)/$(_ArchiveFileName).tar.gz ." | ||
IgnoreExitCode="true" | ||
IgnoreStandardErrorWarningFormat="true" | ||
Condition="!$([MSBuild]::IsOSPlatform(Windows))"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dotnet/aspnetcore creates aspnetcore-targeting-pack-6.0.0-dev.tar.gz on Windows. We execute ./eng/scripts/InstallTar.ps1 on the CI to cover older versions of Windows (some don't include tar
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you have builds, in particular mock official builds, that you can link to? I think artifacts and binlogs would help a lot with review. (I only looked at superficial stuff with this review.)
@@ -0,0 +1,121 @@ | |||
<Project> | |||
<Target Name="_GetCurrentProjectServicingConfiguration"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this target and SetLastReleasedVersionForSourceBuild
copied three times?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the Archives, Installer, and SharedFramework packages can all be used independently (and in dotnet/runtime they will be for the host installers and archives), they all need an implementation of the project servicing configuration check. I'd also be ok adding these targets to the Arcade SDK so they're shared there and defined only once.
src/Microsoft.DotNet.Build.Tasks.Installers/build/Microsoft.DotNet.Build.Tasks.Installers.props
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Build.Tasks.Installers/build/installer.targets
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.SharedFramework.Sdk/src/GenerateSharedFrameworkDepsFile.cs
Outdated
Show resolved
Hide resolved
I'll spin another mock official build. It's been a bit since I've run one and I'm pretty sure the last one I did got deleted. |
Official build running at:
New build with updates in the platform manifest for WebAssembly: https://dev.azure.com/dnceng/internal/_build/results?buildId=785601&view=results |
…ed on the referenced framework packs. Exclude culture assemblies from closure validation (otherwise we get duplicate assembly names).
…it (like the installers and archives packages) Signed-off-by: Jeremy Koritzinsky <[email protected]>
…TFX compat. Use more efficient publicKeyToken-to-string algorithm.
… into pr/jkoritzinsky/5714
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I skimmed most of the relevant tasks / targets that impact packaging. I can't really provide a full sign-off here, but I've left most of my feedback. Thank you for addressing my concerns.
src/Microsoft.DotNet.Build.Tasks.Archives/Microsoft.DotNet.Build.Tasks.Archives.csproj
Outdated
Show resolved
Hide resolved
src/Microsoft.DotNet.Build.Tasks.Installers/Microsoft.DotNet.Build.Tasks.Installers.csproj
Outdated
Show resolved
Hide resolved
Signed-off-by: Jeremy Koritzinsky <[email protected]>
SDK version 5.0.0-dev.20465.2
SDK version 5.0.0-dev.20465.3
Per @NikolaMilosavljevic's suggestion, I've compared the MSIs produced in dotnet/runtime master against the MSIs produced in dotnet/runtime#38457 (which is the PR that uses this SDK) using Orca. I've validated that the Directory and Registry tables match completely and that the Component and File tables match to the expected level (some slight differences in the list of included files for the runtime pack since the current infra includes some .h files that were accidentally included). I've also checked the other tables and they all look identical to the level I can tell (some entries in Orca just say |
…s packages like the old system for Microsoft.DotNet.Build.Tasks.SharedFramework.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The amount of changes is huge, with lots of files moved to new locations and quite a bit of refactoring. Based on extensive discussions about design and various implementation details, and your validations of produced packages, I am signing off on this work.
Merging as Arcade branched-off for 5.0. |
Create a new shared framework SDK + 2 NuGet packages that can fully replace the Microsoft.DotNet.Build.Tasks.SharedFramework SDK as well as ASP.NET Core's custom shared framework construction tooling.
The Microsoft.DotNet.SharedFramework.Sdk can produce the following:
The Microsoft.DotNet.Build.Tasks.Archives package can produce the following:
The Microsoft.DotNet.Build.Tasks.Installers package can produce the following:
Example usages of this SDK and related packages are in dotnet/windowsdesktop#909 and dotnet/runtime#38457.
TODO:
Fixes #2704