Skip to content

Commit

Permalink
Merged PR 732318: Update SdkUpdater to pin transitive dependencies wh…
Browse files Browse the repository at this point in the history 
…en able.

- SdkUpdater will now check for CPM and transitive pinning to be enabled when working with transitive dependencies
- SdkUpdater will add a PackageVersion element to the last ItemGroup which contains PackageVersion elements.
- project_file_parser will now lookup dependency versions from the PackageVersion items in Directory.Packages.props

Also see related smoke test https://dev.azure.com/mseng/1ES/_git/adodependabot-smoke-tests/pullrequest/732336

Related work items: #2078137
  • Loading branch information
@JoeRobich @deivid-rodriguez
JoeRobich authored and deivid-rodriguez committed Nov 14, 2023
1 parent 6cd6c79 commit ec30d3b
Show file tree
Hide file tree
Showing 4 changed files with 263 additions and 22 deletions.
129 changes: 129 additions & 0 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Update/UpdateWorkerTests.Sdk.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1297,5 +1297,134 @@ await TestUpdateForProject("Microsoft.VisualStudio.Sdk.TestFramework.Xunit", "17
""")
});
}

[Fact]
public async Task AddTransitiveDependencyByAddingPackageReferenceAndVersion()
{
await TestUpdateForProject("System.Text.Json", "5.0.0", "5.0.2", isTransitive: true,
// initial
projectContents: """
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net5.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Mongo2Go" />
</ItemGroup>
</Project>
""",
additionalFiles: new[]
{
// initial props files
("Directory.Packages.props", """
<Project>
<PropertyGroup>
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
</PropertyGroup>
<ItemGroup>
<PackageVersion Include="Mongo2Go" Version="3.1.3" />
</ItemGroup>
</Project>
""")
},
// expected
expectedProjectContents: """
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net5.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Mongo2Go" />
<PackageReference Include="System.Text.Json" />
</ItemGroup>
</Project>
""",
additionalFilesExpected: new[]
{
// expected props files
("Directory.Packages.props", """
<Project>
<PropertyGroup>
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
</PropertyGroup>
<ItemGroup>
<PackageVersion Include="Mongo2Go" Version="3.1.3" />
<PackageVersion Include="System.Text.Json" Version="5.0.2" />
</ItemGroup>
</Project>
""")
});
}

[Fact]
public async Task PinTransitiveDependencyByAddingPackageVersion()
{
await TestUpdateForProject("System.Text.Json", "5.0.0", "5.0.2", isTransitive: true,
// initial
projectContents: """
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net5.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Mongo2Go" />
</ItemGroup>
</Project>
""",
additionalFiles: new[]
{
// initial props files
("Directory.Packages.props", """
<Project>
<PropertyGroup>
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
<CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
</PropertyGroup>
<ItemGroup>
<PackageVersion Include="Mongo2Go" Version="3.1.3" />
</ItemGroup>
</Project>
""")
},
// expected
expectedProjectContents: """
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<TargetFramework>net5.0</TargetFramework>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Mongo2Go" />
</ItemGroup>
</Project>
""",
additionalFilesExpected: new[]
{
// expected props files
("Directory.Packages.props", """
<Project>
<PropertyGroup>
<ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
<CentralPackageTransitivePinningEnabled>true</CentralPackageTransitivePinningEnabled>
</PropertyGroup>
<ItemGroup>
<PackageVersion Include="Mongo2Go" Version="3.1.3" />
<PackageVersion Include="System.Text.Json" Version="5.0.2" />
</ItemGroup>
</Project>
""")
});
}
}
}
82 changes: 69 additions & 13 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/SdkPackageUpdater.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,23 +83,87 @@ public static async Task UpdateDependencyAsync(string repoRootPath, string proje

if (isTransitive)
{
await AddTransitiveDependencyAsync(projectPath, dependencyName, previousDependencyVersion, newDependencyVersion, logger);
var directoryPackagesWithPinning = buildFiles.FirstOrDefault(bf => IsCpmTransitivePinningEnabled(bf));
if (directoryPackagesWithPinning is not null)
{
PinTransitiveDependency(directoryPackagesWithPinning, dependencyName, newDependencyVersion, logger);
}
else
{
await AddTransitiveDependencyAsync(projectPath, dependencyName, newDependencyVersion, logger);
}
}
else
{
await UpdateTopLevelDepdendencyAsync(buildFiles, dependencyName, previousDependencyVersion, newDependencyVersion, packagesAndVersions, logger);
}

foreach (var buildFile in buildFiles)
{
if (await buildFile.SaveAsync())
{
logger.Log($" Saved [{buildFile.RepoRelativePath}].");
}
}
}

private static async Task AddTransitiveDependencyAsync(string projectPath, string dependencyName, string previousDependencyVersion, string newDependencyVersion, Logger logger)
private static bool IsCpmTransitivePinningEnabled(BuildFile buildFile)
{
logger.Log($" Adding [{dependencyName}/{previousDependencyVersion}] as a top-level package reference.");
var buildFileName = Path.GetFileName(buildFile.Path);
if (!buildFileName.Equals("Directory.Packages.props", StringComparison.OrdinalIgnoreCase))
{
return false;
}

var propertyElements = buildFile.Xml.RootSyntax
.GetElements("PropertyGroup")
.SelectMany(e => e.Elements);

var isCpmEnabledValue = propertyElements.FirstOrDefault(e => e.Name.Equals("ManagePackageVersionsCentrally", StringComparison.OrdinalIgnoreCase))?.GetContentValue();
if (isCpmEnabledValue is null || !string.Equals(isCpmEnabledValue, "true", StringComparison.OrdinalIgnoreCase))
{
return false;
}

var isTransitivePinningEnabled = propertyElements.FirstOrDefault(e => e.Name.Equals("CentralPackageTransitivePinningEnabled", StringComparison.OrdinalIgnoreCase))?.GetContentValue();
return isTransitivePinningEnabled is not null && string.Equals(isTransitivePinningEnabled, "true", StringComparison.OrdinalIgnoreCase);
}

private static void PinTransitiveDependency(BuildFile directoryPackages, string dependencyName, string newDependencyVersion, Logger logger)
{
logger.Log($" Pinning [{dependencyName}/{newDependencyVersion}] as a package version.");

var lastItemGroup = directoryPackages.Xml.RootSyntax.GetElements("ItemGroup")
.Where(e => e.Elements.Any(se => se.Name.Equals("PackageVersion", StringComparison.OrdinalIgnoreCase)))
.LastOrDefault();

if (lastItemGroup is null)
{
logger.Log($" Transitive dependency [{dependencyName}/{newDependencyVersion}] was not pinned.");
return;
}

var lastPackageVersion = lastItemGroup.Elements.Last(se => se.Name.Equals("PackageVersion", StringComparison.OrdinalIgnoreCase));
var leadingTrivia = lastPackageVersion.AsNode.GetLeadingTrivia();

var packageVersionElement = XmlExtensions.CreateSingleLineXmlElementSyntax("PackageVersion", new SyntaxList<SyntaxNode>(leadingTrivia))
.WithAttribute("Include", dependencyName)
.WithAttribute("Version", newDependencyVersion);

var updatedItemGroup = lastItemGroup.AddChild(packageVersionElement);
var updatedXml = directoryPackages.Xml.ReplaceNode(lastItemGroup.AsNode, updatedItemGroup.AsNode);
directoryPackages.Update(updatedXml);
}

private static async Task AddTransitiveDependencyAsync(string projectPath, string dependencyName, string newDependencyVersion, Logger logger)
{
logger.Log($" Adding [{dependencyName}/{newDependencyVersion}] as a top-level package reference.");

// see https://learn.microsoft.com/nuget/consume-packages/install-use-packages-dotnet-cli
var (exitCode, stdout, stderr) = await ProcessEx.RunAsync("dotnet", $"add {projectPath} package {dependencyName} --version {newDependencyVersion}");
var (exitCode, _, _) = await ProcessEx.RunAsync("dotnet", $"add {projectPath} package {dependencyName} --version {newDependencyVersion}");
if (exitCode != 0)
{
logger.Log($" Transient dependency [{dependencyName}/{previousDependencyVersion}] was not added.");
logger.Log($" Transitive dependency [{dependencyName}/{newDependencyVersion}] was not added.");
}
}

Expand All @@ -116,14 +180,6 @@ private static async Task UpdateTopLevelDepdendencyAsync(ImmutableArray<BuildFil
{
TryUpdateDependencyVersion(buildFiles, packageName, previousDependencyVersion: null, newDependencyVersion: packageVersion, logger);
}

foreach (var buildFile in buildFiles)
{
if (await buildFile.SaveAsync())
{
logger.Log($" Saved [{buildFile.RepoRelativePath}].");
}
}
}

private static ImmutableArray<BuildFile> LoadBuildFiles(string repoRootPath, string projectPath)
Expand Down
37 changes: 29 additions & 8 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/XmlExtensions.cs
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
using System;
using System.Collections.Generic;
using System.Linq;

using Microsoft.Language.Xml;

namespace NuGetUpdater.Core;

public static class XmlExtensions
{
public static IEnumerable<IXmlElementSyntax> GetElements(this IXmlElementSyntax element, string name, StringComparison comparisonType = StringComparison.Ordinal)
{
return element.Elements.Where(a => a.Name.Equals(name, comparisonType));
}

public static IXmlElementSyntax WithChildElement(this IXmlElementSyntax parent, string name)
{
var element = CreateOpenCloseXmlElementSyntax(name);
Expand All @@ -26,42 +33,56 @@ public static XmlElementSyntax WithContent(this XmlElementSyntax element, string

public static IXmlElementSyntax WithAttribute(this IXmlElementSyntax parent, string name, string value)
{
var singleSpanceTrivia = SyntaxFactory.WhitespaceTrivia(" ");

return parent.AddAttribute(SyntaxFactory.XmlAttribute(
SyntaxFactory.XmlName(null, SyntaxFactory.XmlNameToken(name, null, null)),
SyntaxFactory.Punctuation(SyntaxKind.EqualsToken, "=", null, null),
SyntaxFactory.XmlString(
SyntaxFactory.Punctuation(SyntaxKind.SingleQuoteToken, "\"", null, null),
SyntaxFactory.XmlTextLiteralToken(value, null, null),
SyntaxFactory.Punctuation(SyntaxKind.SingleQuoteToken, "\"", null, null))));
SyntaxFactory.Punctuation(SyntaxKind.SingleQuoteToken, "\"", null, singleSpanceTrivia))));
}

public static XmlElementSyntax CreateOpenCloseXmlElementSyntax(string name, int indentation = 2, bool spaces = true)
{
var leadingTrivia = SyntaxFactory.WhitespaceTrivia(" ");
var leadingTrivia = SyntaxFactory.WhitespaceTrivia(new string(' ', indentation));
return CreateOpenCloseXmlElementSyntax(name, new SyntaxList<SyntaxNode>(leadingTrivia));
}

public static XmlElementSyntax CreateOpenCloseXmlElementSyntax(string name, SyntaxList<SyntaxNode> leadingTrivia)
{
var newlineTrivia = SyntaxFactory.WhitespaceTrivia(Environment.NewLine);

return SyntaxFactory.XmlElement(
SyntaxFactory.XmlElementStartTag(
SyntaxFactory.Punctuation(SyntaxKind.LessThanToken, "<", leadingTrivia, null),
SyntaxFactory.Punctuation(SyntaxKind.LessThanToken, "<", leadingTrivia, default),
SyntaxFactory.XmlName(null, SyntaxFactory.XmlNameToken(name, null, null)),
new SyntaxList<XmlAttributeSyntax>(),
SyntaxFactory.Punctuation(SyntaxKind.GreaterThanToken, ">", null, newlineTrivia)),
new SyntaxList<SyntaxNode>(),
SyntaxFactory.XmlElementEndTag(
SyntaxFactory.Punctuation(SyntaxKind.LessThanSlashToken, "</", leadingTrivia, null),
SyntaxFactory.Punctuation(SyntaxKind.LessThanSlashToken, "</", leadingTrivia, default),
SyntaxFactory.XmlName(null, SyntaxFactory.XmlNameToken(name, null, null)),
SyntaxFactory.Punctuation(SyntaxKind.GreaterThanToken, ">", null, null)));
}

public static XmlEmptyElementSyntax CreateSingleLineXmlElementSyntax(string name, int indentation = 2, bool spaces = true)
{
var leadingTrivia = SyntaxFactory.WhitespaceTrivia(" ");
var leadingTrivia = SyntaxFactory.WhitespaceTrivia(new string(' ', indentation));
var followingTrivia = SyntaxFactory.WhitespaceTrivia(Environment.NewLine);

return CreateSingleLineXmlElementSyntax(name, new SyntaxList<SyntaxNode>(leadingTrivia), new SyntaxList<SyntaxNode>(followingTrivia));
}

public static XmlEmptyElementSyntax CreateSingleLineXmlElementSyntax(string name, SyntaxList<SyntaxNode> leadingTrivia, SyntaxList<SyntaxNode> trailingTrivia = default)
{
var singleSpanceTrivia = SyntaxFactory.WhitespaceTrivia(" ");
var newlineTrivia = SyntaxFactory.WhitespaceTrivia(Environment.NewLine);

return SyntaxFactory.XmlEmptyElement(
SyntaxFactory.Punctuation(SyntaxKind.LessThanToken, "<", leadingTrivia, null),
SyntaxFactory.Punctuation(SyntaxKind.LessThanToken, "<", leadingTrivia, default),
SyntaxFactory.XmlName(null, SyntaxFactory.XmlNameToken(name, null, singleSpanceTrivia)),
attributes: new SyntaxList<SyntaxNode>(),
SyntaxFactory.Punctuation(SyntaxKind.SlashGreaterThanToken, "/>", null, newlineTrivia));
SyntaxFactory.Punctuation(SyntaxKind.SlashGreaterThanToken, "/>", default, trailingTrivia));
}
}
37 changes: 36 additions & 1 deletion nuget/lib/dependabot/nuget/file_parser/project_file_parser.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ class ProjectFileParser

PROJECT_REFERENCE_SELECTOR = "ItemGroup > ProjectReference"

PACKAGE_VERSION_SELECTOR = "ItemGroup > PackageVersion"

PROJECT_SDK_REGEX = %r{^([^/]+)/(\d+(?:[.]\d+(?:[.]\d+)?)?(?:[+-].*)?)$}
PROPERTY_REGEX = /\$\((?<property>.*?)\)/
ITEM_REGEX = /\@\((?<property>.*?)\)/
Expand Down Expand Up @@ -227,12 +229,45 @@ def dependency_name(dependency_node, project_file)
end

def dependency_requirement(dependency_node, project_file)
raw_requirement = get_node_version_value(dependency_node)
raw_requirement = get_node_version_value(dependency_node) ||
find_package_version(dependency_node, project_file)
return unless raw_requirement

evaluated_value(raw_requirement, project_file)
end

def find_package_version(dependency_node, project_file)
name = dependency_name(dependency_node, project_file)
return unless name

package_versions[name]
end

def package_versions
@package_versions ||= begin
package_versions = {}
directory_packages_props_files.each do |file|
doc = Nokogiri::XML(file.content)
doc.remove_namespaces!
doc.css(PACKAGE_VERSION_SELECTOR).each do |package_node|
name = dependency_name(package_node, file)
version = dependency_version(package_node, file)
next unless name && version

existing_version = package_versions[name]
next if existing_version && existing_version.numeric_version > version.numeric_version

package_versions[name] = version
end
end
package_versions
end
end

def directory_packages_props_files
dependency_files.select { |df| df.name.match?(/[Dd]irectory.[Pp]ackages.props/) }
end

def dependency_version(dependency_node, project_file)
requirement = dependency_requirement(dependency_node, project_file)
return unless requirement
Expand Down

0 comments on commit ec30d3b

Please sign in to comment.