From 1de7506947ffa2b4ac85e02880f0ce638ff54174 Mon Sep 17 00:00:00 2001 From: pragnya17 <59893188+pragnya17@users.noreply.github.com> Date: Wed, 29 Jun 2022 14:29:59 -0700 Subject: [PATCH 01/16] Add package support for cpm projects --- .../AddPackageReferenceCommandRunner.cs | 7 +- .../Strings.Designer.cs | 9 + .../NuGet.CommandLine.XPlat/Strings.resx | 5 + .../Utility/MSBuildAPIUtility.cs | 306 +++++++++++++++--- .../DotnetAddPackageTests.cs | 264 +++++++++++++++ 5 files changed, 547 insertions(+), 44 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs index 62cc1533ca3..568f31fcacb 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs @@ -276,6 +276,9 @@ private static LibraryDependency GenerateLibraryDependency( // update default packages path if user specified custom package directory var packagesPath = project.RestoreMetadata.PackagesPath; + // get if the project is onboarded to CPM + var isCentralPackageManagementEnabled = project.RestoreMetadata.CentralPackageVersionsEnabled; + if (!string.IsNullOrEmpty(packageReferenceArgs.PackageDirectory)) { packagesPath = packageReferenceArgs.PackageDirectory; @@ -314,6 +317,7 @@ private static LibraryDependency GenerateLibraryDependency( if (dependency != null) { dependency.LibraryRange.VersionRange = version; + dependency.VersionCentrallyManaged = isCentralPackageManagementEnabled; return dependency; } } @@ -324,7 +328,8 @@ private static LibraryDependency GenerateLibraryDependency( LibraryRange = new LibraryRange( name: packageReferenceArgs.PackageId, versionRange: version, - typeConstraint: LibraryDependencyTarget.Package) + typeConstraint: LibraryDependencyTarget.Package), + VersionCentrallyManaged = isCentralPackageManagementEnabled }; } diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs index 0d69d7e6044..196aeabd096 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs @@ -510,6 +510,15 @@ internal static string Info_AddPkgCompatibleWithSubsetFrameworks { } } + /// + /// Looks up a localized string similar to PackageReference for package '{0}' added to '{1}' and PackageVersion added to central package management file '{2}'.. + /// + internal static string Info_AddPkgCPM { + get { + return ResourceManager.GetString("Info_AddPkgCPM", resourceCulture); + } + } + /// /// Looks up a localized string similar to PackageReference for package '{0}' version '{1}' updated in file '{2}'.. /// diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx index a82e8f6fdb6..543c62d47da 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx @@ -753,4 +753,9 @@ The search is a case-insensitive string comparison using the supplied value, whi Non-HTTPS access will be removed in a future version. Consider migrating to 'HTTPS' sources. 0 - The command name. Ex. Push/Delete. 1 - list of server uris + + PackageReference for package '{0}' added to '{1}' and PackageVersion added to central package management file '{2}'. + [2:19 PM] Kartheek Penagamuri +0 - The package ID. 1 - Directory.Packages.props file path. 2 - Project file path. + \ No newline at end of file diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index fcb10d3254e..12067cf496a 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -21,6 +21,7 @@ namespace NuGet.CommandLine.XPlat internal class MSBuildAPIUtility { private const string PACKAGE_REFERENCE_TYPE_TAG = "PackageReference"; + private const string PACKAGE_VERSION_TYPE_TAG = "PackageVersion"; private const string VERSION_TAG = "Version"; private const string FRAMEWORK_TAG = "TargetFramework"; private const string FRAMEWORKS_TAG = "TargetFrameworks"; @@ -32,6 +33,10 @@ internal class MSBuildAPIUtility private const string IncludeAssets = "IncludeAssets"; private const string PrivateAssets = "PrivateAssets"; private const string CollectPackageReferences = "CollectPackageReferences"; + /// + /// The name of the MSBuild property that represents the path to the central package management file, usually Directory.Packages.props. + /// + private const string DirectoryPackagesPropsPathPropertyName = "DirectoryPackagesPropsPath"; public ILogger Logger { get; } @@ -152,27 +157,178 @@ public void AddPackageReferencePerTFM(string projectPath, LibraryDependency libr } } + /// + /// Add package version/package reference to the solution/project based on if the project has been onboarded to CPM or not. + /// + /// Project that needs to be modified. + /// Package Dependency of the package to be added. + /// Package references that already exist in the project. + /// Target Framework for which the package reference should be added. private void AddPackageReference(Project project, LibraryDependency libraryDependency, IEnumerable existingPackageReferences, string framework = null) { + // Getting all the item groups in a given project var itemGroups = GetItemGroups(project); - if (!existingPackageReferences.Any()) + // Add packageReference to the project file only if it does not exist. + var itemGroup = GetItemGroup(itemGroups, PACKAGE_REFERENCE_TYPE_TAG) ?? CreateItemGroup(project, framework); + + if (!libraryDependency.VersionCentrallyManaged) { - // Add packageReference only if it does not exist. - var itemGroup = GetItemGroup(itemGroups, PACKAGE_REFERENCE_TYPE_TAG) ?? CreateItemGroup(project, framework); - AddPackageReferenceIntoItemGroup(itemGroup, libraryDependency); + if (!existingPackageReferences.Any()) + { + //Modify the project file. + AddPackageReferenceIntoItemGroup(itemGroup, libraryDependency); + } + else + { + // If the package already has a reference then try to update the reference. + UpdatePackageReferenceItems(existingPackageReferences, libraryDependency); + } } else { - // If the package already has a reference then try to update the reference. - UpdatePackageReferenceItems(existingPackageReferences, libraryDependency); + // Get package version if it already exists in the props file. Returns null if there is no matching package version. + ProjectItem packageVersion = project.Items.LastOrDefault(i => i.ItemType == PACKAGE_VERSION_TYPE_TAG && i.EvaluatedInclude.Equals(libraryDependency.Name)); + var versionCLIArgument = libraryDependency.LibraryRange.VersionRange.OriginalString; + + //Add to the project file. + AddPackageReferenceIntoItemGroupCPM(project, itemGroup, libraryDependency); + + // If no exists in the Directory.Packages.props file. + if (packageVersion == null) + { + // Modifying the props file if project is onboarded to CPM. + AddPackageVersionIntoItemGroupCPM(project, libraryDependency); + } + else + { + // Modify the Directory.Packages.props file with the version that is passed in. + if (versionCLIArgument != null) + { + UpdatePackageVersion(project, packageVersion, versionCLIArgument); + } + } } + project.Save(); } + /// + /// Add package name and version using PackageVersion tag for projects onboarded to CPM. + /// + /// Project that needs to be modified. + /// Package Dependency of the package to be added. + private void AddPackageVersionIntoItemGroupCPM(Project project, LibraryDependency libraryDependency) + { + // If onboarded to CPM get the directoryBuildPropsRootElement. + ProjectRootElement directoryBuildPropsRootElement = GetDirectoryBuildPropsRootElement(project); + + // Get the ItemGroup to add a PackageVersion to or create a new one. + var propsItemGroup = GetItemGroup(directoryBuildPropsRootElement.ItemGroups, PACKAGE_VERSION_TYPE_TAG) ?? directoryBuildPropsRootElement.AddItemGroup(); + AddPackageVersionIntoPropsItemGroup(project, propsItemGroup, libraryDependency); + + // Save the updated props file. + directoryBuildPropsRootElement.Save(); + } + + /// + /// Get the Directory build props root element for projects onboarded to CPM. + /// + /// Project that needs to be modified. + /// The directory build props root element. + private ProjectRootElement GetDirectoryBuildPropsRootElement(Project project) + { + // Get the Directory.Packages.props path. + string directoryPackagesPropsPath = project.GetPropertyValue(DirectoryPackagesPropsPathPropertyName); + ProjectRootElement directoryBuildPropsRootElement = project.Imports.FirstOrDefault(i => i.ImportedProject.FullPath.Equals(directoryPackagesPropsPath)).ImportedProject; + return directoryBuildPropsRootElement; + } + + /// + /// Add package name and version into the props file. + /// + /// Project that is being modified. + /// Item group that needs to be modified in the props file. + /// Package Dependency of the package to be added. + private void AddPackageVersionIntoPropsItemGroup(Project project, ProjectItemGroupElement itemGroup, + LibraryDependency libraryDependency) + { + // Add both package reference information and version metadata using the PACKAGE_VERSION_TYPE_TAG. + var item = itemGroup.AddItem(PACKAGE_VERSION_TYPE_TAG, libraryDependency.Name); + var packageVersion = AddVersionMetadata(libraryDependency, item); + + Logger.LogInformation(string.Format(CultureInfo.CurrentCulture, + Strings.Info_AddPkgAdded, + libraryDependency.Name, + packageVersion, + itemGroup.ContainingProject.FullPath + )); + + AddExtraMetadataToProjectItemElement(libraryDependency, item); + } + + /// + /// Add package name and version into item group. + /// + /// Item group to add to. + /// Package Dependency of the package to be added. + private void AddPackageReferenceIntoItemGroup(ProjectItemGroupElement itemGroup, + LibraryDependency libraryDependency) + { + // Add both package reference information and version metadata using the PACKAGE_REFERENCE_TYPE_TAG. + var item = itemGroup.AddItem(PACKAGE_REFERENCE_TYPE_TAG, libraryDependency.Name); + var packageVersion = AddVersionMetadata(libraryDependency, item); + + Logger.LogInformation(string.Format(CultureInfo.CurrentCulture, Strings.Info_AddPkgAdded, libraryDependency.Name, packageVersion, itemGroup.ContainingProject.FullPath)); + + AddExtraMetadataToProjectItemElement(libraryDependency, item); + } + + /// + /// Add only the package name into the project file for projects onboarded to CPM. + /// + /// Project to be modified. + /// Item group to add to. + /// Package Dependency of the package to be added. + private void AddPackageReferenceIntoItemGroupCPM(Project project, ProjectItemGroupElement itemGroup, + LibraryDependency libraryDependency) + { + // Only add the package reference information using the PACKAGE_REFERENCE_TYPE_TAG. + ProjectItemElement item = itemGroup.AddItem(PACKAGE_REFERENCE_TYPE_TAG, libraryDependency.Name); + + Logger.LogInformation(string.Format(CultureInfo.CurrentCulture, Strings.Info_AddPkgCPM, libraryDependency.Name, project.GetPropertyValue(DirectoryPackagesPropsPathPropertyName), itemGroup.ContainingProject.FullPath)); + + AddExtraMetadataToProjectItemElement(libraryDependency, item); + } + + /// + /// Add other metadata based on certain flags. + /// + /// Package Dependency of the package to be added. + /// Item to add the metadata to. + private void AddExtraMetadataToProjectItemElement(LibraryDependency libraryDependency, ProjectItemElement item) + { + if (libraryDependency.IncludeType != LibraryIncludeFlags.All) + { + var includeFlags = MSBuildStringUtility.Convert(LibraryIncludeFlagUtils.GetFlagString(libraryDependency.IncludeType)); + item.AddMetadata(IncludeAssets, includeFlags, expressAsAttribute: false); + } + + if (libraryDependency.SuppressParent != LibraryIncludeFlagUtils.DefaultSuppressParent) + { + var suppressParent = MSBuildStringUtility.Convert(LibraryIncludeFlagUtils.GetFlagString(libraryDependency.SuppressParent)); + item.AddMetadata(PrivateAssets, suppressParent, expressAsAttribute: false); + } + } + + /// + /// Get all the item groups in a given project. + /// + /// A specified project. + /// private static IEnumerable GetItemGroups(Project project) { return project @@ -198,6 +354,12 @@ private static ProjectItemGroupElement GetItemGroup(IEnumerable + /// Creating an item group in a project. + /// + /// Project where the item group should be created. + /// Target Framework for which the package reference should be added. + /// An Item Group. private static ProjectItemGroupElement CreateItemGroup(Project project, string framework = null) { // Create a new item group and add a condition if given @@ -209,8 +371,39 @@ private static ProjectItemGroupElement CreateItemGroup(Project project, string f return itemGroup; } + /// + /// Adding version metadata to a given project item element. + /// + /// Package Dependency of the package to be added. + /// The item that the version metadata should be added to. + /// The package version that is added in the metadata. + private string AddVersionMetadata(LibraryDependency libraryDependency, ProjectItemElement item) + { + var packageVersion = libraryDependency.LibraryRange.VersionRange.OriginalString ?? + libraryDependency.LibraryRange.VersionRange.MinVersion.ToString(); + + ProjectMetadataElement versionAttribute = item.Metadata.FirstOrDefault(i => i.Name.Equals("Version")); + + // If version attribute does not exist at all, add it. + if (versionAttribute == null) + { + item.AddMetadata(VERSION_TAG, packageVersion, expressAsAttribute: true); + } + // Else, just update the version in the already existing version attribute. + else + { + versionAttribute.Value = packageVersion; + } + return packageVersion; + } + + /// + /// Update package references for a project that is not onboarded to CPM. + /// + /// Existing package references. + /// Package Dependency of the package to be added. private void UpdatePackageReferenceItems(IEnumerable packageReferencesItems, - LibraryDependency libraryDependency) + LibraryDependency libraryDependency) { // We validate that the operation does not update any imported items // If it does then we throw a user friendly exception without making any changes @@ -223,17 +416,7 @@ private void UpdatePackageReferenceItems(IEnumerable packageReferen packageReferenceItem.SetMetadataValue(VERSION_TAG, packageVersion); - if (libraryDependency.IncludeType != LibraryIncludeFlags.All) - { - var includeFlags = MSBuildStringUtility.Convert(LibraryIncludeFlagUtils.GetFlagString(libraryDependency.IncludeType)); - packageReferenceItem.SetMetadataValue(IncludeAssets, includeFlags); - } - - if (libraryDependency.SuppressParent != LibraryIncludeFlagUtils.DefaultSuppressParent) - { - var suppressParent = MSBuildStringUtility.Convert(LibraryIncludeFlagUtils.GetFlagString(libraryDependency.SuppressParent)); - packageReferenceItem.SetMetadataValue(PrivateAssets, suppressParent); - } + UpdateExtraMetadataInProjectItem(libraryDependency, packageReferenceItem); Logger.LogInformation(string.Format(CultureInfo.CurrentCulture, Strings.Info_AddPkgUpdated, @@ -243,34 +426,31 @@ private void UpdatePackageReferenceItems(IEnumerable packageReferen } } - private void AddPackageReferenceIntoItemGroup(ProjectItemGroupElement itemGroup, - LibraryDependency libraryDependency) + /// + /// Update the element if a version is passed in as a CLI argument. + /// + /// + /// item with a matching package ID. + /// Version that is passed in as a CLI argument. + private static void UpdatePackageVersion(Project project, ProjectItem packageVersion, string versionCLIArgument) { - var packageVersion = libraryDependency.LibraryRange.VersionRange.OriginalString ?? - libraryDependency.LibraryRange.VersionRange.MinVersion.ToString(); - - var item = itemGroup.AddItem(PACKAGE_REFERENCE_TYPE_TAG, libraryDependency.Name); - item.AddMetadata(VERSION_TAG, packageVersion, expressAsAttribute: true); - - if (libraryDependency.IncludeType != LibraryIncludeFlags.All) - { - var includeFlags = MSBuildStringUtility.Convert(LibraryIncludeFlagUtils.GetFlagString(libraryDependency.IncludeType)); - item.AddMetadata(IncludeAssets, includeFlags, expressAsAttribute: false); - } - - if (libraryDependency.SuppressParent != LibraryIncludeFlagUtils.DefaultSuppressParent) - { - var suppressParent = MSBuildStringUtility.Convert(LibraryIncludeFlagUtils.GetFlagString(libraryDependency.SuppressParent)); - item.AddMetadata(PrivateAssets, suppressParent, expressAsAttribute: false); - } - - Logger.LogInformation(string.Format(CultureInfo.CurrentCulture, - Strings.Info_AddPkgAdded, - libraryDependency.Name, - packageVersion, - itemGroup.ContainingProject.FullPath)); + // Determine where the item is decalred + ProjectItemElement packageVersionItemElement = project.GetItemProvenance(packageVersion).LastOrDefault()?.ItemElement; + + // Get the Version attribute on the packageVersionItemElement. + ProjectMetadataElement versionAttribute = packageVersionItemElement.Metadata.FirstOrDefault(i => i.Name.Equals("Version")); + // Update the version + versionAttribute.Value = versionCLIArgument; + packageVersionItemElement.ContainingProject.Save(); } + /// + /// Validate that no imported items in the project are updated with the package version. + /// + /// Existing package reference items. + /// Package Dependency of the package to be added. + /// Operation types such as if a package reference is being updated. + /// private static void ValidateNoImportedItemsAreUpdated(IEnumerable packageReferencesItems, LibraryDependency libraryDependency, string operationType) @@ -300,6 +480,46 @@ private static void ValidateNoImportedItemsAreUpdated(IEnumerable p } } + /// + /// Update other metadata for items based on certain flags. + /// + /// Package Dependency of the package to be added. + /// Item to be modified. + private void UpdateExtraMetadataInProjectItem(LibraryDependency libraryDependency, ProjectItem packageReferenceItem) + { + if (libraryDependency.IncludeType != LibraryIncludeFlags.All) + { + var includeFlags = MSBuildStringUtility.Convert(LibraryIncludeFlagUtils.GetFlagString(libraryDependency.IncludeType)); + packageReferenceItem.SetMetadataValue(IncludeAssets, includeFlags); + } + + if (libraryDependency.SuppressParent != LibraryIncludeFlagUtils.DefaultSuppressParent) + { + var suppressParent = MSBuildStringUtility.Convert(LibraryIncludeFlagUtils.GetFlagString(libraryDependency.SuppressParent)); + packageReferenceItem.SetMetadataValue(PrivateAssets, suppressParent); + } + } + + /// + /// Update other metadata for items based on certain flags. + /// + /// Package Dependency of the package to be added. + /// Item to be modified. + private void UpdateExtraMetadata(LibraryDependency libraryDependency, ProjectItem packageReferenceItem) + { + if (libraryDependency.IncludeType != LibraryIncludeFlags.All) + { + var includeFlags = MSBuildStringUtility.Convert(LibraryIncludeFlagUtils.GetFlagString(libraryDependency.IncludeType)); + packageReferenceItem.SetMetadataValue(IncludeAssets, includeFlags); + } + + if (libraryDependency.SuppressParent != LibraryIncludeFlagUtils.DefaultSuppressParent) + { + var suppressParent = MSBuildStringUtility.Convert(LibraryIncludeFlagUtils.GetFlagString(libraryDependency.SuppressParent)); + packageReferenceItem.SetMetadataValue(PrivateAssets, suppressParent); + } + } + /// /// A simple check for some of the evaluated properties to check /// if the project is package reference project or not diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs index f46a2f198b3..c9e5d984940 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs @@ -613,5 +613,269 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( Assert.Contains($"Installed {packageX} {version} from {packageSource2}", result.AllOutput); Assert.Contains($"NU1100: Unable to resolve '{packageZ} (>= {version})' for 'net5.0'", result.AllOutput); } + + [Fact] + public async Task AddPkgWithNoVersion_WhenProjectOnboardedToCPMAndNoPackagesExistInProps() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution, and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectName = "projectA"; + var projectA = XPlatTestUtils.CreateProject(projectName, pathContext, "net5.0"); + + const string version1 = "1.0.0"; + const string version2 = "2.0.0"; + const string packageX = "X"; + + var packageFrameworks = "net5.0"; + var packageX100 = XPlatTestUtils.CreatePackage(packageX, version1, frameworkString: packageFrameworks); + var packageX200 = XPlatTestUtils.CreatePackage(packageX, version2, frameworkString: packageFrameworks); + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + packageX100); + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + packageX200); + + var propsFile = @$" + + true + + + "; + + solution.Projects.Add(projectA); + solution.Create(pathContext.SolutionRoot); + + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"), propsFile); + + var projectADirectory = Path.Combine(pathContext.SolutionRoot, projectA.ProjectName); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX}", ignoreExitCode: true); + + // Assert + Assert.True(result.Success, result.Output); + Assert.Contains(@$" + + + + true + + + "; + + solution.Projects.Add(projectA); + solution.Create(pathContext.SolutionRoot); + + + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"), propsFile); + var projectADirectory = Path.Combine(pathContext.SolutionRoot, projectA.ProjectName); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX}", ignoreExitCode: true); + + // Assert + Assert.True(result.Success, result.Output); + Assert.Contains(@$" + + + + true + + + + + + "; + + solution.Projects.Add(projectA); + solution.Create(pathContext.SolutionRoot); + + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"), propsFile); + var projectADirectory = Path.Combine(pathContext.SolutionRoot, projectA.ProjectName); + + //Act + //By default the package version used will be 2.0.0 since no version CLI argument is passed in the CLI command. + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX}", ignoreExitCode: true); + + // Assert + Assert.True(result.Success, result.Output); + // Checking that the PackageVersion is not updated. + Assert.Contains(@$"", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + + // Checking that version metadata is not added to the project files. + Assert.Contains(@$"Include=""X""", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + Assert.DoesNotContain(@$"Include=""X"" Version=""1.0.0""", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + } + + [Fact] + public async Task AddPkgWithVersion_WhenProjectOnboardedToCPMAndPackageVersionExistsInProps() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectNameA = "projectA"; + var projectA = XPlatTestUtils.CreateProject(projectNameA, pathContext, "net5.0"); + + const string version = "2.0.0"; + const string packageX = "X"; + + var packageFrameworks = "net5.0"; + var packageX200 = XPlatTestUtils.CreatePackage(packageX, version, frameworkString: packageFrameworks); + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + packageX200); + + var propsFile = @$" + + true + + + + + + "; + + solution.Projects.Add(projectA); + solution.Create(pathContext.SolutionRoot); + + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"), propsFile); + var projectADirectory = Path.Combine(pathContext.SolutionRoot, projectA.ProjectName); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} -v {version}", ignoreExitCode: true); + + // Assert + Assert.True(result.Success, result.Output); + Assert.Contains(@$"", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + + // Checking that version metadata is not added to the project files. + Assert.Contains(@$"Include=""X""", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + Assert.DoesNotContain(@$"Include=""X"" Version=""1.0.0""", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + Assert.DoesNotContain(@$"Include=""X"" Version=""2.0.0""", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + } + + [Fact] + public async Task AddPackageVersionToCorrectItemGroupInPropsFile() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectNameA = "projectA"; + var projectA = XPlatTestUtils.CreateProject(projectNameA, pathContext, "net5.0"); + + const string version = "1.0.0"; + const string packageX = "X"; + + var packageFrameworks = "net5.0"; + var packageX100 = XPlatTestUtils.CreatePackage(packageX, version, frameworkString: packageFrameworks); + + await SimpleTestPackageUtility.CreateFolderFeedV3Async( + pathContext.PackageSource, + PackageSaveMode.Defaultv3, + packageX100); + + var propsFile = @$" + + true + + + + + + + +"; + + solution.Projects.Add(projectA); + solution.Create(pathContext.SolutionRoot); + + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"), propsFile); + var projectADirectory = Path.Combine(pathContext.SolutionRoot, projectA.ProjectName); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} -v {version}", ignoreExitCode: true); + + // Assert + Assert.True(result.Success, result.Output); + + Assert.Contains(@$" + + + ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + + Assert.DoesNotContain($@"< ItemGroup > + < Content Include = ""SomeFile"" /> + + ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + } } } From 41e9474402e4c8f202f6b7e7deead9a1696378e4 Mon Sep 17 00:00:00 2001 From: pragnya17 <59893188+pragnya17@users.noreply.github.com> Date: Wed, 29 Jun 2022 15:21:27 -0700 Subject: [PATCH 02/16] fix bug in test --- .../Dotnet.Integration.Test/DotnetAddPackageTests.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs index c9e5d984940..5c00487bf51 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs @@ -709,7 +709,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( var projectADirectory = Path.Combine(pathContext.SolutionRoot, projectA.ProjectName); //Act - var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX}", ignoreExitCode: true); + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} -v {version1}", ignoreExitCode: true); // Assert Assert.True(result.Success, result.Output); @@ -730,8 +730,7 @@ public async Task AddPkgWithNoVersion_WhenProjectOnboardedToCPMAndPackageVersion var projectNameA = "projectA"; var projectA = XPlatTestUtils.CreateProject(projectNameA, pathContext, "net5.0"); - const string version1 = "1.0.0"; - const string version2 = "2.0.0"; + const string version = "2.0.0"; const string packageX = "X"; var packageFrameworks = "net5.0"; From 8d78db96d9ffea2d56b94c8b947e9aa849523d21 Mon Sep 17 00:00:00 2001 From: pragnya17 <59893188+pragnya17@users.noreply.github.com> Date: Wed, 29 Jun 2022 16:45:01 -0700 Subject: [PATCH 03/16] Added package version parameter --- .../AddPackageReferenceCommandRunner.cs | 7 ++++--- .../Utility/MSBuildAPIUtility.cs | 21 +++++++++++-------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs index 568f31fcacb..c8a67910dcd 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs @@ -57,7 +57,7 @@ public async Task ExecuteCommand(PackageReferenceArgs packageReferenceArgs, typeConstraint: LibraryDependencyTarget.Package) }; - msBuild.AddPackageReference(packageReferenceArgs.ProjectPath, libraryDependency); + msBuild.AddPackageReference(packageReferenceArgs.ProjectPath, libraryDependency, packageReferenceArgs.PackageVersion); return 0; } @@ -219,7 +219,7 @@ public async Task ExecuteCommand(PackageReferenceArgs packageReferenceArgs, // generate a library dependency with all the metadata like Include, Exlude and SuppressParent var libraryDependency = GenerateLibraryDependency(updatedPackageSpec, packageReferenceArgs, restorePreviewResult, userSpecifiedFrameworks, packageDependency); - msBuild.AddPackageReference(packageReferenceArgs.ProjectPath, libraryDependency); + msBuild.AddPackageReference(packageReferenceArgs.ProjectPath, libraryDependency, packageReferenceArgs.PackageVersion); } else { @@ -239,7 +239,8 @@ public async Task ExecuteCommand(PackageReferenceArgs packageReferenceArgs, msBuild.AddPackageReferencePerTFM(packageReferenceArgs.ProjectPath, libraryDependency, - compatibleOriginalFrameworks); + compatibleOriginalFrameworks, + packageReferenceArgs.PackageVersion); } // 6. Commit restore result diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index 12067cf496a..dc9addf3a45 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -125,7 +125,8 @@ public int RemovePackageReference(string projectPath, LibraryDependency libraryD /// /// Path to the csproj file of the project. /// Package Dependency of the package to be added. - public void AddPackageReference(string projectPath, LibraryDependency libraryDependency) + /// The version that is passed in as a CLI argument. Null if no version is passed in the command. + public void AddPackageReference(string projectPath, LibraryDependency libraryDependency, string packageVersion) { var project = GetProject(projectPath); @@ -133,7 +134,7 @@ public void AddPackageReference(string projectPath, LibraryDependency libraryDep // If the project has a conditional reference, then an unconditional reference is not added. var existingPackageReferences = GetPackageReferencesForAllFrameworks(project, libraryDependency); - AddPackageReference(project, libraryDependency, existingPackageReferences); + AddPackageReference(project, libraryDependency, existingPackageReferences, packageVersion); ProjectCollection.GlobalProjectCollection.UnloadProject(project); } @@ -143,8 +144,9 @@ public void AddPackageReference(string projectPath, LibraryDependency libraryDep /// Path to the csproj file of the project. /// Package Dependency of the package to be added. /// Target Frameworks for which the package reference should be added. + /// The version that is passed in as a CLI argument. Null if no version is passed in the command. public void AddPackageReferencePerTFM(string projectPath, LibraryDependency libraryDependency, - IEnumerable frameworks) + IEnumerable frameworks, string packageVersion) { foreach (var framework in frameworks) { @@ -152,7 +154,7 @@ public void AddPackageReferencePerTFM(string projectPath, LibraryDependency libr { { "TargetFramework", framework } }; var project = GetProject(projectPath, globalProperties); var existingPackageReferences = GetPackageReferences(project, libraryDependency); - AddPackageReference(project, libraryDependency, existingPackageReferences, framework); + AddPackageReference(project, libraryDependency, existingPackageReferences, packageVersion, framework); ProjectCollection.GlobalProjectCollection.UnloadProject(project); } } @@ -163,10 +165,12 @@ public void AddPackageReferencePerTFM(string projectPath, LibraryDependency libr /// Project that needs to be modified. /// Package Dependency of the package to be added. /// Package references that already exist in the project. + /// The version that is passed in as a CLI argument. Null if no version is passed in the command. /// Target Framework for which the package reference should be added. private void AddPackageReference(Project project, LibraryDependency libraryDependency, IEnumerable existingPackageReferences, + string packageVersion, string framework = null) { // Getting all the item groups in a given project @@ -191,14 +195,13 @@ private void AddPackageReference(Project project, else { // Get package version if it already exists in the props file. Returns null if there is no matching package version. - ProjectItem packageVersion = project.Items.LastOrDefault(i => i.ItemType == PACKAGE_VERSION_TYPE_TAG && i.EvaluatedInclude.Equals(libraryDependency.Name)); - var versionCLIArgument = libraryDependency.LibraryRange.VersionRange.OriginalString; + ProjectItem packageVersionInProps = project.Items.LastOrDefault(i => i.ItemType == PACKAGE_VERSION_TYPE_TAG && i.EvaluatedInclude.Equals(libraryDependency.Name)); //Add to the project file. AddPackageReferenceIntoItemGroupCPM(project, itemGroup, libraryDependency); // If no exists in the Directory.Packages.props file. - if (packageVersion == null) + if (packageVersionInProps == null) { // Modifying the props file if project is onboarded to CPM. AddPackageVersionIntoItemGroupCPM(project, libraryDependency); @@ -206,9 +209,9 @@ private void AddPackageReference(Project project, else { // Modify the Directory.Packages.props file with the version that is passed in. - if (versionCLIArgument != null) + if (packageVersion != null) { - UpdatePackageVersion(project, packageVersion, versionCLIArgument); + UpdatePackageVersion(project, packageVersionInProps, packageVersion); } } } From 3457cb8c60e7d5417b4822d821c59a46000cb4c6 Mon Sep 17 00:00:00 2001 From: pragnya17 <59893188+pragnya17@users.noreply.github.com> Date: Wed, 29 Jun 2022 17:27:56 -0700 Subject: [PATCH 04/16] Modify test names to be more readable, remove comment in strings.resx --- .../NuGet.CommandLine.XPlat/Strings.resx | 3 +- .../DotnetAddPackageTests.cs | 33 +++++++------------ 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx index 543c62d47da..89d407467f1 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx @@ -755,7 +755,6 @@ Non-HTTPS access will be removed in a future version. Consider migrating to 'HTT PackageReference for package '{0}' added to '{1}' and PackageVersion added to central package management file '{2}'. - [2:19 PM] Kartheek Penagamuri -0 - The package ID. 1 - Directory.Packages.props file path. 2 - Project file path. + 0 - The package ID. 1 - Directory.Packages.props file path. 2 - Project file path. \ No newline at end of file diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs index 5c00487bf51..58088ab4da6 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs @@ -615,14 +615,13 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( } [Fact] - public async Task AddPkgWithNoVersion_WhenProjectOnboardedToCPMAndNoPackagesExistInProps() + public async Task AddPkg_WithCPM_WhenPackageVersionDoesNotExistAndVersionCLIArgNotPassed_Success() { using var pathContext = new SimpleTestPathContext(); // Set up solution, and project var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); - var projectName = "projectA"; - var projectA = XPlatTestUtils.CreateProject(projectName, pathContext, "net5.0"); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net5.0"); const string version1 = "1.0.0"; const string version2 = "2.0.0"; @@ -635,10 +634,7 @@ public async Task AddPkgWithNoVersion_WhenProjectOnboardedToCPMAndNoPackagesExis await SimpleTestPackageUtility.CreateFolderFeedV3Async( pathContext.PackageSource, PackageSaveMode.Defaultv3, - packageX100); - await SimpleTestPackageUtility.CreateFolderFeedV3Async( - pathContext.PackageSource, - PackageSaveMode.Defaultv3, + packageX100, packageX200); var propsFile = @$" @@ -668,14 +664,13 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( } [Fact] - public async Task AddPkgWithVersion_WhenProjectOnboardedToCPMAndNoPackagesExistInProps() + public async Task AddPkg_WithCPM_WhenPackageVersionDoesNotExistAndVersionCLIArgPassed_Success() { using var pathContext = new SimpleTestPathContext(); // Set up solution, and project var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); - var projectName = "projectA"; - var projectA = XPlatTestUtils.CreateProject(projectName, pathContext, "net5.0"); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net5.0"); const string version1 = "1.0.0"; const string version2 = "2.0.0"; @@ -688,10 +683,7 @@ public async Task AddPkgWithVersion_WhenProjectOnboardedToCPMAndNoPackagesExistI await SimpleTestPackageUtility.CreateFolderFeedV3Async( pathContext.PackageSource, PackageSaveMode.Defaultv3, - packageX100); - await SimpleTestPackageUtility.CreateFolderFeedV3Async( - pathContext.PackageSource, - PackageSaveMode.Defaultv3, + packageX100, packageX200); var propsFile = @$" @@ -721,14 +713,13 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( } [Fact] - public async Task AddPkgWithNoVersion_WhenProjectOnboardedToCPMAndPackageVersionExistsInProps() + public async Task AddPkg_WithCPM_WhenPackageVersionExistsAndVersionCLIArgNotPassed_Success() { using var pathContext = new SimpleTestPathContext(); // Set up solution and project var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); - var projectNameA = "projectA"; - var projectA = XPlatTestUtils.CreateProject(projectNameA, pathContext, "net5.0"); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net5.0"); const string version = "2.0.0"; const string packageX = "X"; @@ -772,14 +763,13 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( } [Fact] - public async Task AddPkgWithVersion_WhenProjectOnboardedToCPMAndPackageVersionExistsInProps() + public async Task AddPkg_WithCPM_WhenPackageVersionExistsAndVersionCLIArgPassed_Success() { using var pathContext = new SimpleTestPathContext(); // Set up solution var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); - var projectNameA = "projectA"; - var projectA = XPlatTestUtils.CreateProject(projectNameA, pathContext, "net5.0"); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net5.0"); const string version = "2.0.0"; const string packageX = "X"; @@ -828,8 +818,7 @@ public async Task AddPackageVersionToCorrectItemGroupInPropsFile() // Set up solution var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); - var projectNameA = "projectA"; - var projectA = XPlatTestUtils.CreateProject(projectNameA, pathContext, "net5.0"); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net5.0"); const string version = "1.0.0"; const string packageX = "X"; From 2d5c9e4380e04e2a77c54bf91d4018936f2ccda4 Mon Sep 17 00:00:00 2001 From: pragnya17 <59893188+pragnya17@users.noreply.github.com> Date: Thu, 30 Jun 2022 10:32:04 -0700 Subject: [PATCH 05/16] Change when info is logged --- .../Utility/MSBuildAPIUtility.cs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index dc9addf3a45..242e3fa1427 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -262,15 +262,13 @@ private void AddPackageVersionIntoPropsItemGroup(Project project, ProjectItemGro // Add both package reference information and version metadata using the PACKAGE_VERSION_TYPE_TAG. var item = itemGroup.AddItem(PACKAGE_VERSION_TYPE_TAG, libraryDependency.Name); var packageVersion = AddVersionMetadata(libraryDependency, item); - + AddExtraMetadataToProjectItemElement(libraryDependency, item); Logger.LogInformation(string.Format(CultureInfo.CurrentCulture, Strings.Info_AddPkgAdded, libraryDependency.Name, packageVersion, itemGroup.ContainingProject.FullPath )); - - AddExtraMetadataToProjectItemElement(libraryDependency, item); } /// @@ -284,10 +282,8 @@ private void AddPackageReferenceIntoItemGroup(ProjectItemGroupElement itemGroup, // Add both package reference information and version metadata using the PACKAGE_REFERENCE_TYPE_TAG. var item = itemGroup.AddItem(PACKAGE_REFERENCE_TYPE_TAG, libraryDependency.Name); var packageVersion = AddVersionMetadata(libraryDependency, item); - - Logger.LogInformation(string.Format(CultureInfo.CurrentCulture, Strings.Info_AddPkgAdded, libraryDependency.Name, packageVersion, itemGroup.ContainingProject.FullPath)); - AddExtraMetadataToProjectItemElement(libraryDependency, item); + Logger.LogInformation(string.Format(CultureInfo.CurrentCulture, Strings.Info_AddPkgAdded, libraryDependency.Name, packageVersion, itemGroup.ContainingProject.FullPath)); } /// @@ -301,10 +297,8 @@ private void AddPackageReferenceIntoItemGroupCPM(Project project, ProjectItemGro { // Only add the package reference information using the PACKAGE_REFERENCE_TYPE_TAG. ProjectItemElement item = itemGroup.AddItem(PACKAGE_REFERENCE_TYPE_TAG, libraryDependency.Name); - - Logger.LogInformation(string.Format(CultureInfo.CurrentCulture, Strings.Info_AddPkgCPM, libraryDependency.Name, project.GetPropertyValue(DirectoryPackagesPropsPathPropertyName), itemGroup.ContainingProject.FullPath)); - AddExtraMetadataToProjectItemElement(libraryDependency, item); + Logger.LogInformation(string.Format(CultureInfo.CurrentCulture, Strings.Info_AddPkgCPM, libraryDependency.Name, project.GetPropertyValue(DirectoryPackagesPropsPathPropertyName), itemGroup.ContainingProject.FullPath)); } /// From 991539a8454c4e353ff0fbb82fbbd8cde45f9d53 Mon Sep 17 00:00:00 2001 From: pragnya17 <59893188+pragnya17@users.noreply.github.com> Date: Thu, 30 Jun 2022 15:11:08 -0700 Subject: [PATCH 06/16] fix indentation and existing package references lines --- .../Utility/MSBuildAPIUtility.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index 242e3fa1427..28f7de09c60 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -194,12 +194,15 @@ private void AddPackageReference(Project project, } else { + if (!existingPackageReferences.Any()) + { + //Add to the project file. + AddPackageReferenceIntoItemGroupCPM(project, itemGroup, libraryDependency); + } + // Get package version if it already exists in the props file. Returns null if there is no matching package version. ProjectItem packageVersionInProps = project.Items.LastOrDefault(i => i.ItemType == PACKAGE_VERSION_TYPE_TAG && i.EvaluatedInclude.Equals(libraryDependency.Name)); - //Add to the project file. - AddPackageReferenceIntoItemGroupCPM(project, itemGroup, libraryDependency); - // If no exists in the Directory.Packages.props file. if (packageVersionInProps == null) { @@ -263,11 +266,7 @@ private void AddPackageVersionIntoPropsItemGroup(Project project, ProjectItemGro var item = itemGroup.AddItem(PACKAGE_VERSION_TYPE_TAG, libraryDependency.Name); var packageVersion = AddVersionMetadata(libraryDependency, item); AddExtraMetadataToProjectItemElement(libraryDependency, item); - Logger.LogInformation(string.Format(CultureInfo.CurrentCulture, - Strings.Info_AddPkgAdded, - libraryDependency.Name, - packageVersion, - itemGroup.ContainingProject.FullPath + Logger.LogInformation(string.Format(CultureInfo.CurrentCulture, Strings.Info_AddPkgAdded, libraryDependency.Name, packageVersion, itemGroup.ContainingProject.FullPath )); } From 12118524de75c4437d1cdf5a0b4cc00cdae0e920 Mon Sep 17 00:00:00 2001 From: Kartheek Penagamuri Date: Fri, 1 Jul 2022 10:42:04 -0700 Subject: [PATCH 07/16] [skip ci] add basic unit test for msbuild api --- .../Utility/MSBuildAPIUtility.cs | 7 +- .../MSBuildAPIUtilityTests.cs | 70 +++++++++++++++++++ .../NuGet.CommandLine.Xplat.Tests.csproj | 6 ++ 3 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index 28f7de09c60..493dbf12ee1 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -234,7 +234,7 @@ private void AddPackageVersionIntoItemGroupCPM(Project project, LibraryDependenc // Get the ItemGroup to add a PackageVersion to or create a new one. var propsItemGroup = GetItemGroup(directoryBuildPropsRootElement.ItemGroups, PACKAGE_VERSION_TYPE_TAG) ?? directoryBuildPropsRootElement.AddItemGroup(); - AddPackageVersionIntoPropsItemGroup(project, propsItemGroup, libraryDependency); + AddPackageVersionIntoPropsItemGroup(propsItemGroup, libraryDependency); // Save the updated props file. directoryBuildPropsRootElement.Save(); @@ -245,7 +245,7 @@ private void AddPackageVersionIntoItemGroupCPM(Project project, LibraryDependenc /// /// Project that needs to be modified. /// The directory build props root element. - private ProjectRootElement GetDirectoryBuildPropsRootElement(Project project) + internal ProjectRootElement GetDirectoryBuildPropsRootElement(Project project) { // Get the Directory.Packages.props path. string directoryPackagesPropsPath = project.GetPropertyValue(DirectoryPackagesPropsPathPropertyName); @@ -256,10 +256,9 @@ private ProjectRootElement GetDirectoryBuildPropsRootElement(Project project) /// /// Add package name and version into the props file. /// - /// Project that is being modified. /// Item group that needs to be modified in the props file. /// Package Dependency of the package to be added. - private void AddPackageVersionIntoPropsItemGroup(Project project, ProjectItemGroupElement itemGroup, + private void AddPackageVersionIntoPropsItemGroup(ProjectItemGroupElement itemGroup, LibraryDependency libraryDependency) { // Add both package reference information and version metadata using the PACKAGE_VERSION_TYPE_TAG. diff --git a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs new file mode 100644 index 00000000000..727dd3c8438 --- /dev/null +++ b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs @@ -0,0 +1,70 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.IO; +using Microsoft.Build.Definition; +using Microsoft.Build.Evaluation; +using Microsoft.Build.Locator; +using NuGet.CommandLine.XPlat; +using NuGet.Test.Utility; +using Xunit; +using Project = Microsoft.Build.Evaluation.Project; + +namespace NuGet.CommandLine.Xplat.Tests +{ + public class MSBuildAPIUtilityTests + { + static MSBuildAPIUtilityTests() + { + MSBuildLocator.RegisterDefaults(); + } + + [Fact] + public void MSBuilldTest() + { + var testDirectory = TestDirectory.Create(); + + var projectCollection = new ProjectCollection( + globalProperties: null, + remoteLoggers: null, + loggers: null, + toolsetDefinitionLocations: ToolsetDefinitionLocations.Default, + // Having more than 1 node spins up multiple msbuild.exe instances to run builds in parallel + // However, these targets complete so quickly that the added overhead makes it take longer + maxNodeCount: 1, + onlyLogCriticalEvents: false, + loadProjectsReadOnly: false); + + var projectOptions = new ProjectOptions + { + LoadSettings = ProjectLoadSettings.DoNotEvaluateElementsWithFalseCondition, + ProjectCollection = projectCollection + }; + + var propsFile = +@$" + + true + +"; + File.WriteAllText(Path.Combine(testDirectory, "Directory.Packages.props"), propsFile); + + string projectContent = +@$" + + Exe + net6.0 + enable + enable + +"; + File.WriteAllText(Path.Combine(testDirectory, "projectA.csproj"), projectContent); + + var project = Project.FromFile(Path.Combine(testDirectory, "projectA.csproj"), projectOptions); + + var yes = new MSBuildAPIUtility(logger: new TestLogger()).GetDirectoryBuildPropsRootElement(project); + + Assert.Equal(Path.Combine(testDirectory, "Directory.Packages.props"), yes.FullPath); + } + } +} diff --git a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/NuGet.CommandLine.Xplat.Tests.csproj b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/NuGet.CommandLine.Xplat.Tests.csproj index f0c97435a88..0d12db7b139 100644 --- a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/NuGet.CommandLine.Xplat.Tests.csproj +++ b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/NuGet.CommandLine.Xplat.Tests.csproj @@ -17,6 +17,12 @@ + + + + + + From 9c4e593c4358cd0b6cf2bfd0fb89ecf16b45d21f Mon Sep 17 00:00:00 2001 From: Pragnya Pandrate Date: Tue, 5 Jul 2022 10:36:58 -0700 Subject: [PATCH 08/16] Change name of test --- .../Dotnet.Integration.Test/DotnetAddPackageTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs index 58088ab4da6..32086c5d4fe 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs @@ -812,7 +812,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( } [Fact] - public async Task AddPackageVersionToCorrectItemGroupInPropsFile() + public async Task AddPackageVersionToCorrectItemGroupInPropsFile_Success() { using var pathContext = new SimpleTestPathContext(); From 4dbde7bca21016a033096a2388b5ba99def203ef Mon Sep 17 00:00:00 2001 From: Pragnya Pandrate Date: Wed, 6 Jul 2022 13:47:21 -0700 Subject: [PATCH 09/16] Add unit tests --- .../Utility/MSBuildAPIUtility.cs | 12 +- .../MSBuildAPIUtilityTests.cs | 328 +++++++++++++++++- 2 files changed, 327 insertions(+), 13 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index 493dbf12ee1..d9ab83e593a 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -258,7 +258,7 @@ internal ProjectRootElement GetDirectoryBuildPropsRootElement(Project project) /// /// Item group that needs to be modified in the props file. /// Package Dependency of the package to be added. - private void AddPackageVersionIntoPropsItemGroup(ProjectItemGroupElement itemGroup, + internal void AddPackageVersionIntoPropsItemGroup(ProjectItemGroupElement itemGroup, LibraryDependency libraryDependency) { // Add both package reference information and version metadata using the PACKAGE_VERSION_TYPE_TAG. @@ -290,7 +290,7 @@ private void AddPackageReferenceIntoItemGroup(ProjectItemGroupElement itemGroup, /// Project to be modified. /// Item group to add to. /// Package Dependency of the package to be added. - private void AddPackageReferenceIntoItemGroupCPM(Project project, ProjectItemGroupElement itemGroup, + internal void AddPackageReferenceIntoItemGroupCPM(Project project, ProjectItemGroupElement itemGroup, LibraryDependency libraryDependency) { // Only add the package reference information using the PACKAGE_REFERENCE_TYPE_TAG. @@ -324,7 +324,7 @@ private void AddExtraMetadataToProjectItemElement(LibraryDependency libraryDepen /// /// A specified project. /// - private static IEnumerable GetItemGroups(Project project) + internal IEnumerable GetItemGroups(Project project) { return project .Items @@ -339,7 +339,7 @@ private static IEnumerable GetItemGroups(Project projec /// List of all item groups in the project /// An item type tag that must be in the item group. It if PackageReference in this case. /// An ItemGroup, which could be null. - private static ProjectItemGroupElement GetItemGroup(IEnumerable itemGroups, + internal ProjectItemGroupElement GetItemGroup(IEnumerable itemGroups, string itemType) { var itemGroup = itemGroups? @@ -355,7 +355,7 @@ private static ProjectItemGroupElement GetItemGroup(IEnumerableProject where the item group should be created. /// Target Framework for which the package reference should be added. /// An Item Group. - private static ProjectItemGroupElement CreateItemGroup(Project project, string framework = null) + internal ProjectItemGroupElement CreateItemGroup(Project project, string framework = null) { // Create a new item group and add a condition if given var itemGroup = project.Xml.AddItemGroup(); @@ -427,7 +427,7 @@ private void UpdatePackageReferenceItems(IEnumerable packageReferen /// /// item with a matching package ID. /// Version that is passed in as a CLI argument. - private static void UpdatePackageVersion(Project project, ProjectItem packageVersion, string versionCLIArgument) + internal void UpdatePackageVersion(Project project, ProjectItem packageVersion, string versionCLIArgument) { // Determine where the item is decalred ProjectItemElement packageVersionItemElement = project.GetItemProvenance(packageVersion).LastOrDefault()?.ItemElement; diff --git a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs index 727dd3c8438..4b857d64d2b 100644 --- a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs +++ b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs @@ -2,11 +2,14 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.IO; +using System.Linq; using Microsoft.Build.Definition; using Microsoft.Build.Evaluation; using Microsoft.Build.Locator; using NuGet.CommandLine.XPlat; +using NuGet.LibraryModel; using NuGet.Test.Utility; +using NuGet.Versioning; using Xunit; using Project = Microsoft.Build.Evaluation.Project; @@ -20,7 +23,7 @@ static MSBuildAPIUtilityTests() } [Fact] - public void MSBuilldTest() + public void GetDirectoryBuildPropsRootElementWhenItExists_Success() { var testDirectory = TestDirectory.Create(); @@ -51,20 +54,331 @@ public void MSBuilldTest() string projectContent = @$" - - Exe + net6.0 - enable - enable "; File.WriteAllText(Path.Combine(testDirectory, "projectA.csproj"), projectContent); var project = Project.FromFile(Path.Combine(testDirectory, "projectA.csproj"), projectOptions); - var yes = new MSBuildAPIUtility(logger: new TestLogger()).GetDirectoryBuildPropsRootElement(project); + var result = new MSBuildAPIUtility(logger: new TestLogger()).GetDirectoryBuildPropsRootElement(project); - Assert.Equal(Path.Combine(testDirectory, "Directory.Packages.props"), yes.FullPath); + Assert.Equal(Path.Combine(testDirectory, "Directory.Packages.props"), result.FullPath); } + + [Fact] + public void AddPackageReferenceIntoProjectFileWhenItemGroupDoesNotExist_Success() + { + // Set up + var testDirectory = TestDirectory.Create(); + var projectCollection = new ProjectCollection( + globalProperties: null, + remoteLoggers: null, + loggers: null, + toolsetDefinitionLocations: ToolsetDefinitionLocations.Default, + // Having more than 1 node spins up multiple msbuild.exe instances to run builds in parallel + // However, these targets complete so quickly that the added overhead makes it take longer + maxNodeCount: 1, + onlyLogCriticalEvents: false, + loadProjectsReadOnly: false); + + var projectOptions = new ProjectOptions + { + LoadSettings = ProjectLoadSettings.DoNotEvaluateElementsWithFalseCondition, + ProjectCollection = projectCollection + }; + + // Set up project file + string projectContent = +@$" + +net6.0 + +"; + File.WriteAllText(Path.Combine(testDirectory, "projectA.csproj"), projectContent); + var project = Project.FromFile(Path.Combine(testDirectory, "projectA.csproj"), projectOptions); + + var msObject = new MSBuildAPIUtility(logger: new TestLogger()); + // Creating an item group in the project + var itemGroup = msObject.CreateItemGroup(project, null); + + var libraryDependency = new LibraryDependency + { + LibraryRange = new LibraryRange( + name: "X", + versionRange: VersionRange.Parse("1.0.0"), + typeConstraint: LibraryDependencyTarget.Package) + }; + + // Act + msObject.AddPackageReferenceIntoItemGroupCPM(project, itemGroup, libraryDependency); + project.Save(); + + // Assert + Assert.Contains(@$"", File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); + Assert.DoesNotContain(@$"", File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); + } + + [Fact] + public void AddPackageReferenceIntoProjectFileWhenItemGroupDoesExist_Success() + { + // Set up + var testDirectory = TestDirectory.Create(); + var projectCollection = new ProjectCollection( + globalProperties: null, + remoteLoggers: null, + loggers: null, + toolsetDefinitionLocations: ToolsetDefinitionLocations.Default, + // Having more than 1 node spins up multiple msbuild.exe instances to run builds in parallel + // However, these targets complete so quickly that the added overhead makes it take longer + maxNodeCount: 1, + onlyLogCriticalEvents: false, + loadProjectsReadOnly: false); + + var projectOptions = new ProjectOptions + { + LoadSettings = ProjectLoadSettings.DoNotEvaluateElementsWithFalseCondition, + ProjectCollection = projectCollection + }; + + // Set up project file + string projectContent = +@$" + +net6.0 + + + + +"; + File.WriteAllText(Path.Combine(testDirectory, "projectA.csproj"), projectContent); + var project = Project.FromFile(Path.Combine(testDirectory, "projectA.csproj"), projectOptions); + + var msObject = new MSBuildAPIUtility(logger: new TestLogger()); + // Getting all the item groups in a given project + var itemGroups = msObject.GetItemGroups(project); + // Getting an existing item group that has package reference(s) + var itemGroup = msObject.GetItemGroup(itemGroups, "PackageReference"); + + var libraryDependency = new LibraryDependency + { + LibraryRange = new LibraryRange( + name: "X", + versionRange: VersionRange.Parse("1.0.0"), + typeConstraint: LibraryDependencyTarget.Package) + }; + + // Act + msObject.AddPackageReferenceIntoItemGroupCPM(project, itemGroup, libraryDependency); + project.Save(); + + // Assert + Assert.Contains(@$"", File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); + Assert.DoesNotContain(@$"", File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); + } + + [Fact] + public void AddPackageVersionIntoPropsFileWhenItemGroupDoesNotExist_Success() + { + // Set up + var testDirectory = TestDirectory.Create(); + var projectCollection = new ProjectCollection( + globalProperties: null, + remoteLoggers: null, + loggers: null, + toolsetDefinitionLocations: ToolsetDefinitionLocations.Default, + // Having more than 1 node spins up multiple msbuild.exe instances to run builds in parallel + // However, these targets complete so quickly that the added overhead makes it take longer + maxNodeCount: 1, + onlyLogCriticalEvents: false, + loadProjectsReadOnly: false); + + var projectOptions = new ProjectOptions + { + LoadSettings = ProjectLoadSettings.DoNotEvaluateElementsWithFalseCondition, + ProjectCollection = projectCollection + }; + + // Set up Directory.Packages.props file + var propsFile = +@$" + + true + +"; + File.WriteAllText(Path.Combine(testDirectory, "Directory.Packages.props"), propsFile); + + // Set up project file + string projectContent = +@$" + + net6.0 + +"; + File.WriteAllText(Path.Combine(testDirectory, "projectA.csproj"), projectContent); + var project = Project.FromFile(Path.Combine(testDirectory, "projectA.csproj"), projectOptions); + + // Add item group to Directory.Packages.props + var msObject = new MSBuildAPIUtility(logger: new TestLogger()); + var directoryBuildPropsRootElement = msObject.GetDirectoryBuildPropsRootElement(project); + var propsItemGroup = directoryBuildPropsRootElement.AddItemGroup(); + + var libraryDependency = new LibraryDependency + { + LibraryRange = new LibraryRange( + name: "X", + versionRange: VersionRange.Parse("1.0.0"), + typeConstraint: LibraryDependencyTarget.Package) + }; + + // Act + msObject.AddPackageVersionIntoPropsItemGroup(propsItemGroup, libraryDependency); + // Save the updated props file. + directoryBuildPropsRootElement.Save(); + + // Assert + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(testDirectory, "Directory.Packages.props"))); + } + + [Fact] + public void AddPackageVersionIntoPropsFileWhenItemGroupExists_Success() + { + // Set up + var testDirectory = TestDirectory.Create(); + var projectCollection = new ProjectCollection( + globalProperties: null, + remoteLoggers: null, + loggers: null, + toolsetDefinitionLocations: ToolsetDefinitionLocations.Default, + // Having more than 1 node spins up multiple msbuild.exe instances to run builds in parallel + // However, these targets complete so quickly that the added overhead makes it take longer + maxNodeCount: 1, + onlyLogCriticalEvents: false, + loadProjectsReadOnly: false); + + var projectOptions = new ProjectOptions + { + LoadSettings = ProjectLoadSettings.DoNotEvaluateElementsWithFalseCondition, + ProjectCollection = projectCollection + }; + + // Set up Directory.Packages.props file + var propsFile = +@$" + + true + + + + +"; + File.WriteAllText(Path.Combine(testDirectory, "Directory.Packages.props"), propsFile); + + // Set up project file + string projectContent = +@$" + + net6.0 + + + + +"; + File.WriteAllText(Path.Combine(testDirectory, "projectA.csproj"), projectContent); + var project = Project.FromFile(Path.Combine(testDirectory, "projectA.csproj"), projectOptions); + + // Get existing item group from Directory.Packages.props + var msObject = new MSBuildAPIUtility(logger: new TestLogger()); + var directoryBuildPropsRootElement = msObject.GetDirectoryBuildPropsRootElement(project); + var propsItemGroup = msObject.GetItemGroup(directoryBuildPropsRootElement.ItemGroups, "PackageVersion"); + + var libraryDependency = new LibraryDependency + { + LibraryRange = new LibraryRange( + name: "Y", + versionRange: VersionRange.Parse("1.0.0"), + typeConstraint: LibraryDependencyTarget.Package) + }; + + // Act + msObject.AddPackageVersionIntoPropsItemGroup(propsItemGroup, libraryDependency); + // Save the updated props file + directoryBuildPropsRootElement.Save(); + + // Assert + Assert.Contains(@$"", File.ReadAllText(Path.Combine(testDirectory, "Directory.Packages.props"))); + } + + [Fact] + public void UpdatePackageVersionInPropsFileWhenItExists_Success() + { + // Set up + var testDirectory = TestDirectory.Create(); + var projectCollection = new ProjectCollection( + globalProperties: null, + remoteLoggers: null, + loggers: null, + toolsetDefinitionLocations: ToolsetDefinitionLocations.Default, + // Having more than 1 node spins up multiple msbuild.exe instances to run builds in parallel + // However, these targets complete so quickly that the added overhead makes it take longer + maxNodeCount: 1, + onlyLogCriticalEvents: false, + loadProjectsReadOnly: false); + + var projectOptions = new ProjectOptions + { + LoadSettings = ProjectLoadSettings.DoNotEvaluateElementsWithFalseCondition, + ProjectCollection = projectCollection + }; + + // Set up Directory.Packages.props file + var propsFile = +@$" + + true + + + + +"; + File.WriteAllText(Path.Combine(testDirectory, "Directory.Packages.props"), propsFile); + + // Set up project file + string projectContent = +@$" + + net6.0 + + + + +"; + File.WriteAllText(Path.Combine(testDirectory, "projectA.csproj"), projectContent); + var project = Project.FromFile(Path.Combine(testDirectory, "projectA.csproj"), projectOptions); + + var msObject = new MSBuildAPIUtility(logger: new TestLogger()); + // Get package version if it already exists in the props file. Returns null if there is no matching package version. + ProjectItem packageVersionInProps = project.Items.LastOrDefault(i => i.ItemType == "PackageVersion" && i.EvaluatedInclude.Equals("X")); + + var libraryDependency = new LibraryDependency + { + LibraryRange = new LibraryRange( + name: "X", + versionRange: VersionRange.Parse("2.0.0"), + typeConstraint: LibraryDependencyTarget.Package) + }; + + // Act + msObject.UpdatePackageVersion(project, packageVersionInProps, "2.0.0"); + + // Assert + Assert.Equal(projectContent, File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); + Assert.Contains(@$"", File.ReadAllText(Path.Combine(testDirectory, "Directory.Packages.props"))); + Assert.DoesNotContain(@$"", File.ReadAllText(Path.Combine(testDirectory, "Directory.Packages.props"))); + } + } } From 17403343a66ea67d2dcf0f294d249f0510aa1b39 Mon Sep 17 00:00:00 2001 From: Pragnya Pandrate Date: Wed, 6 Jul 2022 14:44:31 -0700 Subject: [PATCH 10/16] Change name of integration test --- .../Dotnet.Integration.Test/DotnetAddPackageTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs index 32086c5d4fe..9d37977c1ce 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs @@ -812,7 +812,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( } [Fact] - public async Task AddPackageVersionToCorrectItemGroupInPropsFile_Success() + public async Task AddPkg_WithCPM_WhenMultipleItemGroupsExist_Success() { using var pathContext = new SimpleTestPathContext(); From e3c1b34b4f286efd73d1da23754dd18792edf0ab Mon Sep 17 00:00:00 2001 From: Pragnya Pandrate Date: Wed, 6 Jul 2022 14:58:49 -0700 Subject: [PATCH 11/16] Change name to noop test --- .../Dotnet.Integration.Test/DotnetAddPackageTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs index 9d37977c1ce..eea077bdf40 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs @@ -659,7 +659,7 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( Assert.Contains(@$" Date: Thu, 7 Jul 2022 13:43:04 -0700 Subject: [PATCH 12/16] [skip ci] Use boolean to see if package version is passed in as CLI arg or not --- .../AddPackageReferenceCommandRunner.cs | 6 +++--- .../Utility/MSBuildAPIUtility.cs | 19 ++++++++++--------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs index c8a67910dcd..5e1244d29bc 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs @@ -57,7 +57,7 @@ public async Task ExecuteCommand(PackageReferenceArgs packageReferenceArgs, typeConstraint: LibraryDependencyTarget.Package) }; - msBuild.AddPackageReference(packageReferenceArgs.ProjectPath, libraryDependency, packageReferenceArgs.PackageVersion); + msBuild.AddPackageReference(packageReferenceArgs.ProjectPath, libraryDependency, packageReferenceArgs.NoVersion); return 0; } @@ -219,7 +219,7 @@ public async Task ExecuteCommand(PackageReferenceArgs packageReferenceArgs, // generate a library dependency with all the metadata like Include, Exlude and SuppressParent var libraryDependency = GenerateLibraryDependency(updatedPackageSpec, packageReferenceArgs, restorePreviewResult, userSpecifiedFrameworks, packageDependency); - msBuild.AddPackageReference(packageReferenceArgs.ProjectPath, libraryDependency, packageReferenceArgs.PackageVersion); + msBuild.AddPackageReference(packageReferenceArgs.ProjectPath, libraryDependency, packageReferenceArgs.NoVersion); } else { @@ -240,7 +240,7 @@ public async Task ExecuteCommand(PackageReferenceArgs packageReferenceArgs, msBuild.AddPackageReferencePerTFM(packageReferenceArgs.ProjectPath, libraryDependency, compatibleOriginalFrameworks, - packageReferenceArgs.PackageVersion); + packageReferenceArgs.NoVersion); } // 6. Commit restore result diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index d9ab83e593a..a67da984da9 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -125,8 +125,8 @@ public int RemovePackageReference(string projectPath, LibraryDependency libraryD /// /// Path to the csproj file of the project. /// Package Dependency of the package to be added. - /// The version that is passed in as a CLI argument. Null if no version is passed in the command. - public void AddPackageReference(string projectPath, LibraryDependency libraryDependency, string packageVersion) + /// If a version is passed in as a CLI argument. + public void AddPackageReference(string projectPath, LibraryDependency libraryDependency, bool noVersion) { var project = GetProject(projectPath); @@ -134,7 +134,7 @@ public void AddPackageReference(string projectPath, LibraryDependency libraryDep // If the project has a conditional reference, then an unconditional reference is not added. var existingPackageReferences = GetPackageReferencesForAllFrameworks(project, libraryDependency); - AddPackageReference(project, libraryDependency, existingPackageReferences, packageVersion); + AddPackageReference(project, libraryDependency, existingPackageReferences, noVersion); ProjectCollection.GlobalProjectCollection.UnloadProject(project); } @@ -144,9 +144,9 @@ public void AddPackageReference(string projectPath, LibraryDependency libraryDep /// Path to the csproj file of the project. /// Package Dependency of the package to be added. /// Target Frameworks for which the package reference should be added. - /// The version that is passed in as a CLI argument. Null if no version is passed in the command. + /// If a version is passed in as a CLI argument. public void AddPackageReferencePerTFM(string projectPath, LibraryDependency libraryDependency, - IEnumerable frameworks, string packageVersion) + IEnumerable frameworks, bool noVersion) { foreach (var framework in frameworks) { @@ -154,7 +154,7 @@ public void AddPackageReferencePerTFM(string projectPath, LibraryDependency libr { { "TargetFramework", framework } }; var project = GetProject(projectPath, globalProperties); var existingPackageReferences = GetPackageReferences(project, libraryDependency); - AddPackageReference(project, libraryDependency, existingPackageReferences, packageVersion, framework); + AddPackageReference(project, libraryDependency, existingPackageReferences, noVersion, framework); ProjectCollection.GlobalProjectCollection.UnloadProject(project); } } @@ -165,12 +165,12 @@ public void AddPackageReferencePerTFM(string projectPath, LibraryDependency libr /// Project that needs to be modified. /// Package Dependency of the package to be added. /// Package references that already exist in the project. - /// The version that is passed in as a CLI argument. Null if no version is passed in the command. + /// If a version is passed in as a CLI argument. /// Target Framework for which the package reference should be added. private void AddPackageReference(Project project, LibraryDependency libraryDependency, IEnumerable existingPackageReferences, - string packageVersion, + bool noVersion, string framework = null) { // Getting all the item groups in a given project @@ -212,8 +212,9 @@ private void AddPackageReference(Project project, else { // Modify the Directory.Packages.props file with the version that is passed in. - if (packageVersion != null) + if (!noVersion) { + string packageVersion = libraryDependency.LibraryRange.VersionRange.OriginalString; UpdatePackageVersion(project, packageVersionInProps, packageVersion); } } From 4e49d9360afb400af008066d5683c76ea266b30d Mon Sep 17 00:00:00 2001 From: Pragnya Pandrate Date: Thu, 7 Jul 2022 14:58:00 -0700 Subject: [PATCH 13/16] [skip ci] Attempt to modify unit test, still need to debug --- .../NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs index 4b857d64d2b..190a2883e09 100644 --- a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs +++ b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs @@ -3,6 +3,7 @@ using System.IO; using System.Linq; +using Microsoft.Build.Construction; using Microsoft.Build.Definition; using Microsoft.Build.Evaluation; using Microsoft.Build.Locator; @@ -116,8 +117,8 @@ public void AddPackageReferenceIntoProjectFileWhenItemGroupDoesNotExist_Success( project.Save(); // Assert - Assert.Contains(@$"", File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); - Assert.DoesNotContain(@$"", File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); + ProjectItem packageReference = project.Xml.Items.LastOrDefault(i => i.ItemType == "PackageReference" && i.EvaluatedInclude.Equals("X")); + Assert.True(packageReference != null, packageReference.ItemType); } [Fact] From c7aef21df1fab875518949b4ae66bf26f092e719 Mon Sep 17 00:00:00 2001 From: Pragnya Pandrate Date: Fri, 8 Jul 2022 11:52:14 -0700 Subject: [PATCH 14/16] Revert "[skip ci] Attempt to modify unit test, still need to debug" This reverts commit 4e49d9360afb400af008066d5683c76ea266b30d. --- .../NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs index 190a2883e09..4b857d64d2b 100644 --- a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs +++ b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs @@ -3,7 +3,6 @@ using System.IO; using System.Linq; -using Microsoft.Build.Construction; using Microsoft.Build.Definition; using Microsoft.Build.Evaluation; using Microsoft.Build.Locator; @@ -117,8 +116,8 @@ public void AddPackageReferenceIntoProjectFileWhenItemGroupDoesNotExist_Success( project.Save(); // Assert - ProjectItem packageReference = project.Xml.Items.LastOrDefault(i => i.ItemType == "PackageReference" && i.EvaluatedInclude.Equals("X")); - Assert.True(packageReference != null, packageReference.ItemType); + Assert.Contains(@$"", File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); + Assert.DoesNotContain(@$"", File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); } [Fact] From 60655e887ef0ff08c5c89752a09b8ad7e25160d9 Mon Sep 17 00:00:00 2001 From: Pragnya Pandrate Date: Fri, 8 Jul 2022 14:52:30 -0700 Subject: [PATCH 15/16] [skip ci] remove repetition when reading file from disk --- .../DotnetAddPackageTests.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs index eea077bdf40..5c1c25edd50 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs @@ -757,9 +757,11 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( // Checking that the PackageVersion is not updated. Assert.Contains(@$"", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + var projectFileFromDisk = File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj")); + // Checking that version metadata is not added to the project files. - Assert.Contains(@$"Include=""X""", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); - Assert.DoesNotContain(@$"Include=""X"" Version=""1.0.0""", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + Assert.Contains(@$"Include=""X""", projectFileFromDisk); + Assert.DoesNotContain(@$"Include=""X"" Version=""1.0.0""", projectFileFromDisk); } [Fact] @@ -805,10 +807,12 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( Assert.True(result.Success, result.Output); Assert.Contains(@$"", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + var projectFileFromDisk = File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj")); + // Checking that version metadata is not added to the project files. Assert.Contains(@$"Include=""X""", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); - Assert.DoesNotContain(@$"Include=""X"" Version=""1.0.0""", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); - Assert.DoesNotContain(@$"Include=""X"" Version=""2.0.0""", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + Assert.DoesNotContain(@$"Include=""X"" Version=""1.0.0""", projectFileFromDisk); + Assert.DoesNotContain(@$"Include=""X"" Version=""2.0.0""", projectFileFromDisk); } [Fact] @@ -855,15 +859,17 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( // Assert Assert.True(result.Success, result.Output); + var propsFileFromDisk = File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props")); + Assert.Contains(@$" - ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + ", propsFileFromDisk); Assert.DoesNotContain($@"< ItemGroup > < Content Include = ""SomeFile"" /> - ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + ", propsFileFromDisk); } } } From 58a5ddf0f202efb52e0bc7871654aa4913a6cb26 Mon Sep 17 00:00:00 2001 From: Kartheek Penagamuri Date: Mon, 11 Jul 2022 14:02:57 -0700 Subject: [PATCH 16/16] updated tests --- .../MSBuildAPIUtilityTests.cs | 60 ++++++++++--------- 1 file changed, 32 insertions(+), 28 deletions(-) diff --git a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs index 4b857d64d2b..2ef55850e85 100644 --- a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs +++ b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs @@ -21,10 +21,11 @@ static MSBuildAPIUtilityTests() { MSBuildLocator.RegisterDefaults(); } - - [Fact] + + [PlatformFact(Platform.Windows)] public void GetDirectoryBuildPropsRootElementWhenItExists_Success() { + // Arrange var testDirectory = TestDirectory.Create(); var projectCollection = new ProjectCollection( @@ -59,18 +60,19 @@ public void GetDirectoryBuildPropsRootElementWhenItExists_Success() "; File.WriteAllText(Path.Combine(testDirectory, "projectA.csproj"), projectContent); - var project = Project.FromFile(Path.Combine(testDirectory, "projectA.csproj"), projectOptions); + // Act var result = new MSBuildAPIUtility(logger: new TestLogger()).GetDirectoryBuildPropsRootElement(project); + // Assert Assert.Equal(Path.Combine(testDirectory, "Directory.Packages.props"), result.FullPath); } - [Fact] + [PlatformFact(Platform.Windows)] public void AddPackageReferenceIntoProjectFileWhenItemGroupDoesNotExist_Success() { - // Set up + // Arrange var testDirectory = TestDirectory.Create(); var projectCollection = new ProjectCollection( globalProperties: null, @@ -89,7 +91,7 @@ public void AddPackageReferenceIntoProjectFileWhenItemGroupDoesNotExist_Success( ProjectCollection = projectCollection }; - // Set up project file + // Arrange project file string projectContent = @$" @@ -116,14 +118,15 @@ public void AddPackageReferenceIntoProjectFileWhenItemGroupDoesNotExist_Success( project.Save(); // Assert - Assert.Contains(@$"", File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); - Assert.DoesNotContain(@$"", File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); + string updatedProjectFile = File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj")); + Assert.Contains(@$"", updatedProjectFile); + Assert.DoesNotContain(@$"", updatedProjectFile); } - [Fact] + [PlatformFact(Platform.Windows)] public void AddPackageReferenceIntoProjectFileWhenItemGroupDoesExist_Success() { - // Set up + // Arrange var testDirectory = TestDirectory.Create(); var projectCollection = new ProjectCollection( globalProperties: null, @@ -142,7 +145,7 @@ public void AddPackageReferenceIntoProjectFileWhenItemGroupDoesExist_Success() ProjectCollection = projectCollection }; - // Set up project file + // Arrange project file string projectContent = @$" @@ -174,14 +177,15 @@ public void AddPackageReferenceIntoProjectFileWhenItemGroupDoesExist_Success() project.Save(); // Assert - Assert.Contains(@$"", File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); - Assert.DoesNotContain(@$"", File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); + string updatedProjectFile = File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj")); + Assert.Contains(@$"", updatedProjectFile); + Assert.DoesNotContain(@$"", updatedProjectFile); } - [Fact] + [PlatformFact(Platform.Windows)] public void AddPackageVersionIntoPropsFileWhenItemGroupDoesNotExist_Success() { - // Set up + // Arrange var testDirectory = TestDirectory.Create(); var projectCollection = new ProjectCollection( globalProperties: null, @@ -200,7 +204,7 @@ public void AddPackageVersionIntoPropsFileWhenItemGroupDoesNotExist_Success() ProjectCollection = projectCollection }; - // Set up Directory.Packages.props file + // Arrange Directory.Packages.props file var propsFile = @$" @@ -209,7 +213,7 @@ public void AddPackageVersionIntoPropsFileWhenItemGroupDoesNotExist_Success() "; File.WriteAllText(Path.Combine(testDirectory, "Directory.Packages.props"), propsFile); - // Set up project file + // Arrange project file string projectContent = @$" @@ -243,10 +247,10 @@ public void AddPackageVersionIntoPropsFileWhenItemGroupDoesNotExist_Success() ", File.ReadAllText(Path.Combine(testDirectory, "Directory.Packages.props"))); } - [Fact] + [PlatformFact(Platform.Windows)] public void AddPackageVersionIntoPropsFileWhenItemGroupExists_Success() { - // Set up + // Arrange var testDirectory = TestDirectory.Create(); var projectCollection = new ProjectCollection( globalProperties: null, @@ -265,7 +269,7 @@ public void AddPackageVersionIntoPropsFileWhenItemGroupExists_Success() ProjectCollection = projectCollection }; - // Set up Directory.Packages.props file + // Arrange Directory.Packages.props file var propsFile = @$" @@ -277,7 +281,7 @@ public void AddPackageVersionIntoPropsFileWhenItemGroupExists_Success() "; File.WriteAllText(Path.Combine(testDirectory, "Directory.Packages.props"), propsFile); - // Set up project file + // Arrange project file string projectContent = @$" @@ -312,10 +316,10 @@ public void AddPackageVersionIntoPropsFileWhenItemGroupExists_Success() Assert.Contains(@$"", File.ReadAllText(Path.Combine(testDirectory, "Directory.Packages.props"))); } - [Fact] + [PlatformFact(Platform.Windows)] public void UpdatePackageVersionInPropsFileWhenItExists_Success() { - // Set up + // Arrange var testDirectory = TestDirectory.Create(); var projectCollection = new ProjectCollection( globalProperties: null, @@ -334,7 +338,7 @@ public void UpdatePackageVersionInPropsFileWhenItExists_Success() ProjectCollection = projectCollection }; - // Set up Directory.Packages.props file + // Arrange Directory.Packages.props file var propsFile = @$" @@ -346,7 +350,7 @@ public void UpdatePackageVersionInPropsFileWhenItExists_Success() "; File.WriteAllText(Path.Combine(testDirectory, "Directory.Packages.props"), propsFile); - // Set up project file + // Arrange project file string projectContent = @$" @@ -376,9 +380,9 @@ public void UpdatePackageVersionInPropsFileWhenItExists_Success() // Assert Assert.Equal(projectContent, File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); - Assert.Contains(@$"", File.ReadAllText(Path.Combine(testDirectory, "Directory.Packages.props"))); - Assert.DoesNotContain(@$"", File.ReadAllText(Path.Combine(testDirectory, "Directory.Packages.props"))); + string updatedPropsFile = File.ReadAllText(Path.Combine(testDirectory, "Directory.Packages.props")); + Assert.Contains(@$"", updatedPropsFile); + Assert.DoesNotContain(@$"", updatedPropsFile); } - } }