Add support for .NET workload sets #14083

joeloff · 2023-10-05T18:00:29Z

Description

Add support for generating workload set MSIs from a workload set NuGet package as described here.

Changes include

Refactoring common properties for workload creation tasks into a base class since we now have two tasks
Cleaning up the directory template to use fragments and share directory table authoring across multiple MSIs
Additional tests to verify generated workload set installers
Explicit test package versions in the unit test project. These are very specific versions and should rarely change. Placing them in versions.props doesn't provide much benefit.
Additional package group type metadata to distinguish between workload manifest and workload set package groups for inserting to Visual Studio

src/Microsoft.DotNet.Build.Tasks.Workloads/src/Msi/WorkloadPackMsi.wix.cs

src/Microsoft.DotNet.Build.Tasks.Workloads.Tests/PackageTests.cs

Co-authored-by: Noah Gilson <OTAKUPENGUINOP@GMAIL.COM>

...osoft.DotNet.Build.Tasks.Workloads.Tests/Microsoft.DotNet.Build.Tasks.Workloads.Tests.csproj

MiYanni

I might not know the full context of how these changes operate, but the code looked really clean to me. Big thumbs up from me. 👍

MiYanni · 2023-10-06T20:27:29Z

src/Microsoft.DotNet.Build.Tasks.Workloads/src/CreateVisualStudioWorkload.wix.cs

+                                string platform = kvp.Key;
+
+                                //  The key is the paths to the packages included in the pack group, sorted in alphabetical order
+                                string uniquePackGroupKey = string.Join("\r\n", kvp.Value.Select(p => p.PackagePath).OrderBy(p => p));


nit: You can use Environment.NewLine instead of hard-coded "\r\n".

MiYanni · 2023-10-06T20:30:28Z

src/Microsoft.DotNet.Build.Tasks.Workloads/src/CreateVisualStudioWorkload.wix.cs

-                                    swixProjectItem.SetMetadata(Metadata.PackageType, DefaultValues.PackageTypeMsiPack);
-                                    swixProjectItem.SetMetadata(Metadata.IsPreview, "false");
+                            ITaskItem swixProjectItem = new TaskItem(swixProj);
+                            swixProjectItem.SetMetadata(Metadata.SdkFeatureBand, $"{sdkFeatureBand}");


Is $"{sdkFeatureBand}" basically being used as sdkFeatureBand.ToString()?

yup, I'm embracing interpolation

The bold AND the brave only embrace interpolation so... fearlessly. 😜 I wonder if the IL ends up any different. I just hadn't seen someone use string interpolation for single-values before.

src/Microsoft.DotNet.Build.Tasks.Workloads/src/MsiTemplate/WorkloadSetProduct.wxs

Co-authored-by: Michael Yanni <MiYanni@microsoft.com>

...osoft.DotNet.Build.Tasks.Workloads.Tests/Microsoft.DotNet.Build.Tasks.Workloads.Tests.csproj

mmitche · 2023-10-10T17:53:53Z

/azp run

azure-pipelines · 2023-10-10T17:54:05Z

Azure Pipelines successfully started running 1 pipeline(s).

nagilson

I have more questions than really anything else... I don't understand this all too well, but it seems reasonable given the knowledge I do have. If there is someone out there with this domain knowledge, it'd be best to have their review too.

src/Microsoft.DotNet.Build.Tasks.Workloads/src/Msi/MsiUtils.wix.cs

src/Microsoft.DotNet.Build.Tasks.Workloads/src/Msi/WorkloadSetMsi.wix.cs

src/Microsoft.DotNet.Build.Tasks.Workloads/src/CreateVisualStudioWorkload.wix.cs

src/Microsoft.DotNet.Build.Tasks.Workloads.Tests/SwixPackageTests.cs

dsplaisted · 2023-10-16T21:17:23Z

src/Microsoft.DotNet.Build.Tasks.Workloads/src/CreateVisualStudioWorkloadSet.wix.cs

+        /// <summary>
+        /// Set of NuGet packages containing workload sets.
+        /// </summary>
+        public ITaskItem[] WorkloadSetPackageFiles


Do we ever expect there to be more than one of these packages passed to the task?

Probably not, but if we ever needed it, we won't need an Arcade update.

src/Microsoft.DotNet.Build.Tasks.Workloads/src/CreateVisualStudioWorkloadSet.wix.cs

dsplaisted · 2023-10-16T21:28:13Z

src/Microsoft.DotNet.Build.Tasks.Workloads/src/CreateVisualStudioWorkloadSet.wix.cs

+            }
+
+            Msis = msiItems.ToArray();
+            SwixProjects = swixProjectItems.ToArray();


I think for the baseline workload sets, which will be created in the installer repo, we will want to create the MSIs, but we won't insert the authoring into Visual Studio. Will we be able to filter out the VS authoring appropriately?

Yes. Right now the tasks generate the projects for those, but it's up to the repo to decided whether or not it wants to build those projects. Here's an example where the runtime builds the swix projects: https://github.com/dotnet/runtime/blob/ccc475ed5646abe71ef596702d06e78e34981736/src/workloads/workloads.csproj#L179

Alternatively we could add a parameter that controls generating the swixprojects.

src/Microsoft.DotNet.Build.Tasks.Workloads/src/MsiTemplate/WorkloadSetProduct.wxs

joeloff · 2023-10-18T23:06:01Z

Any other feedback or are we good to merge?

dsplaisted · 2023-10-19T01:09:18Z

Good to merge as far as I'm concerned

joeloff · 2023-10-20T19:53:17Z

/backport to release/8.0

github-actions · 2023-10-20T19:53:29Z

Started backporting to release/8.0: https://github.com/dotnet/arcade/actions/runs/6592101683

github-actions · 2023-10-20T19:53:35Z

@joeloff backporting to release/8.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: Add support for .NET workload sets
Applying: Cleanup formatting
Applying: Format cleanup
Applying: PR Feedback
Applying: PR feedback
Applying: Fix iOS package version
error: sha1 information is lacking or useless (eng/Versions.props).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0006 Fix iOS package version
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

github-actions · 2023-10-20T19:53:37Z

@joeloff an error occurred while backporting to release/8.0, please check the run log for details!

Error: git am failed, most likely due to a merge conflict.

Add support for .NET workload sets

eef34f2

joeloff requested a review from a team October 5, 2023 18:00

nagilson reviewed Oct 5, 2023

View reviewed changes

src/Microsoft.DotNet.Build.Tasks.Workloads/src/Msi/WorkloadPackMsi.wix.cs Outdated Show resolved Hide resolved

nagilson reviewed Oct 5, 2023

View reviewed changes

src/Microsoft.DotNet.Build.Tasks.Workloads.Tests/PackageTests.cs Show resolved Hide resolved

Cleanup formatting

b0ff45f

Co-authored-by: Noah Gilson <OTAKUPENGUINOP@GMAIL.COM>

ViktorHofer reviewed Oct 6, 2023

View reviewed changes

...osoft.DotNet.Build.Tasks.Workloads.Tests/Microsoft.DotNet.Build.Tasks.Workloads.Tests.csproj Outdated Show resolved Hide resolved

MiYanni previously approved these changes Oct 6, 2023

View reviewed changes

joeloff dismissed MiYanni’s stale review via 05ddffa October 6, 2023 23:22

joeloff and others added 2 commits October 6, 2023 16:22

Format cleanup

05ddffa

Co-authored-by: Michael Yanni <MiYanni@microsoft.com>

PR Feedback

009007b

ViktorHofer reviewed Oct 7, 2023

View reviewed changes

...osoft.DotNet.Build.Tasks.Workloads.Tests/Microsoft.DotNet.Build.Tasks.Workloads.Tests.csproj Outdated Show resolved Hide resolved

joeloff added 3 commits October 8, 2023 15:38

PR feedback

f683ede

Merge branch 'main' into workload-sets

a75875f

Fix iOS package version

f004ef8

marcpopMSFT assigned joeloff Oct 9, 2023

nagilson previously approved these changes Oct 12, 2023

View reviewed changes

Merge branch 'main' into workload-sets

7433afa

joeloff dismissed nagilson’s stale review via 7433afa October 16, 2023 17:14

dsplaisted previously approved these changes Oct 16, 2023

View reviewed changes

PR feedback

f79ad7d

joeloff dismissed dsplaisted’s stale review via f79ad7d October 16, 2023 22:44

dsplaisted approved these changes Oct 20, 2023

View reviewed changes

joeloff merged commit eed07a0 into dotnet:main Oct 20, 2023
11 checks passed

joeloff mentioned this pull request Oct 20, 2023

Disable flakey test #14144

Closed

marcpopMSFT mentioned this pull request Nov 8, 2023

Support for workload set versions - authoring workload set assets and packages dotnet/sdk#34282

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for .NET workload sets #14083

Add support for .NET workload sets #14083

joeloff commented Oct 5, 2023

MiYanni left a comment

MiYanni Oct 6, 2023

MiYanni Oct 6, 2023

joeloff Oct 6, 2023

MiYanni Oct 7, 2023

mmitche commented Oct 10, 2023

azure-pipelines bot commented Oct 10, 2023

nagilson left a comment

dsplaisted Oct 16, 2023

joeloff Oct 16, 2023

dsplaisted Oct 16, 2023

joeloff Oct 16, 2023

joeloff commented Oct 18, 2023

dsplaisted commented Oct 19, 2023

joeloff commented Oct 20, 2023

github-actions bot commented Oct 20, 2023

github-actions bot commented Oct 20, 2023

github-actions bot commented Oct 20, 2023

Add support for .NET workload sets #14083

Add support for .NET workload sets #14083

Conversation

joeloff commented Oct 5, 2023

Description

MiYanni left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmitche commented Oct 10, 2023

azure-pipelines bot commented Oct 10, 2023

nagilson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joeloff commented Oct 18, 2023

dsplaisted commented Oct 19, 2023

joeloff commented Oct 20, 2023

github-actions bot commented Oct 20, 2023

github-actions bot commented Oct 20, 2023

github-actions bot commented Oct 20, 2023