diff --git a/src/NuGet.Core/NuGet.PackageManagement/NuGetPackageManager.cs b/src/NuGet.Core/NuGet.PackageManagement/NuGetPackageManager.cs index 4bd45b55943..49e09e94fec 100644 --- a/src/NuGet.Core/NuGet.PackageManagement/NuGetPackageManager.cs +++ b/src/NuGet.Core/NuGet.PackageManagement/NuGetPackageManager.cs @@ -1732,11 +1732,6 @@ await PreviewBuildIntegratedProjectActionsAsync(buildIntegratedProject, actions, if (msbuildProject != null) { - // Leaving it as comment for future reference - // Don't use batch API for whole install or uninstall which also includes adding/ removing references - // from project, since it could then create problems while adding binding redirects. - // msbuildProject.MSBuildNuGetProjectSystem.BeginProcessing(); - // raise Nuget batch start event var batchId = Guid.NewGuid().ToString(); string name; diff --git a/src/NuGet.Core/NuGet.ProjectManagement/Projects/MSBuildNuGetProject.cs b/src/NuGet.Core/NuGet.ProjectManagement/Projects/MSBuildNuGetProject.cs index 5316ce3f878..940fbd5dd8c 100644 --- a/src/NuGet.Core/NuGet.ProjectManagement/Projects/MSBuildNuGetProject.cs +++ b/src/NuGet.Core/NuGet.ProjectManagement/Projects/MSBuildNuGetProject.cs @@ -275,64 +275,73 @@ private static bool IsSkipAssemblyReferences(INuGetProjectContext nuGetProjectCo } PackageEventsProvider.Instance.NotifyInstalled(packageEventArgs); - // Step-8: MSBuildNuGetProjectSystem operations - // Step-8.1: Add references to project - if (!IsSkipAssemblyReferences(nuGetProjectContext) && - MSBuildNuGetProjectSystemUtility.IsValid(compatibleReferenceItemsGroup)) + try { - foreach (var referenceItem in compatibleReferenceItemsGroup.Items) + MSBuildNuGetProjectSystem.BeginProcessing(); + + // Step-8: MSBuildNuGetProjectSystem operations + // Step-8.1: Add references to project + if (!IsSkipAssemblyReferences(nuGetProjectContext) && + MSBuildNuGetProjectSystemUtility.IsValid(compatibleReferenceItemsGroup)) { - if (IsAssemblyReference(referenceItem)) + foreach (var referenceItem in compatibleReferenceItemsGroup.Items) { - var referenceItemFullPath = Path.Combine(packageInstallPath, referenceItem); - var referenceName = Path.GetFileName(referenceItem); - - if (MSBuildNuGetProjectSystem.ReferenceExists(referenceName)) + if (IsAssemblyReference(referenceItem)) { - MSBuildNuGetProjectSystem.RemoveReference(referenceName); - } + var referenceItemFullPath = Path.Combine(packageInstallPath, referenceItem); + var referenceName = Path.GetFileName(referenceItem); + + if (MSBuildNuGetProjectSystem.ReferenceExists(referenceName)) + { + MSBuildNuGetProjectSystem.RemoveReference(referenceName); + } - MSBuildNuGetProjectSystem.AddReference(referenceItemFullPath); + MSBuildNuGetProjectSystem.AddReference(referenceItemFullPath); + } } } - } - // Step-8.2: Add Frameworkreferences to project - if (!IsSkipAssemblyReferences(nuGetProjectContext) && - MSBuildNuGetProjectSystemUtility.IsValid(compatibleFrameworkReferencesGroup)) - { - foreach (var frameworkReference in compatibleFrameworkReferencesGroup.Items) + // Step-8.2: Add Frameworkreferences to project + if (!IsSkipAssemblyReferences(nuGetProjectContext) && + MSBuildNuGetProjectSystemUtility.IsValid(compatibleFrameworkReferencesGroup)) { - if (!MSBuildNuGetProjectSystem.ReferenceExists(frameworkReference)) + foreach (var frameworkReference in compatibleFrameworkReferencesGroup.Items) { - MSBuildNuGetProjectSystem.AddFrameworkReference(frameworkReference, packageIdentity.Id); + if (!MSBuildNuGetProjectSystem.ReferenceExists(frameworkReference)) + { + MSBuildNuGetProjectSystem.AddFrameworkReference(frameworkReference, packageIdentity.Id); + } } } - } - // Step-8.3: Add Content Files - if (MSBuildNuGetProjectSystemUtility.IsValid(compatibleContentFilesGroup)) - { - MSBuildNuGetProjectSystemUtility.AddFiles(MSBuildNuGetProjectSystem, - packageReader, compatibleContentFilesGroup, FileTransformers); - } + // Step-8.3: Add Content Files + if (MSBuildNuGetProjectSystemUtility.IsValid(compatibleContentFilesGroup)) + { + MSBuildNuGetProjectSystemUtility.AddFiles(MSBuildNuGetProjectSystem, + packageReader, compatibleContentFilesGroup, FileTransformers); + } - // Step-8.4: Add Build imports - if (MSBuildNuGetProjectSystemUtility.IsValid(compatibleBuildFilesGroup)) - { - foreach (var buildImportFile in compatibleBuildFilesGroup.Items) + // Step-8.4: Add Build imports + if (MSBuildNuGetProjectSystemUtility.IsValid(compatibleBuildFilesGroup)) { - var fullImportFilePath = Path.Combine(packageInstallPath, buildImportFile); - MSBuildNuGetProjectSystem.AddImport(fullImportFilePath, - fullImportFilePath.EndsWith(".props", StringComparison.OrdinalIgnoreCase) ? ImportLocation.Top : ImportLocation.Bottom); + foreach (var buildImportFile in compatibleBuildFilesGroup.Items) + { + var fullImportFilePath = Path.Combine(packageInstallPath, buildImportFile); + MSBuildNuGetProjectSystem.AddImport(fullImportFilePath, + fullImportFilePath.EndsWith(".props", StringComparison.OrdinalIgnoreCase) ? ImportLocation.Top : ImportLocation.Bottom); + } } - } - // Step-9: Install package to PackagesConfigNuGetProject - await PackagesConfigNuGetProject.InstallPackageAsync(packageIdentity, downloadResourceResult, nuGetProjectContext, token); + // Step-9: Install package to PackagesConfigNuGetProject + await PackagesConfigNuGetProject.InstallPackageAsync(packageIdentity, downloadResourceResult, nuGetProjectContext, token); - // Step-10: Add packages.config to MSBuildNuGetProject - MSBuildNuGetProjectSystem.AddExistingFile(Path.GetFileName(PackagesConfigNuGetProject.FullPath)); + // Step-10: Add packages.config to MSBuildNuGetProject + MSBuildNuGetProjectSystem.AddExistingFile(Path.GetFileName(PackagesConfigNuGetProject.FullPath)); + } + finally + { + MSBuildNuGetProjectSystem.EndProcessing(); + } // Step 11: Raise PackageReferenceAdded event PackageReferenceAdded?.Invoke(this, packageEventArgs); @@ -446,61 +455,70 @@ public override async Task UninstallPackageAsync(PackageIdentity packageId compatibleBuildFilesGroup = MSBuildNuGetProjectSystemUtility.Normalize(compatibleBuildFilesGroup); - // Step-5: Remove package reference from packages.config - await PackagesConfigNuGetProject.UninstallPackageAsync(packageIdentity, nuGetProjectContext, token); - - // Step-6: Remove packages.config from MSBuildNuGetProject if there are no packages - // OR Add it again (to ensure that Source Control works), when there are some packages - if (!(await PackagesConfigNuGetProject.GetInstalledPackagesAsync(token)).Any()) + try { - MSBuildNuGetProjectSystem.RemoveFile(Path.GetFileName(PackagesConfigNuGetProject.FullPath)); - } - else - { - MSBuildNuGetProjectSystem.AddExistingFile(Path.GetFileName(PackagesConfigNuGetProject.FullPath)); - } + MSBuildNuGetProjectSystem.BeginProcessing(); - // Step-7: Uninstall package from the msbuild project - // Step-7.1: Remove references - if (MSBuildNuGetProjectSystemUtility.IsValid(compatibleReferenceItemsGroup)) - { - foreach (var item in compatibleReferenceItemsGroup.Items) + // Step-5: Remove package reference from packages.config + await PackagesConfigNuGetProject.UninstallPackageAsync(packageIdentity, nuGetProjectContext, token); + + // Step-6: Remove packages.config from MSBuildNuGetProject if there are no packages + // OR Add it again (to ensure that Source Control works), when there are some packages + if (!(await PackagesConfigNuGetProject.GetInstalledPackagesAsync(token)).Any()) + { + MSBuildNuGetProjectSystem.RemoveFile(Path.GetFileName(PackagesConfigNuGetProject.FullPath)); + } + else { - if (IsAssemblyReference(item)) + MSBuildNuGetProjectSystem.AddExistingFile(Path.GetFileName(PackagesConfigNuGetProject.FullPath)); + } + + // Step-7: Uninstall package from the msbuild project + // Step-7.1: Remove references + if (MSBuildNuGetProjectSystemUtility.IsValid(compatibleReferenceItemsGroup)) + { + foreach (var item in compatibleReferenceItemsGroup.Items) { - MSBuildNuGetProjectSystem.RemoveReference(Path.GetFileName(item)); + if (IsAssemblyReference(item)) + { + MSBuildNuGetProjectSystem.RemoveReference(Path.GetFileName(item)); + } } } - } - // Step-7.2: Framework references are never removed. This is a no-op + // Step-7.2: Framework references are never removed. This is a no-op - // Step-7.3: Remove content files - if (MSBuildNuGetProjectSystemUtility.IsValid(compatibleContentFilesGroup)) - { - var packagesPaths = (await GetInstalledPackagesAsync(token)) - .Select(pr => FolderNuGetProject.GetInstalledPackageFilePath(pr.PackageIdentity)); - - MSBuildNuGetProjectSystemUtility.DeleteFiles(MSBuildNuGetProjectSystem, - zipArchive, - packagesPaths, - compatibleContentFilesGroup, - FileTransformers); - } + // Step-7.3: Remove content files + if (MSBuildNuGetProjectSystemUtility.IsValid(compatibleContentFilesGroup)) + { + var packagesPaths = (await GetInstalledPackagesAsync(token)) + .Select(pr => FolderNuGetProject.GetInstalledPackageFilePath(pr.PackageIdentity)); + + MSBuildNuGetProjectSystemUtility.DeleteFiles(MSBuildNuGetProjectSystem, + zipArchive, + packagesPaths, + compatibleContentFilesGroup, + FileTransformers); + } - // Step-7.4: Remove build imports - if (MSBuildNuGetProjectSystemUtility.IsValid(compatibleBuildFilesGroup)) - { - foreach (var buildImportFile in compatibleBuildFilesGroup.Items) + // Step-7.4: Remove build imports + if (MSBuildNuGetProjectSystemUtility.IsValid(compatibleBuildFilesGroup)) { - var fullImportFilePath = Path.Combine(FolderNuGetProject.GetInstalledPath(packageIdentity), buildImportFile); - MSBuildNuGetProjectSystem.RemoveImport(fullImportFilePath); + foreach (var buildImportFile in compatibleBuildFilesGroup.Items) + { + var fullImportFilePath = Path.Combine(FolderNuGetProject.GetInstalledPath(packageIdentity), buildImportFile); + MSBuildNuGetProjectSystem.RemoveImport(fullImportFilePath); + } } - } - // Step-7.5: Remove binding redirects. This is a no-op - // Binding redirects will be removed when all packages have finished - // uninstalling for performance reasons + // Step-7.5: Remove binding redirects. This is a no-op + // Binding redirects will be removed when all packages have finished + // uninstalling for performance reasons + } + finally + { + MSBuildNuGetProjectSystem.EndProcessing(); + } // Step-8: Raise PackageReferenceRemoved event if (PackageReferenceRemoved != null) diff --git a/src/NuGet.Core/NuGet.ProjectManagement/Utility/MSBuildNuGetProjectSystemUtility.cs b/src/NuGet.Core/NuGet.ProjectManagement/Utility/MSBuildNuGetProjectSystemUtility.cs index 46f06903016..6ffcb4c8b6b 100644 --- a/src/NuGet.Core/NuGet.ProjectManagement/Utility/MSBuildNuGetProjectSystemUtility.cs +++ b/src/NuGet.Core/NuGet.ProjectManagement/Utility/MSBuildNuGetProjectSystemUtility.cs @@ -144,66 +144,59 @@ internal static void TryAddFile(IMSBuildNuGetProjectSystem msBuildNuGetProjectSy var packageItemListAsArchiveEntryNames = frameworkSpecificGroup.Items.ToList(); packageItemListAsArchiveEntryNames.Sort(new PackageItemComparer()); + try { - try - { - var paths = - packageItemListAsArchiveEntryNames.Select( - file => ResolvePath(fileTransformers, fte => fte.InstallExtension, - GetEffectivePathForContentFile(packageTargetFramework, file))); - paths = paths.Where(p => !string.IsNullOrEmpty(p)); - - msBuildNuGetProjectSystem.BeginProcessing(); - msBuildNuGetProjectSystem.RegisterProcessedFiles(paths); - } - catch (Exception) + var paths = + packageItemListAsArchiveEntryNames.Select( + file => ResolvePath(fileTransformers, fte => fte.InstallExtension, + GetEffectivePathForContentFile(packageTargetFramework, file))); + paths = paths.Where(p => !string.IsNullOrEmpty(p)); + + msBuildNuGetProjectSystem.RegisterProcessedFiles(paths); + } + catch (Exception) + { + // Ignore all exceptions for now + } + + foreach (var file in packageItemListAsArchiveEntryNames) + { + if (IsEmptyFolder(file)) { - // Ignore all exceptions for now + continue; } - foreach (var file in packageItemListAsArchiveEntryNames) + var effectivePathForContentFile = GetEffectivePathForContentFile(packageTargetFramework, file); + + // Resolve the target path + IPackageFileTransformer installTransformer; + var path = ResolveTargetPath(msBuildNuGetProjectSystem, + fileTransformers, + fte => fte.InstallExtension, effectivePathForContentFile, out installTransformer); + + if (msBuildNuGetProjectSystem.IsSupportedFile(path)) { - if (IsEmptyFolder(file)) + if (installTransformer != null) { - continue; + installTransformer.TransformFile(() => packageReader.GetStream(file), path, + msBuildNuGetProjectSystem); } - - var effectivePathForContentFile = GetEffectivePathForContentFile(packageTargetFramework, file); - - // Resolve the target path - IPackageFileTransformer installTransformer; - var path = ResolveTargetPath(msBuildNuGetProjectSystem, - fileTransformers, - fte => fte.InstallExtension, effectivePathForContentFile, out installTransformer); - - if (msBuildNuGetProjectSystem.IsSupportedFile(path)) + else { - if (installTransformer != null) + // Ignore uninstall transform file during installation + string truncatedPath; + var uninstallTransformer = + FindFileTransformer(fileTransformers, fte => fte.UninstallExtension, + effectivePathForContentFile, out truncatedPath); + if (uninstallTransformer != null) { - installTransformer.TransformFile(() => packageReader.GetStream(file), path, - msBuildNuGetProjectSystem); - } - else - { - // Ignore uninstall transform file during installation - string truncatedPath; - var uninstallTransformer = - FindFileTransformer(fileTransformers, fte => fte.UninstallExtension, - effectivePathForContentFile, out truncatedPath); - if (uninstallTransformer != null) - { - continue; - } - TryAddFile(msBuildNuGetProjectSystem, path, () => packageReader.GetStream(file)); + continue; } + TryAddFile(msBuildNuGetProjectSystem, path, () => packageReader.GetStream(file)); } } } - finally - { - msBuildNuGetProjectSystem.EndProcessing(); - } } [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope")] @@ -216,136 +209,128 @@ internal static void TryAddFile(IMSBuildNuGetProjectSystem msBuildNuGetProjectSy var packageTargetFramework = frameworkSpecificGroup.TargetFramework; IPackageFileTransformer transformer; - try - { - projectSystem.BeginProcessing(); - var directoryLookup = frameworkSpecificGroup.Items.ToLookup( - p => Path.GetDirectoryName(ResolveTargetPath(projectSystem, - fileTransformers, - fte => fte.UninstallExtension, - GetEffectivePathForContentFile(packageTargetFramework, p), - out transformer))); + var directoryLookup = frameworkSpecificGroup.Items.ToLookup( + p => Path.GetDirectoryName(ResolveTargetPath(projectSystem, + fileTransformers, + fte => fte.UninstallExtension, + GetEffectivePathForContentFile(packageTargetFramework, p), + out transformer))); + + // Get all directories that this package may have added + var directories = from grouping in directoryLookup + from directory in FileSystemUtility.GetDirectories(grouping.Key, altDirectorySeparator: false) + orderby directory.Length descending + select directory; - // Get all directories that this package may have added - var directories = from grouping in directoryLookup - from directory in FileSystemUtility.GetDirectories(grouping.Key, altDirectorySeparator: false) - orderby directory.Length descending - select directory; + string projectFullPath = projectSystem.ProjectFullPath; - string projectFullPath = projectSystem.ProjectFullPath; + // Remove files from every directory + foreach (var directory in directories) + { + var directoryFiles = directoryLookup.Contains(directory) + ? directoryLookup[directory] + : Enumerable.Empty(); - // Remove files from every directory - foreach (var directory in directories) + if (!Directory.Exists(Path.Combine(projectFullPath, directory))) { - var directoryFiles = directoryLookup.Contains(directory) - ? directoryLookup[directory] - : Enumerable.Empty(); + continue; + } - if (!Directory.Exists(Path.Combine(projectFullPath, directory))) + foreach (var file in directoryFiles) + { + if (IsEmptyFolder(file)) { continue; } - foreach (var file in directoryFiles) - { - if (IsEmptyFolder(file)) - { - continue; - } + // Resolve the path + var path = ResolveTargetPath(projectSystem, + fileTransformers, + fte => fte.UninstallExtension, + GetEffectivePathForContentFile(packageTargetFramework, file), + out transformer); - // Resolve the path - var path = ResolveTargetPath(projectSystem, - fileTransformers, - fte => fte.UninstallExtension, - GetEffectivePathForContentFile(packageTargetFramework, file), - out transformer); + if (projectSystem.IsSupportedFile(path)) + { + // Register the file being uninstalled (used by web site project system). + projectSystem.RegisterProcessedFiles(new[] {path}); - if (projectSystem.IsSupportedFile(path)) + if (transformer != null) { - // Register the file being uninstalled (used by web site project system). - projectSystem.RegisterProcessedFiles(new[] {path}); + // TODO: use the framework from packages.config instead of the current framework + // which may have changed during re-targeting + var projectFramework = projectSystem.TargetFramework; - if (transformer != null) + var matchingFiles = new List(); + foreach (var otherPackagePath in otherPackagesPath) { - // TODO: use the framework from packages.config instead of the current framework - // which may have changed during re-targeting - var projectFramework = projectSystem.TargetFramework; - - var matchingFiles = new List(); - foreach (var otherPackagePath in otherPackagesPath) + using (var otherPackageZipReader = new PackageArchiveReader(otherPackagePath)) { - using (var otherPackageZipReader = new PackageArchiveReader(otherPackagePath)) - { - // use the project framework to find the group that would have been installed - var mostCompatibleContentFilesGroup = GetMostCompatibleGroup( - projectFramework, - otherPackageZipReader.GetContentItems()); + // use the project framework to find the group that would have been installed + var mostCompatibleContentFilesGroup = GetMostCompatibleGroup( + projectFramework, + otherPackageZipReader.GetContentItems()); - if (IsValid(mostCompatibleContentFilesGroup)) + if (IsValid(mostCompatibleContentFilesGroup)) + { + // Should not normalize content files group. + // It should be like a ZipFileEntry with a forward slash. + foreach (var otherPackageItem in mostCompatibleContentFilesGroup.Items) { - // Should not normalize content files group. - // It should be like a ZipFileEntry with a forward slash. - foreach (var otherPackageItem in mostCompatibleContentFilesGroup.Items) + if (GetEffectivePathForContentFile(packageTargetFramework, + otherPackageItem) + .Equals( + GetEffectivePathForContentFile(packageTargetFramework, file), + StringComparison.OrdinalIgnoreCase)) { - if (GetEffectivePathForContentFile(packageTargetFramework, - otherPackageItem) - .Equals( - GetEffectivePathForContentFile(packageTargetFramework, file), - StringComparison.OrdinalIgnoreCase)) - { - matchingFiles.Add(new InternalZipFileInfo(otherPackagePath, - otherPackageItem)); - } + matchingFiles.Add(new InternalZipFileInfo(otherPackagePath, + otherPackageItem)); } } } } + } - try - { - var zipArchiveFileEntry = PathUtility.GetEntry(zipArchive, file); - if (zipArchiveFileEntry != null) - { - transformer.RevertFile(zipArchiveFileEntry.Open, path, matchingFiles, - projectSystem); - } - } - catch (Exception e) + try + { + var zipArchiveFileEntry = PathUtility.GetEntry(zipArchive, file); + if (zipArchiveFileEntry != null) { - projectSystem.NuGetProjectContext.Log(MessageLevel.Warning, e.Message); + transformer.RevertFile(zipArchiveFileEntry.Open, path, matchingFiles, + projectSystem); } } - else + catch (Exception e) { - try - { - var zipArchiveFileEntry = PathUtility.GetEntry(zipArchive, file); - if (zipArchiveFileEntry != null) - { - DeleteFileSafe(path, zipArchiveFileEntry.Open, projectSystem); - } - } - catch (Exception e) + projectSystem.NuGetProjectContext.Log(MessageLevel.Warning, e.Message); + } + } + else + { + try + { + var zipArchiveFileEntry = PathUtility.GetEntry(zipArchive, file); + if (zipArchiveFileEntry != null) { - projectSystem.NuGetProjectContext.Log(MessageLevel.Warning, e.Message); + DeleteFileSafe(path, zipArchiveFileEntry.Open, projectSystem); } - } + catch (Exception e) + { + projectSystem.NuGetProjectContext.Log(MessageLevel.Warning, e.Message); + } + } } + } - // If the directory is empty then delete it - if (!GetFilesSafe(projectSystem, directory).Any() - && !GetDirectoriesSafe(projectSystem, directory).Any()) - { - DeleteDirectorySafe(projectSystem, directory); - } + // If the directory is empty then delete it + if (!GetFilesSafe(projectSystem, directory).Any() + && !GetDirectoriesSafe(projectSystem, directory).Any()) + { + DeleteDirectorySafe(projectSystem, directory); } } - finally - { - projectSystem.EndProcessing(); - } } internal static IEnumerable GetFilesSafe(IMSBuildNuGetProjectSystem msBuildNuGetProjectSystem, string path) diff --git a/src/NuGet.Core/Test.Utility/ProjectManagement/TestMSBuildNuGetProjectSystem.cs b/src/NuGet.Core/Test.Utility/ProjectManagement/TestMSBuildNuGetProjectSystem.cs index 646e4cd1462..62dcfd8b7bd 100644 --- a/src/NuGet.Core/Test.Utility/ProjectManagement/TestMSBuildNuGetProjectSystem.cs +++ b/src/NuGet.Core/Test.Utility/ProjectManagement/TestMSBuildNuGetProjectSystem.cs @@ -25,6 +25,9 @@ public class TestMSBuildNuGetProjectSystem : IMSBuildNuGetProjectSystem public Dictionary ScriptsExecuted { get; } public int BindingRedirectsCallCount { get; private set; } public INuGetProjectContext NuGetProjectContext { get; private set; } + public int BatchCount { get; private set; } + public Action AddReferenceAction { get; set; } + public Action RemoveReferenceAction { get; set; } public TestMSBuildNuGetProjectSystem(NuGetFramework targetFramework, INuGetProjectContext nuGetProjectContext, string projectFullPath = null, string projectName = null) @@ -39,6 +42,8 @@ public class TestMSBuildNuGetProjectSystem : IMSBuildNuGetProjectSystem ScriptsExecuted = new Dictionary(); ProcessedFiles = new HashSet(); ProjectName = projectName ?? TestProjectName; + AddReferenceAction = AddReferenceImplementation; + RemoveReferenceAction = RemoveReferenceImplementation; } public void AddFile(string path, Stream stream) @@ -74,12 +79,7 @@ public void AddImport(string targetFullPath, ImportLocation location) public void AddReference(string referencePath) { - var referenceAssemblyName = Path.GetFileName(referencePath); - if (References.ContainsKey(referenceAssemblyName)) - { - throw new InvalidOperationException("Cannot add existing reference. That would be a COMException in VS"); - } - References.Add(referenceAssemblyName, referencePath); + AddReferenceAction(referencePath); } public void RemoveFile(string path) @@ -113,10 +113,7 @@ public void RemoveImport(string targetFullPath) public void RemoveReference(string name) { - if (References.ContainsKey(name)) - { - References.Remove(name); - } + RemoveReferenceAction(name); } public NuGetFramework TargetFramework { get; } @@ -193,6 +190,7 @@ public void RegisterProcessedFiles(IEnumerable files) public void EndProcessing() { + ++BatchCount; ProcessedFiles = FilesInProcessing; FilesInProcessing = null; } @@ -220,5 +218,23 @@ public IEnumerable GetDirectories(string path) { return GetFiles(path, "*.*", recursive: true); } + + private void AddReferenceImplementation(string referencePath) + { + var referenceAssemblyName = Path.GetFileName(referencePath); + if (References.ContainsKey(referenceAssemblyName)) + { + throw new InvalidOperationException("Cannot add existing reference. That would be a COMException in VS"); + } + References.Add(referenceAssemblyName, referencePath); + } + + private void RemoveReferenceImplementation(string name) + { + if (References.ContainsKey(name)) + { + References.Remove(name); + } + } } } diff --git a/src/NuGet.Core/Test.Utility/TestPackages.cs b/src/NuGet.Core/Test.Utility/TestPackages.cs index dc242b888e2..ef2a106b0a6 100644 --- a/src/NuGet.Core/Test.Utility/TestPackages.cs +++ b/src/NuGet.Core/Test.Utility/TestPackages.cs @@ -49,6 +49,18 @@ public static class TestPackagesGroupedByFolder }); } + public static FileInfo GetLegacyTestPackageWithMultipleReferences(string path, + string packageId = "packageA", + string packageVersion = "2.0.3") + { + return GeneratePackage(path, packageId, packageVersion, + new[] + { + "lib/net45/a.dll", + "lib/net45/b.dll" + }); + } + public static FileInfo GetNet45TestPackage(string path, string packageId = "packageA", string packageVersion = "2.0.3") diff --git a/test/NuGet.Core.Tests/NuGet.ProjectManagement.Test/MSBuildNuGetProjectTests.cs b/test/NuGet.Core.Tests/NuGet.ProjectManagement.Test/MSBuildNuGetProjectTests.cs index 17d69e80d12..50126a1cea8 100644 --- a/test/NuGet.Core.Tests/NuGet.ProjectManagement.Test/MSBuildNuGetProjectTests.cs +++ b/test/NuGet.Core.Tests/NuGet.ProjectManagement.Test/MSBuildNuGetProjectTests.cs @@ -188,6 +188,166 @@ public async Task TestMSBuildNuGetProjectUninstallReferences() } } + [Fact] + public async Task TestMSBuildNuGetProjectInstallReferencesEventBatching() + { + // Arrange + var packageIdentity = new PackageIdentity("packageA", new NuGetVersion("1.0.0")); + + using (var randomTestPackageSourcePath = TestFileSystemUtility.CreateRandomTestFolder()) + using (var randomPackagesFolderPath = TestFileSystemUtility.CreateRandomTestFolder()) + using (var randomPackagesConfigFolderPath = TestFileSystemUtility.CreateRandomTestFolder()) + { + var randomPackagesConfigPath = Path.Combine(randomPackagesConfigFolderPath, "packages.config"); + var token = CancellationToken.None; + + var projectTargetFramework = NuGetFramework.Parse("net45"); + var testNuGetProjectContext = new TestNuGetProjectContext(); + var msBuildNuGetProjectSystem = new TestMSBuildNuGetProjectSystem(projectTargetFramework, testNuGetProjectContext); + var msBuildNuGetProject = new MSBuildNuGetProject(msBuildNuGetProjectSystem, randomPackagesFolderPath, randomPackagesConfigFolderPath); + + Assert.Equal(0, msBuildNuGetProjectSystem.References.Count); + Assert.Equal(0, msBuildNuGetProjectSystem.BatchCount); + + var packageFileInfo = TestPackagesGroupedByFolder.GetLegacyTestPackageWithMultipleReferences(randomTestPackageSourcePath, + packageIdentity.Id, packageIdentity.Version.ToNormalizedString()); + using (var packageStream = GetDownloadResourceResult(packageFileInfo)) + { + // Act + await msBuildNuGetProject.InstallPackageAsync(packageIdentity, packageStream, testNuGetProjectContext, token); + } + + // Assert + Assert.Equal(2, msBuildNuGetProjectSystem.References.Count); + Assert.Equal(1, msBuildNuGetProjectSystem.BatchCount); + Assert.Equal("a.dll", msBuildNuGetProjectSystem.References.First().Key); + Assert.Equal("b.dll", msBuildNuGetProjectSystem.References.Skip(1).First().Key); + Assert.Equal(Path.Combine(msBuildNuGetProject.FolderNuGetProject.GetInstalledPath(packageIdentity), + "lib\\net45\\a.dll"), msBuildNuGetProjectSystem.References.First().Value); + Assert.Equal(Path.Combine(msBuildNuGetProject.FolderNuGetProject.GetInstalledPath(packageIdentity), + "lib\\net45\\b.dll"), msBuildNuGetProjectSystem.References.Skip(1).First().Value); + } + } + + [Fact] + public async Task TestMSBuildNuGetProjectInstallReferencesFailureEventBatching() + { + // Arrange + var packageIdentity = new PackageIdentity("packageA", new NuGetVersion("1.0.0")); + + using (var randomTestPackageSourcePath = TestFileSystemUtility.CreateRandomTestFolder()) + using (var randomPackagesFolderPath = TestFileSystemUtility.CreateRandomTestFolder()) + using (var randomPackagesConfigFolderPath = TestFileSystemUtility.CreateRandomTestFolder()) + { + var randomPackagesConfigPath = Path.Combine(randomPackagesConfigFolderPath, "packages.config"); + var token = CancellationToken.None; + + var projectTargetFramework = NuGetFramework.Parse("net45"); + var testNuGetProjectContext = new TestNuGetProjectContext(); + var msBuildNuGetProjectSystem = new TestMSBuildNuGetProjectSystem(projectTargetFramework, testNuGetProjectContext); + var msBuildNuGetProject = new MSBuildNuGetProject(msBuildNuGetProjectSystem, randomPackagesFolderPath, randomPackagesConfigFolderPath); + + msBuildNuGetProjectSystem.AddReferenceAction = (string referenceName) => { throw new InvalidOperationException(); }; + + Assert.Equal(0, msBuildNuGetProjectSystem.References.Count); + Assert.Equal(0, msBuildNuGetProjectSystem.BatchCount); + + var packageFileInfo = TestPackagesGroupedByFolder.GetLegacyTestPackageWithMultipleReferences(randomTestPackageSourcePath, + packageIdentity.Id, packageIdentity.Version.ToNormalizedString()); + using (var packageStream = GetDownloadResourceResult(packageFileInfo)) + { + // Act + await Assert.ThrowsAsync(async () => + { + await msBuildNuGetProject.InstallPackageAsync(packageIdentity, packageStream, testNuGetProjectContext, token); + }); + } + + // Assert + Assert.Equal(0, msBuildNuGetProjectSystem.References.Count); + Assert.Equal(1, msBuildNuGetProjectSystem.BatchCount); + } + } + + [Fact] + public async Task TestMSBuildNuGetProjectUninstallReferencesEventBatching() + { + // Arrange + var packageIdentity = new PackageIdentity("packageA", new NuGetVersion("1.0.0")); + + using (var randomTestPackageSourcePath = TestFileSystemUtility.CreateRandomTestFolder()) + using (var randomPackagesFolderPath = TestFileSystemUtility.CreateRandomTestFolder()) + using (var randomPackagesConfigFolderPath = TestFileSystemUtility.CreateRandomTestFolder()) + { + var randomPackagesConfigPath = Path.Combine(randomPackagesConfigFolderPath, "packages.config"); + var token = CancellationToken.None; + + var projectTargetFramework = NuGetFramework.Parse("net45"); + var testNuGetProjectContext = new TestNuGetProjectContext(); + var msBuildNuGetProjectSystem = new TestMSBuildNuGetProjectSystem(projectTargetFramework, testNuGetProjectContext); + var msBuildNuGetProject = new MSBuildNuGetProject(msBuildNuGetProjectSystem, randomPackagesFolderPath, randomPackagesConfigFolderPath); + + var packageFileInfo = TestPackagesGroupedByFolder.GetLegacyTestPackageWithMultipleReferences(randomTestPackageSourcePath, + packageIdentity.Id, packageIdentity.Version.ToNormalizedString()); + using (var packageStream = GetDownloadResourceResult(packageFileInfo)) + { + await msBuildNuGetProject.InstallPackageAsync(packageIdentity, packageStream, testNuGetProjectContext, token); + } + + Assert.Equal(2, msBuildNuGetProjectSystem.References.Count); + Assert.Equal(1, msBuildNuGetProjectSystem.BatchCount); + + // Act + await msBuildNuGetProject.UninstallPackageAsync(packageIdentity, testNuGetProjectContext, token); + + // Assert + Assert.Equal(0, msBuildNuGetProjectSystem.References.Count); + Assert.Equal(2, msBuildNuGetProjectSystem.BatchCount); + } + } + + [Fact] + public async Task TestMSBuildNuGetProjectUninstallReferencesFailureEventBatching() + { + // Arrange + var packageIdentity = new PackageIdentity("packageA", new NuGetVersion("1.0.0")); + + using (var randomTestPackageSourcePath = TestFileSystemUtility.CreateRandomTestFolder()) + using (var randomPackagesFolderPath = TestFileSystemUtility.CreateRandomTestFolder()) + using (var randomPackagesConfigFolderPath = TestFileSystemUtility.CreateRandomTestFolder()) + { + var randomPackagesConfigPath = Path.Combine(randomPackagesConfigFolderPath, "packages.config"); + var token = CancellationToken.None; + + var projectTargetFramework = NuGetFramework.Parse("net45"); + var testNuGetProjectContext = new TestNuGetProjectContext(); + var msBuildNuGetProjectSystem = new TestMSBuildNuGetProjectSystem(projectTargetFramework, testNuGetProjectContext); + var msBuildNuGetProject = new MSBuildNuGetProject(msBuildNuGetProjectSystem, randomPackagesFolderPath, randomPackagesConfigFolderPath); + + msBuildNuGetProjectSystem.RemoveReferenceAction = (string referenceName) => { throw new InvalidOperationException(); }; + + var packageFileInfo = TestPackagesGroupedByFolder.GetLegacyTestPackageWithMultipleReferences(randomTestPackageSourcePath, + packageIdentity.Id, packageIdentity.Version.ToNormalizedString()); + using (var packageStream = GetDownloadResourceResult(packageFileInfo)) + { + await msBuildNuGetProject.InstallPackageAsync(packageIdentity, packageStream, testNuGetProjectContext, token); + } + + Assert.Equal(2, msBuildNuGetProjectSystem.References.Count); + Assert.Equal(1, msBuildNuGetProjectSystem.BatchCount); + + // Act + await Assert.ThrowsAsync(async () => + { + await msBuildNuGetProject.UninstallPackageAsync(packageIdentity, testNuGetProjectContext, token); + }); + + // Assert + Assert.Equal(2, msBuildNuGetProjectSystem.References.Count); + Assert.Equal(2, msBuildNuGetProjectSystem.BatchCount); + } + } + [Fact] public async Task TestMSBuildNuGetProjectInstallSkipAssemblyReferences() {