puppetlabs / puppetlabs/semantic_puppet

Version range union containing prerelease versions does not work

Open
#55 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
Ruby
Stars
2
Forks
29
Avg merge
21d 4h
Merged PRs (30d)
1

Description

Describe the Bug

Semantic puppet version ranges do not handle prereleases correctly.

Expected Behavior

Given a version range '>=1.0.0 || >=2.0.0-rc0' I would expect both 1.0.0 and 2.0.0-rc0 to be included in the range. But that is not the observed behavior.

Steps to Reproduce

Apply the fix from https://github.com/puppetlabs/semantic_puppet/pull/54

Run the following. I would expect true to be printed both times:

❯ cat test.rb 
require 'semantic_puppet'
range = SemanticPuppet::VersionRange.parse('>=1.0.0 || >=2.0.0-rc0')
puts range.include?(SemanticPuppet::Version.parse('1.0.0'))
puts range.include?(SemanticPuppet::Version.parse('2.0.0-rc0'))


❯ bundle exec ruby test.rb
true
false

SemanticPuppet generally follows the npm implementation of semver. Using that nomenclature, >=1.0.0 is a comparator set containing a single element. Same for >=2.0.0-rc0. And they are joined together using || to create a union. npm says https://github.com/npm/node-semver

A range is composed of one or more comparator sets, joined by ||. A version matches a range if and only if every comparator in at least one of the ||-separated comparator sets is satisfied by the version.

As a result, both 1.0.0 and 2.0.0-rc0 should be included in the range.

The bug is that SemanticPuppet attempts to merge ranges to reduce overlaps, for example >=1.0.0 || >=2.0.0 is redundant as the former includes the latter. So it attempts to merge the two ranges (comparator sets) https://github.com/puppetlabs/semantic_puppet/blob/66f73a5b047d18660f742431173fda3daac6dd91/lib/semantic_puppet/version_range.rb#L247 and drops the second range >=2.0.0 However, it shouldn't do that if the second range contains a prerelease >=2.0.0-rc0 This is because

If a version has a prerelease tag (for example, 1.2.3-alpha.3) then it will only be allowed to satisfy comparator sets if at least one comparator with the same [major, minor, patch] tuple also has a prerelease tag. ... prerelease versions frequently are updated very quickly, and contain many breaking changes that are (by the author's design) not yet fit for public consumption. Therefore, by default, they are excluded from range-matching semantics.

It also behaves inconsistently if the order of ranges is reversed >= 2.0.0-rc0 || >= 1.0.0

Environment

❯ bundle exec ruby -rsemantic_puppet -e "puts SemanticPuppet::VERSION"        
1.1.0

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with lib/semantic_puppet/version_range.rb around the range-merging logic linked in the issue, then run the supplied bundle exec ruby reproduction. The change is done when both versions are included for the shown union and reversing the comparator order does not produce inconsistent results.

Written by the indexing model from the issue text.

Assessment

Tech stack
ruby
Domain
tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.