diff --git a/src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs b/src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs index 87de51b002fe..c6b509a44452 100644 --- a/src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs +++ b/src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Concurrent; using Microsoft.DotNet.Cli.Utils; using Microsoft.DotNet.ToolPackage; using Microsoft.DotNet.Tools; @@ -35,7 +36,7 @@ internal class NuGetPackageDownloader : INuGetPackageDownloader private readonly IReporter _reporter; private readonly IFirstPartyNuGetPackageSigningVerifier _firstPartyNuGetPackageSigningVerifier; private bool _validationMessagesDisplayed = false; - private readonly Dictionary _sourceRepositories; + private readonly ConcurrentDictionary _sourceRepositories; private readonly bool _shouldUsePackageSourceMapping; private readonly bool _verifySignatures; @@ -818,7 +819,7 @@ private SourceRepository GetSourceRepository(PackageSource source) if (!_sourceRepositories.TryGetValue(source, out SourceRepository value)) { value = Repository.Factory.GetCoreV3(source); - _sourceRepositories.Add(source, value); + _sourceRepositories.AddOrUpdate(source, _ => value, (_, _) => value); } return value; diff --git a/src/Cli/dotnet/commands/dotnet-workload/install/FileBasedInstaller.cs b/src/Cli/dotnet/commands/dotnet-workload/install/FileBasedInstaller.cs index 1585c5e33f1f..f7836e920c0b 100644 --- a/src/Cli/dotnet/commands/dotnet-workload/install/FileBasedInstaller.cs +++ b/src/Cli/dotnet/commands/dotnet-workload/install/FileBasedInstaller.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Collections.Concurrent; using System.Text.Json; using Microsoft.DotNet.Cli; using Microsoft.DotNet.Cli.NuGetPackageDownloader; @@ -140,74 +141,82 @@ public void InstallWorkloads(IEnumerable workloadIds, SdkFeatureBand public void InstallWorkloads(IEnumerable workloadIds, SdkFeatureBand sdkFeatureBand, ITransactionContext transactionContext, bool overwriteExistingPacks, DirectoryPath? offlineCache = null) { var packInfos = GetPacksInWorkloads(workloadIds); - + List packsToInstall = []; foreach (var packInfo in packInfos) + { + if (PackIsInstalled(packInfo) && !overwriteExistingPacks) + { + _reporter.WriteLine(string.Format(LocalizableStrings.WorkloadPackAlreadyInstalledMessage, packInfo.ResolvedPackageId, packInfo.Version)); + WritePackInstallationRecord(packInfo, sdkFeatureBand); + } + else + { + packsToInstall.Add(packInfo); + } + } + + var tempFilesToDelete = new ConcurrentBag(); + var packagePaths = packsToInstall.AsParallel().Select(packInfo => + { + if (offlineCache == null || !offlineCache.HasValue) + { + var packagePath = _nugetPackageDownloader + .DownloadPackageAsync(new PackageId(packInfo.ResolvedPackageId), + new NuGetVersion(packInfo.Version), + _packageSourceLocation).GetAwaiter().GetResult(); + tempFilesToDelete.Add(packagePath); + return (packagePath, packInfo); + } + else + { + _reporter.WriteLine(string.Format(LocalizableStrings.UsingCacheForPackInstall, packInfo.ResolvedPackageId, packInfo.Version, offlineCache)); + var packagePath = Path.Combine(offlineCache.Value.Value, $"{packInfo.ResolvedPackageId}.{packInfo.Version}.nupkg"); + if (!File.Exists(packagePath)) + { + throw new Exception(string.Format(LocalizableStrings.CacheMissingPackage, packInfo.ResolvedPackageId, packInfo.Version, offlineCache)); + } + return (packagePath, packInfo); + } + }); + + foreach (var (packagePath, packInfo) in packagePaths) { _reporter.WriteLine(string.Format(LocalizableStrings.InstallingPackVersionMessage, packInfo.ResolvedPackageId, packInfo.Version)); var tempDirsToDelete = new List(); - var tempFilesToDelete = new List(); - bool shouldRollBackPack = false; transactionContext.Run( action: () => { - if (PackIsInstalled(packInfo) && !overwriteExistingPacks) + if (!Directory.Exists(Path.GetDirectoryName(packInfo.Path))) + { + Directory.CreateDirectory(Path.GetDirectoryName(packInfo.Path)); + } + + if (IsSingleFilePack(packInfo)) { - _reporter.WriteLine(string.Format(LocalizableStrings.WorkloadPackAlreadyInstalledMessage, packInfo.ResolvedPackageId, packInfo.Version)); + File.Copy(packagePath, packInfo.Path, overwrite: overwriteExistingPacks); } else { - shouldRollBackPack = true; - string packagePath; - if (offlineCache == null || !offlineCache.HasValue) - { - packagePath = _nugetPackageDownloader - .DownloadPackageAsync(new PackageId(packInfo.ResolvedPackageId), - new NuGetVersion(packInfo.Version), - _packageSourceLocation).GetAwaiter().GetResult(); - tempFilesToDelete.Add(packagePath); - } - else - { - _reporter.WriteLine(string.Format(LocalizableStrings.UsingCacheForPackInstall, packInfo.ResolvedPackageId, packInfo.Version, offlineCache)); - packagePath = Path.Combine(offlineCache.Value.Value, $"{packInfo.ResolvedPackageId}.{packInfo.Version}.nupkg"); - if (!File.Exists(packagePath)) - { - throw new Exception(string.Format(LocalizableStrings.CacheMissingPackage, packInfo.ResolvedPackageId, packInfo.Version, offlineCache)); - } - } + var tempExtractionDir = Path.Combine(_tempPackagesDir.Value, $"{packInfo.ResolvedPackageId}-{packInfo.Version}-extracted"); + tempDirsToDelete.Add(tempExtractionDir); - if (!Directory.Exists(Path.GetDirectoryName(packInfo.Path))) + // This directory should have been deleted, but remove it just in case + if (overwriteExistingPacks && Directory.Exists(tempExtractionDir)) { - Directory.CreateDirectory(Path.GetDirectoryName(packInfo.Path)); + Directory.Delete(tempExtractionDir, recursive: true); } - if (IsSingleFilePack(packInfo)) + Directory.CreateDirectory(tempExtractionDir); + var packFiles = _nugetPackageDownloader.ExtractPackageAsync(packagePath, new DirectoryPath(tempExtractionDir)).GetAwaiter().GetResult(); + + if (overwriteExistingPacks && Directory.Exists(packInfo.Path)) { - File.Copy(packagePath, packInfo.Path, overwrite: overwriteExistingPacks); + Directory.Delete(packInfo.Path, recursive: true); } - else - { - var tempExtractionDir = Path.Combine(_tempPackagesDir.Value, $"{packInfo.ResolvedPackageId}-{packInfo.Version}-extracted"); - tempDirsToDelete.Add(tempExtractionDir); - - // This directory should have been deleted, but remove it just in case - if (overwriteExistingPacks && Directory.Exists(tempExtractionDir)) - { - Directory.Delete(tempExtractionDir, recursive: true); - } - Directory.CreateDirectory(tempExtractionDir); - var packFiles = _nugetPackageDownloader.ExtractPackageAsync(packagePath, new DirectoryPath(tempExtractionDir)).GetAwaiter().GetResult(); - - if (overwriteExistingPacks && Directory.Exists(packInfo.Path)) - { - Directory.Delete(packInfo.Path, recursive: true); - } - - FileAccessRetrier.RetryOnMoveAccessFailure(() => DirectoryPath.MoveDirectory(tempExtractionDir, packInfo.Path)); - } - } + FileAccessRetrier.RetryOnMoveAccessFailure(() => DirectoryPath.MoveDirectory(tempExtractionDir, packInfo.Path)); + } WritePackInstallationRecord(packInfo, sdkFeatureBand); }, @@ -215,14 +224,11 @@ public void InstallWorkloads(IEnumerable workloadIds, SdkFeatureBand { try { - if (shouldRollBackPack) + _reporter.WriteLine(string.Format(LocalizableStrings.RollingBackPackInstall, packInfo.ResolvedPackageId)); + DeletePackInstallationRecord(packInfo, sdkFeatureBand); + if (!PackHasInstallRecords(packInfo)) { - _reporter.WriteLine(string.Format(LocalizableStrings.RollingBackPackInstall, packInfo.ResolvedPackageId)); - DeletePackInstallationRecord(packInfo, sdkFeatureBand); - if (!PackHasInstallRecords(packInfo)) - { - DeletePack(packInfo); - } + DeletePack(packInfo); } } catch (Exception e) diff --git a/test/dotnet-workload-install.Tests/GivenFileBasedWorkloadInstall.cs b/test/dotnet-workload-install.Tests/GivenFileBasedWorkloadInstall.cs index 3e6ce0f632ef..afac5547afeb 100644 --- a/test/dotnet-workload-install.Tests/GivenFileBasedWorkloadInstall.cs +++ b/test/dotnet-workload-install.Tests/GivenFileBasedWorkloadInstall.cs @@ -410,11 +410,11 @@ public void GivenManagedInstallItCanErrorsWhenMissingOfflineCache() var version = "6.0.100"; var cachePath = Path.Combine(dotnetRoot, "MockCache"); - var exceptionThrown = Assert.Throws(() => + var exceptionThrown = Assert.Throws(() => CliTransaction.RunNew(context => installer.InstallWorkloads(new[] { new WorkloadId("android-sdk-workload") }, new SdkFeatureBand(version), context, new DirectoryPath(cachePath)))); - exceptionThrown.Message.Should().Contain(packId); - exceptionThrown.Message.Should().Contain(packVersion); - exceptionThrown.Message.Should().Contain(cachePath); + exceptionThrown.InnerException.Message.Should().Contain(packId); + exceptionThrown.InnerException.Message.Should().Contain(packVersion); + exceptionThrown.InnerException.Message.Should().Contain(cachePath); } private (string, FileBasedInstaller, INuGetPackageDownloader, Func) GetTestInstaller([CallerMemberName] string testName = "", bool failingInstaller = false, string identifier = "", bool manifestDownload = false, diff --git a/test/dotnet-workload-install.Tests/MockNuGetPackageInstaller.cs b/test/dotnet-workload-install.Tests/MockNuGetPackageInstaller.cs index e26e9d963bbe..9fa2cb8c41e0 100644 --- a/test/dotnet-workload-install.Tests/MockNuGetPackageInstaller.cs +++ b/test/dotnet-workload-install.Tests/MockNuGetPackageInstaller.cs @@ -54,7 +54,14 @@ public Task DownloadPackageAsync(PackageId packageId, DownloadCallResult.Add(path); if (_downloadPath != string.Empty) { - File.WriteAllText(path, string.Empty); + try + { + File.WriteAllText(path, string.Empty); + } + catch (IOException) + { + // Do not write this file twice in parallel + } } _lastPackageVersion = packageVersion ?? new NuGetVersion("1.0.42"); return Task.FromResult(path);