Skip to content

Move semver ignore-condition range code into version#10664

Closed
amazimbe wants to merge 4 commits intomainfrom
amazimbe/update-git-ignore-semver
Closed

Move semver ignore-condition range code into version#10664
amazimbe wants to merge 4 commits intomainfrom
amazimbe/update-git-ignore-semver

Conversation

@amazimbe
Copy link
Copy Markdown
Contributor

@amazimbe amazimbe commented Sep 24, 2024

Issue: (Dependabot ignore semantic version not working with latest dependabot-updater-maven) [https://github.com//issues/10634]

What are you trying to accomplish?

Fix an issue where ignore conditions like ["version-update:semver-major", "version-update:semver-minor"] are not applied correctly across version boundaries because we are appending a rather than 0 when calculating version ranges.

The presence of a in a version affects ordering in different ways based on the ecosystem. In the Maven spec, for example, an ignore condition like >=3.a accepts 3.0 but rejects 3.a1 and 3alpha because 3alpha < 3.a1 < 3.0 < 3.a .

By moving the code that calculates the range of ignored versions into version.rb we can calculate correct ranges of which versions to ignore based on the version specification followed by an ecosystem. In maven, for example, a dependency with version 2.9 and settings that say ignore majors would not upgrade to anything >= 3.a0.

@jonjanego @abdulapopoola FYI

How will you know you've accomplished your goal?

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@amazimbe amazimbe self-assigned this Sep 24, 2024
@amazimbe amazimbe marked this pull request as ready for review September 24, 2024 13:56
@amazimbe amazimbe requested a review from a team as a code owner September 24, 2024 13:56
@amazimbe amazimbe force-pushed the amazimbe/update-git-ignore-semver branch from 1fb3f80 to 38f3189 Compare September 25, 2024 20:22
@github-actions github-actions bot added the L: java:maven Maven packages via Maven label Sep 25, 2024
@amazimbe amazimbe force-pushed the amazimbe/update-git-ignore-semver branch 2 times, most recently from 0fcc693 to a3677f4 Compare September 25, 2024 20:50
@amazimbe amazimbe changed the title Use 0 as default in ignore-condition ranges instead of a Move semver ignore-condition range code into version Sep 25, 2024
@amazimbe amazimbe force-pushed the amazimbe/update-git-ignore-semver branch from a3677f4 to 403624e Compare September 26, 2024 10:07
@abdulapopoola
Copy link
Copy Markdown
Contributor

Thanks @amazimbe for the deep analysis, one question

In maven, for example, a dependency with version 2.9 and settings that say ignore majors, for example, would not upgrade to anything > 3.a0.

Does this mean >3.0 or >3.a0?

@amazimbe
Copy link
Copy Markdown
Contributor Author

amazimbe commented Sep 27, 2024

Thanks @amazimbe for the deep analysis, one question

In maven, for example, a dependency with version 2.9 and settings that say ignore majors, for example, would not upgrade to anything > 3.a0.

Does this mean >3.0 or >3.a0?

>=3.a0 is what I meant to say.

In maven 3.a0 is the earliest 3 prerelease. My assumption is that if they are on 2.9 and their settings are to ignore major updates then they don't want any versions with a 3 in the major section e.g. 3.a0.

@amazimbe amazimbe force-pushed the amazimbe/update-git-ignore-semver branch from 403624e to 5c724a7 Compare September 27, 2024 15:58
@amazimbe amazimbe force-pushed the amazimbe/update-git-ignore-semver branch from 5c724a7 to 583a888 Compare September 29, 2024 22:46
lower_parts = version_parts.first(1) + [version_parts[1].to_i + 1] + ["a"]
upper_parts = version_parts.first(0) + [version_parts[0].to_i + 1]
lower_bound = ">= #{lower_parts.join('.')}"
upper_bound = "< #{upper_parts.join('.')}"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we have a unit test covering the regression that exposed this gap?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The issue was only in maven and I already have maven specific tests below

@jakecoffman
Copy link
Copy Markdown
Member

jakecoffman commented Sep 30, 2024

This makes sense, and I poked at it a little using Maven's feature to test versions:

$ java -jar ${MAVEN_HOME}/lib/maven-artifact-3.9.2.jar 3alpha 3.a0 3.a1 3.0 3.a 
Display parameters as parsed by Maven (in canonical form and as a list of tokens) and comparison result:
1. 3alpha -> 3-alpha; tokens: [3, [alpha]]
   3alpha == 3.a0
2. 3.a0 -> 3-alpha; tokens: [3, [alpha]]
   3.a0 < 3.a1
3. 3.a1 -> 3-alpha-1; tokens: [3, [alpha, [1]]]
   3.a1 < 3.0
4. 3.0 -> 3; tokens: [3]
   3.0 < 3.a
5. 3.a -> 3-a; tokens: [3, [a]]

So 3.a is a post 3.0 version, but 3.a0 and 3.a1 get normalized to alpha pre-releases. Interesting!

The change makes sense, but my understanding is Dependabot currently is working correctly now that the previous Maven Version was rolled back here. Will merging this directly into main break something or does this need to get merged into the new Maven Version class branch?

@amazimbe
Copy link
Copy Markdown
Contributor Author

amazimbe commented Oct 1, 2024

The change makes sense, but my understanding is Dependabot currently is working correctly now that the previous Maven Version was rolled back here. Will merging this directly into main break something or does this need to get merged into the new Maven Version class branch?

I agree, probably best to merge this into the new Maven version class although it wouldn't make any difference in the current Maven version implementation that treats any version with a letter e.g. 3.a, 3.a0 as a prerelease.

JamieMagee and others added 2 commits October 1, 2024 10:31
@amazimbe amazimbe requested a review from a team as a code owner October 1, 2024 09:31
@github-actions github-actions bot added the L: dotnet:nuget NuGet packages via nuget or dotnet label Oct 1, 2024
@amazimbe
Copy link
Copy Markdown
Contributor Author

amazimbe commented Oct 1, 2024

I'll close this one as I have merged it into #10704 which has the other new maven spec changes.

@amazimbe amazimbe closed this Oct 2, 2024
@amazimbe amazimbe deleted the amazimbe/update-git-ignore-semver branch January 16, 2025 09:33
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 L: java:maven Maven packages via Maven

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants