From 35507c1a958ac277905211214dcc25ab79ba93c7 Mon Sep 17 00:00:00 2001 From: pragnya17 <59893188+pragnya17@users.noreply.github.com> Date: Tue, 12 Jul 2022 15:09:50 -0700 Subject: [PATCH 01/12] dotnet add package cli support for cpm projects (#4700) --- src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx | 2 +- .../Dotnet.Integration.Test/DotnetAddPackageTests.cs | 1 - .../NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx index 57ce2f0b612..3484b5078b7 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx @@ -783,4 +783,4 @@ Non-HTTPS access will be removed in a future version. Consider migrating to 'HTT '--output-version' option not applicable for console output, it can only be used together with `--format json` option. Don't localize '--output-version' and `--format json` - \ 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 a271d566614..97dee61c28c 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs @@ -684,7 +684,6 @@ private void CopyResourceToDirectory(string resourceName, DirectoryInfo director stream.CopyToFile(destinationFilePath); } } - public async Task AddPkg_WithCPM_WhenPackageVersionDoesNotExistAndVersionCLIArgNotPassed_Success() { using var pathContext = new SimpleTestPathContext(); 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 eed31acbc85..85a16589f0f 100644 --- a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs +++ b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs @@ -21,7 +21,6 @@ static MSBuildAPIUtilityTests() { MSBuildLocator.RegisterDefaults(); } - [PlatformFact(Platform.Windows)] public void GetDirectoryBuildPropsRootElementWhenItExists_Success() { From 8f83ddc27a91328cd6fe9c1b3c6572ce46e1f0f3 Mon Sep 17 00:00:00 2001 From: pragnya17 <59893188+pragnya17@users.noreply.github.com> Date: Tue, 12 Jul 2022 15:09:50 -0700 Subject: [PATCH 02/12] dotnet add package cli support for cpm projects (#4700) --- .../Dotnet.Integration.Test/DotnetAddPackageTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs index 97dee61c28c..81ec2c7e5d2 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs @@ -684,6 +684,8 @@ private void CopyResourceToDirectory(string resourceName, DirectoryInfo director stream.CopyToFile(destinationFilePath); } } + + [Fact] public async Task AddPkg_WithCPM_WhenPackageVersionDoesNotExistAndVersionCLIArgNotPassed_Success() { using var pathContext = new SimpleTestPathContext(); From 3ab944501d5c9d3c21fefff4e0bad3752acd8ca8 Mon Sep 17 00:00:00 2001 From: Martin Ruiz Date: Tue, 13 Sep 2022 09:12:36 -0700 Subject: [PATCH 03/12] add support for dotnet add package command for CPM projects --- .../AddPackageReferenceCommandRunner.cs | 10 + .../Strings.Designer.cs | 8 + .../NuGet.CommandLine.XPlat/Strings.resx | 3 + .../Utility/MSBuildAPIUtility.cs | 82 ++- .../NuGet.LibraryModel/LibraryDependency.cs | 2 +- .../DotnetAddPackageTests.cs | 480 ++++++++++++++++++ .../MSBuildAPIUtilityTests.cs | 69 +++ 7 files changed, 647 insertions(+), 7 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 5e1244d29bc..c65ad04ac41 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs @@ -102,6 +102,16 @@ public async Task ExecuteCommand(PackageReferenceArgs packageReferenceArgs, var originalPackageSpec = matchingPackageSpecs.FirstOrDefault(); + // Check if the project files are correct for CPM + if (originalPackageSpec.RestoreMetadata.CentralPackageVersionsEnabled) + { + var isValid = msBuild.AreCentralVersionRequirementsSatisfied(packageReferenceArgs); + if (!isValid) + { + return 1; + } + } + // 2. Determine the version // Setup the Credential Service before making any potential http calls. diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs index 4fd5508e54f..d26ac56b76e 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs @@ -375,6 +375,14 @@ internal static string Error_InvalidCultureInfo { } } + /// Looks up a localized string similar to Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: {0}. + /// + internal static string Error_CPM_AddPkg_VersionsNotAllowed { + get { + return ResourceManager.GetString("Error_CPM_AddPkg_VersionsNotAllowed", resourceCulture); + } + } + /// /// Looks up a localized string similar to MsBuild was unable to open Project '{0}'.. /// diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx index 3484b5078b7..81fc09914ba 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx @@ -782,5 +782,8 @@ Non-HTTPS access will be removed in a future version. Consider migrating to 'HTT '--output-version' option not applicable for console output, it can only be used together with `--format json` option. Don't localize '--output-version' and `--format json` + + Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: {0} + 0 - package id diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index 0494f1ea780..8ca01b633ba 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -120,6 +120,39 @@ public int RemovePackageReference(string projectPath, LibraryDependency libraryD } } + /// + /// Check if the project files format are correct for CPM + /// + /// Arguments used in the command + /// + public bool AreCentralVersionRequirementsSatisfied(PackageReferenceArgs packageReferenceArgs) + { + var project = GetProject(packageReferenceArgs.ProjectPath); + + // Get package version and VersionOverride if it already exists in the props file. Returns null if there is no matching package version. + ProjectItem packageReferenceInProps = project.Items.LastOrDefault(i => i.ItemType == PACKAGE_REFERENCE_TYPE_TAG && i.EvaluatedInclude.Equals(packageReferenceArgs.PackageId)); + var versionOverrideExists = packageReferenceInProps?.Metadata?.FirstOrDefault(i => i.Name.Equals("VersionOverride")); + + // 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(packageReferenceArgs.PackageId)); + + // Version should not be defined in PackageReference item + if (packageReferenceInProps != null && versionOverrideExists == null && packageVersionInProps == null) + { + packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_CPM_AddPkg_VersionsNotAllowed, packageReferenceArgs.PackageId)); + return false; + } + + // If package reference exists and the user defined a VersionOverride or PackageVersions but didn't specified a version, no-op + if (packageReferenceInProps != null && (versionOverrideExists != null || packageVersionInProps != null) && packageReferenceArgs.NoVersion) + { + return false; + } + + ProjectCollection.GlobalProjectCollection.UnloadProject(project); + return true; + } + /// /// Add an unconditional package reference to the project. /// @@ -194,6 +227,10 @@ private void AddPackageReference(Project project, } else { + // Get package version and VersionOverride if it already exists in the props file. Returns null if there is no matching package version. + ProjectItem packageReferenceInProps = project.Items.LastOrDefault(i => i.ItemType == PACKAGE_REFERENCE_TYPE_TAG && i.EvaluatedInclude.Equals(libraryDependency.Name)); + var versionOverrideExists = packageReferenceInProps?.Metadata.FirstOrDefault(i => i.Name.Equals("VersionOverride")); + if (!existingPackageReferences.Any()) { //Add to the project file. @@ -206,16 +243,30 @@ private void AddPackageReference(Project project, // If no exists in the Directory.Packages.props file. if (packageVersionInProps == null) { - // Modifying the props file if project is onboarded to CPM. - AddPackageVersionIntoItemGroupCPM(project, libraryDependency); + // Update if VersionOverride instead of Directory.Packages.props file + string packageVersion = libraryDependency.LibraryRange.VersionRange.OriginalString; + UpdateVersionOverride(project, packageReferenceInProps, packageVersion); } else { - // Modify the Directory.Packages.props file with the version that is passed in. - if (!noVersion) + // 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)); + + // If no exists in the Directory.Packages.props file. + if (packageVersionInProps == null) { - string packageVersion = libraryDependency.LibraryRange.VersionRange.OriginalString; - UpdatePackageVersion(project, packageVersionInProps, packageVersion); + // 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 (!noVersion) + { + string packageVersion = libraryDependency.LibraryRange.VersionRange.OriginalString; + UpdatePackageVersion(project, packageVersionInProps, packageVersion); + } + } } } @@ -422,6 +473,25 @@ private void UpdatePackageReferenceItems(IEnumerable packageReferen } } + /// + /// Updates VersionOverride from element if version is passed in as a CLI argument + /// + /// + /// + /// + internal void UpdateVersionOverride(Project project, ProjectItem packageReference, string versionCLIArgument) + { + // Determine where the item is decalred + ProjectItemElement packageReferenceItemElement = project.GetItemProvenance(packageReference).LastOrDefault()?.ItemElement; + + // Get the Version attribute on the packageVersionItemElement. + ProjectMetadataElement versionOverrideAttribute = packageReferenceItemElement.Metadata.FirstOrDefault(i => i.Name.Equals("VersionOverride")); + + // Update the version + versionOverrideAttribute.Value = versionCLIArgument; + packageReferenceItemElement.ContainingProject.Save(); + } + /// /// Update the element if a version is passed in as a CLI argument. /// diff --git a/src/NuGet.Core/NuGet.LibraryModel/LibraryDependency.cs b/src/NuGet.Core/NuGet.LibraryModel/LibraryDependency.cs index 3a1684e76a8..84a1848c8be 100644 --- a/src/NuGet.Core/NuGet.LibraryModel/LibraryDependency.cs +++ b/src/NuGet.Core/NuGet.LibraryModel/LibraryDependency.cs @@ -124,7 +124,7 @@ public bool Equals(LibraryDependency other) GeneratePathProperty == other.GeneratePathProperty && VersionCentrallyManaged == other.VersionCentrallyManaged && Aliases == other.Aliases && - VersionOverride == other.VersionOverride && + EqualityUtility.EqualsWithNullCheck(VersionOverride, other.VersionOverride) && ReferenceType == other.ReferenceType; } diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs index 81ec2c7e5d2..ca50b77c348 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs @@ -942,5 +942,485 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( ", propsFileFromDisk); } + + [Fact] + public async Task AddPkg_WithCPM_WithPackageReference_NoVersionOverride_NoPackageVersion_NoVersionCLI_Errors() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net6.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, + 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); + + // Arrange project file + string projectContent = +@$" + + net6.0 + + + + +"; + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "projectA", "projectA.csproj"), projectContent); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} ", ignoreExitCode: true); + + // Assert + Assert.False(result.Success); + Assert.Contains("error: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: X", result.Output); + } + + [Fact] + public async Task AddPkg_WithCPM_WithPackageReference_NoVersionOverride_NoPackageVersion_WithVersionCLI_Errors() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net6.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, + 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); + + // Arrange project file + string projectContent = +@$" + + net6.0 + + + + +"; + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "projectA", "projectA.csproj"), projectContent); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} -v {version2}", ignoreExitCode: true); + + // Assert + Assert.False(result.Success); + Assert.Contains("error: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: X", result.Output); + } + + [Fact] + public async Task AddPkg_WithCPM_WithPackageReference_NoVersionOverride_WithPackageVersion_NoVersionCLI_NoOps() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net6.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, + 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); + + // Arrange project file + string projectContent = +@$" + + net6.0 + + + + +"; + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "projectA", "projectA.csproj"), projectContent); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} ", ignoreExitCode: true); + + // Assert + Assert.False(result.Success); + Assert.DoesNotContain("error: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: X", result.Output); + } + + [Fact] + public async Task AddPkg_WithCPM_WithPackageReference_NoVersionOverride_WithPackageVersion_WithVersionCLI_Success() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net6.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, + 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); + + // Arrange project file + string projectContent = +@$" + + net6.0 + + + + +"; + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "projectA", "projectA.csproj"), projectContent); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} -v {version2}", ignoreExitCode: true); + + // Assert + Assert.True(result.Success, result.Output); + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + } + + [Fact] + public async Task AddPkg_WithCPM_WithPackageReference_WithVersionOverride_NoPackageVersion_NoVersionCLI_NoOps() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net6.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, + 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); + + // Arrange project file + string projectContent = +@$" + + net6.0 + + + + +"; + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "projectA", "projectA.csproj"), projectContent); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX}", ignoreExitCode: true); + + // Assert + Assert.False(result.Success); + Assert.DoesNotContain("error: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: X", result.Output); + } + + [Fact] + public async Task AddPkg_WithCPM_WithPackageReference_WithVersionOverride_NoPackageVersion_WithVersionCLI_Success() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net6.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, + 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); + + // Arrange project file + string projectContent = +@$" + + net6.0 + + + + +"; + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "projectA", "projectA.csproj"), projectContent); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} -v {version2}", ignoreExitCode: true); + + // Assert + Assert.True(result.Success, result.Output); + Assert.DoesNotContain(@$" + + ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + } + + [Fact] + public async Task AddPkg_WithCPM_WithPackageReference_WithVersionOverride_WithPackageVersion_NoVersionCLI_NoOps() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net6.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, + 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); + + // Arrange project file + string projectContent = +@$" + + net6.0 + + + + +"; + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "projectA", "projectA.csproj"), projectContent); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX}", ignoreExitCode: true); + + // Assert + Assert.False(result.Success); + Assert.DoesNotContain("error: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: X", result.Output); + } + + [Fact] + public async Task AddPkg_WithCPM_WithPackageReference_WithVersionOverride_WithPackageVersion_WithVersionCLI_Success() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net6.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, + 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); + + // Arrange project file + string projectContent = +@$" + + net6.0 + + + + +"; + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "projectA", "projectA.csproj"), projectContent); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} -v {version2}", ignoreExitCode: true); + + // Assert + Assert.True(result.Success, result.Output); + Assert.DoesNotContain(@$" + + ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + } } } 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 85a16589f0f..ef266ba2473 100644 --- a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs +++ b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs @@ -383,5 +383,74 @@ public void UpdatePackageVersionInPropsFileWhenItExists_Success() Assert.Contains(@$"", updatedPropsFile); Assert.DoesNotContain(@$"", updatedPropsFile); } + + [PlatformFact(Platform.Windows)] + public void UpdateVersionOverrideInPropsFileWhenItExists_Success() + { + // Arrange + 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 + }; + + // Arrange Directory.Packages.props file + var propsFile = +@$" + + true + + + + +"; + File.WriteAllText(Path.Combine(testDirectory, "Directory.Packages.props"), propsFile); + + // Arrange 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("3.0.0"), + typeConstraint: LibraryDependencyTarget.Package) + }; + + // Act + msObject.UpdateVersionOverride(project, packageVersionInProps, "3.0.0"); + + // Assert + Assert.Equal(projectContent, File.ReadAllText(Path.Combine(testDirectory, "projectA.csproj"))); + string updatedPropsFile = File.ReadAllText(Path.Combine(testDirectory, "Directory.Packages.props")); + Assert.Contains(@$"", updatedPropsFile); + Assert.DoesNotContain(@$"", updatedPropsFile); + } } } From ceb4871543fc3ed73c75f0bda1cbebdcc5d5902a Mon Sep 17 00:00:00 2001 From: Martin Ruiz Date: Thu, 15 Sep 2022 11:33:28 -0700 Subject: [PATCH 04/12] handle more cases and added more tests --- .../Strings.Designer.cs | 27 +++ .../NuGet.CommandLine.XPlat/Strings.resx | 12 + .../Utility/MSBuildAPIUtility.cs | 47 +++- .../DotnetAddPackageTests.cs | 222 ++++++++++++++++++ 4 files changed, 301 insertions(+), 7 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs index d26ac56b76e..20c0a4dbf53 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs @@ -330,6 +330,33 @@ internal static string Err_InvalidValue { } } + /// + /// Looks up a localized string similar to VersionOverride for package '{0}' should not be empty.. + /// + internal static string Error_AddPkg_CentralPackageVersions_EmptyVersionOverride { + get { + return ResourceManager.GetString("Error_AddPkg_CentralPackageVersions_EmptyVersionOverride", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Package reference for package '{0}' defined in incorrect location, PackageReference should be defined in project file.. + /// + internal static string Error_AddPkg_CentralPackageVersions_PackageReference_WrongLocation { + get { + return ResourceManager.GetString("Error_AddPkg_CentralPackageVersions_PackageReference_WrongLocation", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to PackageVersion for package '{0}' defined in incorrect location, PackageVersion should be defined in Directory.Package.props.. + /// + internal static string Error_AddPkg_CentralPackageVersions_PackageVersion_WrongLocation { + get { + return ResourceManager.GetString("Error_AddPkg_CentralPackageVersions_PackageVersion_WrongLocation", resourceCulture); + } + } + /// /// Looks up a localized string similar to Item '{0}' for '{1}' in Imported file '{2}'.. /// diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx index 81fc09914ba..9ab51482c3b 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx @@ -786,4 +786,16 @@ Non-HTTPS access will be removed in a future version. Consider migrating to 'HTT Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: {0} 0 - package id + + VersionOverride for package '{0}' should not be empty. + 0 - package id + + + Package reference for package '{0}' defined in incorrect location, PackageReference should be defined in project file. + 0 - package id + + + PackageVersion for package '{0}' defined in incorrect location, PackageVersion should be defined in Directory.Package.props. + 0 - package id + diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index 8ca01b633ba..d57db383397 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -128,23 +128,56 @@ public int RemovePackageReference(string projectPath, LibraryDependency libraryD public bool AreCentralVersionRequirementsSatisfied(PackageReferenceArgs packageReferenceArgs) { var project = GetProject(packageReferenceArgs.ProjectPath); + string directoryPackagesPropsPath = project.GetPropertyValue(DirectoryPackagesPropsPathPropertyName); + + IEnumerable packageReferences = project.Items.Where(i => i.ItemType == PACKAGE_REFERENCE_TYPE_TAG && i.EvaluatedInclude.Equals(packageReferenceArgs.PackageId)); + ProjectItem packageReference = packageReferences.LastOrDefault(); + + // Check if the package reference is defined in the project file + if (packageReferences != null) + { + foreach (ProjectItem reference in packageReferences) + { + if (!reference.Xml.ContainingProject.FullPath.Equals(project.FullPath)) + { + packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_AddPkg_CentralPackageVersions_PackageReference_WrongLocation, packageReferenceArgs.PackageId)); + return false; + } + } + } - // Get package version and VersionOverride if it already exists in the props file. Returns null if there is no matching package version. - ProjectItem packageReferenceInProps = project.Items.LastOrDefault(i => i.ItemType == PACKAGE_REFERENCE_TYPE_TAG && i.EvaluatedInclude.Equals(packageReferenceArgs.PackageId)); - var versionOverrideExists = packageReferenceInProps?.Metadata?.FirstOrDefault(i => i.Name.Equals("VersionOverride")); + // Get VersionOverride if it exisits in the package reference. + var versionOverride = packageReferences?.LastOrDefault()?.Metadata?.FirstOrDefault(i => i.Name.Equals("VersionOverride") && !string.IsNullOrWhiteSpace(i.EvaluatedValue)); // 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(packageReferenceArgs.PackageId)); + IEnumerable packageVersions = project.Items.Where(i => i.ItemType == PACKAGE_VERSION_TYPE_TAG && i.EvaluatedInclude.Equals(packageReferenceArgs.PackageId)); + ProjectItem packageVersionInProps = packageVersions.LastOrDefault(); + + // Check if the package version is defined in the Directory.Packages.props + if (packageVersions != null) + { + foreach (ProjectItem packageVersion in packageVersions) + { + if (!packageVersion.Xml.ContainingProject.FullPath.Equals(directoryPackagesPropsPath)) + { + packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_AddPkg_CentralPackageVersions_PackageVersion_WrongLocation, packageReferenceArgs.PackageId)); + return false; + } + } + } + + // Get Version if PackageVersion exists + var packageVersionExists = packageVersionInProps?.Metadata?.FirstOrDefault(i => i.Name.Equals("Version")); // Version should not be defined in PackageReference item - if (packageReferenceInProps != null && versionOverrideExists == null && packageVersionInProps == null) + if (packageReference != null && versionOverride == null && packageVersionExists == null) { packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_CPM_AddPkg_VersionsNotAllowed, packageReferenceArgs.PackageId)); return false; } // If package reference exists and the user defined a VersionOverride or PackageVersions but didn't specified a version, no-op - if (packageReferenceInProps != null && (versionOverrideExists != null || packageVersionInProps != null) && packageReferenceArgs.NoVersion) + if (packageReference != null && (versionOverride != null || packageVersionInProps != null) && packageReferenceArgs.NoVersion) { return false; } @@ -229,7 +262,7 @@ private void AddPackageReference(Project project, { // Get package version and VersionOverride if it already exists in the props file. Returns null if there is no matching package version. ProjectItem packageReferenceInProps = project.Items.LastOrDefault(i => i.ItemType == PACKAGE_REFERENCE_TYPE_TAG && i.EvaluatedInclude.Equals(libraryDependency.Name)); - var versionOverrideExists = packageReferenceInProps?.Metadata.FirstOrDefault(i => i.Name.Equals("VersionOverride")); + var versionOverrideExists = packageReferenceInProps?.Metadata.FirstOrDefault(i => i.Name.Equals("VersionOverride") && !string.IsNullOrWhiteSpace(i.EvaluatedValue)); if (!existingPackageReferences.Any()) { diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs index ca50b77c348..cc17528afca 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs @@ -998,6 +998,12 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( // Assert Assert.False(result.Success); Assert.Contains("error: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: X", result.Output); + Assert.DoesNotContain(@$" + + ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); } [Fact] @@ -1055,6 +1061,12 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( // Assert Assert.False(result.Success); Assert.Contains("error: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: X", result.Output); + Assert.DoesNotContain(@$" + + ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); } [Fact] @@ -1115,6 +1127,12 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( // Assert Assert.False(result.Success); Assert.DoesNotContain("error: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: X", result.Output); + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); } [Fact] @@ -1234,6 +1252,12 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( // Assert Assert.False(result.Success); Assert.DoesNotContain("error: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: X", result.Output); + Assert.DoesNotContain(@$" + + ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); } [Fact] @@ -1356,6 +1380,12 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( // Assert Assert.False(result.Success); Assert.DoesNotContain("error: Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: X", result.Output); + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); } [Fact] @@ -1422,5 +1452,197 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); } + + [Fact] + public async Task AddPkg_WithCPM_WithPackageReference_EmptyVersionOverride_WithPackageVersion_WithVersionCLI_IgnoreEmptyVersionOverride_Success() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net6.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, + 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); + + // Arrange project file + string projectContent = +@$" + + net6.0 + + + + +"; + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "projectA", "projectA.csproj"), projectContent); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} -v {version2}", ignoreExitCode: true); + + // Assert + Assert.True(result.Success, result.Output); + // Assert + Assert.True(result.Success, result.Output); + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + Assert.DoesNotContain(@$" + + ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + } + + [Fact] + public async Task AddPkg_WithCPM_WrongPackageReference_WithVersionOverride_WithPackageVersion_WithVersionCLI_WrongConfiguration_Error() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net6.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, + 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); + + // Arrange project file + string projectContent = + @$" + + net6.0 + + "; + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "projectA", "projectA.csproj"), projectContent); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} -v {version2}", ignoreExitCode: true); + + // Assert + Assert.False(result.Success); + Assert.Contains("error: Package reference for package 'X' defined in incorrect location, PackageReference should be defined in project file.", result.Output); + Assert.Contains(@$"", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + Assert.DoesNotContain(@$" + + ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + } + + [Fact] + public async Task AddPkg_WithCPM_WithPackageReference_WithVersionOverride_WrongPackageVersion_WithVersionCLI_WrongConfiguration_Error() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net6.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, + 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); + + // Arrange project file + string projectContent = + @$" + + net6.0 + + + + + + "; + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "projectA", "projectA.csproj"), projectContent); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} -v {version2}", ignoreExitCode: true); + + // Assert + Assert.False(result.Success); + Assert.Contains(" PackageVersion for package 'X' defined in incorrect location, PackageVersion should be defined in Directory.Package.props.", result.Output); + Assert.DoesNotContain(@$"", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + Assert.Contains(@$" + + + ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + } } } From 99aa1744dec8ebb22323bd3c1608353daaf64571 Mon Sep 17 00:00:00 2001 From: Martin Ruiz Date: Thu, 22 Sep 2022 12:51:32 -0700 Subject: [PATCH 05/12] fixed tests and improvments --- .../Utility/MSBuildAPIUtility.cs | 60 ++++++++----------- .../MSBuildAPIUtilityTests.cs | 16 ++--- 2 files changed, 33 insertions(+), 43 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index d57db383397..26e3545d14f 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -128,54 +128,44 @@ public int RemovePackageReference(string projectPath, LibraryDependency libraryD public bool AreCentralVersionRequirementsSatisfied(PackageReferenceArgs packageReferenceArgs) { var project = GetProject(packageReferenceArgs.ProjectPath); + string directoryPackagesPropsPath = project.GetPropertyValue(DirectoryPackagesPropsPathPropertyName); - IEnumerable packageReferences = project.Items.Where(i => i.ItemType == PACKAGE_REFERENCE_TYPE_TAG && i.EvaluatedInclude.Equals(packageReferenceArgs.PackageId)); - ProjectItem packageReference = packageReferences.LastOrDefault(); + var packageReferences = project.Items.Where(item => item.ItemType == PACKAGE_REFERENCE_TYPE_TAG && item.EvaluatedInclude.Equals(packageReferenceArgs.PackageId)); - // Check if the package reference is defined in the project file - if (packageReferences != null) + // PackageReference should not have versions explicitly defined if cpvm is enabled. + var packageReferencesWithDefinedVersion = packageReferences.SelectMany(item => item.Metadata.Where(metadata => metadata.Name.Equals("Version"))); + if (packageReferencesWithDefinedVersion.Any()) { - foreach (ProjectItem reference in packageReferences) - { - if (!reference.Xml.ContainingProject.FullPath.Equals(project.FullPath)) - { - packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_AddPkg_CentralPackageVersions_PackageReference_WrongLocation, packageReferenceArgs.PackageId)); - return false; - } - } + packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_CPM_AddPkg_VersionsNotAllowed, packageReferenceArgs.PackageId)); + return false; } - // Get VersionOverride if it exisits in the package reference. - var versionOverride = packageReferences?.LastOrDefault()?.Metadata?.FirstOrDefault(i => i.Name.Equals("VersionOverride") && !string.IsNullOrWhiteSpace(i.EvaluatedValue)); - - // Get package version if it already exists in the props file. Returns null if there is no matching package version. - IEnumerable packageVersions = project.Items.Where(i => i.ItemType == PACKAGE_VERSION_TYPE_TAG && i.EvaluatedInclude.Equals(packageReferenceArgs.PackageId)); - ProjectItem packageVersionInProps = packageVersions.LastOrDefault(); - - // Check if the package version is defined in the Directory.Packages.props - if (packageVersions != null) + // PackageReference should not be defined outside the project file. + var packageReferencesOutsideProject = packageReferences.Where(item => !item.Xml.ContainingProject.FullPath.Equals(project.FullPath)); + if (packageReferencesOutsideProject.Any()) { - foreach (ProjectItem packageVersion in packageVersions) - { - if (!packageVersion.Xml.ContainingProject.FullPath.Equals(directoryPackagesPropsPath)) - { - packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_AddPkg_CentralPackageVersions_PackageVersion_WrongLocation, packageReferenceArgs.PackageId)); - return false; - } - } + packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_AddPkg_CentralPackageVersions_PackageReference_WrongLocation, packageReferenceArgs.PackageId)); + return false; } - // Get Version if PackageVersion exists - var packageVersionExists = packageVersionInProps?.Metadata?.FirstOrDefault(i => i.Name.Equals("Version")); - - // Version should not be defined in PackageReference item - if (packageReference != null && versionOverride == null && packageVersionExists == null) + // PackageVersion should not be defined outside the project file. + var packageVersions = project.Items.Where(item => item.ItemType == PACKAGE_VERSION_TYPE_TAG && item.EvaluatedInclude.Equals(packageReferenceArgs.PackageId) && !item.Xml.ContainingProject.FullPath.Equals(directoryPackagesPropsPath)); + if (packageVersions.Any()) { - packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_CPM_AddPkg_VersionsNotAllowed, packageReferenceArgs.PackageId)); + packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_AddPkg_CentralPackageVersions_PackageVersion_WrongLocation, packageReferenceArgs.PackageId)); return false; } + ProjectItem packageReference = packageReferences.LastOrDefault(); + ProjectItem packageVersionInProps = packageVersions.LastOrDefault(); + + // Get VersionOverride if it exisits in the package reference. + var versionOverride = packageReference?.Metadata?.FirstOrDefault(i => i.Name.Equals("VersionOverride") && !string.IsNullOrWhiteSpace(i.EvaluatedValue)); + + // Get Version if it exists in PackageVersion + var packageVersionExists = packageVersionInProps?.Metadata?.FirstOrDefault(i => i.Name.Equals("Version")); + // If package reference exists and the user defined a VersionOverride or PackageVersions but didn't specified a version, no-op if (packageReference != null && (versionOverride != null || packageVersionInProps != null) && packageReferenceArgs.NoVersion) { 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 ef266ba2473..c145e5416f9 100644 --- a/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs +++ b/test/NuGet.Core.Tests/NuGet.CommandLine.Xplat.Tests/MSBuildAPIUtilityTests.cs @@ -420,20 +420,20 @@ public void UpdateVersionOverrideInPropsFileWhenItExists_Success() // Arrange project file string projectContent = -@$" - - net6.0 - - - - +@$" + + 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")); + ProjectItem packageVersionInProps = project.Items.LastOrDefault(i => i.ItemType == "PackageReference" && i.EvaluatedInclude.Equals("X")); var libraryDependency = new LibraryDependency { From b3b8cf6c9f40120ad2dfff5624541f2374883f5f Mon Sep 17 00:00:00 2001 From: Martin Ruiz Date: Fri, 23 Sep 2022 10:46:55 -0700 Subject: [PATCH 06/12] using dgspec to check if project config is correct --- .../AddPackageReferenceCommandRunner.cs | 8 +-- .../Strings.Designer.cs | 31 +++++++++- .../NuGet.CommandLine.XPlat/Strings.resx | 11 +++- .../Utility/MSBuildAPIUtility.cs | 61 +++++++++++++------ 4 files changed, 84 insertions(+), 27 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 c65ad04ac41..8ba60ab4194 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Commands/PackageReferenceCommands/AddPackageReferenceCommandRunner.cs @@ -103,13 +103,9 @@ public async Task ExecuteCommand(PackageReferenceArgs packageReferenceArgs, var originalPackageSpec = matchingPackageSpecs.FirstOrDefault(); // Check if the project files are correct for CPM - if (originalPackageSpec.RestoreMetadata.CentralPackageVersionsEnabled) + if (originalPackageSpec.RestoreMetadata.CentralPackageVersionsEnabled && !msBuild.AreCentralVersionRequirementsSatisfied(packageReferenceArgs, originalPackageSpec)) { - var isValid = msBuild.AreCentralVersionRequirementsSatisfied(packageReferenceArgs); - if (!isValid) - { - return 1; - } + return 1; } // 2. Determine the version diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs index 20c0a4dbf53..4aa481d578d 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs @@ -402,11 +402,38 @@ internal static string Error_InvalidCultureInfo { } } + /// Looks up a localized string similar to The packages {0} are implicitly referenced. You do not typically need to reference them from your project or in your central package versions management file. For more information, see https://aka.ms/sdkimplicitrefs. + /// + internal static string Error_CentralPackageVersions_AutoreferencedReferencesNotAllowed { + get { + return ResourceManager.GetString("Error_CentralPackageVersions_AutoreferencedReferencesNotAllowed", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to Centrally defined floating package versions are not allowed.. + /// + internal static string Error_CentralPackageVersions_FloatingVersionsAreNotAllowed { + get { + return ResourceManager.GetString("Error_CentralPackageVersions_FloatingVersionsAreNotAllowed", resourceCulture); + } + } + + /// + /// Looks up a localized string similar to The PackageReference items {0} do not have corresponding PackageVersion.. + /// + internal static string Error_CentralPackageVersions_MissingPackageVersion { + get { + return ResourceManager.GetString("Error_CentralPackageVersions_MissingPackageVersion", resourceCulture); + } + } + + /// /// Looks up a localized string similar to Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: {0}. /// - internal static string Error_CPM_AddPkg_VersionsNotAllowed { + internal static string Error_CentralPackageVersions_VersionsNotAllowed { get { - return ResourceManager.GetString("Error_CPM_AddPkg_VersionsNotAllowed", resourceCulture); + return ResourceManager.GetString("Error_CentralPackageVersions_VersionsNotAllowed", resourceCulture); } } diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx index 9ab51482c3b..6146d980999 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx @@ -782,7 +782,7 @@ Non-HTTPS access will be removed in a future version. Consider migrating to 'HTT '--output-version' option not applicable for console output, it can only be used together with `--format json` option. Don't localize '--output-version' and `--format json` - + Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: {0} 0 - package id @@ -798,4 +798,13 @@ Non-HTTPS access will be removed in a future version. Consider migrating to 'HTT PackageVersion for package '{0}' defined in incorrect location, PackageVersion should be defined in Directory.Package.props. 0 - package id + + The packages {0} are implicitly referenced. You do not typically need to reference them from your project or in your central package versions management file. For more information, see https://aka.ms/sdkimplicitrefs + + + Centrally defined floating package versions are not allowed. + + + The PackageReference items {0} do not have corresponding PackageVersion. + diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index 26e3545d14f..115a9aa39bd 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -124,28 +124,50 @@ public int RemovePackageReference(string projectPath, LibraryDependency libraryD /// Check if the project files format are correct for CPM /// /// Arguments used in the command + /// /// - public bool AreCentralVersionRequirementsSatisfied(PackageReferenceArgs packageReferenceArgs) + public bool AreCentralVersionRequirementsSatisfied(PackageReferenceArgs packageReferenceArgs, PackageSpec packageSpec) { var project = GetProject(packageReferenceArgs.ProjectPath); - string directoryPackagesPropsPath = project.GetPropertyValue(DirectoryPackagesPropsPathPropertyName); - var packageReferences = project.Items.Where(item => item.ItemType == PACKAGE_REFERENCE_TYPE_TAG && item.EvaluatedInclude.Equals(packageReferenceArgs.PackageId)); + // Get VersionOverride if it exisits in the package reference. + IEnumerable dependenciesWithVersionOverride = packageSpec.TargetFrameworks.SelectMany(tfm => tfm.Dependencies.Where(d => !d.AutoReferenced && d.VersionOverride != null)); - // PackageReference should not have versions explicitly defined if cpvm is enabled. - var packageReferencesWithDefinedVersion = packageReferences.SelectMany(item => item.Metadata.Where(metadata => metadata.Name.Equals("Version"))); - if (packageReferencesWithDefinedVersion.Any()) + if (packageSpec.RestoreMetadata.CentralPackageVersionOverrideDisabled) { - packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_CPM_AddPkg_VersionsNotAllowed, packageReferenceArgs.PackageId)); + // Emit a error if VersionOverride was specified for a package reference but that functionality is disabled + foreach (var item in dependenciesWithVersionOverride) + { + //await _logger.LogAsync(RestoreLogMessage.CreateError(NuGetLogCode.NU1013, string.Format(CultureInfo.CurrentCulture, Strings.Error_CentralPackageVersions_VersionOverrideDisabled, item.Name))); + } + return false; } - // PackageReference should not be defined outside the project file. - var packageReferencesOutsideProject = packageReferences.Where(item => !item.Xml.ContainingProject.FullPath.Equals(project.FullPath)); - if (packageReferencesOutsideProject.Any()) + // The dependencies should not have versions explicitly defined if cpvm is enabled. + IEnumerable dependenciesWithDefinedVersion = packageSpec.TargetFrameworks.SelectMany(tfm => tfm.Dependencies.Where(d => !d.VersionCentrallyManaged && !d.AutoReferenced && d.VersionOverride == null)); + if (dependenciesWithDefinedVersion.Any()) { - packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_AddPkg_CentralPackageVersions_PackageReference_WrongLocation, packageReferenceArgs.PackageId)); + packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_CentralPackageVersions_VersionsNotAllowed, string.Join(";", dependenciesWithDefinedVersion.Select(d => d.Name)))); + return false; + } + IEnumerable autoReferencedAndDefinedInCentralFile = packageSpec.TargetFrameworks.SelectMany(tfm => tfm.Dependencies.Where(d => d.AutoReferenced && tfm.CentralPackageVersions.ContainsKey(d.Name))); + if (autoReferencedAndDefinedInCentralFile.Any()) + { + packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_CentralPackageVersions_AutoreferencedReferencesNotAllowed, string.Join(";", autoReferencedAndDefinedInCentralFile.Select(d => d.Name)))); + return false; + } + IEnumerable packageReferencedDependenciesWithoutCentralVersionDefined = packageSpec.TargetFrameworks.SelectMany(tfm => tfm.Dependencies.Where(d => d.LibraryRange.VersionRange == null)); + if (packageReferencedDependenciesWithoutCentralVersionDefined.Any()) + { + packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_CentralPackageVersions_MissingPackageVersion, string.Join(";", packageReferencedDependenciesWithoutCentralVersionDefined.Select(d => d.Name)))); + return false; + } + var floatingVersionDependencies = packageSpec.TargetFrameworks.SelectMany(tfm => tfm.CentralPackageVersions.Values).Where(cpv => cpv.VersionRange.IsFloating); + if (floatingVersionDependencies.Any()) + { + packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_CentralPackageVersions_FloatingVersionsAreNotAllowed)); return false; } @@ -157,14 +179,17 @@ public bool AreCentralVersionRequirementsSatisfied(PackageReferenceArgs packageR return false; } - ProjectItem packageReference = packageReferences.LastOrDefault(); - ProjectItem packageVersionInProps = packageVersions.LastOrDefault(); - - // Get VersionOverride if it exisits in the package reference. - var versionOverride = packageReference?.Metadata?.FirstOrDefault(i => i.Name.Equals("VersionOverride") && !string.IsNullOrWhiteSpace(i.EvaluatedValue)); + // PackageReference should not be defined in Directory.Packages.props + var packageReferenceOutsideProjectFile = project.Items.Where(item => item.ItemType == PACKAGE_REFERENCE_TYPE_TAG && item.Xml.ContainingProject.FullPath.Equals(directoryPackagesPropsPath)); + if (packageReferenceOutsideProjectFile.Any()) + { + packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_AddPkg_CentralPackageVersions_PackageReference_WrongLocation, packageReferenceArgs.PackageId)); + return false; + } - // Get Version if it exists in PackageVersion - var packageVersionExists = packageVersionInProps?.Metadata?.FirstOrDefault(i => i.Name.Equals("Version")); + ProjectItem packageReference = project.Items.Where(item => item.ItemType == PACKAGE_REFERENCE_TYPE_TAG && item.EvaluatedInclude.Equals(packageReferenceArgs.PackageId)).LastOrDefault(); + ProjectItem packageVersionInProps = packageVersions.LastOrDefault(); + var versionOverride = dependenciesWithVersionOverride.FirstOrDefault(d => d.Name.Equals(packageReferenceArgs.PackageId)); // If package reference exists and the user defined a VersionOverride or PackageVersions but didn't specified a version, no-op if (packageReference != null && (versionOverride != null || packageVersionInProps != null) && packageReferenceArgs.NoVersion) From da5c61bbb23e4f877d1cb56839de1eb68cfcb5b3 Mon Sep 17 00:00:00 2001 From: Martin Ruiz Date: Mon, 26 Sep 2022 09:00:35 -0700 Subject: [PATCH 07/12] added disabled overrideversion error message --- .../NuGet.CommandLine.XPlat/Strings.Designer.cs | 9 +++++++++ src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx | 4 ++++ .../NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs | 5 ++--- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs index 4aa481d578d..adb96817b0d 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs @@ -428,6 +428,15 @@ internal static string Error_CentralPackageVersions_MissingPackageVersion { } } + /// + /// Looks up a localized string similar to The package reference {0} specifies a VersionOverride but the ability to override a centrally defined version is currently disabled.. + /// + internal static string Error_CentralPackageVersions_VersionOverrideDisabled { + get { + return ResourceManager.GetString("Error_CentralPackageVersions_VersionOverrideDisabled", resourceCulture); + } + } + /// /// Looks up a localized string similar to Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: {0}. /// diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx index 6146d980999..672137457fa 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx @@ -807,4 +807,8 @@ Non-HTTPS access will be removed in a future version. Consider migrating to 'HTT The PackageReference items {0} do not have corresponding PackageVersion. + + The package reference {0} specifies a VersionOverride but the ability to override a centrally defined version is currently disabled. + 0 - packagereference name + diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index 115a9aa39bd..d565cb472b8 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -139,10 +139,9 @@ public bool AreCentralVersionRequirementsSatisfied(PackageReferenceArgs packageR // Emit a error if VersionOverride was specified for a package reference but that functionality is disabled foreach (var item in dependenciesWithVersionOverride) { - //await _logger.LogAsync(RestoreLogMessage.CreateError(NuGetLogCode.NU1013, string.Format(CultureInfo.CurrentCulture, Strings.Error_CentralPackageVersions_VersionOverrideDisabled, item.Name))); + packageReferenceArgs.Logger.LogError(string.Format(CultureInfo.CurrentCulture, Strings.Error_CentralPackageVersions_VersionOverrideDisabled, string.Join(";", dependenciesWithVersionOverride.Select(d => d.Name)))); + return false; } - - return false; } // The dependencies should not have versions explicitly defined if cpvm is enabled. From ea3a450f78673121ab7dade98c1a7c6c635f52ae Mon Sep 17 00:00:00 2001 From: Martin Ruiz Date: Mon, 26 Sep 2022 10:24:16 -0700 Subject: [PATCH 08/12] fixing tests --- .../DotnetAddPackageTests.cs | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs index cc17528afca..db724dac282 100644 --- a/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs +++ b/test/NuGet.Core.FuncTests/Dotnet.Integration.Test/DotnetAddPackageTests.cs @@ -1644,5 +1644,67 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); } + + [Fact] + public async Task AddPkg_WithCPM_WithPackageReference_DisabledVersionOverride_WrongPackageVersion_WithVersionCLI_WrongConfiguration_Error() + { + using var pathContext = new SimpleTestPathContext(); + + // Set up solution and project + var solution = new SimpleTestSolutionContext(pathContext.SolutionRoot); + var projectA = XPlatTestUtils.CreateProject("projectA", pathContext, "net6.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, + 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); + + // Arrange project file + string projectContent = + @$" + + net6.0 + false + + + + + "; + File.WriteAllText(Path.Combine(pathContext.SolutionRoot, "projectA", "projectA.csproj"), projectContent); + + //Act + var result = _fixture.RunDotnet(projectADirectory, $"add {projectA.ProjectPath} package {packageX} -v {version2}", ignoreExitCode: true); + + // Assert + Assert.False(result.Success); + Assert.Contains("The package reference X specifies a VersionOverride but the ability to override a centrally defined version is currently disabled.", result.Output); + Assert.DoesNotContain(@$"", File.ReadAllText(Path.Combine(pathContext.SolutionRoot, "Directory.Packages.props"))); + Assert.Contains(@$" + + ", File.ReadAllText(Path.Combine(projectADirectory, "projectA.csproj"))); + } } } From 12fdaf992a73ceadb238443ad2cffc924725866c Mon Sep 17 00:00:00 2001 From: Martin Ruiz Date: Wed, 28 Sep 2022 10:43:36 -0700 Subject: [PATCH 09/12] pr comment --- .../NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index d565cb472b8..6c8294184a2 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -132,10 +132,11 @@ public bool AreCentralVersionRequirementsSatisfied(PackageReferenceArgs packageR string directoryPackagesPropsPath = project.GetPropertyValue(DirectoryPackagesPropsPathPropertyName); // Get VersionOverride if it exisits in the package reference. - IEnumerable dependenciesWithVersionOverride = packageSpec.TargetFrameworks.SelectMany(tfm => tfm.Dependencies.Where(d => !d.AutoReferenced && d.VersionOverride != null)); + IEnumerable dependenciesWithVersionOverride = null; if (packageSpec.RestoreMetadata.CentralPackageVersionOverrideDisabled) { + dependenciesWithVersionOverride = packageSpec.TargetFrameworks.SelectMany(tfm => tfm.Dependencies.Where(d => !d.AutoReferenced && d.VersionOverride != null)); // Emit a error if VersionOverride was specified for a package reference but that functionality is disabled foreach (var item in dependenciesWithVersionOverride) { @@ -188,7 +189,7 @@ public bool AreCentralVersionRequirementsSatisfied(PackageReferenceArgs packageR ProjectItem packageReference = project.Items.Where(item => item.ItemType == PACKAGE_REFERENCE_TYPE_TAG && item.EvaluatedInclude.Equals(packageReferenceArgs.PackageId)).LastOrDefault(); ProjectItem packageVersionInProps = packageVersions.LastOrDefault(); - var versionOverride = dependenciesWithVersionOverride.FirstOrDefault(d => d.Name.Equals(packageReferenceArgs.PackageId)); + var versionOverride = dependenciesWithVersionOverride?.FirstOrDefault(d => d.Name.Equals(packageReferenceArgs.PackageId)); // If package reference exists and the user defined a VersionOverride or PackageVersions but didn't specified a version, no-op if (packageReference != null && (versionOverride != null || packageVersionInProps != null) && packageReferenceArgs.NoVersion) From f17aae5ebb4040719daf839fb8f11f01f95472ed Mon Sep 17 00:00:00 2001 From: Martin Ruiz Date: Thu, 15 Dec 2022 10:05:03 -0800 Subject: [PATCH 10/12] fix xaml --- src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs index adb96817b0d..5f1fc789bcd 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.Designer.cs @@ -1,4 +1,4 @@ -//------------------------------------------------------------------------------ +//------------------------------------------------------------------------------ // // This code was generated by a tool. // Runtime Version:4.0.30319.42000 @@ -401,7 +401,8 @@ internal static string Error_InvalidCultureInfo { return ResourceManager.GetString("Error_InvalidCultureInfo", resourceCulture); } } - + + /// /// Looks up a localized string similar to The packages {0} are implicitly referenced. You do not typically need to reference them from your project or in your central package versions management file. For more information, see https://aka.ms/sdkimplicitrefs. /// internal static string Error_CentralPackageVersions_AutoreferencedReferencesNotAllowed { From 4dd0d420e4f4f121aee61b7f13c80b6285f7c83a Mon Sep 17 00:00:00 2001 From: Martin Ruiz Date: Thu, 15 Dec 2022 11:03:52 -0800 Subject: [PATCH 11/12] xaml parse fix --- src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx index 672137457fa..ab94427ff28 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Strings.resx @@ -782,6 +782,7 @@ Non-HTTPS access will be removed in a future version. Consider migrating to 'HTT '--output-version' option not applicable for console output, it can only be used together with `--format json` option. Don't localize '--output-version' and `--format json` + Projects that use central package version management should not define the version on the PackageReference items but on the PackageVersion items: {0} 0 - package id From e60912f7d188914d01e58889ab7ccdc06fe50bc1 Mon Sep 17 00:00:00 2001 From: Martin Ruiz Date: Thu, 15 Dec 2022 12:18:37 -0800 Subject: [PATCH 12/12] test --- .../NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs index 6c8294184a2..aaef4a08a76 100644 --- a/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs +++ b/src/NuGet.Core/NuGet.CommandLine.XPlat/Utility/MSBuildAPIUtility.cs @@ -285,11 +285,7 @@ private void AddPackageReference(Project project, 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, StringComparison.OrdinalIgnoreCase)); - - // If no exists in the Directory.Packages.props file. - if (packageVersionInProps == null) + if (versionOverrideExists != null) { // Update if VersionOverride instead of Directory.Packages.props file string packageVersion = libraryDependency.LibraryRange.VersionRange.OriginalString;