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.

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

Conversation

brettfo
Copy link
Contributor

@brettfo brettfo commented Feb 5, 2025

This requires a simultaneous merge of PR dependabot/smoke-tests#255

This PR has a temporary commit that redirects the smoke tests to the changes in the above linked PR

When considering a package as an update candidate, we need to consider the original project's target framework. We already have the values and just need to forward them through to a framework checker utility that we already have. The only weird instance is when considering updates to repo-level packages specified in dotnet-tools.json or global.json; those have no project target framework so we have to consider an empty framework list as always compatible.

@brettfo brettfo requested review from a team as code owners February 5, 2025 00:09
@github-actions github-actions bot added the L: dotnet:nuget NuGet packages via nuget or dotnet label Feb 5, 2025
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.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: dotnet:nuget NuGet packages via nuget or dotnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant