search

jbeder / yaml-cpp

A YAML parser and emitter in C++
MIT License
4.91k stars 1.78k forks source link

CMake: Allow to disable uninstall #1215

Closed Megamouse closed 8 months ago

Megamouse commented 10 months ago

add_custom_target(uninstall ... causes a clash with the same target in rtmidi (https://github.com/thestk/rtmidi). "add_custom_target cannot create target "uninstall" because another target with the same name already exists. The existing target is a custom target created in source directory"

So it's probably easiest to simply allow to disable the uninstall in the first place.

Also gates the uninstall target unless YAML_CPP_INSTALL is set, which fixes #1151.

jbeder commented 9 months ago

That strikes me as really weird; why can't every project have an uninstall? Are you sure it's not misconfigured somehow? (Note I'm not a CMake expert so maybe it's best to ask the CMake people.)

HunterBelanger commented 9 months ago

I have just run into the problem that was mentioned by @Megamouse above and in #1151 . My particular case comes from trying to use FetchContent to get both yaml-cpp and Eigen in the same project. Both define an uninstall target, and this then leads to a CMake error. The only way to avoid this problem is to disable the creation of the uninstall target.

I would prefer the approach @duncanspumpkin recommended, shielding the uninstall target creation with the YAML_CPP_INSTALL option. You should certainly only make the uninstall target if you also have an install target. That being said, there might still be cases where you want an install target, but no uninstall target. I think the option @Megamouse added to disable the uninstall target should be kept, but a check should also be added to ensure that YAML_CPP_INSTALL is also set to ON.

Megamouse commented 9 months ago

I applied the suggested change as well.