Gitlab::VersionInfo cannot compare some suffixes, and its hash disagrees with eql?

Summary

Gitlab::VersionInfo in gems/gitlab-utils has two defects around the parsed version suffix:

  1. #<=> returns nil when two versions with the same major, minor and patch have suffixes whose first differing parts are a word and a number, for example 16.0.0-rc1 and 16.0.0-1. sort, max, < and > then raise ArgumentError.
  2. #eql? compares the parsed suffix, but #hash uses the raw text, so versions that are eql? can have different hashes. Array#uniq, Hash keys and group_by then treat equal versions as different.

Steps to reproduce

  1. cd gems/gitlab-utils && bundle install

  2. Save this as repro.rb and run bundle exec ruby -r active_support/core_ext/object/blank -r ./lib/gitlab/version_info repro.rb:

    a = Gitlab::VersionInfo.parse('16.0.0-rc1', parse_suffix: true)
b = Gitlab::VersionInfo.parse('16.0.0-1', parse_suffix: true)
p(a <=> b)
begin; p [a, b].sort; rescue => e; p e; end
c = Gitlab::VersionInfo.parse('1.0.1-rc1', parse_suffix: true)
d = Gitlab::VersionInfo.parse('1.0.1.pre.rc1', parse_suffix: true)
p [c.eql?(d), c.hash == d.hash, [c, d].uniq.size]

Example Project

Not applicable: the defect is in a library class, and the script above reproduces it from a checkout of this repository.

What is the current bug behavior?

On master (f0464c0449f) the script prints:

nil
#<ArgumentError: comparison of Gitlab::VersionInfo with Gitlab::VersionInfo failed>
[true, false, 2]

a <=> b is nil and sorting raises. c.eql?(d) is true, but the hashes differ and uniq keeps both.

What is the expected correct behavior?

Any two valid versions get an order, so sorting never raises. Versions that are eql? have the same hash, so uniq keeps one of them.

Relevant logs and/or screenshots

The suffix is parsed into an Array of Integers and Strings (version_info.rb#L87-91): 16.0.0-rc1 gives ["pre", "rc", 1] and 16.0.0-1 gives ["pre", 1]. Array#<=> returns nil when it meets "rc" against 1.

Gitlab::Ci::RunnerReleases parses runner release names with parse_suffix: true, sorts them and takes the max per minor version (runner_releases.rb#L55, #L82-93), so two releases of the same version with such suffixes would make that raise. I have not seen it happen with real runner release names.

Output of checks

Not from an installation: the output above comes from the script, run in a checkout of master with Ruby 3.3.12 and 3.4.10.

Possible fixes

  • version_info.rb#L69: compare the suffix parts with their kind first, so that a word sorts before a number, as Gem::Version does.
  • version_info.rb#L97-99: build the hash from the major, minor and patch numbers and the parsed suffix instead of to_s.
Edited by 🤖 GitLab Bot 🤖