cmake improvements and fixes

[waebbl]

Fixes some of the issues in #101

I've not yet found the source of why imathnumpy.so is going to get installed.
Looks like the missing install call has been the source for this.

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Bernd (550bd82, a4507b4, fa43561, 77846b7, d3567de, 0f1e3b3, f3c63de, 391c6ed, 0d652fc)

[meshula]

I assume the reason for the removal of several chunks is that they are redundant with the Imath setup?

[meshula]

This looks good to me! Let's get some more eyes on it before landing.

[waebbl]

I assume the reason for the removal of several chunks is that they are redundant with the Imath setup?

Yes, these snippets are all defined in Imath/config/ImathSetup.cmake.

[cary-ilm]

This all looks good to me. But shouldn't the reference to "Python3" be agnostic regarding 3 vs 2? Will this work for python2?

[waebbl]

Good catch. I doubt this works with python2, although I haven't tested it.

Can I safely assume, that Imath only supports one python version at any time, i.e. no parallel installations of e.g. libPyImath_Python2_7-3_0.so and libPyImath_Python3_8-3_0.so?

If so, we could add a variable, something like PYIMATH_PYTHON_SUFFIX in the blocks at https://github.com/AcademySoftwareFoundation/Imath/blob/master/src/python/CMakeLists.txt#L106 and line 114, by using the PYIMATH_LIB_PYTHONVER_ROOT and corresponding Python{2,3}_VERSION_{MAJOR,MINOR} vars and substitue this one variable in PyImath.pc.in. What do you think?

[waebbl]

This patch works for python3.
I don't have an easy way to test for python2 though, because it's already disabled in Gentoo. The interpreter is still available, but no support for packages using python2. I would need to manually install a copy of boost and numpy with py2 support enabled.

[cary-ilm]

I tested this change with python2 and it indeed produces an incorrect .pc file that references a "-lPyImath_Python3" library. But don't we actually need two pkg-config files, one for python2 and one for python3? The CMake files are currently set up to build for both if both are installed, so in this case a single .pc file won't do.

[waebbl]

I tested this change with python2 and it indeed produces an incorrect .pc file that references a "-lPyImath_Python3" library. But don't we actually need two pkg-config files, one for python2 and one for python3? The CMake files are currently set up to build for both if both are installed, so in this case a single .pc file won't do.

This only works if build for one python version. If both are build, the set statement in the python3 block will override the setting from the python2 block.

[waebbl]

Revert the last patch.

[cary-ilm]

#103 removed the PYIMATH_ENABLE_EXCEPTIONS option, which is conflicting here, can you fix that?

Our emerging consensus is that we'd like to simplify this further by eliminating the dual simultaneous build and instead build for a single python version, with a configuration variable to specify which.

But this PR is a good step in that direction and improves the current state dramatically. I'll merge it in if you can resolve the conflict, and we'll go from there. Thanks for the contributions.

[waebbl]

Rebased and fixed the conflict.
Still todo is the -lPyImath${libsuffix} entry in PyImath.pc.in, which needs to be covered by the python variable changes as well.
Thank you for considering the PR.

[cary-ilm]

LGTM