iLCSoft / LCIO

Linear Collider I/O
BSD 3-Clause "New" or "Revised" License
17 stars 34 forks source link

Don't make the uninstall target if it already exists #172

Closed jmcarcell closed 1 year ago

jmcarcell commented 1 year ago

BEGINRELEASENOTES

ENDRELEASENOTES

CMake will throw an error and stop if the uninstall target already exists

jmcarcell commented 1 year ago

If we remove cmake_uninstall.cmake then ilcsoft_default_uninstall_target.cmake can also be removed; I wasn't proposing that but for me it would be fine since I never make use of the uninstall target.

tmadlener commented 1 year ago

My main concern would be to not have "dead" code around and then having to wonder down the line, whether it can be removed.

Maybe @andresailer or @gaede can shed some light on the use cases of the uninstall target (and how it was used in the past)?

jmcarcell commented 1 year ago

The only useful case that I have found has been when installing to the system in /usr/local, for example, and then when you have multiple packages installed like this uninstalling them is hard since they share installation folders

tmadlener commented 1 year ago

Looking at cmake_uninstall.cmake.in it looks like it pretty much follows what is suggested by cmake: https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

So, I think we keep this PR as it is and just guard against a duplicate definition.

jmcarcell commented 1 year ago

Sounds good, https://github.com/iLCSoft/iLCUtil/pull/29 is the same PR