-
Notifications
You must be signed in to change notification settings - Fork 556
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
dev versions don't respect semver #1220
Comments
The reasoning behind this change is in #1198. Changing back to I think the model used by Go (which is actually not what you are proposing -- using
In addition, ideally nobody should be using the In addition (as I mentioned in #1221), you keep using the word "should" but the specification explicitly uses the word MAY -- meaning that us deviating from these recommendations is perfectly acceptable and requires no justification (with SHOULD we need to have some justification -- which I think we do -- but it would still be allowed). And, SemVer doesn't cover unreleased in-development "versions" of the spec. Go's behaviour exists because they needed to figure out a way to retro-fit their previous "versions shouldn't exist" mantra onto the Go module system (where everything is SemVer). It's a very clever hack, but it is still a hack. |
We do this in runc all the time. I don't see why you think it should be unusual, it's the opposite: we even implement some spec changes on runc to validate the changes we propose to the spec. To do that, we use an unreleased version of the spec. And unless we want to start releasing new versions of the spec every few months, I don't see how this can change (and I'm not sure releasing every few months will be enough to not need this). And I don't think adding more bureaucracy, like requiring a spec release to use it in runc, is going to help in any way.
If I don't use it in capital letters, don't take it as the RFC meaning. |
For what it's worth, I think the usage of "should" here is incorrect in the regular meaning as well. Titling the issue "dev versions don't respect semver" implies that this is a spec violation (if it's not a violation, there is nothing to "respect") and you used the phrase:
Even if it's not in capitals, the implication is that the spec is saying we should do something, when that is not the case (neither in the RFC sense of the word, nor in the regular sense of the word). |
What I meant with that is that the "pre-release" info is supposed to be used in the "-pre-release" part of the format, the "+build" thing is for the build information. We are not respecting that and I think we should. Simply because things that are different, under semver, evaluate to equal. Now that the |
My point was that "pre-release" in SemVer explicitly refers to alpha and beta releases, it doesn't refer to unreleased HEAD commits (what you are referring to as "pre-releases"). |
Closing for the same reason as stated here: #1221 (comment) |
When we switch to dev after a release, we are not respecting semver (as defined in semver.org version 2.0.0). The issue is that we just change the patchDev to "-dev", without changing the minor or anything else.
After release x.y.0, we almost always switch that field to "-dev". The issue is that according the spec, x.y.0-dev is a lower version than x.y.0. Therefore, we should increase some other component while switching to "-dev". Like x.y.1-dev.
This is not a big issue generally, as this dev versions are not included in releases, but it become an issue since we expose this in "runc features". This is incovenient, as using modules like https://pkg.go.dev/golang.org/x/mod/semver will not give the desired result when we compare versions (with our versioning, "1.0.2-dev" is greater than "1.0.2", but not with that module that correctly implements the spec).
Looking at the git history, we've almost always done that, the only exception is recent: 3f552ce (what is currently on main). The
+
sign should be used to indicate a build, which is incorrect here.In fact, if we look at the go.mod in runc release-1.1, it shows this that is in sync with what I propose in the next paragraph:
I propose the following:
Note there is nothing to do for crun (it implemented it in 1.8.6, that uses an rc runtime spec release all good in 1.8.7 and main)
cc @giuseppe @AkihiroSuda
The text was updated successfully, but these errors were encountered: