Skip to content

Commit

Permalink
Handle Update PackageReferences more correctly (dependabot#8827)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ryanbrandenburg
ryanbrandenburg authored Jan 18, 2024
1 parent 03b552c commit a10872c
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 9 deletions.
97 changes: 97 additions & 0 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Utilities/MSBuildHelperTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,37 @@ public async Task AllPackageDependenciesCanBeTraversed()
Assert.Equal(expectedDependencies, actualDependencies);
}

[Fact]
public async Task AllPackageDependencies_DoNotIncludeUpdateOnlyPackages()
{
using var temp = new TemporaryDirectory();
var expectedDependencies = new Dependency[]
{
new("Microsoft.Bcl.AsyncInterfaces", "7.0.0", DependencyType.Unknown),
new("Microsoft.Extensions.DependencyInjection", "7.0.0", DependencyType.Unknown),
new("Microsoft.Extensions.DependencyInjection.Abstractions", "7.0.0", DependencyType.Unknown),
new("Microsoft.Extensions.Http", "7.0.0", DependencyType.Unknown),
new("Microsoft.Extensions.Logging", "7.0.0", DependencyType.Unknown),
new("Microsoft.Extensions.Logging.Abstractions", "7.0.0", DependencyType.Unknown),
new("Microsoft.Extensions.Options", "7.0.0", DependencyType.Unknown),
new("Microsoft.Extensions.Primitives", "7.0.0", DependencyType.Unknown),
new("System.Buffers", "4.5.1", DependencyType.Unknown),
new("System.ComponentModel.Annotations", "5.0.0", DependencyType.Unknown),
new("System.Diagnostics.DiagnosticSource", "7.0.0", DependencyType.Unknown),
new("System.Memory", "4.5.5", DependencyType.Unknown),
new("System.Numerics.Vectors", "4.4.0", DependencyType.Unknown),
new("System.Runtime.CompilerServices.Unsafe", "6.0.0", DependencyType.Unknown),
new("System.Threading.Tasks.Extensions", "4.5.4", DependencyType.Unknown),
new("NETStandard.Library", "2.0.3", DependencyType.Unknown),
};
var packages = new[] {
new Dependency("Microsoft.Extensions.Http", "7.0.0", DependencyType.Unknown),
new Dependency("Newtonsoft.Json", "12.0.1", DependencyType.Unknown, IsUpdate: true)
};
var actualDependencies = await MSBuildHelper.GetAllPackageDependenciesAsync(temp.DirectoryPath, temp.DirectoryPath, "netstandard2.0", packages);
Assert.Equal(expectedDependencies, actualDependencies);
}

[Fact]
public async Task AllPackageDependenciesCanBeFoundWithNuGetConfig()
{
Expand Down Expand Up @@ -349,6 +380,72 @@ public static IEnumerable<object[]> GetTopLevelPackageDependenyInfosTestData()
new("Newtonsoft.Json", "12.0.1", DependencyType.Unknown)
}
};

// version is set in one file, used in another
yield return new object[]
{
// build file contents
new[]
{
("Packages.props", """
<Project>
<ItemGroup>
<PackageReference Update="Azure.Identity" Version="1.6.0" />
<PackageReference Update="Microsoft.Data.SqlClient" Version="5.1.4" />
</ItemGroup>
</Project>
"""),
("project.csproj", """
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Azure.Identity" Version="1.6.1" />
</ItemGroup>
</Project>
""")
},
// expected dependencies
new Dependency[]
{
new("Azure.Identity", "1.6.0", DependencyType.Unknown),
new("Microsoft.Data.SqlClient", "5.1.4", DependencyType.Unknown, IsUpdate: true)
}
};

// version is set in one file, used in another
yield return new object[]
{
// build file contents
new[]
{
("project.csproj", """
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>netstandard2.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Azure.Identity" />
</ItemGroup>
</Project>
"""),
("Packages.props", """
<Project>
<ItemGroup>
<PackageReference Update="Azure.Identity" Version="1.6.0" />
<PackageReference Update="Microsoft.Data.SqlClient" Version="5.1.4" />
</ItemGroup>
</Project>
""")
},
// expected dependencies
new Dependency[]
{
new("Azure.Identity", "1.6.0", DependencyType.Unknown),
new("Microsoft.Data.SqlClient", "5.1.4", DependencyType.Unknown, IsUpdate: true)
}
};
}

public static IEnumerable<object[]> SolutionProjectPathTestData()
Expand Down
2 changes: 1 addition & 1 deletion nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Dependency.cs
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
namespace NuGetUpdater.Core;

public sealed record Dependency(string Name, string? Version, DependencyType Type, bool IsDevDependency = false, bool IsOverride = false);
public sealed record Dependency(string Name, string? Version, DependencyType Type, bool IsDevDependency = false, bool IsOverride = false, bool IsUpdate = false);
4 changes: 2 additions & 2 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/SdkPackageUpdater.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public static async Task UpdateDependencyAsync(string repoRootPath, string proje
foreach (var tfm in tfms)
{
var dependencies = await MSBuildHelper.GetAllPackageDependenciesAsync(repoRootPath, projectPath, tfm, topLevelDependencies, logger);
foreach (var (packageName, packageVersion, _, _, _) in dependencies)
foreach (var (packageName, packageVersion, _, _, _, _) in dependencies)
{
if (packageName.Equals(dependencyName, StringComparison.OrdinalIgnoreCase))
{
Expand Down Expand Up @@ -76,7 +76,7 @@ public static async Task UpdateDependencyAsync(string repoRootPath, string proje
var packagesAndVersions = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
foreach (var (tfm, dependencies) in tfmsAndDependencies)
{
foreach (var (packageName, packageVersion, _, _, _) in dependencies)
foreach (var (packageName, packageVersion, _, _, _, _) in dependencies)
{
if (packagesAndVersions.TryGetValue(packageName, out var existingVersion) &&
existingVersion != packageVersion)
Expand Down
29 changes: 23 additions & 6 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/MSBuildHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ public static IEnumerable<string> GetProjectPathsFromProject(string projFilePath

public static IEnumerable<Dependency> GetTopLevelPackageDependenyInfos(ImmutableArray<ProjectBuildFile> buildFiles)
{
Dictionary<string, string> packageInfo = new(StringComparer.OrdinalIgnoreCase);
Dictionary<string, (string, bool)> packageInfo = new(StringComparer.OrdinalIgnoreCase);
Dictionary<string, string> packageVersionInfo = new(StringComparer.OrdinalIgnoreCase);
Dictionary<string, string> propertyInfo = new(StringComparer.OrdinalIgnoreCase);

Expand All @@ -154,7 +154,22 @@ public static IEnumerable<Dependency> GetTopLevelPackageDependenyInfos(Immutable
{
if (!string.IsNullOrWhiteSpace(attributeValue))
{
packageInfo[attributeValue] = versionSpecification;
if (packageInfo.TryGetValue(attributeValue, out var existingInfo))
{
var existingVersion = existingInfo.Item1;
var existingUpdate = existingInfo.Item2;
// Retain the version from the Update reference since the intention
// would be to override the version of the Include reference.
var vSpec = string.IsNullOrEmpty(versionSpecification) || existingUpdate ? existingVersion : versionSpecification;

var isUpdate = existingUpdate && string.IsNullOrEmpty(packageItem.Include);
packageInfo[attributeValue] = (vSpec, isUpdate);
}
else
{
var isUpdate = !string.IsNullOrEmpty(packageItem.Update);
packageInfo[attributeValue] = (versionSpecification, isUpdate);
}
}
}
}
Expand All @@ -180,8 +195,9 @@ public static IEnumerable<Dependency> GetTopLevelPackageDependenyInfos(Immutable
}
}

foreach (var (name, version) in packageInfo)
foreach (var (name, info) in packageInfo)
{
var (version, isUpdate) = info;
if (version.Length != 0 || !packageVersionInfo.TryGetValue(name, out var packageVersion))
{
packageVersion = version;
Expand All @@ -195,8 +211,8 @@ public static IEnumerable<Dependency> GetTopLevelPackageDependenyInfos(Immutable
// We don't know the version for range requirements or wildcard
// requirements, so return "" for these.
yield return packageVersion.Contains(',') || packageVersion.Contains('*')
? new Dependency(name, string.Empty, DependencyType.Unknown)
: new Dependency(name, packageVersion, DependencyType.Unknown);
? new Dependency(name, string.Empty, DependencyType.Unknown, IsUpdate: isUpdate)
: new Dependency(name, packageVersion, DependencyType.Unknown, IsUpdate: isUpdate);
}
}

Expand Down Expand Up @@ -284,7 +300,8 @@ private static async Task<string> CreateTempProjectAsync(DirectoryInfo tempDir,
Environment.NewLine,
packages
.Where(p => !string.IsNullOrWhiteSpace(p.Version)) // empty `Version` attributes will cause the temporary project to not build
.Select(static p => $"<PackageReference Include=\"{p.Name}\" Version=\"[{p.Version}]\" />"));
// If all PackageReferences for a package are update-only mark it as such, otherwise it can cause package incoherence errors which do not exist in the repo.
.Select(static p => $"<PackageReference {(p.IsUpdate ? "Update" : "Include")}=\"{p.Name}\" Version=\"[{p.Version}]\" />"));

var projectContents = $"""
<Project Sdk="Microsoft.NET.Sdk">
Expand Down

0 comments on commit a10872c

Please sign in to comment.