search

torch / rocks

Rocks for torch
72 stars 70 forks source link

Using CMake to build the rocks #115

Open Ark-kun opened 8 years ago

Ark-kun commented 8 years ago

CMake is already used by many rocks to generate the Makefile. I want to modify the rockspec files to also build and install the code via cmake as opposed to make. CMake already knows how to properly invoke the chosen make program and we should use this ability to improve the cross-platform, compatibility. For example, on Windows with Visual Studio installed, CMake generates msbuild projects by default. luarocks then tries to build them using nmake and fails. CMake on the other hand should handle all builds properly.

What I propose: In all .rockspec files I want to replace $(MAKE) => cmake --build . --config release $(MAKE) install => cmake --build . --config release --target install

What's the best way for me to handle this? Should I go to each rock's repository and submit PR for the .rockspec?

soumith commented 8 years ago

this sounds okay to me. it's going to be a bit tedious for you to modify each package's rockspec but that's the only way forward, looks like it.

hishamhm commented 8 years ago

Note that LuaRocks includes built-in support for cmake, so it accepts build.type = "cmake" and can construct these commands automatically. So your rockspecs will probably become shorter. :)

soumith commented 8 years ago

But from the last time I looked (year-ish ago), luarocks-cmake didn't have the ability to do builds in a separate build folder. That's the reason I think we do it this way.

hishamhm commented 8 years ago

You mean passing --build? (I'm not a CMake user myself) It seems to me it currently does: https://github.com/keplerproject/luarocks/blob/master/src/luarocks/build/cmake.lua

howard0su commented 7 years ago

Current luarocks didn't do exact same thing as most rockspec is doing. one common example is the below. this is very common pattern in many rocks files. It will be great to replace this with type='cmake'. A big advantage is bringing windows support for those rocks.

type = "command", build_command = [[ cmake -E make_directory build; cd build; cmake .. -DCMAKE_BUILD_TYPE=Release -DCMAKE_PREFIX_PATH="$(LUA_BINDIR)/.." -DCMAKE_INSTALL_PREFIX="$(PREFIX)"; $(MAKE) ]], install_command = "cd build && $(MAKE) install"

the above section should be replaced by this: type = "cmake", variables = { CMAKE_BUILD_TYPE="Release", CMAKE_PREFIX_PATH="$(LUA_BINDIR)/..", CMAKE_INSTALL_PREFIX="$(PREFIX)" }

This will touch many many files. are u okay with that? @soumith