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

Exclude pre-releases from NuGet latest version check #3468

Merged
merged 5 commits into from
Sep 30, 2024

Conversation

brentos99
Copy link
Contributor

Description

This PR addressed the issue of pre-release nuget packages being returned as the latest version.

The Nuget Documentation identifies packages with a "-" in their version string as a pre-release package.

The change is to filter the list of Nuget Packages that contain an "-" when determining the latestVersion. If the component being inspected is a pre-release itself, then pre-release versions are not filtered.

Addressed Issue

fixes #3467

Additional Details

There are 2 tests in this PR (NugetMetaAnalyzerTest.java), but due to the transient nature of the Nuget packages, these tests could fail if there is a non "pre-release" as the latest nuget version of the Microsoft.Extensions.DependencyInjection package.

I would have liked to have created a mock dataset for this scenario, but I the current tests appear tied to connecting directly to the live nuget package urls.

These 2 tests have their @Test commented as to not break future builds.

Checklist

  • I have read and understand the contributing guidelines
  • This PR fixes a defect, and I have provided tests to verify that the fix is effective
  • This PR implements an enhancement, and I have provided tests to verify that it works as intended
  • This PR introduces changes to the database model, and I have added corresponding update logic
  • This PR introduces new or alters existing behavior, and I have updated the documentation accordingly

Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.01% (target: -1.00%) 100.00% (target: 70.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (502a768) 21360 15775 73.85%
Head commit (72b6131) 21367 (+7) 15782 (+7) 73.86% (+0.01%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#3468) 12 12 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

You may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation

@Doxoh
Copy link

Doxoh commented May 27, 2024

@sebD how is the state for this pull-request. Is it possible to merge? :)

Kind Regards
Till

@Doxoh
Copy link

Doxoh commented May 27, 2024

//cc @nscuro

@taladar
Copy link

taladar commented Jul 1, 2024

It would be quite useful if this was merged as we have the same issue.

Copy link

@sebastien-lp sebastien-lp left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@brentos99
Copy link
Contributor Author

Is there something more I need to do on this pull request or some process I have not followed. I'm new to this, and not sure if this is standard timeframe or you are waiting on something.

@nscuro nscuro added this to the 4.12 milestone Jul 21, 2024
@nscuro nscuro added the enhancement New feature or request label Jul 21, 2024
@Doxoh
Copy link

Doxoh commented Sep 5, 2024

What about this pull request, whats the state?

Kind Regards
Till

@brentos99
Copy link
Contributor Author

brentos99 commented Sep 5, 2024

Based on the feedback, the pr doesn't work with the way the data is stored in the database. Their can only be one latest version of a package and this pr was to dynamically return different versions depending on if the version testing against is a prerelease or not.

Two options could be

  1. Always exclude prerelease packages from being the latest version.

  2. Provide configuration flag to allow prerelease versions to be a latest version. This would require the ui (different repo) to be updated as well to support the option as well.

Alternatively further investigation on how other repositories store prerelease data is required.

@Gepardgame
Copy link
Contributor

Whats the state of this PR? 4.12 is soon to be released and this has the milestone for this and there was long no development?

@nscuro
Copy link
Member

nscuro commented Sep 30, 2024

Will likely un-assign from v4.12. We can still ship this with a bugfix release so that doesn't mean having to wait for it for too long.

@brentos99
Copy link
Contributor Author

Hi @nscuro,

Could I get some direction on this PR?

The simplest approach would be to always ignore packages with a - in the version, as this aligns with NuGet's pre-release convention. According to the NuGet spec, versions with suffixes are treated as pre-release, and versions without a suffix take precedence.

Would this approach be acceptable as a global solution, or do you think a more configurable option is needed?

Thanks!

@brentos99 brentos99 requested a review from nscuro September 30, 2024 12:18
@nscuro
Copy link
Member

nscuro commented Sep 30, 2024

@brentos99 Would this approach be acceptable as a global solution [...]?

Sounds fine to me for now.

Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
Report missing for e746db01 100.00% (target: 70.00%)
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (e746db0) Report Missing Report Missing Report Missing
Head commit (b198fb5) 22353 17668 79.04%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#3468) 10 10 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

Footnotes

  1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct.

@nscuro nscuro changed the title Added feature to NugetMetaAnalyzer to exclude pre-release versions. Exclude pre-releases from NuGet latest version check Sep 30, 2024
@nscuro nscuro merged commit 8c7d5a9 into DependencyTrack:master Sep 30, 2024
10 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 31, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove nuget pre-release packages from being returned.
8 participants