From 0a2e4c10a40538736136ff34115b2b3c22b4ea96 Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Wed, 7 Aug 2024 17:07:34 -0400 Subject: [PATCH 1/7] Permit workload restore to work with new global.json Specifying a workload set version in the global.json file is a supported scenario. If that workload set has not yet been installed, and the user tries to take an action that relies on workloads, we should error and indicate that they should make sure that workload set is installed first via update, install, or restore. We do that correctly, but when the user subsequently restores, we do not escape from the error case and instead direct the user to run restore. We should handle it properly, essentially installing the workload set before we discover which workloads the user needs and installing those. --- .../restore/WorkloadRestoreCommand.cs | 14 ++++++++++++++ .../SdkDirectoryWorkloadManifestProvider.cs | 2 ++ 2 files changed, 16 insertions(+) diff --git a/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs b/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs index 4f5bfc994bdb..b056c8a98372 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs @@ -8,6 +8,7 @@ using Microsoft.DotNet.Cli; using Microsoft.DotNet.Cli.Utils; using Microsoft.DotNet.Workloads.Workload.Install; +using Microsoft.DotNet.Workloads.Workload.Update; using Microsoft.Extensions.EnvironmentAbstractions; using Microsoft.NET.Sdk.WorkloadManifestReader; @@ -31,6 +32,19 @@ public WorkloadRestoreCommand( public override int Execute() { + var globalJsonPath = SdkDirectoryWorkloadManifestProvider.GetGlobalJsonPath(Environment.CurrentDirectory); + var workloadSetVersionFromGlobalJson = SdkDirectoryWorkloadManifestProvider.GlobalJsonReader.GetWorkloadVersionFromGlobalJson(globalJsonPath); + + // If there's a workload set version specified in the global.json file, we need to make sure the workload set is installed before we try to discover workload ids. + if (!string.IsNullOrWhiteSpace(workloadSetVersionFromGlobalJson)) + { + var creationResult = new WorkloadResolverFactory().Create(); + if (creationResult.WorkloadResolver.GetWorkloadManifestProvider() is SdkDirectoryWorkloadManifestProvider provider && provider.WouldThrowException()) + { + new WorkloadUpdateCommand(_result).Execute(); + } + } + var allProjects = DiscoverAllProjects(Directory.GetCurrentDirectory(), _slnOrProjectArgument).Distinct(); List allWorkloadId = RunTargetToGetWorkloadIds(allProjects); Reporter.WriteLine(string.Format(LocalizableStrings.InstallingWorkloads, string.Join(" ", allWorkloadId))); diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs index 4acd8945ff9a..5977b31865c3 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs @@ -235,6 +235,8 @@ private static int VersionCompare(string first, string second) return new ReleaseVersion(modifiedFirst).CompareTo(new ReleaseVersion(modifiedSecond)); } + public bool WouldThrowException() => _exceptionToThrow is not null; + void ThrowExceptionIfManifestsNotAvailable() { if (_exceptionToThrow != null) From e963dec7f7456142a6f09bfce86a4406c40567ef Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Wed, 7 Aug 2024 18:36:27 -0400 Subject: [PATCH 2/7] move up workload history recorder --- .../commands/InstallingWorkloadCommand.cs | 2 +- .../install/WorkloadInstallCommand.cs | 168 ++++++++++-------- .../restore/WorkloadRestoreCommand.cs | 45 +++-- .../update/WorkloadUpdateCommand.cs | 77 ++++---- 4 files changed, 169 insertions(+), 123 deletions(-) diff --git a/src/Cli/dotnet/commands/InstallingWorkloadCommand.cs b/src/Cli/dotnet/commands/InstallingWorkloadCommand.cs index 4ef78e76a211..d5ab676fce1c 100644 --- a/src/Cli/dotnet/commands/InstallingWorkloadCommand.cs +++ b/src/Cli/dotnet/commands/InstallingWorkloadCommand.cs @@ -208,7 +208,7 @@ protected void UpdateWorkloadManifests(WorkloadHistoryRecorder recorder, ITransa _workloadInstaller.UpdateInstallMode(_sdkFeatureBand, true); } - if (SpecifiedWorkloadSetVersionInGlobalJson) + if (SpecifiedWorkloadSetVersionInGlobalJson && recorder is not null) { recorder.HistoryRecord.GlobalJsonVersion = _workloadSetVersionFromGlobalJson; } diff --git a/src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs b/src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs index cf93d982b185..c7c203500c45 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs @@ -127,90 +127,21 @@ public override int Execute() else { WorkloadHistoryRecorder recorder = new WorkloadHistoryRecorder(_workloadResolver, _workloadInstaller, () => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, null)); - recorder.HistoryRecord.CommandName = IsRunningRestore ? "restore" : "install"; + recorder.HistoryRecord.CommandName = "install"; try { - recorder.Run(() => + if (!IsRunningRestore) { - // Normally we want to validate that the workload IDs specified were valid. However, if there is a global.json file with a workload - // set version specified, and we might install that workload version, then we don't do that check here, because we might not have the right - // workload set installed yet, and trying to list the available workloads would throw an error - if (_skipManifestUpdate || string.IsNullOrEmpty(_workloadSetVersionFromGlobalJson)) + recorder.Run(() => { - ValidateWorkloadIdsInput(); - } - - Reporter.WriteLine(); - - DirectoryPath? offlineCache = string.IsNullOrWhiteSpace(_fromCacheOption) ? null : new DirectoryPath(_fromCacheOption); - - if (!_skipManifestUpdate) - { - var installStateFilePath = Path.Combine(WorkloadInstallType.GetInstallStateFolder(_sdkFeatureBand, _workloadRootDir), "default.json"); - if (string.IsNullOrWhiteSpace(_fromRollbackDefinition) && - !SpecifiedWorkloadSetVersionOnCommandLine && - !SpecifiedWorkloadSetVersionInGlobalJson && - InstallStateContents.FromPath(installStateFilePath) is InstallStateContents installState && - (installState.Manifests != null || installState.WorkloadVersion != null)) - { - // If the workload version is pinned in the install state, then we don't want to automatically update workloads when a workload is installed - // To update to a new version, the user would need to run "dotnet workload update" - _skipManifestUpdate = true; - } - } - - RunInNewTransaction(context => - { - if (!_skipManifestUpdate) - { - if (Verbosity != VerbosityOptions.quiet && Verbosity != VerbosityOptions.q) - { - Reporter.WriteLine(LocalizableStrings.CheckForUpdatedWorkloadManifests); - } - UpdateWorkloadManifests(recorder, context, offlineCache); - } - - // Add workload Ids that already exist to our collection to later trigger an update in those installed workloads - var workloadIds = _workloadIds.Select(id => new WorkloadId(id)); - var installedWorkloads = _workloadInstaller.GetWorkloadInstallationRecordRepository().GetInstalledWorkloads(_sdkFeatureBand); - var previouslyInstalledWorkloads = installedWorkloads.Intersect(workloadIds); - if (previouslyInstalledWorkloads.Any()) - { - Reporter.WriteLine(string.Format(LocalizableStrings.WorkloadAlreadyInstalled, string.Join(" ", previouslyInstalledWorkloads)).Yellow()); - } - workloadIds = workloadIds.Concat(installedWorkloads).Distinct(); - workloadIds = WriteSDKInstallRecordsForVSWorkloads(workloadIds); - - _workloadInstaller.InstallWorkloads(workloadIds, _sdkFeatureBand, context, offlineCache); - - // Write workload installation records - var recordRepo = _workloadInstaller.GetWorkloadInstallationRecordRepository(); - var newWorkloadInstallRecords = workloadIds.Except(recordRepo.GetInstalledWorkloads(_sdkFeatureBand)); - context.Run( - action: () => - { - foreach (var workloadId in newWorkloadInstallRecords) - { - recordRepo.WriteWorkloadInstallationRecord(workloadId, _sdkFeatureBand); - } - }, - rollback: () => - { - foreach (var workloadId in newWorkloadInstallRecords) - { - recordRepo.DeleteWorkloadInstallationRecord(workloadId, _sdkFeatureBand); - } - }); - - TryRunGarbageCollection(_workloadInstaller, Reporter, Verbosity, workloadSetVersion => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, workloadSetVersion), offlineCache); - - Reporter.WriteLine(); - Reporter.WriteLine(string.Format(LocalizableStrings.InstallationSucceeded, string.Join(" ", newWorkloadInstallRecords))); - Reporter.WriteLine(); - + InstallWorkloads(recorder); }); - }); + } + else + { + InstallWorkloads(null); + } } catch (Exception e) { @@ -225,6 +156,87 @@ public override int Execute() return _workloadInstaller.ExitCode; } + private void InstallWorkloads(WorkloadHistoryRecorder recorder) + { + // Normally we want to validate that the workload IDs specified were valid. However, if there is a global.json file with a workload + // set version specified, and we might install that workload version, then we don't do that check here, because we might not have the right + // workload set installed yet, and trying to list the available workloads would throw an error + if (_skipManifestUpdate || string.IsNullOrEmpty(_workloadSetVersionFromGlobalJson)) + { + ValidateWorkloadIdsInput(); + } + + Reporter.WriteLine(); + + DirectoryPath? offlineCache = string.IsNullOrWhiteSpace(_fromCacheOption) ? null : new DirectoryPath(_fromCacheOption); + + if (!_skipManifestUpdate) + { + var installStateFilePath = Path.Combine(WorkloadInstallType.GetInstallStateFolder(_sdkFeatureBand, _workloadRootDir), "default.json"); + if (string.IsNullOrWhiteSpace(_fromRollbackDefinition) && + !SpecifiedWorkloadSetVersionOnCommandLine && + !SpecifiedWorkloadSetVersionInGlobalJson && + InstallStateContents.FromPath(installStateFilePath) is InstallStateContents installState && + (installState.Manifests != null || installState.WorkloadVersion != null)) + { + // If the workload version is pinned in the install state, then we don't want to automatically update workloads when a workload is installed + // To update to a new version, the user would need to run "dotnet workload update" + _skipManifestUpdate = true; + } + } + + RunInNewTransaction(context => + { + if (!_skipManifestUpdate) + { + if (Verbosity != VerbosityOptions.quiet && Verbosity != VerbosityOptions.q) + { + Reporter.WriteLine(LocalizableStrings.CheckForUpdatedWorkloadManifests); + } + UpdateWorkloadManifests(recorder, context, offlineCache); + } + + // Add workload Ids that already exist to our collection to later trigger an update in those installed workloads + var workloadIds = _workloadIds.Select(id => new WorkloadId(id)); + var installedWorkloads = _workloadInstaller.GetWorkloadInstallationRecordRepository().GetInstalledWorkloads(_sdkFeatureBand); + var previouslyInstalledWorkloads = installedWorkloads.Intersect(workloadIds); + if (previouslyInstalledWorkloads.Any()) + { + Reporter.WriteLine(string.Format(LocalizableStrings.WorkloadAlreadyInstalled, string.Join(" ", previouslyInstalledWorkloads)).Yellow()); + } + workloadIds = workloadIds.Concat(installedWorkloads).Distinct(); + workloadIds = WriteSDKInstallRecordsForVSWorkloads(workloadIds); + + _workloadInstaller.InstallWorkloads(workloadIds, _sdkFeatureBand, context, offlineCache); + + // Write workload installation records + var recordRepo = _workloadInstaller.GetWorkloadInstallationRecordRepository(); + var newWorkloadInstallRecords = workloadIds.Except(recordRepo.GetInstalledWorkloads(_sdkFeatureBand)); + context.Run( + action: () => + { + foreach (var workloadId in newWorkloadInstallRecords) + { + recordRepo.WriteWorkloadInstallationRecord(workloadId, _sdkFeatureBand); + } + }, + rollback: () => + { + foreach (var workloadId in newWorkloadInstallRecords) + { + recordRepo.DeleteWorkloadInstallationRecord(workloadId, _sdkFeatureBand); + } + }); + + TryRunGarbageCollection(_workloadInstaller, Reporter, Verbosity, workloadSetVersion => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, workloadSetVersion), offlineCache); + + Reporter.WriteLine(); + Reporter.WriteLine(string.Format(LocalizableStrings.InstallationSucceeded, string.Join(" ", newWorkloadInstallRecords))); + Reporter.WriteLine(); + + }); + } + internal static void TryRunGarbageCollection(IInstaller workloadInstaller, IReporter reporter, VerbosityOptions verbosity, Func getResolverForWorkloadSet, DirectoryPath? offlineCache = null) { try diff --git a/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs b/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs index b056c8a98372..78f64dffbec1 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs @@ -6,6 +6,7 @@ using Microsoft.Build.Execution; using Microsoft.Build.Logging; using Microsoft.DotNet.Cli; +using Microsoft.DotNet.Cli.NuGetPackageDownloader; using Microsoft.DotNet.Cli.Utils; using Microsoft.DotNet.Workloads.Workload.Install; using Microsoft.DotNet.Workloads.Workload.Update; @@ -35,25 +36,43 @@ public override int Execute() var globalJsonPath = SdkDirectoryWorkloadManifestProvider.GetGlobalJsonPath(Environment.CurrentDirectory); var workloadSetVersionFromGlobalJson = SdkDirectoryWorkloadManifestProvider.GlobalJsonReader.GetWorkloadVersionFromGlobalJson(globalJsonPath); - // If there's a workload set version specified in the global.json file, we need to make sure the workload set is installed before we try to discover workload ids. - if (!string.IsNullOrWhiteSpace(workloadSetVersionFromGlobalJson)) + var workloadResolverFactory = new WorkloadResolverFactory(); + var creationResult = workloadResolverFactory.Create(); + var workloadInstaller = WorkloadInstallerFactory.GetWorkloadInstaller(NullReporter.Instance, new SdkFeatureBand(creationResult.SdkVersion), + creationResult.WorkloadResolver, Verbosity, creationResult.UserProfileDir, VerifySignatures, PackageDownloader, + creationResult.DotnetPath, TempDirectoryPath, null, RestoreActionConfiguration, elevationRequired: true); + var recorder = new WorkloadHistoryRecorder( + creationResult.WorkloadResolver, + workloadInstaller, + () => workloadResolverFactory.CreateForWorkloadSet( + creationResult.DotnetPath, + creationResult.SdkVersion.ToString(), + creationResult.UserProfileDir, + null)); + recorder.HistoryRecord.CommandName = "restore"; + + recorder.Run(() => { - var creationResult = new WorkloadResolverFactory().Create(); - if (creationResult.WorkloadResolver.GetWorkloadManifestProvider() is SdkDirectoryWorkloadManifestProvider provider && provider.WouldThrowException()) + // If there's a workload set version specified in the global.json file, we need to make sure the workload set is installed before we try to discover workload ids. + if (!string.IsNullOrWhiteSpace(workloadSetVersionFromGlobalJson)) { - new WorkloadUpdateCommand(_result).Execute(); + if (creationResult.WorkloadResolver.GetWorkloadManifestProvider() is SdkDirectoryWorkloadManifestProvider provider && provider.WouldThrowException()) + { + new WorkloadUpdateCommand(_result).Execute(); + } } - } - var allProjects = DiscoverAllProjects(Directory.GetCurrentDirectory(), _slnOrProjectArgument).Distinct(); - List allWorkloadId = RunTargetToGetWorkloadIds(allProjects); - Reporter.WriteLine(string.Format(LocalizableStrings.InstallingWorkloads, string.Join(" ", allWorkloadId))); + var allProjects = DiscoverAllProjects(Directory.GetCurrentDirectory(), _slnOrProjectArgument).Distinct(); + List allWorkloadId = RunTargetToGetWorkloadIds(allProjects); + Reporter.WriteLine(string.Format(LocalizableStrings.InstallingWorkloads, string.Join(" ", allWorkloadId))); - var workloadInstallCommand = new WorkloadInstallCommand(_result, - workloadIds: allWorkloadId.Select(a => a.ToString()).ToList().AsReadOnly()); - workloadInstallCommand.IsRunningRestore = true; + var workloadInstallCommand = new WorkloadInstallCommand(_result, + workloadIds: allWorkloadId.Select(a => a.ToString()).ToList().AsReadOnly()); + workloadInstallCommand.IsRunningRestore = true; - workloadInstallCommand.Execute(); + workloadInstallCommand.Execute(); + }); + return 0; } diff --git a/src/Cli/dotnet/commands/dotnet-workload/update/WorkloadUpdateCommand.cs b/src/Cli/dotnet/commands/dotnet-workload/update/WorkloadUpdateCommand.cs index a71770a3f038..5d40f9c0d637 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/update/WorkloadUpdateCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/update/WorkloadUpdateCommand.cs @@ -22,6 +22,7 @@ internal class WorkloadUpdateCommand : InstallingWorkloadCommand private readonly bool _printRollbackDefinitionOnly; private readonly bool _fromPreviousSdk; private WorkloadHistoryRecorder _recorder; + private readonly bool _isRestoring; public WorkloadUpdateCommand( ParseResult parseResult, IReporter reporter = null, @@ -29,7 +30,8 @@ public WorkloadUpdateCommand( IInstaller workloadInstaller = null, INuGetPackageDownloader nugetPackageDownloader = null, IWorkloadManifestUpdater workloadManifestUpdater = null, - string tempDirPath = null) + string tempDirPath = null, + bool isRestoring = false) : base(parseResult, reporter: reporter, workloadResolverFactory: workloadResolverFactory, workloadInstaller: workloadInstaller, nugetPackageDownloader: nugetPackageDownloader, workloadManifestUpdater: workloadManifestUpdater, tempDirPath: tempDirPath) @@ -53,6 +55,7 @@ public WorkloadUpdateCommand( _fromHistorySpecified = parseResult.GetValue(WorkloadUpdateCommandParser.FromHistoryOption); _historyManifestOnlyOption = !string.IsNullOrWhiteSpace(parseResult.GetValue(WorkloadUpdateCommandParser.HistoryManifestOnlyOption)); + _isRestoring = isRestoring; } public override int Execute() @@ -104,39 +107,17 @@ public override int Execute() Reporter.WriteLine(); try { - _recorder.Run(() => + if (!_isRestoring) { - DirectoryPath? offlineCache = string.IsNullOrWhiteSpace(_fromCacheOption) ? null : new DirectoryPath(_fromCacheOption); - var workloadIds = Enumerable.Empty(); - RunInNewTransaction(context => + _recorder.Run(() => { - UpdateWorkloadManifests(_recorder, context, offlineCache); - - // This depends on getting the available workloads, so it needs to run after manifests have potentially been installed - workloadIds = WriteSDKInstallRecordsForVSWorkloads(GetUpdatableWorkloads()); - - if (FromHistory) - { - if (!_historyManifestOnlyOption) - { - UpdateInstalledWorkloadsFromHistory(context, offlineCache); - } - } - else - { - _workloadInstaller.InstallWorkloads(workloadIds, _sdkFeatureBand, context, offlineCache); - } + UpdateWorkloads(); }); - - WorkloadInstallCommand.TryRunGarbageCollection(_workloadInstaller, Reporter, Verbosity, workloadSetVersion => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, workloadSetVersion), offlineCache); - - // TODO: potentially only do this in some cases (ie not if global.json specifies workload set) - _workloadManifestUpdater.DeleteUpdatableWorkloadsFile(); - - Reporter.WriteLine(); - Reporter.WriteLine(string.Format(LocalizableStrings.UpdateSucceeded, string.Join(" ", workloadIds))); - Reporter.WriteLine(); - }); + } + else + { + UpdateWorkloads(); + } } catch (Exception e) { @@ -149,6 +130,40 @@ public override int Execute() return _workloadInstaller.ExitCode; } + private void UpdateWorkloads() + { + DirectoryPath? offlineCache = string.IsNullOrWhiteSpace(_fromCacheOption) ? null : new DirectoryPath(_fromCacheOption); + var workloadIds = Enumerable.Empty(); + RunInNewTransaction(context => + { + UpdateWorkloadManifests(_recorder, context, offlineCache); + + // This depends on getting the available workloads, so it needs to run after manifests have potentially been installed + workloadIds = WriteSDKInstallRecordsForVSWorkloads(GetUpdatableWorkloads()); + + if (FromHistory) + { + if (!_historyManifestOnlyOption) + { + UpdateInstalledWorkloadsFromHistory(context, offlineCache); + } + } + else + { + _workloadInstaller.InstallWorkloads(workloadIds, _sdkFeatureBand, context, offlineCache); + } + }); + + WorkloadInstallCommand.TryRunGarbageCollection(_workloadInstaller, Reporter, Verbosity, workloadSetVersion => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, workloadSetVersion), offlineCache); + + // TODO: potentially only do this in some cases (ie not if global.json specifies workload set) + _workloadManifestUpdater.DeleteUpdatableWorkloadsFile(); + + Reporter.WriteLine(); + Reporter.WriteLine(string.Format(LocalizableStrings.UpdateSucceeded, string.Join(" ", workloadIds))); + Reporter.WriteLine(); + } + private void UpdateInstalledWorkloadsFromHistory(ITransactionContext context, DirectoryPath? offlineCache) { if (FromHistory) From c8dacc6ba15cbb50de1f8239e69b79e9aa18273a Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Wed, 7 Aug 2024 17:07:34 -0400 Subject: [PATCH 3/7] Permit workload restore to work with new global.json Specifying a workload set version in the global.json file is a supported scenario. If that workload set has not yet been installed, and the user tries to take an action that relies on workloads, we should error and indicate that they should make sure that workload set is installed first via update, install, or restore. We do that correctly, but when the user subsequently restores, we do not escape from the error case and instead direct the user to run restore. We should handle it properly, essentially installing the workload set before we discover which workloads the user needs and installing those. --- .../restore/WorkloadRestoreCommand.cs | 14 ++++++++++++++ .../SdkDirectoryWorkloadManifestProvider.cs | 2 ++ 2 files changed, 16 insertions(+) diff --git a/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs b/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs index 4f5bfc994bdb..b056c8a98372 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs @@ -8,6 +8,7 @@ using Microsoft.DotNet.Cli; using Microsoft.DotNet.Cli.Utils; using Microsoft.DotNet.Workloads.Workload.Install; +using Microsoft.DotNet.Workloads.Workload.Update; using Microsoft.Extensions.EnvironmentAbstractions; using Microsoft.NET.Sdk.WorkloadManifestReader; @@ -31,6 +32,19 @@ public WorkloadRestoreCommand( public override int Execute() { + var globalJsonPath = SdkDirectoryWorkloadManifestProvider.GetGlobalJsonPath(Environment.CurrentDirectory); + var workloadSetVersionFromGlobalJson = SdkDirectoryWorkloadManifestProvider.GlobalJsonReader.GetWorkloadVersionFromGlobalJson(globalJsonPath); + + // If there's a workload set version specified in the global.json file, we need to make sure the workload set is installed before we try to discover workload ids. + if (!string.IsNullOrWhiteSpace(workloadSetVersionFromGlobalJson)) + { + var creationResult = new WorkloadResolverFactory().Create(); + if (creationResult.WorkloadResolver.GetWorkloadManifestProvider() is SdkDirectoryWorkloadManifestProvider provider && provider.WouldThrowException()) + { + new WorkloadUpdateCommand(_result).Execute(); + } + } + var allProjects = DiscoverAllProjects(Directory.GetCurrentDirectory(), _slnOrProjectArgument).Distinct(); List allWorkloadId = RunTargetToGetWorkloadIds(allProjects); Reporter.WriteLine(string.Format(LocalizableStrings.InstallingWorkloads, string.Join(" ", allWorkloadId))); diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs index 4acd8945ff9a..5977b31865c3 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs @@ -235,6 +235,8 @@ private static int VersionCompare(string first, string second) return new ReleaseVersion(modifiedFirst).CompareTo(new ReleaseVersion(modifiedSecond)); } + public bool WouldThrowException() => _exceptionToThrow is not null; + void ThrowExceptionIfManifestsNotAvailable() { if (_exceptionToThrow != null) From 9840fbd07e4f535d18f24da7002c0d6cc5c3c546 Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Wed, 7 Aug 2024 18:36:27 -0400 Subject: [PATCH 4/7] move up workload history recorder --- .../commands/InstallingWorkloadCommand.cs | 2 +- .../install/WorkloadInstallCommand.cs | 168 ++++++++++-------- .../restore/WorkloadRestoreCommand.cs | 45 +++-- .../update/WorkloadUpdateCommand.cs | 77 ++++---- 4 files changed, 169 insertions(+), 123 deletions(-) diff --git a/src/Cli/dotnet/commands/InstallingWorkloadCommand.cs b/src/Cli/dotnet/commands/InstallingWorkloadCommand.cs index 4ef78e76a211..d5ab676fce1c 100644 --- a/src/Cli/dotnet/commands/InstallingWorkloadCommand.cs +++ b/src/Cli/dotnet/commands/InstallingWorkloadCommand.cs @@ -208,7 +208,7 @@ protected void UpdateWorkloadManifests(WorkloadHistoryRecorder recorder, ITransa _workloadInstaller.UpdateInstallMode(_sdkFeatureBand, true); } - if (SpecifiedWorkloadSetVersionInGlobalJson) + if (SpecifiedWorkloadSetVersionInGlobalJson && recorder is not null) { recorder.HistoryRecord.GlobalJsonVersion = _workloadSetVersionFromGlobalJson; } diff --git a/src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs b/src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs index cf93d982b185..c7c203500c45 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs @@ -127,90 +127,21 @@ public override int Execute() else { WorkloadHistoryRecorder recorder = new WorkloadHistoryRecorder(_workloadResolver, _workloadInstaller, () => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, null)); - recorder.HistoryRecord.CommandName = IsRunningRestore ? "restore" : "install"; + recorder.HistoryRecord.CommandName = "install"; try { - recorder.Run(() => + if (!IsRunningRestore) { - // Normally we want to validate that the workload IDs specified were valid. However, if there is a global.json file with a workload - // set version specified, and we might install that workload version, then we don't do that check here, because we might not have the right - // workload set installed yet, and trying to list the available workloads would throw an error - if (_skipManifestUpdate || string.IsNullOrEmpty(_workloadSetVersionFromGlobalJson)) + recorder.Run(() => { - ValidateWorkloadIdsInput(); - } - - Reporter.WriteLine(); - - DirectoryPath? offlineCache = string.IsNullOrWhiteSpace(_fromCacheOption) ? null : new DirectoryPath(_fromCacheOption); - - if (!_skipManifestUpdate) - { - var installStateFilePath = Path.Combine(WorkloadInstallType.GetInstallStateFolder(_sdkFeatureBand, _workloadRootDir), "default.json"); - if (string.IsNullOrWhiteSpace(_fromRollbackDefinition) && - !SpecifiedWorkloadSetVersionOnCommandLine && - !SpecifiedWorkloadSetVersionInGlobalJson && - InstallStateContents.FromPath(installStateFilePath) is InstallStateContents installState && - (installState.Manifests != null || installState.WorkloadVersion != null)) - { - // If the workload version is pinned in the install state, then we don't want to automatically update workloads when a workload is installed - // To update to a new version, the user would need to run "dotnet workload update" - _skipManifestUpdate = true; - } - } - - RunInNewTransaction(context => - { - if (!_skipManifestUpdate) - { - if (Verbosity != VerbosityOptions.quiet && Verbosity != VerbosityOptions.q) - { - Reporter.WriteLine(LocalizableStrings.CheckForUpdatedWorkloadManifests); - } - UpdateWorkloadManifests(recorder, context, offlineCache); - } - - // Add workload Ids that already exist to our collection to later trigger an update in those installed workloads - var workloadIds = _workloadIds.Select(id => new WorkloadId(id)); - var installedWorkloads = _workloadInstaller.GetWorkloadInstallationRecordRepository().GetInstalledWorkloads(_sdkFeatureBand); - var previouslyInstalledWorkloads = installedWorkloads.Intersect(workloadIds); - if (previouslyInstalledWorkloads.Any()) - { - Reporter.WriteLine(string.Format(LocalizableStrings.WorkloadAlreadyInstalled, string.Join(" ", previouslyInstalledWorkloads)).Yellow()); - } - workloadIds = workloadIds.Concat(installedWorkloads).Distinct(); - workloadIds = WriteSDKInstallRecordsForVSWorkloads(workloadIds); - - _workloadInstaller.InstallWorkloads(workloadIds, _sdkFeatureBand, context, offlineCache); - - // Write workload installation records - var recordRepo = _workloadInstaller.GetWorkloadInstallationRecordRepository(); - var newWorkloadInstallRecords = workloadIds.Except(recordRepo.GetInstalledWorkloads(_sdkFeatureBand)); - context.Run( - action: () => - { - foreach (var workloadId in newWorkloadInstallRecords) - { - recordRepo.WriteWorkloadInstallationRecord(workloadId, _sdkFeatureBand); - } - }, - rollback: () => - { - foreach (var workloadId in newWorkloadInstallRecords) - { - recordRepo.DeleteWorkloadInstallationRecord(workloadId, _sdkFeatureBand); - } - }); - - TryRunGarbageCollection(_workloadInstaller, Reporter, Verbosity, workloadSetVersion => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, workloadSetVersion), offlineCache); - - Reporter.WriteLine(); - Reporter.WriteLine(string.Format(LocalizableStrings.InstallationSucceeded, string.Join(" ", newWorkloadInstallRecords))); - Reporter.WriteLine(); - + InstallWorkloads(recorder); }); - }); + } + else + { + InstallWorkloads(null); + } } catch (Exception e) { @@ -225,6 +156,87 @@ public override int Execute() return _workloadInstaller.ExitCode; } + private void InstallWorkloads(WorkloadHistoryRecorder recorder) + { + // Normally we want to validate that the workload IDs specified were valid. However, if there is a global.json file with a workload + // set version specified, and we might install that workload version, then we don't do that check here, because we might not have the right + // workload set installed yet, and trying to list the available workloads would throw an error + if (_skipManifestUpdate || string.IsNullOrEmpty(_workloadSetVersionFromGlobalJson)) + { + ValidateWorkloadIdsInput(); + } + + Reporter.WriteLine(); + + DirectoryPath? offlineCache = string.IsNullOrWhiteSpace(_fromCacheOption) ? null : new DirectoryPath(_fromCacheOption); + + if (!_skipManifestUpdate) + { + var installStateFilePath = Path.Combine(WorkloadInstallType.GetInstallStateFolder(_sdkFeatureBand, _workloadRootDir), "default.json"); + if (string.IsNullOrWhiteSpace(_fromRollbackDefinition) && + !SpecifiedWorkloadSetVersionOnCommandLine && + !SpecifiedWorkloadSetVersionInGlobalJson && + InstallStateContents.FromPath(installStateFilePath) is InstallStateContents installState && + (installState.Manifests != null || installState.WorkloadVersion != null)) + { + // If the workload version is pinned in the install state, then we don't want to automatically update workloads when a workload is installed + // To update to a new version, the user would need to run "dotnet workload update" + _skipManifestUpdate = true; + } + } + + RunInNewTransaction(context => + { + if (!_skipManifestUpdate) + { + if (Verbosity != VerbosityOptions.quiet && Verbosity != VerbosityOptions.q) + { + Reporter.WriteLine(LocalizableStrings.CheckForUpdatedWorkloadManifests); + } + UpdateWorkloadManifests(recorder, context, offlineCache); + } + + // Add workload Ids that already exist to our collection to later trigger an update in those installed workloads + var workloadIds = _workloadIds.Select(id => new WorkloadId(id)); + var installedWorkloads = _workloadInstaller.GetWorkloadInstallationRecordRepository().GetInstalledWorkloads(_sdkFeatureBand); + var previouslyInstalledWorkloads = installedWorkloads.Intersect(workloadIds); + if (previouslyInstalledWorkloads.Any()) + { + Reporter.WriteLine(string.Format(LocalizableStrings.WorkloadAlreadyInstalled, string.Join(" ", previouslyInstalledWorkloads)).Yellow()); + } + workloadIds = workloadIds.Concat(installedWorkloads).Distinct(); + workloadIds = WriteSDKInstallRecordsForVSWorkloads(workloadIds); + + _workloadInstaller.InstallWorkloads(workloadIds, _sdkFeatureBand, context, offlineCache); + + // Write workload installation records + var recordRepo = _workloadInstaller.GetWorkloadInstallationRecordRepository(); + var newWorkloadInstallRecords = workloadIds.Except(recordRepo.GetInstalledWorkloads(_sdkFeatureBand)); + context.Run( + action: () => + { + foreach (var workloadId in newWorkloadInstallRecords) + { + recordRepo.WriteWorkloadInstallationRecord(workloadId, _sdkFeatureBand); + } + }, + rollback: () => + { + foreach (var workloadId in newWorkloadInstallRecords) + { + recordRepo.DeleteWorkloadInstallationRecord(workloadId, _sdkFeatureBand); + } + }); + + TryRunGarbageCollection(_workloadInstaller, Reporter, Verbosity, workloadSetVersion => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, workloadSetVersion), offlineCache); + + Reporter.WriteLine(); + Reporter.WriteLine(string.Format(LocalizableStrings.InstallationSucceeded, string.Join(" ", newWorkloadInstallRecords))); + Reporter.WriteLine(); + + }); + } + internal static void TryRunGarbageCollection(IInstaller workloadInstaller, IReporter reporter, VerbosityOptions verbosity, Func getResolverForWorkloadSet, DirectoryPath? offlineCache = null) { try diff --git a/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs b/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs index b056c8a98372..78f64dffbec1 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs @@ -6,6 +6,7 @@ using Microsoft.Build.Execution; using Microsoft.Build.Logging; using Microsoft.DotNet.Cli; +using Microsoft.DotNet.Cli.NuGetPackageDownloader; using Microsoft.DotNet.Cli.Utils; using Microsoft.DotNet.Workloads.Workload.Install; using Microsoft.DotNet.Workloads.Workload.Update; @@ -35,25 +36,43 @@ public override int Execute() var globalJsonPath = SdkDirectoryWorkloadManifestProvider.GetGlobalJsonPath(Environment.CurrentDirectory); var workloadSetVersionFromGlobalJson = SdkDirectoryWorkloadManifestProvider.GlobalJsonReader.GetWorkloadVersionFromGlobalJson(globalJsonPath); - // If there's a workload set version specified in the global.json file, we need to make sure the workload set is installed before we try to discover workload ids. - if (!string.IsNullOrWhiteSpace(workloadSetVersionFromGlobalJson)) + var workloadResolverFactory = new WorkloadResolverFactory(); + var creationResult = workloadResolverFactory.Create(); + var workloadInstaller = WorkloadInstallerFactory.GetWorkloadInstaller(NullReporter.Instance, new SdkFeatureBand(creationResult.SdkVersion), + creationResult.WorkloadResolver, Verbosity, creationResult.UserProfileDir, VerifySignatures, PackageDownloader, + creationResult.DotnetPath, TempDirectoryPath, null, RestoreActionConfiguration, elevationRequired: true); + var recorder = new WorkloadHistoryRecorder( + creationResult.WorkloadResolver, + workloadInstaller, + () => workloadResolverFactory.CreateForWorkloadSet( + creationResult.DotnetPath, + creationResult.SdkVersion.ToString(), + creationResult.UserProfileDir, + null)); + recorder.HistoryRecord.CommandName = "restore"; + + recorder.Run(() => { - var creationResult = new WorkloadResolverFactory().Create(); - if (creationResult.WorkloadResolver.GetWorkloadManifestProvider() is SdkDirectoryWorkloadManifestProvider provider && provider.WouldThrowException()) + // If there's a workload set version specified in the global.json file, we need to make sure the workload set is installed before we try to discover workload ids. + if (!string.IsNullOrWhiteSpace(workloadSetVersionFromGlobalJson)) { - new WorkloadUpdateCommand(_result).Execute(); + if (creationResult.WorkloadResolver.GetWorkloadManifestProvider() is SdkDirectoryWorkloadManifestProvider provider && provider.WouldThrowException()) + { + new WorkloadUpdateCommand(_result).Execute(); + } } - } - var allProjects = DiscoverAllProjects(Directory.GetCurrentDirectory(), _slnOrProjectArgument).Distinct(); - List allWorkloadId = RunTargetToGetWorkloadIds(allProjects); - Reporter.WriteLine(string.Format(LocalizableStrings.InstallingWorkloads, string.Join(" ", allWorkloadId))); + var allProjects = DiscoverAllProjects(Directory.GetCurrentDirectory(), _slnOrProjectArgument).Distinct(); + List allWorkloadId = RunTargetToGetWorkloadIds(allProjects); + Reporter.WriteLine(string.Format(LocalizableStrings.InstallingWorkloads, string.Join(" ", allWorkloadId))); - var workloadInstallCommand = new WorkloadInstallCommand(_result, - workloadIds: allWorkloadId.Select(a => a.ToString()).ToList().AsReadOnly()); - workloadInstallCommand.IsRunningRestore = true; + var workloadInstallCommand = new WorkloadInstallCommand(_result, + workloadIds: allWorkloadId.Select(a => a.ToString()).ToList().AsReadOnly()); + workloadInstallCommand.IsRunningRestore = true; - workloadInstallCommand.Execute(); + workloadInstallCommand.Execute(); + }); + return 0; } diff --git a/src/Cli/dotnet/commands/dotnet-workload/update/WorkloadUpdateCommand.cs b/src/Cli/dotnet/commands/dotnet-workload/update/WorkloadUpdateCommand.cs index a71770a3f038..5d40f9c0d637 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/update/WorkloadUpdateCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/update/WorkloadUpdateCommand.cs @@ -22,6 +22,7 @@ internal class WorkloadUpdateCommand : InstallingWorkloadCommand private readonly bool _printRollbackDefinitionOnly; private readonly bool _fromPreviousSdk; private WorkloadHistoryRecorder _recorder; + private readonly bool _isRestoring; public WorkloadUpdateCommand( ParseResult parseResult, IReporter reporter = null, @@ -29,7 +30,8 @@ public WorkloadUpdateCommand( IInstaller workloadInstaller = null, INuGetPackageDownloader nugetPackageDownloader = null, IWorkloadManifestUpdater workloadManifestUpdater = null, - string tempDirPath = null) + string tempDirPath = null, + bool isRestoring = false) : base(parseResult, reporter: reporter, workloadResolverFactory: workloadResolverFactory, workloadInstaller: workloadInstaller, nugetPackageDownloader: nugetPackageDownloader, workloadManifestUpdater: workloadManifestUpdater, tempDirPath: tempDirPath) @@ -53,6 +55,7 @@ public WorkloadUpdateCommand( _fromHistorySpecified = parseResult.GetValue(WorkloadUpdateCommandParser.FromHistoryOption); _historyManifestOnlyOption = !string.IsNullOrWhiteSpace(parseResult.GetValue(WorkloadUpdateCommandParser.HistoryManifestOnlyOption)); + _isRestoring = isRestoring; } public override int Execute() @@ -104,39 +107,17 @@ public override int Execute() Reporter.WriteLine(); try { - _recorder.Run(() => + if (!_isRestoring) { - DirectoryPath? offlineCache = string.IsNullOrWhiteSpace(_fromCacheOption) ? null : new DirectoryPath(_fromCacheOption); - var workloadIds = Enumerable.Empty(); - RunInNewTransaction(context => + _recorder.Run(() => { - UpdateWorkloadManifests(_recorder, context, offlineCache); - - // This depends on getting the available workloads, so it needs to run after manifests have potentially been installed - workloadIds = WriteSDKInstallRecordsForVSWorkloads(GetUpdatableWorkloads()); - - if (FromHistory) - { - if (!_historyManifestOnlyOption) - { - UpdateInstalledWorkloadsFromHistory(context, offlineCache); - } - } - else - { - _workloadInstaller.InstallWorkloads(workloadIds, _sdkFeatureBand, context, offlineCache); - } + UpdateWorkloads(); }); - - WorkloadInstallCommand.TryRunGarbageCollection(_workloadInstaller, Reporter, Verbosity, workloadSetVersion => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, workloadSetVersion), offlineCache); - - // TODO: potentially only do this in some cases (ie not if global.json specifies workload set) - _workloadManifestUpdater.DeleteUpdatableWorkloadsFile(); - - Reporter.WriteLine(); - Reporter.WriteLine(string.Format(LocalizableStrings.UpdateSucceeded, string.Join(" ", workloadIds))); - Reporter.WriteLine(); - }); + } + else + { + UpdateWorkloads(); + } } catch (Exception e) { @@ -149,6 +130,40 @@ public override int Execute() return _workloadInstaller.ExitCode; } + private void UpdateWorkloads() + { + DirectoryPath? offlineCache = string.IsNullOrWhiteSpace(_fromCacheOption) ? null : new DirectoryPath(_fromCacheOption); + var workloadIds = Enumerable.Empty(); + RunInNewTransaction(context => + { + UpdateWorkloadManifests(_recorder, context, offlineCache); + + // This depends on getting the available workloads, so it needs to run after manifests have potentially been installed + workloadIds = WriteSDKInstallRecordsForVSWorkloads(GetUpdatableWorkloads()); + + if (FromHistory) + { + if (!_historyManifestOnlyOption) + { + UpdateInstalledWorkloadsFromHistory(context, offlineCache); + } + } + else + { + _workloadInstaller.InstallWorkloads(workloadIds, _sdkFeatureBand, context, offlineCache); + } + }); + + WorkloadInstallCommand.TryRunGarbageCollection(_workloadInstaller, Reporter, Verbosity, workloadSetVersion => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, workloadSetVersion), offlineCache); + + // TODO: potentially only do this in some cases (ie not if global.json specifies workload set) + _workloadManifestUpdater.DeleteUpdatableWorkloadsFile(); + + Reporter.WriteLine(); + Reporter.WriteLine(string.Format(LocalizableStrings.UpdateSucceeded, string.Join(" ", workloadIds))); + Reporter.WriteLine(); + } + private void UpdateInstalledWorkloadsFromHistory(ITransactionContext context, DirectoryPath? offlineCache) { if (FromHistory) From 8e820df275afda3dc0d24e9dc2b3fd94cfffa859 Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Mon, 19 Aug 2024 15:59:39 -0400 Subject: [PATCH 5/7] Comments --- .../restore/WorkloadRestoreCommand.cs | 2 +- .../IWorkloadManifestProvider.cs | 2 ++ .../TempDirectoryWorkloadManifestProvider.cs | 1 + .../WorkloadResolver.cs | 1 + .../FakeManifestProvider.cs | 2 ++ .../WorkloadSetTests.cs | 22 ++++++------------- .../MockManifestProvider.cs | 1 + 7 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs b/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs index 78f64dffbec1..95be1aea70da 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs @@ -56,7 +56,7 @@ public override int Execute() // If there's a workload set version specified in the global.json file, we need to make sure the workload set is installed before we try to discover workload ids. if (!string.IsNullOrWhiteSpace(workloadSetVersionFromGlobalJson)) { - if (creationResult.WorkloadResolver.GetWorkloadManifestProvider() is SdkDirectoryWorkloadManifestProvider provider && provider.WouldThrowException()) + if (creationResult.WorkloadResolver.GetWorkloadManifestProvider().WouldThrowException()) { new WorkloadUpdateCommand(_result).Execute(); } diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/IWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/IWorkloadManifestProvider.cs index f6a5e9676624..1af98c2c982d 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/IWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/IWorkloadManifestProvider.cs @@ -17,5 +17,7 @@ public interface IWorkloadManifestProvider string? GetWorkloadVersion(); Dictionary GetAvailableWorkloadSets(); + + bool WouldThrowException(); } } diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs index 79d1351d9f90..466d62825462 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs @@ -55,5 +55,6 @@ public IEnumerable GetManifestDirectories() public string GetSdkFeatureBand() => _sdkVersionBand; public string? GetWorkloadVersion() => _sdkVersionBand.ToString() + ".2"; public Dictionary GetAvailableWorkloadSets() => new(); + public bool WouldThrowException() => false; } } diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs index 3d9eba645554..c789eb54ee40 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs @@ -779,6 +779,7 @@ public void RefreshWorkloadManifests() { } public IEnumerable GetManifests() => Enumerable.Empty(); public string GetSdkFeatureBand() => _sdkFeatureBand; public string? GetWorkloadVersion() => _sdkFeatureBand.ToString() + ".2"; + public bool WouldThrowException() => false; } } diff --git a/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs b/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs index 354aeb7ad8ee..b7bc94775b93 100644 --- a/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs +++ b/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs @@ -41,6 +41,7 @@ public IEnumerable GetManifests() public string GetSdkFeatureBand() => "8.0.100"; public Dictionary GetAvailableWorkloadSets() => throw new NotImplementedException(); public string? GetWorkloadVersion() => "8.0.100.2"; + public bool WouldThrowException() => false; } internal class InMemoryFakeManifestProvider : IWorkloadManifestProvider, IEnumerable<(string id, string content)> @@ -68,5 +69,6 @@ public IEnumerable GetManifests() public string GetSdkFeatureBand() => "8.0.100"; public Dictionary GetAvailableWorkloadSets() => throw new NotImplementedException(); public string? GetWorkloadVersion() => "8.0.100.2"; + public bool WouldThrowException() => false; } } diff --git a/test/dotnet-MsiInstallation.Tests/WorkloadSetTests.cs b/test/dotnet-MsiInstallation.Tests/WorkloadSetTests.cs index a42b6f263f43..629821804cbf 100644 --- a/test/dotnet-MsiInstallation.Tests/WorkloadSetTests.cs +++ b/test/dotnet-MsiInstallation.Tests/WorkloadSetTests.cs @@ -274,24 +274,16 @@ void SetupWorkloadSetInGlobalJson(out WorkloadSet originalRollback) AddNuGetSource(@"C:\SdkTesting\workloadsets", SdkTestingDirectory); } - [Fact] - public void UpdateWorkloadSetViaGlobalJson() - { - SetupWorkloadSetInGlobalJson(out var originalRollback); - - VM.CreateRunCommand("dotnet", "workload", "update").WithWorkingDirectory(SdkTestingDirectory).Execute().Should().Pass(); - GetRollback(SdkTestingDirectory).Should().NotBe(originalRollback); - } - - [Fact] - public void InstallWorkloadSetViaGlobalJson() + [Theory] + [InlineData("restore")] + [InlineData("update")] + [InlineData("install")] + public void UseGlobalJsonToSpecifyWorkloadSet(string command) { SetupWorkloadSetInGlobalJson(out var originalRollback); - VM.CreateRunCommand("dotnet", "workload", "install", "aspire") - .WithWorkingDirectory(SdkTestingDirectory) - .Execute().Should().Pass(); - + string[] args = command.Equals("install") ? ["dotnet", "workload", "install", "aspire"] : ["dotnet", "workload", command]; + VM.CreateRunCommand(args).WithWorkingDirectory(SdkTestingDirectory).Execute().Should().Pass(); GetRollback(SdkTestingDirectory).Should().NotBe(originalRollback); } diff --git a/test/dotnet-workload-install.Tests/MockManifestProvider.cs b/test/dotnet-workload-install.Tests/MockManifestProvider.cs index 94cc85ae5f1d..36753ea06da4 100644 --- a/test/dotnet-workload-install.Tests/MockManifestProvider.cs +++ b/test/dotnet-workload-install.Tests/MockManifestProvider.cs @@ -49,5 +49,6 @@ public IEnumerable GetManifests() public string GetSdkFeatureBand() => SdkFeatureBand.ToString(); public string GetWorkloadVersion() => SdkFeatureBand.ToString() + ".2"; + public bool WouldThrowException() => false; } } From 2fd02c7351ef5988eb9f7d901939b5c56940e4bc Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Fri, 23 Aug 2024 16:53:09 -0400 Subject: [PATCH 6/7] Remove WouldThrowException --- .../IWorkloadManifestProvider.cs | 2 -- .../SdkDirectoryWorkloadManifestProvider.cs | 2 -- .../TempDirectoryWorkloadManifestProvider.cs | 1 - .../WorkloadResolver.cs | 1 - .../FakeManifestProvider.cs | 2 -- test/dotnet-workload-install.Tests/MockManifestProvider.cs | 1 - 6 files changed, 9 deletions(-) diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/IWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/IWorkloadManifestProvider.cs index 1af98c2c982d..f6a5e9676624 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/IWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/IWorkloadManifestProvider.cs @@ -17,7 +17,5 @@ public interface IWorkloadManifestProvider string? GetWorkloadVersion(); Dictionary GetAvailableWorkloadSets(); - - bool WouldThrowException(); } } diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs index 5977b31865c3..4acd8945ff9a 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs @@ -235,8 +235,6 @@ private static int VersionCompare(string first, string second) return new ReleaseVersion(modifiedFirst).CompareTo(new ReleaseVersion(modifiedSecond)); } - public bool WouldThrowException() => _exceptionToThrow is not null; - void ThrowExceptionIfManifestsNotAvailable() { if (_exceptionToThrow != null) diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs index 466d62825462..79d1351d9f90 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs @@ -55,6 +55,5 @@ public IEnumerable GetManifestDirectories() public string GetSdkFeatureBand() => _sdkVersionBand; public string? GetWorkloadVersion() => _sdkVersionBand.ToString() + ".2"; public Dictionary GetAvailableWorkloadSets() => new(); - public bool WouldThrowException() => false; } } diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs index c789eb54ee40..3d9eba645554 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs @@ -779,7 +779,6 @@ public void RefreshWorkloadManifests() { } public IEnumerable GetManifests() => Enumerable.Empty(); public string GetSdkFeatureBand() => _sdkFeatureBand; public string? GetWorkloadVersion() => _sdkFeatureBand.ToString() + ".2"; - public bool WouldThrowException() => false; } } diff --git a/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs b/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs index b7bc94775b93..354aeb7ad8ee 100644 --- a/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs +++ b/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs @@ -41,7 +41,6 @@ public IEnumerable GetManifests() public string GetSdkFeatureBand() => "8.0.100"; public Dictionary GetAvailableWorkloadSets() => throw new NotImplementedException(); public string? GetWorkloadVersion() => "8.0.100.2"; - public bool WouldThrowException() => false; } internal class InMemoryFakeManifestProvider : IWorkloadManifestProvider, IEnumerable<(string id, string content)> @@ -69,6 +68,5 @@ public IEnumerable GetManifests() public string GetSdkFeatureBand() => "8.0.100"; public Dictionary GetAvailableWorkloadSets() => throw new NotImplementedException(); public string? GetWorkloadVersion() => "8.0.100.2"; - public bool WouldThrowException() => false; } } diff --git a/test/dotnet-workload-install.Tests/MockManifestProvider.cs b/test/dotnet-workload-install.Tests/MockManifestProvider.cs index 36753ea06da4..94cc85ae5f1d 100644 --- a/test/dotnet-workload-install.Tests/MockManifestProvider.cs +++ b/test/dotnet-workload-install.Tests/MockManifestProvider.cs @@ -49,6 +49,5 @@ public IEnumerable GetManifests() public string GetSdkFeatureBand() => SdkFeatureBand.ToString(); public string GetWorkloadVersion() => SdkFeatureBand.ToString() + ".2"; - public bool WouldThrowException() => false; } } From 58ffe11ba6b660245e31efeca71fc4fb793393ef Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Tue, 27 Aug 2024 13:53:51 -0400 Subject: [PATCH 7/7] Harden against double-install --- .../Framework/VMTestBase.cs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/dotnet-MsiInstallation.Tests/Framework/VMTestBase.cs b/test/dotnet-MsiInstallation.Tests/Framework/VMTestBase.cs index dd11aeb112a0..4b704c299070 100644 --- a/test/dotnet-MsiInstallation.Tests/Framework/VMTestBase.cs +++ b/test/dotnet-MsiInstallation.Tests/Framework/VMTestBase.cs @@ -53,6 +53,7 @@ public virtual void Dispose() } Lazy _sdkInstallerVersion; + private bool _sdkInstalled = false; protected string SdkInstallerVersion => _sdkInstallerVersion.Value; @@ -60,6 +61,11 @@ public virtual void Dispose() protected void InstallSdk(bool deployStage2 = true) { + if (_sdkInstalled) + { + return; + } + VM.CreateRunCommand("setx", "DOTNET_NOLOGO", "true") .WithDescription("Disable .NET SDK first run message") .Execute() @@ -70,12 +76,12 @@ protected void InstallSdk(bool deployStage2 = true) .WithDescription($"Install SDK {SdkInstallerVersion}") .Execute().Should().Pass(); - - if (deployStage2) { DeployStage2Sdk(); } + + _sdkInstalled = true; } protected void UninstallSdk()