Add support for versions using git revision suffixes for Maven and Gradle

[yeikel]

What are you trying to accomplish?

Adds support for detecting versions that include embedded Git commits in both short and long format.

The updated logic correctly parses version strings with Git revision suffixes, as used by some Maven projects (e.g., the Jenkins plugins organization). Examples: 2.9.2-29.v717aac953ff3, 2.9.3-29.v717aac953ff3

This is a follow-up for #13818 #13999

Fixes #14102

How will you know you've accomplished your goal?

Both the existing and new specs pass. I was able to reproduce the issue with the tests, and the new code resolves it

Note to reviewers

• The change may look more intimidating than it actually is due to the large diff.
• A bulk of the effort went into adding as many scenarios/specs as possible.
• As the method grew too complex, I refactored it to prevent another RuboCop finding/muting RuboCop
• A big chunk of the changes are documentation improvements

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.

[jglick]

Would suggest a test checking explicitly the case most recently observed to fail, that https://github.com/jenkinsci/plugin-pom/releases/tag/6.2108.v08c2b_01b_cf4d is not considered superseded by https://github.com/jenkinsci/plugin-pom/releases/tag/6.2122.v70b_7b_f659d72 which I suppose was a regression from #13818. None of the examples here seem to include a prefix prior to the revision count, as used for example in https://github.com/jenkinsci/plugin-pom/blob/623345749f5978fee57ab0e670f1f980dd5b05cd/pom.xml#L57 or even https://github.com/jenkinsci/jjwt-api-plugin/blob/b2c204d6cd36e8597dddade93d7cc95f12074943/pom.xml#L16

[yeikel]

Thank you for the feedback @jglick. I updated the tests cases to consider that format as well as others I could find. Could you please review and let me know if I am missing any scenario?

Thanks in advance for your time and understanding

[kbukum1]

@jglick @timja Thanks for your feedback. I believe I’ve updated the changes to cover all the scenarios you mentioned. The main update in the recent commits is that the digest is now treated as irrelevant, allowing updates to proceed even if only one artifact includes a digest.

@kbukum1 Given the impact (see #14102, affecting many repositories in the Jenkins organization), I would appreciate your review to help determine if we can move forward with releasing this change.

Thanks @yeikel . I will check it hopefully today otherwise next week after Tuesday.

[yeikel]

@jglick @timja Thanks for your feedback. I believe I’ve updated the changes to cover all the scenarios you mentioned. The main update in the recent commits is that the digest is now treated as irrelevant, allowing updates to proceed even if only one artifact includes a digest.
@kbukum1 Given the impact (see #14102, affecting many repositories in the Jenkins organization), I would appreciate your review to help determine if we can move forward with releasing this change.

Thanks @yeikel . I will check it hopefully today otherwise next week after Tuesday.

@jglick @timja Thanks for your feedback. I believe I’ve updated the changes to cover all the scenarios you mentioned. The main update in the recent commits is that the digest is now treated as irrelevant, allowing updates to proceed even if only one artifact includes a digest.
@kbukum1 Given the impact (see #14102, affecting many repositories in the Jenkins organization), I would appreciate your review to help determine if we can move forward with releasing this change.

Thanks @yeikel . I will check it hopefully today otherwise next week after Tuesday.

Hey @kbukum1 just pinging to see if you got a chance to review this?

Thanks

[kbukum1]

@jglick @timja Thanks for your feedback. I believe I’ve updated the changes to cover all the scenarios you mentioned. The main update in the recent commits is that the digest is now treated as irrelevant, allowing updates to proceed even if only one artifact includes a digest.
@kbukum1 Given the impact (see #14102, affecting many repositories in the Jenkins organization), I would appreciate your review to help determine if we can move forward with releasing this change.

Thanks @yeikel . I will check it hopefully today otherwise next week after Tuesday.

@jglick @timja Thanks for your feedback. I believe I’ve updated the changes to cover all the scenarios you mentioned. The main update in the recent commits is that the digest is now treated as irrelevant, allowing updates to proceed even if only one artifact includes a digest.
@kbukum1 Given the impact (see #14102, affecting many repositories in the Jenkins organization), I would appreciate your review to help determine if we can move forward with releasing this change.

Thanks @yeikel . I will check it hopefully today otherwise next week after Tuesday.

Hey @kbukum1 just pinging to see if you got a chance to review this?

Thanks

Hi @yeikel , sorry I have some workload and I really couldn't review that. I am hoping by mid of next week I will do review.

[yeikel]

@kbukum1 any chance you can get to it this week?

[kbukum1]

@yeikel

Nice work! Can you evaluate the following additional specs and if it is valid, it will be great if we can cover them. Other than this it looks good to me.

1. Valid length but invalid hex characters:

context "when suffix has valid length but invalid hex characters" do
  let(:dependency_version) { "100-vghijklm" }
  let(:comparison_version) { "200-vhijklmn" }

  it { is_expected.to be false }
end

2. All-numeric string (no a-f letters):

context "when suffix is all-numeric and could be confused for a SHA" do
  let(:dependency_version) { "100.v1234567" }
  let(:comparison_version) { "200.v7654321" }

  it { is_expected.to be false }
end

3. SHA exceeds max length (41+ chars):

context "when git SHA exceeds maximum length (41+ chars)" do
  let(:dependency_version) { "100.va1b2c3d4e5f6789012345678901234567890ab" }
  let(:comparison_version) { "200.vb2c3d4e5f6789012345678901234567890abc" }

  it { is_expected.to be false }
end

4. Reverse direction — current has SHA, candidate is plain semver:

context "when current has git SHA and candidate is standard semver" do
  let(:dependency_version) { "1.2.3.va1b2c3d" }
  let(:comparison_version) { "1.2.4" }

  it { is_expected.to be true }
end

[yeikel]

@yeikel

Nice work! Can you evaluate the following additional specs and if it is valid, it will be great if we can cover them. Other than this it looks good to me.

1. Valid length but invalid hex characters:

context "when suffix has valid length but invalid hex characters" do
  let(:dependency_version) { "100-vghijklm" }
  let(:comparison_version) { "200-vhijklmn" }

  it { is_expected.to be false }
end

2. All-numeric string (no a-f letters):

context "when suffix is all-numeric and could be confused for a SHA" do
  let(:dependency_version) { "100.v1234567" }
  let(:comparison_version) { "200.v7654321" }

  it { is_expected.to be false }
end

3. SHA exceeds max length (41+ chars):

context "when git SHA exceeds maximum length (41+ chars)" do
  let(:dependency_version) { "100.va1b2c3d4e5f6789012345678901234567890ab" }
  let(:comparison_version) { "200.vb2c3d4e5f6789012345678901234567890abc" }

  it { is_expected.to be false }
end

4. Reverse direction — current has SHA, candidate is plain semver:

context "when current has git SHA and candidate is standard semver" do
  let(:dependency_version) { "1.2.3.va1b2c3d" }
  let(:comparison_version) { "1.2.4" }

  it { is_expected.to be true }
end

Thank you for the feedback @kbukum1. I applied it in the latest revision: https://github.com/yeikel/dependabot-core/blob/d11b4a3ad3ed9f9ac0bad8e382521781eb465aa8/maven/spec/dependabot/maven/shared/shared_version_finder_spec.rb#L641-L667

[kbukum1]

@yeikel, I checked with copilot and seems like the cause is matches_dependency_version_type?. Can you check the following and confirm the change and accordingly you can apply proper fix the way you think is right.

Analyse:
There are 4 CI failures across the Maven and Gradle test suites, all seems to be from the same root cause:

The "RELEASE802" (non-standard version) test case is now returning nil instead of "23.0".

When dependency_version is "RELEASE802", your matches_dependency_version_type? method rejects every candidate:

1. pre_release_compatible?("RELEASE802", "23.0") → false
2. upgrade_to_stable?("RELEASE802", "23.0") → false (not recognized as pre-release/snapshot)
3. suffix_compatible?("RELEASE802", "23.0") → false (extract_version_suffix returns a non-nil suffix for "RELEASE802" but nil for "23.0", so they don't match)

The fix should ensure that unrecognized/non-standard version formats don't block upgrades. A couple of options:

• Add a fallback in matches_dependency_version_type? — if the current version doesn't match any known pattern, return true to allow the upgrade.
• Or adjust extract_version_suffix so versions like "RELEASE802" return nil (no meaningful suffix), matching "23.0".

One more thing on test coverage: Consider adding a test for a suffix with valid SHA length but invalid hex characters (e.g., "1.0.0-vghijklm") to confirm git_sha? rejects it correctly. The GIT_COMMIT regex already handles this with the [0-9a-f] character class, but having an explicit spec would prevent future regressions.

[yeikel]

@yeikel, I checked with copilot and seems like the cause is matches_dependency_version_type?. Can you check the following and confirm the change and accordingly you can apply proper fix the way you think is right.

Analyse: There are 4 CI failures across the Maven and Gradle test suites, all seems to be from the same root cause:

The "RELEASE802" (non-standard version) test case is now returning nil instead of "23.0".

When dependency_version is "RELEASE802", your matches_dependency_version_type? method rejects every candidate:

1. `pre_release_compatible?("RELEASE802", "23.0")` → `false`

2. `upgrade_to_stable?("RELEASE802", "23.0")` → `false` (not recognized as pre-release/snapshot)

3. `suffix_compatible?("RELEASE802", "23.0")` → `false` (`extract_version_suffix` returns a non-nil suffix for `"RELEASE802"` but `nil` for `"23.0"`, so they don't match)

The fix should ensure that unrecognized/non-standard version formats don't block upgrades. A couple of options:

* Add a fallback in `matches_dependency_version_type?` — if the current version doesn't match any known pattern, return `true` to allow the upgrade.

* Or adjust `extract_version_suffix` so versions like `"RELEASE802"` return `nil` (no meaningful suffix), matching `"23.0"`.

One more thing on test coverage: Consider adding a test for a suffix with valid SHA length but invalid hex characters (e.g., "1.0.0-vghijklm") to confirm git_sha? rejects it correctly. The GIT_COMMIT regex already handles this with the [0-9a-f] character class, but having an explicit spec would prevent future regressions.

Thank you @kbukum1. I updated it based on your feedback and added these additional tests cases to the test suite to protect ourselves from regressions as you suggested

[kbukum1]

LGTM