fix VersionSetSpecifier calculation for versions with pre-release identifiers #6492
Conversation
tomerd
requested review from
abertelrud,
neonichu and
MaxDesiatov
as code owners
tomerd
force-pushed
the
fix/beta-versions
branch
4 times, most recently
from
0924330
to
7674645
Compare
tomerd
changed the title
…ntitiers motivation: some version calculations were incorrect when dealing with pre-release identitiers, leading to dependency resolution errors changes: * fix Version::nextPatch() to take into pre-release identitiers into account * add tests
tomerd
force-pushed
the
fix/beta-versions
branch
from
7674645
to
8dc83fb
Compare
|
@swift-ci smoke test |
|
|
||
| state.derive(unsatisfiedTerm.inverse, cause: incompatibility) | ||
| self.delegate?.derived(term: unsatisfiedTerm.inverse) |
There was a problem hiding this comment.
Is it meaningful that the order changed here?
There was a problem hiding this comment.
no, it was part of a change I reverted (I made state.derive throwing so wanted to call delegate only if succeeded), but should not have impact - the delegate is used for logging etc and in any case should be called after the fact not before
| XCTAssertEqual(VersionSetSpecifier.exact("2.0.0-beta").difference(.exact("1.0.0-beta")), .exact("2.0.0-beta")) | ||
|
|
||
| XCTAssertEqual(VersionSetSpecifier.range("1.0.0-beta"..<"1.0.0-beta").difference(.range("1.0.0-beta"..<"1.0.0-beta")), .empty) | ||
| XCTAssertEqual(VersionSetSpecifier.range("2.0.0-beta"..<"2.0.0-beta").difference(.range("1.0.0"..<"2.0.0")), .range("2.0.0-beta"..<"2.0.0-beta")) |
There was a problem hiding this comment.
Possibly not a new thing, but I wonder whether this should just be .empty?
There was a problem hiding this comment.
I think this is the same as
XCTAssertEqual(VersionSetSpecifier.range("2.0.0"..<"2.0.0").difference(.range("3.0.0"..<"4.0.0")), .range("2.0.0"..<"2.0.0"))
There was a problem hiding this comment.
I guess we simply don't touch the first operand if the second one is not a subset of it. It doesn't make a practical difference since I have previously fixed equality in 96ed023, so the two are equal, but could be interesting for readability maybe. In any case, not relevant for this PR since it is pre-existing.
…ntifiers (apple#6492) motivation: some version calculations were incorrect when dealing with pre-release identifiers, leading to dependency resolution errors changes: * fix Version::nextPatch() to take into pre-release identifiers into account * add tests
motivation: some version calculations were incorrect when dealing with pre-release identifiers, leading to dependency resolution errors
changes: