diff --git a/src/NuGet.Core/NuGet.ProjectModel/ProjectLockFile/PackagesLockFileUtilities.cs b/src/NuGet.Core/NuGet.ProjectModel/ProjectLockFile/PackagesLockFileUtilities.cs index a40ba670ba0..410997e09a2 100644 --- a/src/NuGet.Core/NuGet.ProjectModel/ProjectLockFile/PackagesLockFileUtilities.cs +++ b/src/NuGet.Core/NuGet.ProjectModel/ProjectLockFile/PackagesLockFileUtilities.cs @@ -244,7 +244,7 @@ public static LockFileValidationResult IsLockFileValid(DependencyGraphSpec dgSpe if (hasChanged) { - // P2P transitive package dependencies have changed + // P2P transitive package dependencies have changed invalidReasons.Add(message); } @@ -265,12 +265,20 @@ public static LockFileValidationResult IsLockFileValid(DependencyGraphSpec dgSpe } else { - invalidReasons.Add(string.Format( - CultureInfo.CurrentCulture, - Strings.PackagesLockFile_ProjectReferenceHasNoCompatibleTargetFramework, - p2pProjectName, - useAliasForMessages ? restoreMetadataFramework.TargetAlias : restoreMetadataFramework.FrameworkName.GetShortFolderName() - )); + // When no compatible TFM is found, restore contributes no transitive dependencies + // for this P2P project. Non-empty deps in the lock file indicate a stale entry + // from when the P2P previously had a compatible TFM. + if (projectDependency.Dependencies.Count > 0) + { + invalidReasons.Add( + string.Format( + CultureInfo.CurrentCulture, + Strings.PackagesLockFile_ProjectReferenceHasNoCompatibleTargetFramework, + p2pProjectName, + useAliasForMessages ? restoreMetadataFramework.TargetAlias : restoreMetadataFramework.FrameworkName.GetShortFolderName() + ) + ); + } } } else // This can't happen. When adding the queue, the referenceSpec HAS to be discovered. If the project is otherwise missing, it will be discovered in HasP2PDependencyChanged diff --git a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreNETCoreTest.cs b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreNETCoreTest.cs index 95342d932c5..4d7ee6c43e3 100644 --- a/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreNETCoreTest.cs +++ b/test/NuGet.Clients.Tests/NuGet.CommandLine.Test/RestoreNETCoreTest.cs @@ -8531,8 +8531,8 @@ public void RestoreNetCore_PackagesLockFile_WithDependentProjectChangeOfNotCompa string[] initialFrameworks, string[] updatedFrameworks) { - // A project with RestoreLockedMode should fail restore if the frameworks of a dependent project were changed - // with incompatible frameworks between restores. + // In locked mode, restore should fail when a dependent project's frameworks change to be incompatible. + // B has no packages, so the lock file is still valid (0 deps), and failure comes from NU1201 not NU1004. using (var pathContext = new SimpleTestPathContext()) { // Set up solution, project, and packages @@ -8574,9 +8574,9 @@ public void RestoreNetCore_PackagesLockFile_WithDependentProjectChangeOfNotCompa // Assert r.Success.Should().BeFalse(); - Assert.Contains("NU1004", r.Errors); + Assert.Contains("NU1201", r.Errors); var logCodes = projectA.AssetsFile.LogMessages.Select(e => e.Code); - Assert.Contains(NuGetLogCode.NU1004, logCodes); + Assert.Contains(NuGetLogCode.NU1201, logCodes); } } diff --git a/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommand_PackagesLockFileTests.cs b/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommand_PackagesLockFileTests.cs index 394173e4c51..c04bc087259 100644 --- a/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommand_PackagesLockFileTests.cs +++ b/test/NuGet.Core.FuncTests/NuGet.Commands.FuncTest/RestoreCommand_PackagesLockFileTests.cs @@ -512,14 +512,13 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( } [Fact] - public async Task RestoreCommand_PackagesLockFile_InLockedMode_WhenANewDirectProjectReferenceChangesFramework_FailsWithNU1004() + public async Task RestoreCommand_PackagesLockFile_InLockedMode_WhenANewDirectProjectReferenceChangesFramework_FailsWithNU1201() { // Arrange using (var pathContext = new SimpleTestPathContext()) { var logger = new TestLogger(); var packageA = new SimpleTestPackageContext("a", "1.0.0"); - var packageB = new SimpleTestPackageContext("b", "1.0.0"); await SimpleTestPackageUtility.CreateFolderFeedV3Async( pathContext.PackageSource, @@ -556,29 +555,30 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( restoreLockedMode: true); logger.Clear(); + // Change the P2P to an incompatible framework. PackageSpecOperationsUtility.RemoveTargetFramework(projectReferenceSpec, "net46"); PackageSpecOperationsUtility.AddTargetFramework(projectReferenceSpec, "net47"); // Act. result = await new RestoreCommand(ProjectTestHelpers.CreateRestoreRequest(pathContext, logger, rootPackageSpec, projectReferenceSpec)).ExecuteAsync(); - // Assert. + // Assert: the lock file remains valid (no transitive dep change), so failure is NU1201, + // not NU1004. result.Success.Should().BeFalse(); logger.ErrorMessages.Count.Should().Be(1); - logger.ErrorMessages.Single().Should().Contain("NU1004"); - logger.ErrorMessages.Single().Should().Contain($"The project IntermediateProject has no compatible target framework."); + logger.ErrorMessages.Single().Should().Contain("NU1201"); + logger.ErrorMessages.Single().Should().Contain("Project IntermediateProject is not compatible"); } } [Fact] - public async Task RestoreCommand_PackagesLockFile_InLockedMode_WhenANewTransitiveProjectReferenceChangesFramework_FailsWithNU1004() + public async Task RestoreCommand_PackagesLockFile_InLockedMode_WhenANewTransitiveProjectReferenceChangesFramework_FailsWithNU1201() { // Arrange using (var pathContext = new SimpleTestPathContext()) { var logger = new TestLogger(); var packageA = new SimpleTestPackageContext("a", "1.0.0"); - var packageB = new SimpleTestPackageContext("b", "1.0.0"); await SimpleTestPackageUtility.CreateFolderFeedV3Async( pathContext.PackageSource, @@ -622,17 +622,19 @@ await SimpleTestPackageUtility.CreateFolderFeedV3Async( restoreLockedMode: true); logger.Clear(); + // Change the leaf P2P to an incompatible framework. PackageSpecOperationsUtility.RemoveTargetFramework(leafProjectReferenceSpec, "net46"); PackageSpecOperationsUtility.AddTargetFramework(leafProjectReferenceSpec, "net47"); // Act. result = await new RestoreCommand(ProjectTestHelpers.CreateRestoreRequest(pathContext, logger, rootPackageSpec, intermediateProjectReferenceSpec, leafProjectReferenceSpec)).ExecuteAsync(); - // Assert. + // Assert: the lock file remains valid (no transitive dep change), so failure is NU1201, + // not NU1004. result.Success.Should().BeFalse(); logger.ErrorMessages.Count.Should().Be(1); - logger.ErrorMessages.Single().Should().Contain("NU1004"); - logger.ErrorMessages.Single().Should().Contain($"The project LeafProject has no compatible target framework."); + logger.ErrorMessages.Single().Should().Contain("NU1201"); + logger.ErrorMessages.Single().Should().Contain("Project LeafProject is not compatible"); } } @@ -1678,5 +1680,60 @@ public async Task RestoreCommand_PackagesLockFile_InLockedMode_WithAliasedFramew logger.ErrorMessages.Single().Should().Contain("NU1004"); logger.ErrorMessages.Single().Should().Contain("The project reference project2 has changed"); } + + [Fact] + public async Task RestoreCommand_PackagesLockFile_InLockedMode_WhenP2PProjectHasNoCompatibleTargetFramework_Succeeds() + { + // Arrange + // Root uses an AssetTargetFallback (ATF) framework. IsLockFileValid does not expand ATF + // when checking P2P compatibility, so the lock file sees the P2P as contributing 0 deps — + // which is valid with this fix. The actual restore does expand ATF and succeeds. + using (var pathContext = new SimpleTestPathContext()) + { + var logger = new TestLogger(); + + var rootProjectName = "RootProject"; + var rootProjectDirectory = Path.Combine(pathContext.SolutionRoot, rootProjectName); + + var atfFramework = new AssetTargetFallbackFramework( + CommonFrameworks.NetStandard20, + new List { CommonFrameworks.Net46 }); + var rootTFI = new TargetFrameworkInformation { FrameworkName = atfFramework, TargetAlias = "netstandard2.0" }; + + var rootPackageSpec = PackageReferenceSpecBuilder.Create(rootProjectName, rootProjectDirectory) + .WithTargetFrameworks(new[] { rootTFI }) + .WithPackagesLockFile() + .Build(); + + var p2pProjectName = "P2PProject"; + var p2pProjectDirectory = Path.Combine(pathContext.SolutionRoot, p2pProjectName); + + // P2P targets net46, which is only reachable from the root via ATF fallback. + var p2pPackageSpec = PackageReferenceSpecBuilder.Create(p2pProjectName, p2pProjectDirectory) + .WithTargetFrameworks(["net46"]) + .Build(); + + PackageSpecOperationsUtility.AddProjectReference(rootPackageSpec, p2pPackageSpec, atfFramework); + + // Preconditions: initial restore uses ATF expansion to resolve the P2P. + var result = await new RestoreCommand(ProjectTestHelpers.CreateRestoreRequest(pathContext, logger, rootPackageSpec, p2pPackageSpec)).ExecuteAsync(); + await result.CommitAsync(logger, CancellationToken.None); + result.Success.Should().BeTrue(); + + // Enable locked mode and clear the logger. + rootPackageSpec.RestoreMetadata.RestoreLockProperties = new RestoreLockProperties( + restorePackagesWithLockFile: "true", + rootPackageSpec.RestoreMetadata.RestoreLockProperties.NuGetLockFilePath, + restoreLockedMode: true); + logger.Clear(); + + // Act + result = await new RestoreCommand(ProjectTestHelpers.CreateRestoreRequest(pathContext, logger, rootPackageSpec, p2pPackageSpec)).ExecuteAsync(); + + // Assert: the lock file is valid (P2P contributes 0 deps, which is correct), so locked restore succeeds. + result.Success.Should().BeTrue(); + logger.ErrorMessages.Should().BeEmpty(); + } + } } } diff --git a/test/NuGet.Core.Tests/NuGet.ProjectModel.Test/ProjectLockFile/LockFileUtilitiesTests.cs b/test/NuGet.Core.Tests/NuGet.ProjectModel.Test/ProjectLockFile/LockFileUtilitiesTests.cs index 8c64b23a2cf..45a91a09d2b 100644 --- a/test/NuGet.Core.Tests/NuGet.ProjectModel.Test/ProjectLockFile/LockFileUtilitiesTests.cs +++ b/test/NuGet.Core.Tests/NuGet.ProjectModel.Test/ProjectLockFile/LockFileUtilitiesTests.cs @@ -508,7 +508,7 @@ public void IsLockFileStillValid_TransitiveVersionsMovedToCentralFile_Invalidate .WithType(PackageDependencyType.Transitive))) .Build(); - // The central package version cpvm2 has was changed from transitive to central + // The central package version cpvm2 has was changed from transitive to central var actual = PackagesLockFileUtilities.IsLockFileValid(dgSpec, lockFile); Assert.False(actual.IsValid); Assert.Contains("Transitive dependency cpvm2 moved to be centraly managed invalidated the lock file.", actual.InvalidReasons); @@ -932,7 +932,7 @@ public void IsLockFileStillValid_WithProjectToProjectWithPrivateAssets_IgnoresSu var projectB = ProjectTestHelpers.GetPackageSpec("B", framework: frameworkShortName).WithTestRestoreMetadata(); var projectC = ProjectTestHelpers.GetPackageSpec("C", framework: frameworkShortName).WithTestRestoreMetadata(); - // B (PrivateAssets.All) -> C + // B (PrivateAssets.All) -> C projectB = projectB.WithTestProjectReference(projectC, privateAssets: LibraryIncludeFlags.All); // A -> B @@ -982,7 +982,7 @@ public void IsLockFileStillValid_WithProjectToProject_MultipleEdgesWithDifferent aliases: null, versionOverride: null); - // B (PrivateAssets.All) -> C + // B (PrivateAssets.All) -> C projectB = projectB.WithTestProjectReference(projectC, privateAssets: LibraryIncludeFlags.All); // A -> B @@ -1056,7 +1056,7 @@ public void IsLockFileStillValid_WithProjectToProjectPackagesConfig_IncludesAllD aliases: null, versionOverride: null); - // B -> C + // B -> C projectB = projectB.WithTestProjectReference(projectC); // A -> B @@ -1162,5 +1162,74 @@ public void IsLockFileValid_WithNullVersionRange_DoesNotThrow() var invalidReason = actual.InvalidReasons.Single(); Assert.Contains("PackageA", invalidReason); } + + /// + /// A(PR) -> B(PR), where A targets netstandard2.0 and B targets net46 (incompatible frameworks). + /// The lock file records B with no deps, consistent with what restore produces for an incompatible TFM. + /// Regression for https://github.com/NuGet/Home/issues/12010 + /// + [Fact] + public void IsLockFileValid_WithPackageReferenceP2PAndIncompatibleFramework_WithNoDepsInLockFile_IsValid() + { + // Arrange + var parentFramework = CommonFrameworks.NetStandard20; + var projectA = ProjectTestHelpers.GetPackageSpec("A", framework: parentFramework.GetShortFolderName()).WithTestRestoreMetadata(); + var projectB = ProjectTestHelpers.GetPackageSpec("B", framework: "net46").WithTestRestoreMetadata(); + + // A -> B + projectA = projectA.WithTestProjectReference(projectB); + var dgSpec = ProjectTestHelpers.GetDGSpecForFirstProject(projectA, projectB); + + var lockFile = new PackagesLockFileBuilder() + .WithTarget(target => target + .WithFramework(parentFramework) + .WithDependency(dep => dep + .WithId("B") + .WithType(PackageDependencyType.Project) + .WithRequestedVersion(VersionRange.Parse("1.0.0")))) + .Build(); + + // Act + var actual = PackagesLockFileUtilities.IsLockFileValid(dgSpec, lockFile); + + // Assert + actual.IsValid.Should().BeTrue(); + actual.InvalidReasons.Should().BeEmpty(); + } + + /// + /// A(PR) -> B(PR), where A targets netstandard2.0 and B targets net46 (incompatible frameworks). + /// A lock file with non-empty deps for an incompatible-TFM P2P is stale and must be detected as invalid. + /// + [Fact] + public void IsLockFileValid_WithPackageReferenceP2PAndIncompatibleFramework_WithNonEmptyDepsInLockFile_IsInvalid() + { + // Arrange + var parentFramework = CommonFrameworks.NetStandard20; + var projectA = ProjectTestHelpers.GetPackageSpec("A", framework: parentFramework.GetShortFolderName()).WithTestRestoreMetadata(); + var projectB = ProjectTestHelpers.GetPackageSpec("B", framework: "net46").WithTestRestoreMetadata(); + + // A -> B + projectA = projectA.WithTestProjectReference(projectB); + var dgSpec = ProjectTestHelpers.GetDGSpecForFirstProject(projectA, projectB); + + // B has a stale transitive dependency from when it previously had a compatible TFM. + var lockFile = new PackagesLockFileBuilder() + .WithTarget(target => target + .WithFramework(parentFramework) + .WithDependency(dep => dep + .WithId("B") + .WithType(PackageDependencyType.Project) + .WithRequestedVersion(VersionRange.Parse("1.0.0")) + .WithDependency(new PackageDependency("SomePackage", VersionRange.Parse("1.0.0"))))) + .Build(); + + // Act + var actual = PackagesLockFileUtilities.IsLockFileValid(dgSpec, lockFile); + + // Assert + actual.IsValid.Should().BeFalse(); + actual.InvalidReasons.Should().ContainSingle().Which.Should().Contain("The project B has no compatible target framework"); + } } }