nigels-com / glew

The OpenGL Extension Wrangler Library
Other
2.62k stars 614 forks source link

Moving the glew/build/cmake/CMakeLists.txt to glew/CMakeLists.txt #149

Open jasjuang opened 7 years ago

jasjuang commented 7 years ago

I am wondering if it's possible to change the file arrangement a bit so the head CMakeLists at glew/build/cmake/ could be moved to glew/? Most of the libraries (ex: glm, glfw, opencv, pcl, glog, gflags etc) have their head CMakeLists in the root directory and the build folder is actually created by the user himself. All these libraries can be installed in the below way

For example:

git clone https://github.com/glfw/glfw
cd glfw && mkdir build && cd build
cmake ..
sudo make install
git clone https://github.com/g-truc/glm
cd glfw && mkdir build && cd build
cmake ..
sudo make install

but only glew is installed like this

cd glew-2.0.0/build/
cmake ./cmake
sudo make install

The "build" folder is technically in the source tree so this is actually performing an in-source build which is bad. The main problem with this non standard setup is I am having trouble integrating glew with vcpkg with the CMake build.

It would be great if you can consider changing this!

nigels-com commented 7 years ago

I'm aware of the convention and the pain being caused. I still consider the GNU makefile the primary build system, and when I tried putting CMakeLists along side it, it was trouble.

Short of making cmake the primary build system for GLEW, can you propose an arrangement that does not conflict with the Makefile ?

jasjuang commented 7 years ago

@nigels-com Got it, now I understand your concern. Unfortunately I am not well versed enough in GNU makefiles to propose an arrangement.

I would like to try to convince you to consider throwing away the GNU makefiles entirely. The primary reason for using cmake is cross platform, meaning it allows code to compile on both linux (generates GNU makefiles) and windows (generates Visual Studio Solutions). If cmake is able to do what plain GNU makefiles can do and more, there isn't a strong reason to have the GNU makefiles in source. Do you agree?

nigels-com commented 7 years ago

I disagree because I have a severe dislike of maintaining cmake (too much like a day-job), and the GNU makefiles has been esssentially stable for 10+ years. (If it's not broke...)

jasjuang commented 7 years ago

@nigels-com That's too bad. I will figure out with the people at vcpkg to see how to make glew's cmake build work then. Could you make a new release (2.0.1?) that contains the change to allow compilation on Visual Studio Debug mode?

nigels-com commented 7 years ago

I can see the merit in making a change. There just isn't much in it for me personally. I'll reconsider, but I don't think I can spare much time for GLEW at the moment.

bitminer49er commented 5 years ago

I'm aware of the convention and the pain being caused. I still consider the GNU makefile the primary build system, and when I tried putting CMakeLists along side it, it was trouble.

Certainly it is on Linux... Windows not so much. Agreed?

Short of making cmake the primary build system for GLEW, can you propose an arrangement that does not conflict with the Makefile ?

How and in what way did it conflict with GNU Make / Makefile? Could I get some clarification here.

git checkout glew-2.1.0

vc12.sln (sigh)

Trying to build with 1>------ Build started: Project: glew_static, Configuration: Debug Win32 ------ 2>------ Build started: Project: glew_shared, Configuration: Debug Win32 ------ 1> glew.c 1>c1 : fatal error C1083: Cannot open source file: '....\src\glew.c': No such file or directory 2> glew.c 2>c1 : fatal error C1083: Cannot open source file: '....\src\glew.c': No such file or directory 3>------ Build started: Project: glewinfo, Configuration: Debug Win32 ------ 4>------ Build started: Project: visualinfo, Configuration: Debug Win32 ------ 4> visualinfo.c 3> glewinfo.c 3>c1 : fatal error C1083: Cannot open source file: '....\src\glewinfo.c': No such file or directory 4>....\src\visualinfo.c(36): fatal error C1083: Cannot open include file: 'GL/glew.h': No such file or directory ========== Build: 0 succeeded, 4 failed, 0 up-to-date, 0 skipped ==========

Ok lets try vc10.sln (sigh)

After upgrade (sigh sigh)

1>------ Build started: Project: glew_static, Configuration: Debug Win32 ------ 2>------ Build started: Project: glew_shared, Configuration: Debug Win32 ------ 2> glew.c 1> glew.c 2>c1 : fatal error C1083: Cannot open source file: '....\src\glew.c': No such file or directory 1>c1 : fatal error C1083: Cannot open source file: '....\src\glew.c': No such file or directory 3>------ Build started: Project: glewinfo, Configuration: Debug Win32 ------ 4>------ Build started: Project: visualinfo, Configuration: Debug Win32 ------ 3> glewinfo.c 4> visualinfo.c 3>c1 : fatal error C1083: Cannot open source file: '....\src\glewinfo.c': No such file or directory 4>....\src\visualinfo.c(36): fatal error C1083: Cannot open include file: 'GL/glew.h': No such file or directory ========== Build: 0 succeeded, 4 failed, 0 up-to-date, 0 skipped ==========

Boy this sure is awesome... sooo glad wrangler is not using CMake.

nigels-com commented 5 years ago

As mentioned in the documentation:

It is highly recommended to build from a tgz or zip release snapshot.

powderluv commented 5 years ago

@bitminer49er @jasjuang You may want to use https://github.com/Perlmint/glew-cmake

It is a nightly mirror of this repo with a sane CMakelists.txt in the root dir. Works on Linux/Windows/OSX out the box. And some downstream repos like https://github.com/stevenlovegrove/Pangolin/ use it instead of this repo.

The following commands work on Windows10(yay to VS2017 native cmake/ninja support)/Linux/OSX:

50 git clone https://github.com/Perlmint/glew-cmake 51 cd .\glew-cmake\ 52 ls 53 mkdir b 54 cd b 55 cmake .. -GNinja 56 ninja PS C:\Users\yeahright\github\glew-cmake> ls -r *.lib Directory: C:\Users\yeahright\github\glew-cmake\b\lib Mode LastWriteTime Length Name


-a---- 10/7/2018 7:52 PM 1856266 glewd.lib -a---- 10/7/2018 7:52 PM 708496 glewinfo.lib -a---- 10/7/2018 7:52 PM 1856282 glewmxd.lib -a---- 10/7/2018 7:52 PM 697628 libglew_sharedd.lib -a---- 10/7/2018 7:52 PM 704836 libglewmx_sharedd.lib -a---- 10/7/2018 7:52 PM 715704 visualinfo.lib

@nigels-com Would you consider a PR to make CMakelists.txt work in the root dir without messing the current Makefile ? (There is no reason they can't coexist). I verified the above fork works with both Makefiles and CMake on Linux.

There are bunch of other fixes to CMakelists to make it easy to integrate into other projects and it would be good to have it in the original repo rather than the @Perlmint fork. I'll open PRs based on your feedback. Thanks

nigels-com commented 5 years ago

I'm currently revving on GLEW with a view to making a release. I'll go see what's going on in the other fork. It's clear enough that cmake isn't going away. Currently I don't use cmake for maintaining GLEW, and cmake doesn't seem suitable or preferable for that purpose. So far cmake support has been mostly user-contributed, and that's the mode I'd prefer, but I'm open to discussion based on popular and persistent demand.

In a nutshell the poor experience from a Linux or Mac point of view is making the "mistake" of running cmake . in the root directory which overwrites the GLEW Makefile. It seemed to me I'd be doomed to have constant complaints about that, and possibly smash my own work-in-progress too. Best practice is out-of-source building, but there are various aspects of GLEW that is not really set up for that, going back over a decade.

Let's get some PRs going on things we can agree on easily. I'll go have a look for other arrangements of having both a Makefile and CMakeLists.txt co-existing in the root directory. One "problem" is that GLEW is still reasonably popular and the fallout for any kind of mis-step or even well-intentioned change tends to be unpleasant.

jasjuang commented 5 years ago

@nigels-com The problem you described can be easily avoided by adding

set(CMAKE_DISABLE_SOURCE_CHANGES ON)
set(CMAKE_DISABLE_IN_SOURCE_BUILD ON)

at the top CMakeLists.txt, so cmake . won't work if someone accidentally does it.

powderluv commented 5 years ago

@nigels-com Thanks for being open to changes. No one wants to maintain a fork for just the build. @jasjuang suggestion above should help alleviate your concern on "cmake ." Having said that most cmake folks will almost always create a directory called "build". Also if the root CMakeLists.txt is a concern you can always put it in a contrib/cmake dir similar to tensorflow.

I think with Visual Studio 2017 now natively supporting cmake / ninja it is here to stay :)

I would propose a CMakeLists.txt in the root dir with CMAKE_DISABLE_SOURCE_CHANGES that builds on OSX/Linux/Windows. I am confident we can achieve what you have in the Makefile with the same CMake so it should be as close as possible.

nigels-com commented 5 years ago

The next hurdle is what to do about the build directory. Move it elsewhere to accommodate cmake conventions?

powderluv commented 5 years ago

Maybe rename build/ to "contrib/" or "deprecated" similar to what tensorflow does: https://github.com/tensorflow/tensorflow/tree/master/tensorflow/contrib Inside it there is a "makefile" dir and a "cmake" dir.

Ideally CMake will generate for whichever VS version you are using, and with the right toolchain (32bit, 64bit etc)

nigels-com commented 5 years ago
-- Looking for IceConnectionNumber in ICE
-- Looking for IceConnectionNumber in ICE - found
-- Found X11: /usr/lib/x86_64-linux-gnu/libX11.so
CMake Error at CMakeLists.txt:215 (configure_file):
  configure_file attempted to configure a file: /home/nigels/dev/glew/glew.pc
  into a source directory.

Ouch! It's already looking like a rabbit-hole with CMAKE_DISABLE_SOURCE_CHANGES

powderluv commented 5 years ago

Possibly use ${CMAKE_CURRENT_SOURCE_DIR}/ instead of ${GLEW_DIR} in the code below

configure_file (${GLEW_DIR}/glew.pc.in ${GLEW_DIR}/glew.pc @ONLY)

install(FILES ${GLEW_DIR}/glew.pc DESTINATION ${CMAKE_INSTALL_LIBDIR}/pkgconfig )

the glew-config.cmake does that a few lines below.

astralchan commented 7 months ago

It would be convenient if there was just a "cmake" folder. Currently I have this:

glew.cmake

set (GLEW_VERSION "2.1.0")
set (GLEW_URL "https://download.sourceforge.net/sourceforge/project/glew/glew/${GLEW_VERSION}/glew-${GLEW_VERSION}.zip")
set (GLEW_HASH "SHA256=2700383d4de2455f06114fbaf872684f15529d4bdc5cdea69b5fb0e9aa7763f1")

FetchContent_Declare(glew
    URL "${GLEW_URL}"
    URL_HASH "${GLEW_HASH}"
    SOURCE_SUBDIR "build/cmake"
)

FetchContent_MakeAvailable(glew)

This does seem to build glew, however the current setup fails to properly include the header files, resulting in an error when trying to build with FetchContent. (fatal error C1083: Cannot open include file: 'GL/glew.h': No such file or directory). For libraries, the cmake configuration should include these include directories, such as GLFW. Here is what I have for GLFW:

glfw.cmake

set (GLFW_VERSION "3.3.9")
set (GLFW_URL "https://github.com/glfw/glfw/releases/download/${GLFW_VERSION}/glfw-${GLFW_VERSION}.zip")
set (GLFW_HASH "SHA256=55261410f8c3a9cc47ce8303468a90f40a653cd8f25fb968b12440624fb26d08")

FetchContent_Declare (glfw
    URL "${GLFW_URL}"
    URL_HASH "${GLFW_HASH}"
)

Fetchcontent_MakeAvailable(glfw)

Moving build/cmake to cmake would also change SOURCE_SUBDIR "build/cmake" to the more common (if subdirs are used) SOURCE_SUBDIR "cmake" (not that it matters, though.)

As for how to keep on parity with native UNIX Makefiles, one could use if (UNIX) / etc. Although, the current Makefile uses many, many non-standard extensions to IEEE / Open Group 1003.1-2017 (POSIX.1-2017) make (1p), anyway. Switching to cmake could simply make it more portable, possibly. bmake, pdpmake, and other make implementations may not have the GNU extensions used. If portability with other make isn't desired, GNU Make actually searches for GNUmakefile first, if GNU make is supposed to be the explicit build system. Specifically: GNUmakefile, makefile and Makefile (https://www.gnu.org/software/make/manual/make.html#Makefile-Names). If this project is intended to work specifically with GNU Make, Makefile -> GNUmakefile can help clarify (e.g. OpenDOAS).

The first name checked, GNUmakefile, is not recommended for most makefiles. You should use this name if you have a makefile that is specific to GNU make, and will not be understood by other versions of make. Other make programs look for makefile and Makefile, but not GNUmakefile.

I was looking through the CMakeLists.txt included with snapshot release "2.1.0" (the one currently listed on the glew website) and did see commands that, should, include the headers / etc. I'm not sure why it's not working. Also, when using FetchContent on Windows, I get this warning from building glew: LINK : warning LNK4281: undesirable base address 0x62AA0000 for x64 image; set base address above 4GB for best ASLR optimization.

nigels-com commented 7 months ago

This does seem to build glew, however the current setup fails to properly include the header files, resulting in an error when trying to build with FetchContent.

Sounds like something that can be fixed. I don't think this has been reported before. A separate issue makes sense for that I think.

As for how to keep on parity with native UNIX Makefiles

For maintenance purposes I've been content using the Makefiles and have others contribute and maintain the cmake path. I'm still reluctant to use cmake for the maintainer stuff, and GLEW has always been nearly trivial to build. But recently I've been using just as a modern make alternative. One advantage of that would be that the CMakeLists.txt can move to the normal place and there would be no chance of cmake overwriting the Makefiles. So it's a possibility, in principle. (any volunteers?)

LINK : warning LNK4281: undesirable base address 0x62AA0000 for x64 image; set base address above 4GB for best ASLR optimization.

Yes, that's a legit issue. Looks like it is fixed via Issue #309

astralchan commented 7 months ago

I notice there is a worry of cmake . overriding the Makefile (GNUmakefile?). This can be disabled in the top CMakeLists.txt as mentioned previously. I usually do something like:

$ cmake -B build_dir # or build, but build_dir here since build exists
# windows
$ cmake --build build_dir --parallel %NUMBER_OF_PROCESSORS% --config Debug
# unix
$ cmake --build build_dir --parallel $(nproc --all) --config Debug

[...] GLEW has always been nearly trivial to build.

I imagine maintaining UNIX Makefiles and vcxproj configurations needs about twice the work. The ideal would be to have one cross-platform build system for C. Historically this has been cmake, though meson, ninja, premake / etc other build systems do exist, as well (which cmake could build with ExternalProject if needed).

I believe the appeal of cmake for glew, specifically is that other common C graphics / audio libraries use cmake - e.g. glfw, glm, openal, etc. If glew had a cmake configuration, it would be near trivial to add it to a cmake project using such libraries.

I'm still reluctant to use cmake for the maintainer stuff[...]

Fair. To be honest I'm more familiar with makefile / etc. The only huge appeal to cmake is that Visual Studio Build Tools comes with cmake, and can use and setup msvc for native builds on windows, rather than having to use something like mingw to get a version of make on windows.

nigels-com commented 7 months ago

I imagine maintaining UNIX Makefiles and vcxproj configurations needs about twice the work.

From a maintainer point of view, barely any work at all. They work.

Yes, the world has changed in the meantime though.