Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How is the latest version determined? #185

Closed
gabro opened this issue Jul 21, 2015 · 32 comments
Closed

How is the latest version determined? #185

gabro opened this issue Jul 21, 2015 · 32 comments

Comments

@gabro
Copy link

gabro commented Jul 21, 2015

The latest version of ReactiveCocoa is 3.0.0-RC.1 and that's also the latest Podspec pushed to the Spec repo.

However CocoaPods picks up 3.0.0-alpha.1 as the latest version, as also shown on https://cocoapods.org/?q=ReactiveCocoa

This causes some issues, especially with pods that go through a long beta period such as this one.

How's the latest version determined?

@orta
Copy link
Member

orta commented Jul 21, 2015

Looks like ReactiveCocoa isn't using semantic versioning so the ordering isn't working correctly for them.

screen shot 2015-07-21 at 15 29 37

@orta orta closed this as completed Jul 21, 2015
@gabro
Copy link
Author

gabro commented Jul 21, 2015

Thanks @orta for the quick answer, if I get this right, is it because the patch version is missing?

@orta
Copy link
Member

orta commented Jul 21, 2015

Yep 👍

@segiddins
Copy link
Member

Actually, this might be a bug in our version sorting, since I'm not sure it's supposed to be case sensitive?

@kylef
Copy link
Contributor

kylef commented Jul 21, 2015

@segiddins not the case, 3.0.0-RC.1 isn't actually the latest version of ReactiveCocoa, 3.0-RC.1 is the latest.

3.0.0 (alpha 1) is newer than 3.0 (rc1) instead.

@gabro
Copy link
Author

gabro commented Jul 21, 2015

@kylef you're correct. I asked the ReactiveCocoa maintainers to add the patch number so that future releases will be fully compliant with semver.

To be fair, the ReactiveCocoa versions seem to be ordered correctly (the patch number is missing in all the 3.0 pre-releases) but the 3.0.0-alpha.1 breaks the ordering.

In the meantime, is it possible to either amend or remove the 3.0.0-alpha.1 spec from the repo? Apart from being wrongly picked as the current version, it's pointing to a fork rather than the official repo.

@segiddins
Copy link
Member

@kylef the version class should be padding the missing 0, so that shouldn't be it

@floere
Copy link
Member

floere commented Jul 21, 2015

@kylef These two are treated as equals, and sort order between them is not defined.

@floere
Copy link
Member

floere commented Jul 21, 2015

Uppercase and lowercase are both allowed on pre-release versions, and are compared lexically in ASCII sort order. See this excerpt from http://semver.org:

Precedence for two pre-release versions with the same major, minor, and patch version MUST be determined by comparing each dot separated identifier from left to right until a difference is found as follows: identifiers consisting of only digits are compared numerically and identifiers with letters or hyphens are compared lexically in ASCII sort order. Numeric identifiers always have lower precedence than non-numeric identifiers. A larger set of pre-release fields has a higher precedence than a smaller set, if all of the preceding identifiers are equal. Example: 1.0.0-alpha < 1.0.0-alpha.1 < 1.0.0-alpha.beta < 1.0.0-beta < 1.0.0-beta.2 < 1.0.0-beta.11 < 1.0.0-rc.1 < 1.0.0.

So an uppercase RC is < to a lowercase alpha (see an ASCII table).

@floere
Copy link
Member

floere commented Jul 21, 2015

So I assume this will be resolved by using 3.0.0-rc.1 (lowercase rc instead of uppercase RC).

@segiddins
Copy link
Member

cool!

@gabro
Copy link
Author

gabro commented Jul 21, 2015

@floere if I get this right, considering what @segiddins said about padding of the patch version, then shouldn't

3.0.0-alpha.1 < 3.0-beta.9?

@floere
Copy link
Member

floere commented Jul 21, 2015

@gabro Not 100% sure about the padding (I haven't seen that particular part of the source code) – but assuming he's right, then yes, that would evaluate to true. If not, we need to look into the patch version comparison code.

@gabro
Copy link
Author

gabro commented Jul 21, 2015

@floere Well 3.0-beta.9 spec is out and 3.0.0-alpha.1 gets picked up as the latest version.
So either the padding is not really in place, or something is wrong with the ordering.

@segiddins segiddins reopened this Jul 21, 2015
@floere
Copy link
Member

floere commented Jul 21, 2015

@gabro Thanks for the feedback!

How it works is much simpler than I thought. It splits the version string into pieces of numeric or A-Za-z pieces.Then it moves through them in order, and compares each piece.

So 1.0.z will lose against 1.0.0.a. Solution: get onto semantic versioning 2.0.0 asap 🚀

@floere
Copy link
Member

floere commented Jul 21, 2015

P.S: Code is in e.g. Ruby 2.3.0 in file lib/ruby/site_ruby/2.3.0/rubygems/version.rb.

@gabro
Copy link
Author

gabro commented Jul 22, 2015

@floere oh I see! That makes sense, thanks!
To be fair, the ReactiveCocoa guys are consistently leaving the patch number out, so the ordering should be correct. It's definitively the alpha with the "extra" patch number that is messing with things, but I'll get it fixed by pushing the new rc version as soon as CocoaPods/CocoaPods#3855 is fixed!

Thanks again for the prompt support 👍

@floere
Copy link
Member

floere commented Jul 23, 2015

@gabro Oh yeah! I did not want to be unfair :) Just wanted to point out that if you add a patch number, it will always win against a version without. So once you move onto patch numbers, you need to use them … forever ;)
Cheers, glad to help! 🚀

@alloy
Copy link
Member

alloy commented Jul 23, 2015

How it works is much simpler than I thought. It splits the version string into pieces of numeric or A-Za-z pieces.Then it moves through them in order, and compares each piece.

This seems very unsemantic, though, we could fix this.

So 1.0.z will lose against 1.0.0.a. Solution: get onto semantic versioning 2.0.0 asap 🚀

Is that really a semver v2 specification?

@gabro
Copy link
Author

gabro commented Jul 23, 2015

@alloy it doesn't seem like an explicit semver specification, although the comparison algorithm is well detailed

Precedence for two pre-release versions with the same major, minor, and patch version MUST be determined by comparing each dot separated identifier from left to right until a difference is found as follows

The key here is that they (rightfully) assume major, minor and patch versions to be present in the identifier. I personally think padding the missing ones before performing the comparison wouldn't hurt.

@gabro
Copy link
Author

gabro commented Jul 23, 2015

By the way, @segiddins is correct: the padding is in place when accessing the major, minor or patch accessors provided by Pod::Version

However those accessors are not used by the <=> implementation, which is provided by the superclass Gem::Version.

One would need to override <=> in Pod::Version and explicitly compare major, minor and patch before moving onto comparing the remaining tokens.
Probably that would also be more compliant with the semver v2 specification above.

@floere
Copy link
Member

floere commented Jul 23, 2015

Is that really a semver v2 specification?

@alloy No, hence the "get onto it". What I did was explain how Gem::Version (that we use) works. Not how SemVer 2.0 works.

@gabro
Copy link
Author

gabro commented Jul 23, 2015

@floere I think the issue right now is that CocoaPods allows for pods with a version not compliant with the semver specification, and then treats them as semver (for instance when comparing them)

So, I think two sensible alternatives would be:

  • either reject non-strictly-semver versions, e.g. with a missing patch or minor numbers (I believe there's already a check in place, but it's probably not so strict)
  • or allow for versions with missing patch/minor numbers and compensate explicitly when manipulating them

@floere
Copy link
Member

floere commented Jul 23, 2015

@gabro I know – we explicitly did not design it with SemVer in mind and left it open, but then started using Gem::Version. This can work if Pod owners stick to a scheme – and I think this is already sensible.
Your first point – that ship has sailed. Your second point – we already allow for these versions, however, mixing the schemes results in undefined behaviour (like in this case).

@alloy
Copy link
Member

alloy commented Jul 23, 2015

Rejecting is not an option. 1 is the semantically the same as 1.0.0.0.0.0.0000000.0.0 so we should just treat it like that.

@gabro
Copy link
Author

gabro commented Jul 23, 2015

Yes, I agree rejecting doesn't seem like a viable option right now and I very much prefer option 2 as well.

If 1-rc is a accepted as a valid version number, then it should be greater than 1.0.0-alpha, so the current sorting implementation has to be fixed to take this into account.

My ruby is quite rusty, but I can I can give it a shot during this weekend, if you guys agree this is a desirable change in behavior.

@alloy
Copy link
Member

alloy commented Jul 23, 2015

@floere I guess I don’t understand what you were saying then at all 😿

@alloy
Copy link
Member

alloy commented Jul 23, 2015

@gabro That would be awesome, thanks! So yeah, trailing zeroes in segments (before the prerelease segment) that the other version does not have can be ignored.

@floere
Copy link
Member

floere commented Jul 23, 2015

@alloy No worries 🌳 I was just explaining what is currently happening and why patch versions always (well, almost) win over non-patch versions.
@gabro For writing tests for this functionality, here's the complete list of all existing versions (including funky cases) from the search engine: http://search.cocoapods.org/api/v1/pods.facets.json?include=version&only=version&counts=false

One thing… currently, the order is defined 3.0.0 wins over 3.0. If you have 3.0 <=> 3.0.0 return 0, then the order is pretty much random (or random seeming). Just a note. In this case, it will help, but I am unsure about the implications (e.g. in Trunk) of having different version strings that mean the same thing.

@gabro
Copy link
Author

gabro commented Jul 23, 2015

@floere right, do we want to set an explicit order for "synonyms"?

It could be something as simple as having

return this <=> other

at the end, as opposed to the current

return 0

That would get us

2.0.0 < 3-alpha < 3.0-alpha < 3.0.0-alpha < 4

@segiddins
Copy link
Member

Yes, sorting needs to be stable (and these changes would need to go into CocoaPods-core)

@gabro
Copy link
Author

gabro commented Jul 27, 2015

CocoaPods/Core#256

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants