Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

consider project target framework for package update eligibility #11485

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

consider project target framework for package update eligibility #11485

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c92bb37
consider project target framework for package update eligibility
brettfo Feb 4, 2025
fe1399c
TEMP: redirect smoke tests
brettfo Feb 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/smoke.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ concurrency:

env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
SMOKE_TEST_BRANCH: main
SMOKE_TEST_BRANCH: dev/brettfo/nuget-smoke-test-tfm
jobs:
discover:
runs-on: ubuntu-latest
Expand Down
33 changes: 33 additions & 0 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core.Test/Analyze/VersionFinderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
using System.Collections.Immutable;

using NuGet.Frameworks;
using NuGet.Versioning;

using NuGetUpdater.Core.Analyze;
using NuGetUpdater.Core.Test.Update;
using NuGetUpdater.Core.Test.Utilities;

using Xunit;

Expand Down Expand Up @@ -190,4 +195,32 @@ public void VersionFilter_WildcardPreviewVersion_ReturnsTrue()

Assert.True(result);
}

[Fact]
public async Task TargetFrameworkIsConsideredForUpdatedVersions()
{
// arrange
using var tempDir = new TemporaryDirectory();
await UpdateWorkerTestBase.MockNuGetPackagesInDirectory(
[
MockNuGetPackage.CreateSimplePackage("Some.Package", "1.0.0", "net8.0"),
MockNuGetPackage.CreateSimplePackage("Some.Package", "2.0.0", "net8.0"), // can only update to this version because of the tfm
MockNuGetPackage.CreateSimplePackage("Some.Package", "3.0.0", "net9.0"),
],
tempDir.DirectoryPath);

// act
var projectTfms = new[] { "net8.0" }.Select(NuGetFramework.Parse).ToImmutableArray();
var packageId = "Some.Package";
var currentVersion = NuGetVersion.Parse("1.0.0");
var logger = new TestLogger();
var nugetContext = new NuGetContext(tempDir.DirectoryPath);
var versionResult = await VersionFinder.GetVersionsAsync(projectTfms, packageId, currentVersion, nugetContext, logger, CancellationToken.None);
var versions = versionResult.GetVersions();

// assert
var actual = versions.Select(v => v.ToString()).ToArray();
var expected = new[] { "2.0.0" };
AssertEx.Equal(expected, actual);
}
}
2 changes: 2 additions & 0 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/AnalyzeWorker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ private static async Task<T> DeserializeJsonFileAsync<T>(string filePath, string
CancellationToken cancellationToken)
{
var versionResult = await VersionFinder.GetVersionsAsync(
projectFrameworks,
dependencyInfo,
nugetContext,
logger,
Expand All @@ -262,6 +263,7 @@ private static async Task<T> DeserializeJsonFileAsync<T>(string filePath, string
CancellationToken cancellationToken)
{
var versionResult = await VersionFinder.GetVersionsAsync(
projectFrameworks,
packageIds.First(),
version,
nugetContext,
Expand Down
13 changes: 6 additions & 7 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/CompatabilityChecker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,19 +108,18 @@ internal static bool PerformCheck(

foreach (var d in dependencyGroups)
{
var libItems = (await readers.ContentReader.GetLibItemsAsync(cancellationToken)).ToList();

foreach (var item in libItems)
{
tfms.Add(item.TargetFramework);
}

if (!d.TargetFramework.IsAny)
{
tfms.Add(d.TargetFramework);
}
}

var refItems = await readers.ContentReader.GetReferenceItemsAsync(cancellationToken);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was pulled out of the above loop because it's possible for a NuGet package to not list any dependency groups, but still have values in the lib/ directory that need to be considered.

foreach (var refItem in refItems)
{
tfms.Add(refItem.TargetFramework);
}

if (!tfms.Any())
{
tfms.Add(NuGetFramework.AnyFramework);
Expand Down
33 changes: 29 additions & 4 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/VersionFinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using NuGet.Common;
using NuGet.Configuration;
using NuGet.Frameworks;
using NuGet.Packaging.Core;
using NuGet.Protocol;
using NuGet.Protocol.Core.Types;
Expand All @@ -12,6 +13,7 @@ namespace NuGetUpdater.Core.Analyze;
internal static class VersionFinder
{
public static Task<VersionResult> GetVersionsAsync(
ImmutableArray<NuGetFramework> projectTfms,
string packageId,
NuGetVersion currentVersion,
NuGetContext nugetContext,
Expand All @@ -20,10 +22,11 @@ public static Task<VersionResult> GetVersionsAsync(
{
var versionFilter = CreateVersionFilter(currentVersion);

return GetVersionsAsync(packageId, currentVersion, versionFilter, nugetContext, logger, cancellationToken);
return GetVersionsAsync(projectTfms, packageId, currentVersion, versionFilter, nugetContext, logger, cancellationToken);
}

public static Task<VersionResult> GetVersionsAsync(
ImmutableArray<NuGetFramework> projectTfms,
DependencyInfo dependencyInfo,
NuGetContext nugetContext,
ILogger logger,
Expand All @@ -34,10 +37,11 @@ public static Task<VersionResult> GetVersionsAsync(
var currentVersion = versionRange.MinVersion!;
var versionFilter = CreateVersionFilter(dependencyInfo, versionRange);

return GetVersionsAsync(packageId, currentVersion, versionFilter, nugetContext, logger, cancellationToken);
return GetVersionsAsync(projectTfms, packageId, currentVersion, versionFilter, nugetContext, logger, cancellationToken);
}

public static async Task<VersionResult> GetVersionsAsync(
ImmutableArray<NuGetFramework> projectTfms,
string packageId,
NuGetVersion currentVersion,
Func<NuGetVersion, bool> versionFilter,
Expand All @@ -62,7 +66,14 @@ public static async Task<VersionResult> GetVersionsAsync(
var feed = await sourceRepository.GetResourceAsync<MetadataResource>();
if (feed is null)
{
logger.Warn($"Failed to get MetadataResource for [{source.Source}]");
logger.Warn($"Failed to get {nameof(MetadataResource)} for [{source.Source}]");
continue;
}

var packageFinder = await sourceRepository.GetResourceAsync<FindPackageByIdResource>();
if (packageFinder is null)
{
logger.Warn($"Failed to get {nameof(FindPackageByIdResource)} for [{source.Source}]");
continue;
}

Expand Down Expand Up @@ -100,7 +111,21 @@ public static async Task<VersionResult> GetVersionsAsync(
result.AddCurrentVersionSource(source);
}

result.AddRange(source, feedVersions.Where(versionFilter));
var versions = feedVersions.Where(versionFilter).ToArray();
foreach (var version in versions)
{
var isTfmCompatible = await CompatibilityChecker.CheckAsync(
new PackageIdentity(packageId, version),
projectTfms,
nugetContext,
logger,
CancellationToken.None);
if (isTfmCompatible || projectTfms.IsEmpty)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual fix.

{
// dotnet-tools.json and global.json packages won't specify a TFM, so they're always compatible
result.Add(source, version);
}
}
}

return result;
Expand Down
17 changes: 7 additions & 10 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Analyze/VersionResult.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,15 @@ public void AddCurrentVersionSource(PackageSource source)
_currentVersionSources.Add(source);
}

public void AddRange(PackageSource source, IEnumerable<NuGetVersion> versions)
public void Add(PackageSource source, NuGetVersion version)
{
foreach (var version in versions)
if (_versions.ContainsKey(version))
{
if (_versions.ContainsKey(version))
{
_versions[version].Add(source);
}
else
{
_versions.Add(version, [source]);
}
_versions[version].Add(source);
}
else
{
_versions.Add(version, [source]);
}
}

Expand Down
7 changes: 5 additions & 2 deletions nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Utilities/DependencyConflictResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,8 @@ public async Task<string> UpdateVersion(List<PackageToUpdate> existingPackages,
}
}

var projectFramework = NuGetFramework.Parse(targetFramework);

// Get the parent packages of the package and check the compatibility between its family
HashSet<PackageToUpdate> parentPackages = GetParentPackages(package);

Expand Down Expand Up @@ -458,7 +460,7 @@ public async Task<string> UpdateVersion(List<PackageToUpdate> existingPackages,
string currentVersionString = parent.CurrentVersion;
NuGetVersion currentVersionParent = NuGetVersion.Parse(currentVersionString);

var result = await VersionFinder.GetVersionsAsync(parent.PackageName, currentVersionParent, nugetContext, logger, CancellationToken.None);
var result = await VersionFinder.GetVersionsAsync([projectFramework], parent.PackageName, currentVersionParent, nugetContext, logger, CancellationToken.None);
var versions = result.GetVersions();
NuGetVersion latestVersion = versions.Where(v => !v.IsPrerelease).Max();

Expand Down Expand Up @@ -565,8 +567,9 @@ public async Task<NuGetVersion> FindCompatibleVersionAsync(List<PackageToUpdate>

// Create a NugetContext instance to get the latest versions of the parent
NuGetContext nugetContext = new NuGetContext(Path.GetDirectoryName(projectPath));
var projectFramework = NuGetFramework.Parse(targetFramework);

var result = await VersionFinder.GetVersionsAsync(possibleParent.PackageName, CurrentVersion, nugetContext, logger, CancellationToken.None);
var result = await VersionFinder.GetVersionsAsync([projectFramework], possibleParent.PackageName, CurrentVersion, nugetContext, logger, CancellationToken.None);
var versions = result.GetVersions();

// If there are no versions
Expand Down
Loading