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

fix nuget ignored package version handling #9824

Merged
merged 1 commit into from
May 27, 2024

Conversation

brettfo
Copy link
Contributor

@brettfo brettfo commented May 23, 2024

The current handling of ignored_versions in a job.yml file is incorrect. Consider an example from the unit tests; a job.yml could list that for a certain package, ignored_versions should be < 1.1.1, > 1.1.1 which should be read as "ignore anything below 1.1.1 and ignore anything above 1.1.1" which boils down to "only accept version 1.1.1". The previous implementation did a slightly different thing of "negate (version is less than 1.1.1 AND version is greater than 1.1.1)" which is impossible; this was a wrong application of our good friend De Morgan.

The proper fix here is to turn that check into "negate (version is less than 1.1.1 OR version is greater than 1.1.1)". Thankfully the Maven updater does the correct thing so I could almost directly copy what was done there.

Some additional changes had to be made, namely ensuring that the ignored_versions property was passed around everywhere and fixing some unit tests that weren't correct.

@github-actions github-actions bot added the L: dotnet:nuget NuGet packages via nuget or dotnet label May 23, 2024
@@ -226,7 +226,7 @@
end

context "when a version range is specified using Ruby syntax" do
let(:ignored_versions) { [">= 2.a, < 3.0.0"] }
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 test was incorrect because it originally said to exclude anything greater than or equal to 2.a and less than 3, which is an empty set, and given that this test was really only checking the Ruby-specific syntax of 2.a I simplified it.

@@ -671,7 +677,7 @@ def create_nupkg(nuspec_name, nuspec_content)
its([:version]) { is_expected.to eq(version_class.new("2.0.0")) }

context "when the user is ignoring the lowest version" do
let(:ignored_versions) { [">= 2.a, <= 2.0.0"] }
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 test was originally incorrect for the same reason as the one above so it has been simplified.

@brettfo brettfo force-pushed the dev/brettfo/nuget-ignored-versions branch from 3bb609b to 3a16d12 Compare May 23, 2024 18:45
@brettfo brettfo marked this pull request as ready for review May 23, 2024 18:51
@brettfo brettfo requested a review from a team as a code owner May 23, 2024 18:51
Copy link
Contributor

@honeyankit honeyankit left a comment

Choose a reason for hiding this comment

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

LGTM

@JamieMagee JamieMagee force-pushed the dev/brettfo/nuget-ignored-versions branch from 3a16d12 to 96eee27 Compare May 24, 2024 17:27
@sachin-sandhu sachin-sandhu force-pushed the dev/brettfo/nuget-ignored-versions branch from 96eee27 to cb8dcdd Compare May 27, 2024 13:10
@sachin-sandhu sachin-sandhu merged commit c6745da into main May 27, 2024
67 checks passed
@sachin-sandhu sachin-sandhu deleted the dev/brettfo/nuget-ignored-versions branch May 27, 2024 13:36
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.

None yet

4 participants