Skip to content

Commit 779eff1

Browse files
nkolev92nkolev92
authored
Fix cross product no-op (#7250)
1 parent 39d7da6 commit 779eff1

4 files changed

Lines changed: 229 additions & 32 deletions

File tree

‎src/NuGet.Core/NuGet.ProjectModel/DependencyGraphSpec.cs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -350,12 +350,20 @@ public void Save(Stream stream)
350350
public string GetHash()
351351
{
352352
// Use the faster FNV hash function for hashing unless the user has specified to use the legacy SHA512 hash function
353-
using (IHashFunction hashFunc = UseLegacyHashFunction == true ? new Sha512HashFunction() : new FnvHash64Function())
354-
using (var writer = new HashObjectWriter(hashFunc))
353+
return GetHash(() => UseLegacyHashFunction == true ? new Sha512HashFunction() : new FnvHash64Function());
354+
}
355+
356+
internal string GetHash(Func<IHashFunction> getHashFunction)
357+
{
358+
if (getHashFunction == null)
355359
{
356-
Write(writer, hashing: true, PackageSpecWriter.Write);
357-
return writer.GetHash();
360+
throw new ArgumentNullException(nameof(getHashFunction));
358361
}
362+
363+
using IHashFunction hashFunc = getHashFunction();
364+
using var writer = new HashObjectWriter(hashFunc);
365+
Write(writer, hashing: true, PackageSpecWriter.Write);
366+
return writer.GetHash();
359367
}
360368

361369
private void Write(RuntimeModel.IObjectWriter writer, bool hashing, Action<PackageSpec, RuntimeModel.IObjectWriter, bool, IEnvironmentVariableReader> writeAction)

‎src/NuGet.Core/NuGet.ProjectModel/PackageSpecWriter.cs‎

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
using NuGet.Frameworks;
1212
using NuGet.LibraryModel;
1313
using NuGet.RuntimeModel;
14+
using NuGet.Shared;
1415
using NuGet.Versioning;
1516

1617
namespace NuGet.ProjectModel
@@ -589,19 +590,9 @@ private static void SetCentralDependencies(IObjectWriter writer, int count, IEnu
589590

590591
writer.WriteObjectStart("centralPackageVersions");
591592

592-
if (hashing)
593+
foreach (var dependency in centralPackageVersions.OrderBy(dep => dep.Name, StringComparer.OrdinalIgnoreCase))
593594
{
594-
foreach (var dependency in centralPackageVersions)
595-
{
596-
writer.WriteNameValue(name: dependency.Name, value: dependency.VersionRange.OriginalString ?? dependency.VersionRange.ToNormalizedString());
597-
}
598-
}
599-
else
600-
{
601-
foreach (var dependency in centralPackageVersions.OrderBy(dep => dep.Name, StringComparer.OrdinalIgnoreCase))
602-
{
603-
writer.WriteNameValue(name: dependency.Name, value: dependency.VersionRange.OriginalString ?? dependency.VersionRange.ToNormalizedString());
604-
}
595+
writer.WriteNameValue(name: dependency.Name, value: dependency.VersionRange.OriginalString ?? dependency.VersionRange.ToNormalizedString());
605596
}
606597

607598
writer.WriteObjectEnd();
@@ -616,19 +607,9 @@ private static void SetPackagesToPrune(IObjectWriter writer, IReadOnlyDictionary
616607

617608
writer.WriteObjectStart("packagesToPrune");
618609

619-
if (hashing)
610+
foreach (var dependency in packagesToPrune.OrderBy(dep => dep.Key, StringComparer.OrdinalIgnoreCase))
620611
{
621-
foreach (var dependency in packagesToPrune)
622-
{
623-
writer.WriteNameValue(name: dependency.Key, value: dependency.Value.VersionRange.OriginalString ?? dependency.Value.VersionRange.ToNormalizedString());
624-
}
625-
}
626-
else
627-
{
628-
foreach (var dependency in packagesToPrune.OrderBy(dep => dep.Key, StringComparer.OrdinalIgnoreCase))
629-
{
630-
writer.WriteNameValue(name: dependency.Key, value: dependency.Value.VersionRange.OriginalString ?? dependency.Value.VersionRange.ToNormalizedString());
631-
}
612+
writer.WriteNameValue(name: dependency.Key, value: dependency.Value.VersionRange.OriginalString ?? dependency.Value.VersionRange.ToNormalizedString());
632613
}
633614

634615
writer.WriteObjectEnd();

‎test/EndToEnd/tests/NetCoreProjectTest.ps1‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ function Test-NetCoreConsoleAppRebuildDoesNotDeleteCacheFile {
6666
}
6767

6868
function Test-NetCoreVSandMSBuildNoOp {
69-
[SkipTest('https://github.com/NuGet/Home/issues/13003')]
7069
param ()
7170

7271
# Arrange
@@ -92,7 +91,6 @@ function Test-NetCoreVSandMSBuildNoOp {
9291
}
9392

9493
function Test-NetCoreTargetFrameworksVSandMSBuildNoOp {
95-
[SkipTest('https://github.com/NuGet/Home/issues/13003')]
9694
param ()
9795

9896
# Arrange
@@ -118,7 +116,6 @@ function Test-NetCoreTargetFrameworksVSandMSBuildNoOp {
118116
}
119117

120118
function Test-NetCoreMultipleTargetFrameworksVSandMSBuildNoOp {
121-
[SkipTest('https://github.com/NuGet/Home/issues/11231')]
122119
param ()
123120

124121
# Arrange
@@ -144,7 +141,6 @@ function Test-NetCoreMultipleTargetFrameworksVSandMSBuildNoOp {
144141
}
145142

146143
function Test-NetCoreToolsVSandMSBuildNoOp {
147-
[SkipTest('https://github.com/NuGet/Home/issues/11231')]
148144
param ()
149145

150146
# Arrange

‎test/NuGet.Core.Tests/NuGet.ProjectModel.Test/DependencyGraphSpecTests.cs‎

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,11 @@
99
using System.IO;
1010
using System.Linq;
1111
using Microsoft.Internal.NuGet.Testing.SignedPackages;
12+
using NuGet.Commands.Test;
1213
using NuGet.Configuration;
1314
using NuGet.Frameworks;
1415
using NuGet.LibraryModel;
16+
using NuGet.Packaging;
1517
using NuGet.Test.Utility;
1618
using NuGet.Versioning;
1719
using Xunit;
@@ -447,6 +449,164 @@ public void Save_WithCentralVersionDependencies_SerializesMembersAsJson()
447449
Assert.Equal(expectedJson, actualJson);
448450
}
449451

452+
[Theory]
453+
[InlineData(false)]
454+
[InlineData(true)]
455+
public void GetHash_WithCentralPackageVersionsAndPackagesToPrune_IgnoresDictionaryCreationOrder(bool useLegacyHashFunction)
456+
{
457+
// Arrange
458+
DependencyGraphSpec first = CreateDependencyGraphSpecWithCentralDependenciesAndPruning(
459+
centralPackageVersionNames: ["PackageB", "PackageA"],
460+
packagesToPruneNames: ["TransitiveB", "TransitiveA"]);
461+
462+
DependencyGraphSpec second = CreateDependencyGraphSpecWithCentralDependenciesAndPruning(
463+
centralPackageVersionNames: ["PackageA", "PackageB"],
464+
packagesToPruneNames: ["TransitiveA", "TransitiveB"]);
465+
466+
// Act
467+
string firstHash = GetHash(first, useLegacyHashFunction);
468+
string secondHash = GetHash(second, useLegacyHashFunction);
469+
470+
// Assert
471+
Assert.Equal(firstHash, secondHash);
472+
}
473+
474+
[Theory]
475+
[InlineData(false)]
476+
[InlineData(true)]
477+
public void GetHash_WithPackagesToPruneInDifferentJsonOrder_IgnoresInputOrder(bool useLegacyHashFunction)
478+
{
479+
// Arrange
480+
DependencyGraphSpec first = CreateDependencyGraphSpecFromJson(@"
481+
{
482+
""restore"": {
483+
""projectUniqueName"": ""Project1"",
484+
""centralPackageVersionsManagementEnabled"": true
485+
},
486+
""frameworks"": {
487+
""net8.0"": {
488+
""dependencies"": {
489+
""PackageA"": {
490+
""target"": ""Package"",
491+
""version"": ""[1.0.0,)"",
492+
""versionCentrallyManaged"": true
493+
}
494+
},
495+
""centralPackageVersions"": {
496+
""PackageA"": ""[1.0.0]"",
497+
""PackageB"": ""[2.0.0]""
498+
},
499+
""packagesToPrune"": {
500+
""TransitiveB"": ""(,2.0.0]"",
501+
""TransitiveA"": ""(,1.0.0]""
502+
}
503+
}
504+
}
505+
}");
506+
507+
DependencyGraphSpec second = CreateDependencyGraphSpecFromJson(@"
508+
{
509+
""restore"": {
510+
""projectUniqueName"": ""Project1"",
511+
""centralPackageVersionsManagementEnabled"": true
512+
},
513+
""frameworks"": {
514+
""net8.0"": {
515+
""dependencies"": {
516+
""PackageA"": {
517+
""target"": ""Package"",
518+
""version"": ""[1.0.0,)"",
519+
""versionCentrallyManaged"": true
520+
}
521+
},
522+
""centralPackageVersions"": {
523+
""PackageA"": ""[1.0.0]"",
524+
""PackageB"": ""[2.0.0]""
525+
},
526+
""packagesToPrune"": {
527+
""TransitiveA"": ""(,1.0.0]"",
528+
""TransitiveB"": ""(,2.0.0]""
529+
}
530+
}
531+
}
532+
}");
533+
534+
// Act
535+
string firstHash = GetHash(first, useLegacyHashFunction);
536+
string secondHash = GetHash(second, useLegacyHashFunction);
537+
538+
// Assert
539+
Assert.Equal(firstHash, secondHash);
540+
}
541+
542+
[Theory]
543+
[InlineData(false)]
544+
[InlineData(true)]
545+
public void GetHash_WithCentralPackageVersionsInDifferentJsonOrder_IgnoresInputOrder(bool useLegacyHashFunction)
546+
{
547+
// Arrange
548+
DependencyGraphSpec first = CreateDependencyGraphSpecFromJson(@"
549+
{
550+
""restore"": {
551+
""projectUniqueName"": ""Project1"",
552+
""centralPackageVersionsManagementEnabled"": true
553+
},
554+
""frameworks"": {
555+
""net8.0"": {
556+
""dependencies"": {
557+
""PackageA"": {
558+
""target"": ""Package"",
559+
""version"": ""[1.0.0,)"",
560+
""versionCentrallyManaged"": true
561+
}
562+
},
563+
""centralPackageVersions"": {
564+
""PackageB"": ""[2.0.0]"",
565+
""PackageA"": ""[1.0.0]""
566+
},
567+
""packagesToPrune"": {
568+
""TransitiveA"": ""(,1.0.0]"",
569+
""TransitiveB"": ""(,2.0.0]""
570+
}
571+
}
572+
}
573+
}");
574+
575+
DependencyGraphSpec second = CreateDependencyGraphSpecFromJson(@"
576+
{
577+
""restore"": {
578+
""projectUniqueName"": ""Project1"",
579+
""centralPackageVersionsManagementEnabled"": true
580+
},
581+
""frameworks"": {
582+
""net8.0"": {
583+
""dependencies"": {
584+
""PackageA"": {
585+
""target"": ""Package"",
586+
""version"": ""[1.0.0,)"",
587+
""versionCentrallyManaged"": true
588+
}
589+
},
590+
""centralPackageVersions"": {
591+
""PackageA"": ""[1.0.0]"",
592+
""PackageB"": ""[2.0.0]""
593+
},
594+
""packagesToPrune"": {
595+
""TransitiveA"": ""(,1.0.0]"",
596+
""TransitiveB"": ""(,2.0.0]""
597+
}
598+
}
599+
}
600+
}");
601+
602+
// Act
603+
string firstHash = GetHash(first, useLegacyHashFunction);
604+
string secondHash = GetHash(second, useLegacyHashFunction);
605+
606+
// Assert
607+
Assert.Equal(firstHash, secondHash);
608+
}
609+
450610
[Fact]
451611
public void AddProject_WhenRestoreMetadataIsNull_AddsProject()
452612
{
@@ -595,6 +755,45 @@ private static DependencyGraphSpec CreateDependencyGraphSpecWithCentralDependenc
595755
return dgSpec;
596756
}
597757

758+
private static DependencyGraphSpec CreateDependencyGraphSpecWithCentralDependenciesAndPruning(
759+
IReadOnlyList<string> centralPackageVersionNames,
760+
IReadOnlyList<string> packagesToPruneNames)
761+
{
762+
string centralPackageVersions = string.Join(
763+
$",{Environment.NewLine}",
764+
centralPackageVersionNames.Select(packageId => $@" ""{packageId}"": ""[1.0.0]"""));
765+
string packagesToPrune = string.Join(
766+
$",{Environment.NewLine}",
767+
packagesToPruneNames.Select(packageId => $@" ""{packageId}"": ""(,1.0.0]"""));
768+
769+
string rootProject = $@"
770+
{{
771+
""restore"": {{
772+
""projectUniqueName"": ""Project1"",
773+
""centralPackageVersionsManagementEnabled"": true
774+
}},
775+
""frameworks"": {{
776+
""net8.0"": {{
777+
""dependencies"": {{
778+
""PackageA"": {{
779+
""version"": ""[1.0.0,)"",
780+
""target"": ""Package"",
781+
""versionCentrallyManaged"": true
782+
}}
783+
}},
784+
""centralPackageVersions"": {{
785+
{centralPackageVersions}
786+
}},
787+
""packagesToPrune"": {{
788+
{packagesToPrune}
789+
}}
790+
}}
791+
}}
792+
}}";
793+
794+
return CreateDependencyGraphSpecFromJson(rootProject);
795+
}
796+
598797
private static TargetFrameworkInformation CreateTargetFrameworkInformation()
599798
{
600799
var nugetFramework = new NuGetFramework("net40");
@@ -654,6 +853,19 @@ private static TargetFrameworkInformation CreateTargetFrameworkInformation(Immut
654853
return tfi;
655854
}
656855

856+
private static DependencyGraphSpec CreateDependencyGraphSpecFromJson(string json)
857+
{
858+
PackageSpec packageSpec = ProjectTestHelpers.GetPackageSpecWithProjectNameAndSpec("Project1", @"c:\", json);
859+
return ProjectTestHelpers.GetDGSpecForFirstProject(packageSpec);
860+
}
861+
862+
private static string GetHash(DependencyGraphSpec dgSpec, bool useLegacyHashFunction)
863+
{
864+
return useLegacyHashFunction
865+
? dgSpec.GetHash(() => new Sha512HashFunction())
866+
: dgSpec.GetHash(() => new FnvHash64Function());
867+
}
868+
657869
private static string GetJson(DependencyGraphSpec dgSpec)
658870
{
659871
using (TestDirectory testDirectory = TestDirectory.Create())

0 commit comments

Comments
 (0)