Greater-than comparison of an unknown version yields true

[ernestask]

Trying to build tracker-miners with some dependencies missing results in rather interesting errors. I’m missing libcue-devel in my Fedora installation, but the support for it is optional, depending on whether it was found and which version was found: https://gitlab.gnome.org/GNOME/tracker-miners/blob/f9e7de0fe0549b009defc68ba37ba2516a03f49c/meson.build#L317, however, the code ends up trying to include one of the headers anyway.

Adding a debug print for libcue.version() yields “unknown”, but then printing libcue.version() >= '2.0.0' results in “True”.

[ernestask]

I guess one could argue that there should be another libcue.found() check, but I’m thinking that “unknown” version comparisons should always be false.

[ernestask]

I forgot to mention that the version I’m using right now is 0.48.0.

[jon-turney]

conf.set('HAVE_LIBCUE2', libcue.version() >= '2.0.0')

This is a string comparison. I guess libcue.version().version_compare('>=2.0.0') is what's meant, but I'm not sure that will work as expected.

but I’m thinking that “unknown” version comparisons should always be false.

Yeah, I agree. That's implemented for dependency(version:) constraints (6a4c2d6), but I don't think version_compare does the right thing forunknown yet.

[ernestask]

This is a string comparison. I guess libcue.version().version_compare('>=2.0.0') is what's meant, but I'm not sure that will work as expected.

Oh, so it is, I totally forgot about that.

[ernestask]

It seems to do the right thing, thanks! And sorry for the noise.

[jon-turney]

It seems to do the right thing, thanks!

I think version_compare() only happens to work here because it compares the first element of the versions (unknown and 2) and considers alphabetic elements less than numeric ones.

This won't work as expected ('“unknown” version comparisons should always be false') if the comparision was the other way round e.g. 'unknown'.version_compare('<=2.0.0'), or when applied to a version where the first element is alphabetic. e.g. 'unknown'.version_compare('>=alpha').

And sorry for the noise.

No problem. Thanks for raising this issue.

[ernestask]

I think version_compare() only happens to work here because it compares the first element of the versions (unknown and 2) and considers alphabetic elements less than numeric ones.

Yeah, I see it now:

meson/mesonbuild/mesonlib.py

Line 442 in 646a073

if self._v[i].isdigit() != other._v[i].isdigit():

This won't work as expected ('“unknown” version comparisons should always be false') if the comparision was the other way round e.g. 'unknown'.version_compare('<=2.0.0'), or when applied to a version where the first element is alphabetic. e.g. 'unknown'.version_compare('>=alpha').

Was there ever a discussion about such cases and whether version() on a dependency that hasn’t been found should return a value? Seems easier than making sure that version comparison works intuitively 100% of the time.

[ernestask]

Maybe a special Version object could be returned that unconditionally returns false in __cmp__() et al.