mesonbuild / mesonwrap

Meson wraps service and tools, please use https://github.com/mesonbuild/wrapdb for wraps issues
https://wrapdb.mesonbuild.com
Apache License 2.0
26 stars 7 forks source link

Sort project versions using distutils #57

Closed sarum9in closed 6 years ago

ignatenkobrain commented 6 years ago

why not to use pkg-config's (aka RPM) sorting?

sarum9in commented 6 years ago

Because calling an external binary is probably expensive. Also the main reason to use distutils is that it is a part of a standard python library.

ignatenkobrain commented 6 years ago

no one said that we need to call anything externally. we can either use rpm's python module or write our own code ;)

sarum9in commented 6 years ago

What is the advantage of rpm module compared to distutils.LooseVersion? What is the advantage of our own code compared to distutils.LooseVersion?

nirbheek commented 6 years ago

There is no advantage to using rpm's version comparison because our versions are well-specified: x.y.z + revision, and all components are numeric.

We should definitely use rpm's version comparison algorithm in Meson's str.version_compare() because it is supposed to be for free-form version strings.

ignatenkobrain commented 6 years ago

I can't describe advantage before I get answer for the question: should we implement some version comparison as the pkg-config's or not ;)

If yes, then it's trivial to reuse rpm's vercmp or use meson's or implement our own (or copy from somewhere) because pkg-config uses same algorithm.

nirbheek commented 6 years ago

use meson's

Meson's version comparison is wrong. There's a bug about that too, but I can't find it right now.

jpakkane commented 6 years ago

From memory, rpm's version comparison algorithm is fairly close to what Meson does. IIRC it should be the same for the number part, but probably differs in the followup string part.

sarum9in commented 6 years ago

Okay, since there is no further feedback I am going to merge this. Current implementation is incorrect, we already chose to use distutils algorithm and we should be consistent. If anyone wishes to change this let's switch it for both sorting and max selection at the same time consistently in the other PR.