-
Notifications
You must be signed in to change notification settings - Fork 262
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
De-couple root Makefile from root CMakeLists.txt #611
Conversation
Run `cmake --build . --target purge` (or `make purge` if CMake uses a Makefile generator) to remove all files in the build tree. Remove a target of the same name from the build rules of Mongoose to avoid a target name conflict.
Sure, we could figure out something that works. It would be nice to have several options:
My "make purge" is my own usage; the phrase is kind of unusual. We could just remove it or repurpose it. The more common practice is "make clean" or "make distclean" which is why I have those as synonyms in each Makefile. "make distclean" is more a common usage, and it removes all files not in the original distribution. So it's more like option (3) above. It should clean everything. What if we gave (1) and (2) their own names? Inside a cmake script, instead of using "rm -rf", the better thing in cmake would be to use
https://cmake.org/cmake/help/latest/command/file.html#remove That would be the portable way to delete files. The only downside to using cmake to delete files is that removing build/* deletes the constructed cmake files with the target itself. The Mongoose CMakeLists.txt uses |
The
|
Wrt the above points: |
OK. I can do that. Will this PR do both (1) and (2) then? |
I didn't touch much in the root Makefile. If there are parts of (1) that the root Makefile doesn't do currently, it won't do it after this change. If you find it confusing that there are two |
To clarify, I'd prefer if running |
That won't be confusing. This is all fine. And if I want a single command to clean everything, I can always do that locally in an alias or shell script that's completely outside the SuiteSparse repo. Just for my own use. |
2f0ee55
into
DrTimothyAldenDavis:dev2
Maybe, that is not a supported use-case. But I'm often using the same source tree to test building with the root Makefile and with the root CMakeLists.txt. However, the
purge
target of the root Makefile also removes the build tree of the root CMakeLists.txt. That is annoying sometimes.I'm working around that by using a different folder to
./build
when building with the root CMakeLists.txt. So, I don't mind if you would prefer to not accept this change. I could just continue with that "workaround".The proposed change would remove that the root Makefile interacts with the (default) folder for the build tree of the root CMakeLists.txt. Instead, it would introduce a (new) target "purge" to the root CMakeLists.txt that would remove all files from its own build tree.
That new target doesn't do much more than executing
rm -rf *
(or whatever command that corresponds to on a platform without POSIX-compatible binutils). A developer could probably also just use that command instead ofmake purge
. So, it would also be fine if just the second part (of removing the interaction of the root Makefile with the build tree of the root CMakeLists.txt) would be accepted.