AcademySoftwareFoundation / Imath

Imath is a C++ and python library of 2D and 3D vector, matrix, and math operations for computer graphics
https://imath.readthedocs.io
BSD 3-Clause "New" or "Revised" License
356 stars 104 forks source link

Add uninstall target #366

Closed cary-ilm closed 4 months ago

cary-ilm commented 4 months ago

Satisfy the OpenSSF Best Practices Badge requirement for an insta/uninstall process: https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the community recommends implementing an "uninstal" target that remove files named in the install_manifest.txt: https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare" library, i.e. the symlink from libImath-3_2.so to libImath.so, fails to add the symlink to the manifest, so "make uninstall" misses the symlink. The existing mechanism uses "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and "install(FILES)" to accomplish the same thing while also registering the symlink the the manifest.

This change also adds the cmake option IMATH_INSTALL, on by default and for normal usage, but which allows disabling of the installation when Imath is configured as a subproject of another project. OpenEXR propgates the OPENEXR_INSTALL option to IMATH_INSTALL.

meshula commented 4 months ago

but which allows disabling of the installation when Imath is configured as a subproject of another project.

I haven't seen this pattern in other projects, I think I missed the motivation when it was introduced upstream at OpenEXR?

cary-ilm commented 4 months ago

OPENEXR_INSTALL has been there all along, since 3.0: https://github.com/AcademySoftwareFoundation/openexr/blob/44d2d0eaf289f54d4f86f45f4b6b68fac2d15f0e/CMakeLists.txt#L35

It's needed by the python bindings because the cmake target that installs the wheels should not install the libraries, so this option makes it possible to suppress installing the libraries. Imath needs it, too, otherwise the OpenEXR target that installs the wheels would also install the Imath library.

meshula commented 4 months ago

Gotcha, lgtm!