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..a80755c6af41 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs @@ -31,12 +31,13 @@ public WorkloadInstallCommand( INuGetPackageDownloader nugetPackageDownloader = null, IWorkloadManifestUpdater workloadManifestUpdater = null, string tempDirPath = null, - IReadOnlyCollection workloadIds = null) + IReadOnlyCollection workloadIds = null, + bool? skipWorkloadManifestUpdate = null) : base(parseResult, reporter: reporter, workloadResolverFactory: workloadResolverFactory, workloadInstaller: workloadInstaller, nugetPackageDownloader: nugetPackageDownloader, workloadManifestUpdater: workloadManifestUpdater, tempDirPath: tempDirPath) { - _skipManifestUpdate = parseResult.GetValue(WorkloadInstallCommandParser.SkipManifestUpdateOption); + _skipManifestUpdate = skipWorkloadManifestUpdate ?? parseResult.GetValue(WorkloadInstallCommandParser.SkipManifestUpdateOption); _workloadIds = workloadIds ?? parseResult.GetValue(WorkloadInstallCommandParser.WorkloadIdArgument).ToList().AsReadOnly(); var resolvedReporter = _printDownloadLinkOnly ? NullReporter.Instance : Reporter; @@ -126,91 +127,22 @@ public override int Execute() } else { - WorkloadHistoryRecorder recorder = new WorkloadHistoryRecorder(_workloadResolver, _workloadInstaller, () => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, null)); - recorder.HistoryRecord.CommandName = IsRunningRestore ? "restore" : "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)) - { - ValidateWorkloadIdsInput(); - } + WorkloadHistoryRecorder recorder = new(_workloadResolver, _workloadInstaller, () => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, null)); + recorder.HistoryRecord.CommandName = "install"; - Reporter.WriteLine(); - - DirectoryPath? offlineCache = string.IsNullOrWhiteSpace(_fromCacheOption) ? null : new DirectoryPath(_fromCacheOption); - - if (!_skipManifestUpdate) + recorder.Run(() => { - 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 +157,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 4f5bfc994bdb..d02a72c2d6d6 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/restore/WorkloadRestoreCommand.cs @@ -6,8 +6,10 @@ 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; using Microsoft.Extensions.EnvironmentAbstractions; using Microsoft.NET.Sdk.WorkloadManifestReader; @@ -31,15 +33,38 @@ public WorkloadRestoreCommand( public override int Execute() { - var allProjects = DiscoverAllProjects(Directory.GetCurrentDirectory(), _slnOrProjectArgument).Distinct(); - List allWorkloadId = RunTargetToGetWorkloadIds(allProjects); - Reporter.WriteLine(string.Format(LocalizableStrings.InstallingWorkloads, string.Join(" ", allWorkloadId))); + 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(() => + { + // First update manifests and install a workload set as necessary + new WorkloadUpdateCommand(_result, recorder: recorder).Execute(); - var workloadInstallCommand = new WorkloadInstallCommand(_result, - workloadIds: allWorkloadId.Select(a => a.ToString()).ToList().AsReadOnly()); - workloadInstallCommand.IsRunningRestore = true; + var allProjects = DiscoverAllProjects(Directory.GetCurrentDirectory(), _slnOrProjectArgument).Distinct(); + List allWorkloadId = RunTargetToGetWorkloadIds(allProjects); + Reporter.WriteLine(string.Format(LocalizableStrings.InstallingWorkloads, string.Join(" ", allWorkloadId))); - workloadInstallCommand.Execute(); + new WorkloadInstallCommand(_result, + workloadIds: allWorkloadId.Select(a => a.ToString()).ToList().AsReadOnly(), + skipWorkloadManifestUpdate: true) + { + IsRunningRestore = true + }.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..a403f5266329 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,9 @@ public WorkloadUpdateCommand( IInstaller workloadInstaller = null, INuGetPackageDownloader nugetPackageDownloader = null, IWorkloadManifestUpdater workloadManifestUpdater = null, - string tempDirPath = null) + string tempDirPath = null, + bool isRestoring = false, + WorkloadHistoryRecorder recorder = null) : base(parseResult, reporter: reporter, workloadResolverFactory: workloadResolverFactory, workloadInstaller: workloadInstaller, nugetPackageDownloader: nugetPackageDownloader, workloadManifestUpdater: workloadManifestUpdater, tempDirPath: tempDirPath) @@ -48,11 +51,17 @@ public WorkloadUpdateCommand( _workloadManifestUpdater = _workloadManifestUpdaterFromConstructor ?? new WorkloadManifestUpdater(resolvedReporter, _workloadResolver, PackageDownloader, _userProfileDir, _workloadInstaller.GetWorkloadInstallationRecordRepository(), _workloadInstaller, _packageSourceLocation, sdkFeatureBand: _sdkFeatureBand); - _recorder = new(_workloadResolver, _workloadInstaller, () => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, null)); - _recorder.HistoryRecord.CommandName = "update"; + _recorder = recorder; + if (_recorder is null) + { + _recorder = new(_workloadResolver, _workloadInstaller, () => _workloadResolverFactory.CreateForWorkloadSet(_dotnetPath, _sdkVersion.ToString(), _userProfileDir, null)); + _recorder.HistoryRecord.CommandName = "update"; + + } _fromHistorySpecified = parseResult.GetValue(WorkloadUpdateCommandParser.FromHistoryOption); _historyManifestOnlyOption = !string.IsNullOrWhiteSpace(parseResult.GetValue(WorkloadUpdateCommandParser.HistoryManifestOnlyOption)); + _isRestoring = isRestoring; } public override int Execute() @@ -104,39 +113,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 +136,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) 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() diff --git a/test/dotnet-MsiInstallation.Tests/WorkloadSetTests.cs b/test/dotnet-MsiInstallation.Tests/WorkloadSetTests.cs index a42b6f263f43..04135fd74845 100644 --- a/test/dotnet-MsiInstallation.Tests/WorkloadSetTests.cs +++ b/test/dotnet-MsiInstallation.Tests/WorkloadSetTests.cs @@ -275,23 +275,35 @@ void SetupWorkloadSetInGlobalJson(out WorkloadSet originalRollback) } [Fact] - public void UpdateWorkloadSetViaGlobalJson() + public void RestoreWorkloadSetViaGlobalJson() { + InstallSdk(); + + var testProjectFolder = Path.Combine(SdkTestingDirectory, "ConsoleApp"); + VM.CreateRunCommand("dotnet", "new", "console", "-o", "ConsoleApp") + .WithWorkingDirectory(SdkTestingDirectory) + .Execute().Should().Pass(); + SetupWorkloadSetInGlobalJson(out var originalRollback); - VM.CreateRunCommand("dotnet", "workload", "update").WithWorkingDirectory(SdkTestingDirectory).Execute().Should().Pass(); + VM.CreateRunCommand("dotnet", "workload", "restore") + .WithWorkingDirectory(testProjectFolder) + .Execute().Should().Pass(); + + GetWorkloadVersion(SdkTestingDirectory).Should().Be(WorkloadSetVersion2); + GetRollback(SdkTestingDirectory).Should().NotBe(originalRollback); } - [Fact] - public void InstallWorkloadSetViaGlobalJson() + [Theory] + [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); }