velum icon indicating copy to clipboard operation
velum copied to clipboard

Use a limited subset of characters for version

Open davidcassany opened this issue 6 years ago • 10 comments

https://github.com/kubic-project/velum/blob/b67a614f1900c58674e0de37797339511f040afe/packaging/suse/make_spec.sh#L33

@MaximilianMeister This line introduces a quite complex version of the package. I am wondering if we could simplify it. I have seen + is used in other openSUSE package versions, thus I am wondering if this format of <version>+gir_r<revision_num>_<commit> is some kind of policy. If not, couldn't we make it more simple? something like <version>_git_<short commit hash>?

All this came across while trying to tag the velum image with the velum package version. Umoci only supports a limited subset of characters for tags.

In

https://github.com/openSUSE/umoci/blob/8b89fabfac6c176b95078cb106c2704303564e6d/cmd/umoci/utils_ux.go#L30

umoci states the limited character set available for image names and tags. @cyphar is there any mandatory reason for that limited subset of characters for tags? Anyway, I'd prefer to simplify our version numbers rather than enabling insane tags in umoci.

davidcassany avatar Mar 21 '18 16:03 davidcassany

The vBLAH+rNUM_other_crap versioning scheme is done so that upgrades work properly. If you have vBLAH_git_crap then RPM might not see vBLAH_00af8231 as being newer than vBLAH_59125ad. The +rBLAH means that RPM will always consider vBLAH+r10 as older than vBLAH+r50. It's not really a policy, but you have to do it this way if you want to upgrade between different git commits without changing the "real" version. You can play around with sort -V to see what different version strings do to the ordering.

@cyphar is there any mandatory reason for that limited subset of characters for tags?

The character set is defined by the image specification. They intentionally made it overly restrictive so it could be relaxed in the future, but they started with it being URL-safe. However, I just checked and it looks like they expanded it a while ago and I must've missed that change.

cyphar avatar Mar 22 '18 00:03 cyphar

Right, I understand the revision number is needed to sort versions. I was more concerned about the usage of + sing and by the usage of the full commit hash, that leads to a terribly long version string.

However, I just checked and it looks like they expanded it a while ago and I must've missed that change.

Does the expansion include +?

davidcassany avatar Mar 22 '18 08:03 davidcassany

I was more concerned about the usage of + sing and by the usage of the full commit hash, that leads to a terribly long version string.

+ is needed in certain cases (it's treated differently as are ~, _, and -). For instance: v1.3.1-dev < v1.3.1 < v1.3.1+dev. But shouldn't be required in the r430 case -- though personally I think it looks clearer than _r430.

Does the expansion include +?

Yes. They added a really weird URL-like schema. From here:

ref       ::= component ("/" component)*
component ::= alphanum (separator alphanum)*
alphanum  ::= [A-Za-z0-9]+
separator ::= [-._:@+] | "--"

cyphar avatar Mar 22 '18 10:03 cyphar

Packaging wise, we made up this "policy", so we can also change it. The only requirement is that zypper always sees a new version as a new version. You can use "zypper versioncmp" to check different formats.

For example:

zypper versioncmp 1.0.0_git_r50 1.0.0+git_r50 1.0.0_git_r50 matches 1.0.0+git_r50

zypper versioncmp 1.0.0_git_r50 1.0.0+git_r60 1.0.0_git_r50 is older than 1.0.0+git_r60

zypper versioncmp 1.0.0_git_r70 1.0.0+git_r60 1.0.0_git_r70 is newer than 1.0.0+git_r60

Thus, looks like zypper does not make a difference between "+" and "_", so you could replace that.

The commit hash could also be shortened. Since we have the revision first, the commit hash is not really used other than for humans to visually know which commit is in, but nor zypper nor our tools make use of that.

So, packaging wise, it is ok to do these changes.

jordimassaguerpla avatar Mar 22 '18 12:03 jordimassaguerpla

i also dont have any strong opinion on this, feel free to submit a change. but then we mustn't forget to do it consistently for the other packages in kubic-project as well

MaximilianMeister avatar Mar 22 '18 15:03 MaximilianMeister

I've written a patch to handle the extended character set for tags openSUSE/umoci#234. It will be included in the 0.4.1 update that I'm planning on sending out next week.

cyphar avatar Mar 22 '18 23:03 cyphar

@cyphar, great thanks!

davidcassany avatar Mar 23 '18 09:03 davidcassany

@cyphar I think openSUSE/umoci#234 did not get into Factory yet. Is there any change to get it there?

davidcassany avatar Jun 06 '18 13:06 davidcassany

is this still an issue?

stefsuse avatar Jan 09 '19 23:01 stefsuse

This GitHub issue/PR is unactive since long time. Is this GitHub ISSUE/PR still needed? Please close or update it accordingly. This reminder is autogenerated by https://github.com/MalloZup/blacktango

MalloZup avatar Mar 18 '19 14:03 MalloZup