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

Jump to bottom

cmake: support sudo make uninstall #1674

Closed
wants to merge 1 commit into from
Closed

cmake: support sudo make uninstall #1674

wants to merge 1 commit into from

Conversation

jasjuang
Copy link
Contributor

This features reduces the pain of having to remove the installed libraries and headers in /usr/local/ one by one.

Before this change, after sudo make install

Install the project...
-- Install configuration: ""
-- Up-to-date: /usr/local/bin/curl-config
-- Up-to-date: /usr/local/lib/pkgconfig/libcurl.pc
-- Up-to-date: /usr/local/include/curl/curlbuild.h
-- Up-to-date: /usr/local/include/curl
-- Up-to-date: /usr/local/include/curl/multi.h
-- Up-to-date: /usr/local/include/curl/easy.h
-- Up-to-date: /usr/local/include/curl/mprintf.h
-- Up-to-date: /usr/local/include/curl/curl.h
-- Up-to-date: /usr/local/include/curl/curlrules.h
-- Up-to-date: /usr/local/include/curl/typecheck-gcc.h
-- Up-to-date: /usr/local/include/curl/curlver.h
-- Up-to-date: /usr/local/include/curl/stdcheaders.h
-- Up-to-date: /usr/local/lib/libcurl.so
-- Up-to-date: /usr/local/bin/curl

there is no easy ways to remove these files, but with this change we can simply type sudo make uninstall and

Scanning dependencies of target uninstall
/usr/local
-- Uninstalling /usr/local/bin/curl-config
-- Uninstalling /usr/local/lib/pkgconfig/libcurl.pc
-- Uninstalling /usr/local/include/curl/curlbuild.h
-- Uninstalling /usr/local/include/curl/multi.h
-- Uninstalling /usr/local/include/curl/easy.h
-- Uninstalling /usr/local/include/curl/mprintf.h
-- Uninstalling /usr/local/include/curl/curl.h
-- Uninstalling /usr/local/include/curl/curlrules.h
-- Uninstalling /usr/local/include/curl/typecheck-gcc.h
-- Uninstalling /usr/local/include/curl/curlver.h
-- Uninstalling /usr/local/include/curl/stdcheaders.h
-- Uninstalling /usr/local/lib/libcurl.so
-- Uninstalling /usr/local/bin/curl
Built target uninstall

@mention-bot
Copy link

@jasjuang, thanks for your PR! By analyzing the history of the files in this pull request, we identified @billhoffman, @Sukender and @jzakrzewski to be potential reviewers.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 76.062% when pulling badda39 on jasjuang:master into e909de6 on curl:master.

@bagder bagder added the cmake label Jul 11, 2017
@bagder bagder changed the title support sudo make uninstall cmake: support sudo make uninstall Jul 13, 2017
@snikulov
Copy link
Member

@jasjuang Bad idea. If you required sudo to install why don't you call uninstall without sudo and push it into build system?

@bagder I suggest declining this PR.

@snikulov
Copy link
Member

@jasjuang @bagder Sorry for my previous comment. Didn't get it for the first time. Will check it today evening.
Again, sorry for the noise.

@snikulov
Copy link
Member

@bagder this PR just copy-paste from CMake FAQ https://cmake.org/Wiki/CMake_FAQ#Can_I_do_.22make_uninstall.22_with_CMake.3F

So depending on your decision.
If make uninstall is required, you can merge it.
Personally, I don't like this idea and suggesting @jasjuang focus on CPack support instead.

@jasjuang
Copy link
Contributor Author

@snikulov Not sure why you don't like this idea. This is a common feature for most open source libraries that supports cmake like OpenCV, glm, glfw, glew, pcl, ceres, glog, gflags, tinyxml2 etc. Without this PR, how would you remove the files installed in /usr/local easily?

@snikulov
Copy link
Member

@jasjuang As I already mentioned, I prefer to use CPack to make a package for installation.
I rarely keep build directory to run make uninstall from it.
To uninstall something from build directory I will use

xargs rm < install_manifest.txt

as mentioned on the link where you copy it.
It simply shorter.

@jasjuang
Copy link
Contributor Author

jasjuang commented Jul 27, 2017 •
edited

@snikulov Well, sudo make uninstall is shorter than sudo xargs rm < install_manifest.txt. Also, this is beneficial for windows as well, I can run the uninstall target to remove the installed files without having to manually delete it.

@snikulov
Copy link
Member

@jasjuang matter of taste. It's shorter because of increased code size under your control and support.

@bagder
Copy link
Member

bagder commented Jul 28, 2017

In general I think it makes sense to support "make uninstall" as an opposite to "make install", and the autotools build supports it already.

This code seems to look for and use a install_manifest.txt file to know what specific files to uninstall? What's that and how does it get created?

@jasjuang
Copy link
Contributor Author

@bagder install_manifest.txt is created by cmake as long as the install functions are been written inside the CMakeLists. It records what files should be installed.

@snikulov
Copy link
Member

@bagder then merge it. As I've already mentioned it's copy-paste from cmake F.A.Q., so I see no issue with it.

@bagder bagder closed this in 27e2a47 Jul 29, 2017
@bagder
Copy link
Member

bagder commented Jul 29, 2017

Thanks!

@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
cmake
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jasjuang @mention-bot @coveralls @snikulov @bagder