Skip to content

Conversation

@isra-fel
Copy link
Member

@isra-fel isra-fel commented Nov 24, 2025

Introduce a new cmdlet Compare-DevPackageDep that compares dependencies between two versions of a NuGet package. This cmdlet reports added, removed, and changed dependencies, including their versions, and provides a detailed output for dependency differences.

Useful for example when updating Azure.Core.

@azure-client-tools-bot-prd
Copy link

Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status.

@isra-fel isra-fel requested a review from Copilot November 24, 2025 04:13
Copilot finished reviewing on behalf of isra-fel November 24, 2025 04:17
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request introduces a new cmdlet Compare-DevPackageDep to the AzDev PowerShell module that compares NuGet package dependencies between two versions. The cmdlet identifies added, removed, and changed dependencies, and recursively compares changed dependencies to reveal all transitive dependency changes in the dependency tree.

Key Changes

  • New dependency comparison service with recursive comparison logic
  • PowerShell cmdlet with tab completion for target framework selection
  • Comprehensive unit tests covering add, remove, and version change scenarios

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
tools/AzDev/src/Services/Dep/IDepComparisonService.cs New interface defining the dependency comparison service contract
tools/AzDev/src/Services/Dep/DefaultDepComparisonService.cs Implementation of recursive dependency comparison logic
tools/AzDev/src/Services/AzDevModule.cs Registers the new dependency comparison service in the DI container
tools/AzDev/src/Services/Assembly/INugetService.cs Extends interface to support querying package dependencies
tools/AzDev/src/Services/Assembly/DefaultNugetService.cs Implements GetPackageDependencies using NuGet SDK
tools/AzDev/src/Models/PSModels/PSPackageDepDiff.cs PowerShell output model for dependency differences
tools/AzDev/src/Models/Dep/PackageDepDiff.cs Internal model representing a dependency change
tools/AzDev/src/Models/Dep/PackageDep.cs Internal model representing a package dependency
tools/AzDev/src/Cmdlets/Dep/ComparePackageDepCmdlet.cs PowerShell cmdlet implementation with argument completer
tools/AzDev/spec/package-comparing-cmdlet.md Design specification and usage examples
tools/AzDev/Tests/ServiceTests/PackageComparisonServiceTests.cs Unit tests for the comparison service
tools/AzDev/README.md Documentation and usage examples for the new cmdlet
tools/AzDev/CHANGELOG.md Release notes entry for the new feature
tools/AzDev/AzDev/AzDev.psd1 Exports the new cmdlet from the module
tools/AzDev/AzDev/AzDev.format.ps1xml Custom formatting definition for the output object

// Convert NuGet SDK model to our model
return nugetDependencies.Select(d => new PackageDep
{
Id = d.Id,
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Potential issue with version range handling. The code uses d.VersionRange.MinVersion.ToString() to extract the version, but NuGet version ranges can be complex (e.g., "[1.0.0, 2.0.0)", "(1.0.0, )", etc.). Using only MinVersion may not accurately represent the dependency requirement.

Consider:

  1. Documenting this limitation in the code comments
  2. Or handling version ranges more comprehensively by using VersionRange.ToShortString() or similar
  3. Or logging a warning when non-exact version ranges are encountered

For the current implementation, at minimum add a comment explaining the decision:

// Note: Using MinVersion for simplicity. This may not fully represent 
// complex version ranges like "[1.0, 2.0)" but is sufficient for 
// identifying dependency changes.
Version = d.VersionRange.MinVersion.ToString()
Suggested change
Id = d.Id,
Id = d.Id,
// Note: Using MinVersion for simplicity. This may not fully represent
// complex version ranges like "[1.0, 2.0)" but is sufficient for
// identifying dependency changes.

Copilot uses AI. Check for mistakes.
Comment on lines +86 to +93
// Recursively compare the changed dependency
var recursiveChanges = ComparePackageDependencies(
newDep.Id,
oldDep.Version,
newDep.Version,
targetFramework);

results.AddRange(recursiveChanges);
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The recursive comparison lacks protection against infinite loops. If package A depends on B and B depends on A (circular dependency), or if there's a deep dependency chain, this could cause a stack overflow or extremely long execution time.

Consider adding:

  1. A visited set to track already-compared package version pairs: HashSet<(string packageName, string oldVersion, string newVersion)>
  2. A maximum depth parameter to limit recursion depth
  3. Early return if the current comparison has already been processed

Example:

private readonly HashSet<string> _visitedComparisons = new();

public IEnumerable<PackageDepDiff> ComparePackageDependencies(...)
{
    var comparisonKey = $"{packageName}:{oldVersion}:{newVersion}";
    if (!_visitedComparisons.Add(comparisonKey))
    {
        _logger.Debug($"Skipping already visited comparison: {comparisonKey}");
        return Enumerable.Empty<PackageDepDiff>();
    }
    // ... rest of the method
}

Copilot uses AI. Check for mistakes.
Comment on lines +99 to +130

using var packageStream = new MemoryStream();
_findPackageByIdResource.Value.CopyNupkgToStreamAsync(
packageName,
new NuGetVersion(packageVersion),
packageStream,
_cache,
NullLogger.Instance,
default).ConfigureAwait(false).GetAwaiter().GetResult();

using var packageReader = new PackageArchiveReader(packageStream);
var dependencyGroups = packageReader.GetPackageDependencies();

var framework = NuGetFramework.Parse(targetFramework);
var reducer = new FrameworkReducer();
var nearestFramework = reducer.GetNearest(framework, dependencyGroups.Select(g => g.TargetFramework));

if (nearestFramework == null)
{
_logger.Debug($"[{nameof(DefaultNugetService)}] No compatible dependency group found for {targetFramework}");
return Enumerable.Empty<PackageDep>();
}

var dependencyGroup = dependencyGroups.FirstOrDefault(g => g.TargetFramework.Equals(nearestFramework));
var nugetDependencies = dependencyGroup?.Packages ?? Enumerable.Empty<NuGet.Packaging.Core.PackageDependency>();

// Convert NuGet SDK model to our model
return nugetDependencies.Select(d => new PackageDep
{
Id = d.Id,
Version = d.VersionRange.MinVersion.ToString()
});
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing error handling for NuGet API failures. The GetPackageDependencies method can fail due to:

  • Network connectivity issues
  • Invalid package name or version
  • Package not found on NuGet.org
  • API rate limiting

When called recursively, a single failure will propagate and stop the entire comparison. Consider adding try-catch blocks to handle these scenarios gracefully:

try
{
    using var packageStream = new MemoryStream();
    _findPackageByIdResource.Value.CopyNupkgToStreamAsync(...)
        .ConfigureAwait(false).GetAwaiter().GetResult();
    // ... rest of the method
}
catch (Exception ex)
{
    _logger.Warning($"Failed to get dependencies for {packageName} {packageVersion}: {ex.Message}");
    return Enumerable.Empty<PackageDep>();
}
Suggested change
using var packageStream = new MemoryStream();
_findPackageByIdResource.Value.CopyNupkgToStreamAsync(
packageName,
new NuGetVersion(packageVersion),
packageStream,
_cache,
NullLogger.Instance,
default).ConfigureAwait(false).GetAwaiter().GetResult();
using var packageReader = new PackageArchiveReader(packageStream);
var dependencyGroups = packageReader.GetPackageDependencies();
var framework = NuGetFramework.Parse(targetFramework);
var reducer = new FrameworkReducer();
var nearestFramework = reducer.GetNearest(framework, dependencyGroups.Select(g => g.TargetFramework));
if (nearestFramework == null)
{
_logger.Debug($"[{nameof(DefaultNugetService)}] No compatible dependency group found for {targetFramework}");
return Enumerable.Empty<PackageDep>();
}
var dependencyGroup = dependencyGroups.FirstOrDefault(g => g.TargetFramework.Equals(nearestFramework));
var nugetDependencies = dependencyGroup?.Packages ?? Enumerable.Empty<NuGet.Packaging.Core.PackageDependency>();
// Convert NuGet SDK model to our model
return nugetDependencies.Select(d => new PackageDep
{
Id = d.Id,
Version = d.VersionRange.MinVersion.ToString()
});
try
{
using var packageStream = new MemoryStream();
_findPackageByIdResource.Value.CopyNupkgToStreamAsync(
packageName,
new NuGetVersion(packageVersion),
packageStream,
_cache,
NullLogger.Instance,
default).ConfigureAwait(false).GetAwaiter().GetResult();
using var packageReader = new PackageArchiveReader(packageStream);
var dependencyGroups = packageReader.GetPackageDependencies();
var framework = NuGetFramework.Parse(targetFramework);
var reducer = new FrameworkReducer();
var nearestFramework = reducer.GetNearest(framework, dependencyGroups.Select(g => g.TargetFramework));
if (nearestFramework == null)
{
_logger.Debug($"[{nameof(DefaultNugetService)}] No compatible dependency group found for {targetFramework}");
return Enumerable.Empty<PackageDep>();
}
var dependencyGroup = dependencyGroups.FirstOrDefault(g => g.TargetFramework.Equals(nearestFramework));
var nugetDependencies = dependencyGroup?.Packages ?? Enumerable.Empty<NuGet.Packaging.Core.PackageDependency>();
// Convert NuGet SDK model to our model
return nugetDependencies.Select(d => new PackageDep
{
Id = d.Id,
Version = d.VersionRange.MinVersion.ToString()
});
}
catch (Exception ex)
{
_logger.Warning($"Failed to get dependencies for {packageName} {packageVersion}: {ex.Message}");
return Enumerable.Empty<PackageDep>();
}

Copilot uses AI. Check for mistakes.
# Use -Debug to see all dependencies for both versions
PS /> Compare-DevPackageDep -PackageName "Azure.Core" -OldVersion "1.47.3" -NewVersion "1.50.0" -Debug
DEBUG: Comparing Azure.Core from 1.47.3 to 1.50.0 for netstandard2.0
DEBUG: [DefaultPackageComparisonService] Old version 1.47.3 dependencies:
Copy link

Copilot AI Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The service class name in the debug message is inconsistent with the actual class name. The message uses DefaultPackageComparisonService but the class is named DefaultDepComparisonService.

Change line 109 to use the correct class name:

DEBUG: [DefaultDepComparisonService] Old version 1.47.3 dependencies:
Suggested change
DEBUG: [DefaultPackageComparisonService] Old version 1.47.3 dependencies:
DEBUG: [DefaultDepComparisonService] Old version 1.47.3 dependencies:

Copilot uses AI. Check for mistakes.
@isra-fel isra-fel changed the title Add new cmdlet Compare-DevPackageDep for dependency comparison [AzDev] Add Compare-DevPackageDep for dependency comparison Nov 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant