EasyCL icon indicating copy to clipboard operation
EasyCL copied to clipboard

Why install clew system-wide?

Open firegurafiku opened this issue 8 years ago • 8 comments

I'm trying to use EasyCL via as a Git submodule which is included into my project by using CMake's add_subdirectory. Unfortunately, building fails with a message:

src/cmake_install.cmake:56: error: file INSTALL cannot copy file "/home/firegurafiku/PROJECTS/padawans/simple-fdtd-cpu-computing/build/clew/src/clew-external-build/src/libclew.so.1.0.0" to "/usr/local/lib/libclew.so.1.0.0". cmake_install.cmake:37 (include)

I think, instead clew should be installed locally right into building tree. And why not simply add clew's CMakeLists.txt as a subdirectory, and statically link into EasyCL? On clew's README it's stated that static linking is the default behavior, and I cannot see any benefit of linking it dynamically.

Also, maybe it's worth converting thirdparty/lua-5.1.5 into another Git submodule, pointing, say, to https://github.com/lua/lua — it's not official, but seems reliable.

firegurafiku avatar May 19 '16 15:05 firegurafiku

And why not simply add clew's CMakeLists.txt as a subdirectory, and statically link into EasyCL?

I originally had it all built as one single thing eg https://github.com/hughperkins/EasyCL/tree/a03261662d263f14ce8e8e6f4dce1f649b400bd0

The problems with building it as one single thing include:

  • the options from the child modules pollute the parent modules
  • you have to use the exact same compile options etc for parent and child modules (at least, it's non-trivial to not do so, and not very maintainable)

Having clew as a submodule works very well for me in general. Now, it's not perfect, it has its issues, but so does the other way around too.

As far as your specific problem...

src/cmake_install.cmake:56: error: file INSTALL cannot copy file "/home/firegurafiku/PROJECTS/padawans/simple-fdtd-cpu-computing/build/clew/src/clew-external-build/src/libclew.so.1.0.0" to "/usr/local/lib/libclew.so.1.0.0". cmake_install.cmake:37 (include)

Normally, I am installing into a subdirectory dist of EasyCL, ie see https://github.com/hughperkins/EasyCL/blob/master/CMakeLists.txt#L28

IF(CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT)
  SET(CMAKE_INSTALL_PREFIX
    "${CMAKE_CURRENT_SOURCE_DIR}/dist" CACHE PATH "Installation prefix, default 'dist'" FORCE
   )
ENDIF(CMAKE_INSTALL_PREFIX_INITIALIZED_TO_DEFAULT)

... and thats what I test with. I suppose you are saying that:

  • you are running the make install with sudo, but
  • the prior make already fails, is that right?

An obvious solution would be to sudo chown uog+w /usr/local, not sure if you want to do that? That's what the Mac brew guys recommend https://github.com/Homebrew/brew/blob/master/share/doc/homebrew/Installation.md#installation

Otherwise, I guess you could install in two steps:

  • build into dist, as per default
  • copy the files up into /usr/local, from dist, using sudo

Of course, this will mess with RPATH, if you are using it, but shouldnt matter if everything is installed up to /usr/local I think?

hughperkins avatar May 20 '16 11:05 hughperkins

Currently, I have worked around the issue in a dirty way, as didn't want to bother with sudo and system-wide installation at all:

--- a/cmake/build_clew.cmake
+++ b/cmake/build_clew.cmake
@@ -1,6 +1,7 @@
 INCLUDE(ExternalProject)

 #message("CMAKE_INSTALL_PREFIX ${CMAKE_INSTALL_PREFIX}")
+set(CMAKE_INSTALL_PREFIX "${CMAKE_CURRENT_BINARY_DIR}")
 ExternalProject_Add(
     clew-external
     STAMP_DIR ${CMAKE_BINARY_DIR}/clew/stamp

Let me also comment on some of you points and explain why statically linking clew would be a better solution, in my opinion.

the options from the child modules pollute the parent modules

That's a perfect reason for each library to have its CMake options prefixed with certain unique string. For example, clew should have names its options as CLEW_BUILD_TESTS, CLEW_BUILD_SHARED_LIBRARY, and CLEW_INSTALL_CL_HEADER. Of course, we would have to convince clew's author to rename those options, I hope it's doable.

you have to use the exact same compile options etc for parent and child modules (at least, it's non-trivial to not do so, and not very maintainable)

Generally speaking, you're right. But in the particular case of clew in EasyCL where is almost no benefit having separate CMake caches for EasyCL and clew:

  • CMAKE_INSTALL_PREFIX — not needed anymore if clew is a part of tree.
  • BUILD_SHARED_LIBRARY — reduces the very value of having clew, as it was designed to prevent the program from failing when no runtime library found; linking it dynamically just introduce another dependency.
  • BUILD_TESTS — pretty fine if renamed as CLEW_BUILD_TESTS.
  • ADD_RPATH — not needed if statically linked.
  • CMAKE_BUILD_TYPE — why choose different build type for clew?

firegurafiku avatar May 20 '16 12:05 firegurafiku

Well, in the big picture, I dont use EasyCL directly: it's a submodule eg in cltorch https://github.com/hughperkins/cltorch/blob/master/.gitmodules#L1 I need to use submodules anyway, to get clBLAS to build ok https://github.com/hughperkins/cltorch/blob/master/.gitmodules#L4 And this way is consistent.

If I understand correctly, your issue is not with the use of git submodules, but the fact that there are multiple cmake ... domains ... projects ?

I doubt you're going to convince me to change from decoupled cmake projects to monolithic ones, because I tried monolithic ones, and they were totally painful for me :-)

However, if you want, if you add an option to EasyCL/CMakeLists.txt, like 'CLEW_STATIC' or 'CLEW_INTERNAL' or something similar, make sure all the logic for how to handle that is in some separate file say cmake/easycl_clew_static, which I can basically ignore, then I'll (probably) be happy to merge that :-)

hughperkins avatar May 20 '16 12:05 hughperkins

[...] then I'll (probably) be happy to merge that :-)

Okay :) BTW, how about the Lua part of the original post?

firegurafiku avatar May 20 '16 12:05 firegurafiku

BTW, how about the Lua part of the original post

Are you trying to bait me? :-P You want the clew bit to be monolithic, statically linked, instead of a separate so as now, but you want the lua bit to be a separate so, instead of monolithically statically linked as now? :-P

hughperkins avatar May 20 '16 14:05 hughperkins

but you want the lua bit to be a separate so, instead of monolithically statically linked as now? :-P

Now, just propose to replace actual Lua source files with a submodule. :-) Also, many Linux (and other Unixes) distributions have Lua installed system-wide, so it may actually be worth to add an option to compile Lua dynamically. Or user may already have Lua linked against they program, so linking another Lua may introduce a conflict.

firegurafiku avatar May 20 '16 14:05 firegurafiku

Also, many Linux (and other Unixes) distributions have Lua installed system-wide, so it may actually be worth to add an option to compile Lua dynamically.

It's one more dependency, and loads of people have different versions etc. I got enough support issues because of this, that including it internally fixes a lot of issues, and gives me a quiet life :-)

Or user may already have Lua linked against they program, so linking another Lua may introduce a conflict.

This is hte case for torch, and is the reason why there is an option PROVIDE_LUA_ENGINE https://github.com/hughperkins/EasyCL/blob/master/CMakeLists.txt#L18 , which is set to OFF when I build for cltorch :-)

hughperkins avatar May 20 '16 14:05 hughperkins

@firegurafiku FYI https://github.com/hughperkins/EasyCL/pull/20 which should do the trick for you (I had also noticed sandbox violations when building).

+1 for providing an option for all "non-obscure" 3rd party dependencies to be system-linked (eg. lua, gtest); it's probably the wiser choice to have this by default for a vast majority of the libs (once the interface gets kind of stable). And the 3rd parties which are bundled, it's probably better for traceability, to use a submodule vs. doing a code dump.

@hughperkins note that the way the bundled libs are handled by the cmake scripts doesn't provide the way to configure their options.

zougloub avatar Nov 04 '16 04:11 zougloub