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

Tag upcoming release as v1.0.1-rc.1 to fix go modules / SemVer comparison #2399

Closed
thaJeztah opened this issue May 12, 2020 · 30 comments
Closed

Comments

@thaJeztah
Copy link
Member

(Assuming we don't reach "GA" yet)

I was updating a dependency in a project using go modules, and noticed that the runc dependency was downgraded from v1.0.0-rc10 to v1.0.0-rc6 (a version specified in another dependency). Scratching my head why v1.0.0-rc9 "stuck", but v1.0.0-rc10 was "ignored" by go modules, I had a 🤦 moment, and recalled that the runc pre-release tags are missing a dot separator.

This worked until -rc9, but broke when -rc10 was released;

Due to the missing separator in the release candidates:

v1.0.0-rc10
  • v (prefix)
  • 1 (major)
  • 0 (minor)
  • 0 (patch)
  • - (it's a pre-release)
  • rc10 (pre-release suffix)

So for v1.0.0-rc6, the pre-release suffix (which is just anything you name it), is rc6, and for v1.0.0-rc10, the suffix is rc10.

These suffices are compared alphabetically, not numerically, only releases with the same pre-release suffix are comparable.

So, when sorted alphabetically;

v1.0.0-rc1
v1.0.0-rc10
v1.0.0-rc2
v1.0.0-rc3
v1.0.0-rc4
v1.0.0-rc5
v1.0.0-rc6
v1.0.0-rc7
v1.0.0-rc8
v1.0.0-rc9

If a dot separator was used, the situation would be:

v1.0.0-rc.10
  • v (prefix)
  • 1 (major)
  • 0 (minor)
  • 0 (patch)
  • - (it's a pre-release)
  • rc (pre-release suffix)
  • 10 (pre-release version for rc suffix)

In which case, the order (oldest -> newest) would be:

v1.0.0-rc.1
v1.0.0-rc.2
v1.0.0-rc.3
v1.0.0-rc.4
v1.0.0-rc.5
v1.0.0-rc.6
v1.0.0-rc.7
v1.0.0-rc.8
v1.0.0-rc.9
v1.0.0-rc.10

You can see the behavior here;

I propose to skip v1.0.0, and tag the next -rc as v1.0.1-rc.0 or v1.0.1-rc.1, in which case version comparison works correct again

v1.0.0-rc1
v1.0.0-rc10
v1.0.0-rc2
v1.0.0-rc20
v1.0.1-rc.1
v1.0.1-rc.2
v1.0.1-rc.10
v1.0.1-rc.20
@cyphar
Copy link
Member

cyphar commented May 12, 2020

🤦 🤦 🤦 🤦 🤦 🤦 🤦 🤦

While this is a correct implementation of SemVer, and it does match the behaviour described in the spec (though it is quite strange since I've never seen 1.x.y-rc.z in any project, despite many projects using SemVer -- and sort -V does work properly):

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.

I don't think doing a 1.0.1-rc.1 release is reasonable. Honestly, I'd be happier to go back and rename all of the releases (or make rc10 rc90) than to skip over 1.0.0 over such a silly technicality. It also won't help us deal with 1.0.0-rc10.

@thaJeztah
Copy link
Member Author

It won't help with -rc10, but any project that would depend on v1.0.1-rc.1 would not be downgraded to v1.0.0-rc9 (or lower). I considered adding new tags for each existing rc (v1.0.0-rc1 -> v1.0.0-rc.1 and so on), but those new versions would be considered lower than rc1; https://semvercompare.azurewebsites.net/?version=1.0.0-rc1&version=1.0.0-rc2&version=1.0.0-rc10&version=1.0.0-rc.1&version=1.0.0-rc.2&version=1.0.0-rc.10

such a silly technicality

I'm not sure if it's a silly technicality at this point; projects getting an older version than expected looks like an actual problem to me.

@dims
Copy link
Contributor

dims commented May 12, 2020

I agree @thaJeztah it will trip up people if they are not paying close attention and hard to deal with using go modules as well.

@cyphar
Copy link
Member

cyphar commented May 12, 2020

My suggestion would be to just rename all of the releases -- since this only really affects go modules we just need to change the names of the tags (distributions don't care because they use sort -V order). Yeah, the version string in the binary will be a little bit different but nobody is using that from code. And we can make sure we avoid this problem in future releases.

I will also note that there are many projects (including Moby/Docker) which use -rcN so I'd suggest everyone else also make sure they avoid this pitfall. I guess we're the lucky ones that hit it because we've been on release candidates for 3 years at this point.

I'm not sure if it's a silly technicality at this point; projects getting an older version than expected looks like an actual problem to me.

I wasn't trying to say it wasn't a problem, just that releasing a post-1.0.0 release over this when compared to all of the other reasons why we've had to delay 1.0.0 does make this reason look like a "silly technicality" in comparison. I agree we should fix it.

@thaJeztah
Copy link
Member Author

I will also note that there are many projects (including Moby/Docker) which use -rcN

Moby/Docker uses CalVer, so it would not be compatible anyway (well, until you have a v17.12.x release, and go modules thinks that's SemVer 😓))

I know containerd follows the SemVer notation (https://github.com/containerd/containerd/releases/tag/v1.3.0-rc.0), although there's still the discrepancy between SemVer and "go" SemVer (which requires the v prefix, which is explicitly not a SemVer version, but that's another topic)

My suggestion would be to just rename all of the releases

You mean removing the existing tags, and replacing them? That sounds scary (and would break anyone that's already using on of these tags)

@cyphar
Copy link
Member

cyphar commented May 12, 2020

That sounds scary (and would break anyone that's already using on of these tags).

Ah, right. Well then there's always the nuclear option -- tag 1.0.0-rc90 right now as being the same as 1.0.0-rc10 and then make rc11 be rc91. That way we don't need to skip over 1.0.0 and we don't touch the old releases -- it's just that the rc10 release becomes defunct as everyone uses the rc90 release instead.

@thaJeztah
Copy link
Member Author

tbh, the diff between v1.0.0-rc1 and v1.0.0-rc10 is so big, that there's no relation whatsoever between them (it'd almost qualify as v2.0.0) 😂

tag 1.0.0-rc90 right now as being the same as 1.0.0-rc10 and then make rc11 be rc91.

That should work; assuming we're confident that we don't reach -rc100 😬 as that would be lower than -rc2 again

https://semvercompare.azurewebsites.net/?version=1.0.0-rc1&version=1.0.0-rc2&version=1.0.0-rc9&version=1.0.0-rc10&version=1.0.0-rc90&version=1.0.0-rc100

We're not alone in this, there's a reason Microsoft never released a Windows 9 😉

@cyphar
Copy link
Member

cyphar commented May 12, 2020

tbh, the diff between v1.0.0-rc1 and v1.0.0-rc10 is so big, that there's no relation whatsoever between them (it'd almost qualify as v2.0.0)

Yeah, this is something we've talked about before. Ultimately we've already broken SemVer more than a little -- but the only thing that is super critical is that runc 1.0.0 must be spec-compliant with the runtime-spec. After that we can do proper release management rather than the situation we're in now.

That should work; assuming we're confident that we don't reach -rc100 grimacing as that would be lower than -rc2 again

That's when we roll out rc990 😆 😿. But I agree, let's hope it never comes to that.

@cyphar
Copy link
Member

cyphar commented May 12, 2020

@opencontainers/runc-maintainers Are there any objections to the above plan? I've already got an -rc90 tag ready and waiting. Here is the description:

v1.0.0-rc90

This release is *identical* to v1.0.0-rc10.

The purpose of this release is to resolve an issue with our versioning
scheme (in particular, the format we've used under SemVer means that the
"-rcNN" string suffix is sorted lexicographically rather than in the
classic `sort -V` order).

Because we cannot do a post-1.0 release yet, this is a workaround to
make sure that systems such as Go modules correctly update to the latest
runc release. See [1] for more details.

The next release (which would've originally been called -rc11) will be
1.0.0-rc91. I'm sorry.

[1]: https://github.com/opencontainers/runc/issues/2399

Signed-off-by: Aleksa Sarai <asarai@suse.de>

I don't think we need a new vote for this since it's just an alias for an existing release, but I don't want to go push a tag without at least one other person saying it's not a bad idea.

@dims
Copy link
Contributor

dims commented May 12, 2020

rc990 LOL!!! good one

@AkihiroSuda
Copy link
Member

😕

Can we just switch away from "rc" to another string. e.g. v1.0.0-milestone.11 or v1.0.0-rtm.11

@AkihiroSuda
Copy link
Member

Or just give up the v1.0.0 dream and release runc v11.
This might be the best for reflecting the reality.

@AkihiroSuda
Copy link
Member

Or if we can release the v1.0.0 in a month, I think we can just close this issue and call it a day.

@thaJeztah
Copy link
Member Author

Can we just switch away from "rc" to another string. e.g. v1.0.0-milestone.11 or v1.0.0-rtm.11

milestone would be < rc (alphabetically). I was thinking along those lines as well, but would be equally confusing I think. That's why my first suggestion was to skip v1.0.0 and use v1.0.1-rc.X.

@cyphar
Copy link
Member

cyphar commented May 13, 2020

Or if we can release the v1.0.0 in a month, I think we can just close this issue and call it a day.

The issue is our existing release (rc10) is already causing this issue. At the very least we should have an rc90 alias purely to stop people from downgrading their runc go modules.

@kolyshkin
Copy link
Contributor

kolyshkin commented May 13, 2020

rtm looks good. I.e.

v1.0.0-rc9 -> v1.0.0-rtm.9
v1.0.0-rc10 -> v1.0.0-rtm.10
...

rc90 is OKish, too, but I like rtm more since we don't have to jump the number.

@AkihiroSuda
Copy link
Member

But it is not really "RTM" ¯_(ツ)_/¯

@cyphar
Copy link
Member

cyphar commented May 13, 2020

I mean, let's just admit it and call it 1.0.0-runc.11 ;)

@kolyshkin
Copy link
Contributor

We can call it 1.0.0-sigma.11 without admitting anything :)

@thaJeztah
Copy link
Member Author

v1.0.0-real-rc.11, v1.0.0-really-almost-there.1 or v1.0.0-remember-to-add-a-.11 should all work 😂🤣

@cyphar
Copy link
Member

cyphar commented May 13, 2020

My vote is for v1.0.0-really-sorry-but-still-an-rc.11. In all seriousness, I'll bring this up at the OCI call tomorrow.

@thaJeztah
Copy link
Member Author

Thanks @cyphar! (Just trying to keep a bit of "fun" in the situation)

@AkihiroSuda
Copy link
Member

What's the conclusion?

@mrunalp
Copy link
Contributor

mrunalp commented May 28, 2020

I propose that we cut one more rc after #2229 , let other projects soak it, fix bugs if any and then call it 1.0.0 after a couple of weeks.

@cyphar
Copy link
Member

cyphar commented May 28, 2020

Right, but the issue is we need to switch to proper SemVer /somehow/ for our current releases because they're breaking downstream projects. My vote is still for rc90 because it doesn't require anyone to change how they're dealing with the rc releases (it just looks like we've done 80 of them in one go).

@AkihiroSuda AkihiroSuda added this to the 1.0.0-rc90 (tentative) milestone May 28, 2020
@mrunalp
Copy link
Contributor

mrunalp commented May 28, 2020

Agree that rc90 is least disruptive for the next release.

@cyphar
Copy link
Member

cyphar commented Jun 2, 2020

I've pushed v1.0.0-rc90 as a tag for v1.0.0-rc10 (a reminder that the the issue is that rc10 already has triggered this problem). So v1.0.0-rc91 will be v1.0.0-rc11.

@cyphar cyphar closed this as completed Jun 2, 2020
@AkihiroSuda
Copy link
Member

@cyphar could you add release note (#2399 (comment)) ?

@cyphar
Copy link
Member

cyphar commented Jun 2, 2020

Yup, I was editing them just as you posted your comment. 😉 https://github.com/opencontainers/runc/releases/tag/v1.0.0-rc90

@thaJeztah
Copy link
Member Author

It's a SameVer release

anton-johansson added a commit to anton-johansson/kubernetes-the-right-way that referenced this issue Aug 5, 2020
Due to some versioning issues, the version that is currently use for `runc`
has been rebuilt, which causes a new checksum.

Info:
> NOTE: For those who are confused by the massive version jump (rc10
> to rc91), this was done to avoid issues with SemVer and lexical
> comparisons -- there haven't been 90 other release candidates. Please
> also note that runc 1.0.0-rc90 is identical to 1.0.0-rc10. See #2399
> for more details.

See more here:
opencontainers/runc#2399
amimof pushed a commit to amimof/kubernetes-the-right-way that referenced this issue Aug 11, 2020
* Bump Kubernetes to v1.16.13

* Use Xenial image in Travis builds

* Add `sudo` to centos containers

* Update the incorrect checksum for `runc`

Due to some versioning issues, the version that is currently use for `runc`
has been rebuilt, which causes a new checksum.

Info:
> NOTE: For those who are confused by the massive version jump (rc10
> to rc91), this was done to avoid issues with SemVer and lexical
> comparisons -- there haven't been 90 other release candidates. Please
> also note that runc 1.0.0-rc90 is identical to 1.0.0-rc10. See #2399
> for more details.

See more here:
opencontainers/runc#2399

Co-authored-by: Amir Mofasser <amir.mofasser@gmail.com>
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