Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding an uninstall target to CMake #22

Closed
vmagnin opened this issue Jan 25, 2024 · 12 comments
Closed

Adding an uninstall target to CMake #22

vmagnin opened this issue Jan 25, 2024 · 12 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@vmagnin
Copy link
Owner

vmagnin commented Jan 25, 2024

I have added an uninstall target to CMake, providing the ability to use make uninstall. Perhaps you could also add it for forcolormap. See gha3mi/forimage#17 and the corresponding changes: gha3mi/forimage@f1b4056. The same approach can be applied to forcolormap.

Originally posted by @gha3mi in #21 (comment)

@vmagnin vmagnin self-assigned this Jan 25, 2024
@vmagnin vmagnin added this to the 0.9 milestone Jan 25, 2024
@vmagnin vmagnin added the enhancement New feature or request label Jan 25, 2024
@vmagnin
Copy link
Owner Author

vmagnin commented Jan 25, 2024

See also:
https://github.com/vmagnin/gtk-fortran/blob/gtk4/cmake/cmake_uninstall.cmake.in

@vmagnin
Copy link
Owner Author

vmagnin commented Jan 29, 2024

I tried to add such code copied from gtk-fortran (but I did not pushed it on GitHub).

The problem is that when I try to uninstall ForColormap, CMake is looking also for the ForImage install_manifest.txt, which does not exist:

forcolormap/build (main)$ sudo make uninstall
CMake Error at cmake/cmake_uninstall.cmake:2 (message):
  Cannot find install manifest: "forcolormap/build/_deps/forimage-build/install_manifest.txt"

make[3]: *** [_deps/forimage-build/CMakeFiles/uninstall.dir/build.make:70 : _deps/forimage-build/CMakeFiles/uninstall] Erreur 1
make[2]: *** [CMakeFiles/Makefile2:254 : _deps/forimage-build/CMakeFiles/uninstall.dir/all] Erreur 2
make[1]: *** [CMakeFiles/Makefile2:261 : _deps/forimage-build/CMakeFiles/uninstall.dir/rule] Erreur 2
make: *** [Makefile:195 : uninstall] Erreur 2

The install_manifest.txt contains both ForImage and ForColormap files:

forcolormap (main)$ cat build/install_manifest.txt
/usr/local/lib/libforimage.so.0.3.0
/usr/local/lib/libforimage.so.0
/usr/local/lib/libforimage.so
/usr/local/include/lut.mod
/usr/local/include/pnm.mod
/usr/local/include/forimage_parameters.mod
/usr/local/include/forcolor.mod
/usr/local/include/forimage.mod
/usr/local/lib/cmake/forimage/forimageTargets.cmake
/usr/local/lib/cmake/forimage/forimageTargets-release.cmake
/usr/local/lib/cmake/forimage/forimageConfig.cmake
/usr/local/lib/cmake/forimage/forimageConfigVersion.cmake
/usr/local/lib/pkgconfig/forimage.pc
/usr/local/lib/libforcolormap.so.0.8.0
/usr/local/lib/libforcolormap.so.0
/usr/local/lib/libforcolormap.so
/usr/local/include/miscellaneous_colormaps.mod
/usr/local/include/matplotlib_colormaps.mod
/usr/local/include/colormap_parameters.mod
/usr/local/include/forcolormap.mod
/usr/local/include/forcolormap_info.mod
/usr/local/include/scientific_colour_maps.mod
/usr/local/lib/cmake/forcolormap/forcolormapTargets.cmake
/usr/local/lib/cmake/forcolormap/forcolormapTargets-release.cmake
/usr/local/lib/cmake/forcolormap/forcolormapConfig.cmake
/usr/local/lib/cmake/forcolormap/forcolormapConfigVersion.cmake

@gha3mi
Copy link
Collaborator

gha3mi commented Jan 29, 2024

The issue arises from assigning the same name to the CMake target uninstall:

add_custom_target(uninstall
  COMMAND ${CMAKE_COMMAND} -P ${CMAKE_CURRENT_BINARY_DIR}/cmake/cmake_uninstall.cmake)

I suggest using distinct custom target names. For instance, you may consider using uninstall_forcolormap, since the name uninstall is already used in the ForImage custom target. Consequently, I need to update it to uninstall_forimage.

@gha3mi
Copy link
Collaborator

gha3mi commented Jan 29, 2024

I don't know if this is a problem or if it's intentional: uninstall_forcolormap also removes ForImage!

I believe it is correct because it removes all installed libraries and dependencies. If someone wants to have ForImage, they must install ForImage again!

@gha3mi
Copy link
Collaborator

gha3mi commented Jan 29, 2024

The ideal approach would be to have uninstall, uninstall_forcolormap, and uninstall_forimage. If we could separate the install_manifest into two files, one for ForImage and one for ForColormap, then we would be able to uninstall each library separately or all of them collectively.

@vmagnin
Copy link
Owner Author

vmagnin commented Jan 29, 2024

The issue arises from assigning the same name to the CMake target uninstall:
I suggest using distinct custom target names. For instance, you may consider using uninstall_forcolormap, since the name uninstall is already used in the ForImage custom target. Consequently, I need to update it to uninstall_forimage.

Thanks Ali, I will try the name uninstall_forcolormap tomorrow.

@vmagnin
Copy link
Owner Author

vmagnin commented Jan 29, 2024

If we could separate the install_manifest into two files, one for ForImage and one for ForColormap, then we would be able to uninstall each library separately or all of them collectively.

Indeed, we have both libraries in the same build/install_manifest.txt...

@gha3mi
Copy link
Collaborator

gha3mi commented Jan 29, 2024

Another issue to note is that the CMake installation relies on the dependencies directory. If you remove the dependencies directory, CMake won't be able to clone the necessary dependencies. Maybe we can somehow make CMake do a git clone?
[Edited]: https://github.com/vmagnin/forcolormap/blob/8e14b89f1e14e8d20a4294a5bc00fa2bda31de2f/CMakeLists.txt#L32C1-L32C31

My apologies, it was my mistake. There is no issue; the functionality is correct!

@vmagnin
Copy link
Owner Author

vmagnin commented Jan 30, 2024

I have pushed the CMake uninstall functionality:

$ sudo make uninstall_forcolormap
-- Uninstalling /usr/local/lib/libforimage.so.0.3.0
-- Uninstalling /usr/local/lib/libforimage.so.0
-- Uninstalling /usr/local/lib/libforimage.so
-- Uninstalling /usr/local/include/forimage.mod
-- Uninstalling /usr/local/include/forimage_parameters.mod
-- Uninstalling /usr/local/include/forcolor.mod
-- Uninstalling /usr/local/include/lut.mod
-- Uninstalling /usr/local/include/pnm.mod
-- Uninstalling /usr/local/lib/cmake/forimage/forimageTargets.cmake
-- Uninstalling /usr/local/lib/cmake/forimage/forimageTargets-release.cmake
-- Uninstalling /usr/local/lib/cmake/forimage/forimageConfig.cmake
-- Uninstalling /usr/local/lib/cmake/forimage/forimageConfigVersion.cmake
-- Uninstalling /usr/local/lib/pkgconfig/forimage.pc
-- Uninstalling /usr/local/lib/libforcolormap.so.0.8.0
-- Uninstalling /usr/local/lib/libforcolormap.so.0
-- Uninstalling /usr/local/lib/libforcolormap.so
-- Uninstalling /usr/local/include/forcolormap_info.mod
-- Uninstalling /usr/local/include/forcolormap.mod
-- Uninstalling /usr/local/include/colormap_parameters.mod
-- Uninstalling /usr/local/include/matplotlib_colormaps.mod
-- Uninstalling /usr/local/include/miscellaneous_colormaps.mod
-- Uninstalling /usr/local/include/scientific_colour_maps.mod
-- Uninstalling /usr/local/lib/cmake/forcolormap/forcolormapTargets.cmake
-- Uninstalling /usr/local/lib/cmake/forcolormap/forcolormapTargets-release.cmake
-- Uninstalling /usr/local/lib/cmake/forcolormap/forcolormapConfig.cmake
-- Uninstalling /usr/local/lib/cmake/forcolormap/forcolormapConfigVersion.cmake
-- Uninstalling /usr/local/lib/pkgconfig/forcolormap.pc
Built target uninstall_forcolormap

And I have added in the README.md a comment about the consequences for ForImage.

I don't know the best way to separate ForImage/ForColormap uninstall, but note that cmake/cmake_uninstall.cmake.in is just running a loop on the content of build/install_manifest.txt:

file(READ "@CMAKE_BINARY_DIR@/install_manifest.txt" files)
string(REGEX REPLACE "\n" ";" files "${files}")
foreach(file ${files})
...

It would be (relatively) easy to add a test to avoid removing files whose path contains forimage.

@gha3mi
Copy link
Collaborator

gha3mi commented Jan 30, 2024

And I have added in the README.md a comment about the consequences for ForImage.

I think this is good for now.

@gha3mi
Copy link
Collaborator

gha3mi commented Jan 30, 2024

(I have also moved all CMake-related directories for ForImage, including install (and dependencies), to the cmake directory. Having the install directory in the main path was somehow confusing for me, as it is specifically intended for CMake-related functionality.)

@vmagnin
Copy link
Owner Author

vmagnin commented Jan 30, 2024

Thanks @gha3mi , I have now also moved install/ and dependencies/ to the cmake/ directory. Everything seems to run.

@vmagnin vmagnin closed this as completed Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants