Set IMATH_VERSION from Imath_VERSION instead of CMAKE_PROJECT_VERSION

[cary-ilm]

When invoked via FetchContent() from another project, the
CMAKE_PROJECT_VERSION variables inherit values from the parent project
rather than coming from Imath itself.

Signed-off-by: Cary Phillips [email protected]

[lgritz]

Not for this patch -- because I'm hesitant to introduce any potentially breaking change to the backports -- but if IMATH_VERSION_MAJOR is never different from Imath_VERSION_MAJOR, do we really need the former to exist at all?

[cary-ilm]

Agreed, not for this patch. I created a v3.0.3 tag which will suffice for the current issue. I'll dig a little deeper afterwards.

[cary-ilm]

I removed the IMATH_VERSION_{MAJOR,MINOR,PATCH} variables and replaced their use with the Imath_ ones.

It bugs me that we have some variables prefixed with IMATH_ and some (cmake-provided) prefixed with Imath_, but I'm leery of changing the ones that are already public settings.

[meshula]

Oh geez, I had to look up if cmake variables are case sensitive or not (they are case sensitive). This pattern is moderately confusing.

[lgritz]

The old Cmake convention was all-caps PACKAGENAME_FOO, but nowadays preference seem to be PackageName_FOO matching the real name of the package (and PackageNameConfig.cmake, FindPackageName.cmake, etc., for consistency).

[cary-ilm]

The project(Imath VERSION 3.2.1) call sets Imath_VERSION_MAJOR=3, Imath_VERSION_MINOR=2, and Imath_VERSION_PATCH=1, which is the incentive to use make use of the Imath_ variables.

Previously, we were setting the IMATH_ variables and then forming the string to use as the VERSION for the package() call, which seemed backwards. It seems like the fewer variables, the better.

[lgritz]

This turns out not to be ok. IMATH_VERSION cascades down to getting put in the exported ImathConfigVersion.cmake as

set(PACKAGE_VERSION "3.1.0-dev")

and that's in fact an invalid cmake package version, which has to be of the form MAJOR.MINOR[.PATCH[.TWEAK]]

We don't want the "-dev" to show up in this variable.

In OIIO, I use the dev designator as a totally separate variable:

set (PROJECT_VERSION_RELEASE_TYPE "dev")   # "dev", "betaX", "RCY", ""

and then it can be directly tested in cmake code, it gets folded into the string that prints for versions, but not into the OpenImageIO_VERSION variable or the exported configs.

[lgritz]

Current master Imath's exported config appears to be broken for this reason. (But luckily, 3.0.3 is not. It's mildly broken for a totally different reason. :-))

[lgritz]

We've had a rough patch of bad luck in the last few days that isn't really anybody's fault, but I think there are lessons that we can draw from it to make things smoother in the future. One of those lessons is that even the minor patch releases should not have changes to the build scripts added mere hours before a tag is published. All patches should go into the RB-x.y branch and sit for 48 hours without any bugs found (including by downstream projects' CI), and only then should the tag (and NO other changes) be added and release made. (Of course, in a TRUE emergency release, where the prior one is simply too broken to be made any worse even if there are mistakes.)

[lgritz]

I'm aware that I'm the one who urged the "-dev" stuff, and I also didn't spot this problem during the PR reviews. I didn't realize the problem until it had already been merged and trickled downstream. Sorry about that.

[lgritz]

I'll try proposing a fix for this, since I feel like I'm the one who set us off in the wrong direction.

[cary-ilm]

I don't think we have any reason to do anything special or fancy or non-standard with Imath or OpenEXR, it should be just basic version specification/identification stuff, so parroting exactly what other projects do seems like the best approach. I took inspiration for the original cleanup from OIIO but obviously didn't replicated it properly.

[lgritz]

See #146