SFML community forums

General => General discussions => Topic started by: Klaim on January 05, 2013, 11:47:52 pm

Title: Some CMake remarks
Post by: Klaim on January 05, 2013, 11:47:52 pm

I think there is a CMake related bug.
When I use SFML, or any other dependency on a project using CMake, I put a version in my project repository and use CMake

add_subdirectory( sfml ) # sfml is a directory

Which build CMake for SFML.

The problem is that the current CMake files assume that it's the only project being built.
First error is line 14 of the root SFML CMakeList.txt file:

# include the configuration file
include(${CMAKE_SOURCE_DIR}/cmake/Config.cmake)
 

${CMAKE_SOURCE_DIR} is the directory of the CMakeLists.txt file that is at the root of the cmake command. So in my case I have:

myproject/CMakeLists.txt
myproject/dependencies/CMakeLists.txt
myproject/dependencies/sfml/*all files in the current V2 branch - I checked out few minutes ago*


So the include cannot find the directory. It shouldn't be relative to the cmake call directory, but to the CMakeFiles.txt being executed directory.

Solution: NEVER use ${CMAKE_SOURCE_DIR} but use ${CMAKE_CURRENT_SOURCE_DIR} instead.

This problem appear at several places in the CMake files, where what you want is not the current file directory, but the root directory of SFML sources.
To solve this the simplest way (that I use in my projects) is to define exactly where it is, in the root CMakeLists.txt file :

set( SFML_SOURCES_ROOT_DIR ${CMAKE_CURRENT_SOURCE_DIR} )

In the root directory of the library, then use it in all CMake scripts that need a directory relative to that directory.

I made a patch (attached) that does this, apparently fix the problem on CMake generation but I didn't try yet to compile the generated projects because...well, it's late.  ;D
I think it should be reviewed with care before admission.

Also, while I'm on the CMake subject:
 - why don't you use module paths declaration instead of including modules by hand? I see you do use module paths sometimes, but only in depths of the files. It would have been simpler to set the module dir in the root scripts and then just use modules where you need them. Is there a specific reason for this?
 - there is a lot of paths used that should be just the file name, because the file is in the same directory. Is it  voluntary?

Maybe it just need some cleanup from CMake litterate people. It took me a full year to get to the level I can understand some CMake organisation patterns that work, but I'm far from being an expert.


[attachment deleted by admin]

Title: Re: CMake add_subdirectory( sfml ) cannot work with SFML2.0 & other CMake problems
Post by: Laurent on January 06, 2013, 11:09:59 am

It seems that you work with an old revision of SFML 2, I think this kind of stuff is fixed now.

And I didn't understand your two additional suggestions. Paths, files, modules, ... ?? Would you mind explaining clearly, with examples of what you refer to?

Title: Re: CMake add_subdirectory( sfml ) cannot work with SFML2.0 & other CMake problems
Post by: Klaim on January 06, 2013, 01:54:48 pm

Quote from: Laurent on January 06, 2013, 11:09:59 am

It seems that you work with an old revision of SFML 2, I think this kind of stuff is fixed now.

Argh I just found that you're using git now. I totally forgot about that and my version was under SVN so I thought it was the right repository... that explains why I there have been no changes since march of last year...  :-X

Ignore my bug report, sorry again. (should have sleep first)

Quote

And I didn't understand your two additional suggestions. Paths, files, modules, ... ?? Would you mind explaining clearly, with examples of what you refer to?

Sorry for the lack of clarity, seems that these remarks are still valid for the git repo.
I was talking about setting the cmake directory as a cmake module path, like this:

set( CMAKE_MODULE_PATH ${CMAKE_MODULE_PATH} ${PROJECT_SOURCE_DIR}/cmake )
 

If you do this in the root cmake file you just have to do include( ModuleName ) instead of the full path. This is a minor change but it should help maintainability.


For the sources paths, there is a lot of full path to the source files that should just be relative to the current file. What I mean is for example in SFML/Main you have:

add_library(sfml-main STATIC ${PROJECT_SOURCE_DIR}/src/SFML/Main/SFML_Main.cpp)

Which could be written

add_library(sfml-main STATIC SFML_Main.cpp)
 

I suppose the source directory and the target declaration directory should be the same so it might help modifying the files to remove this noise. I had the same patterns before in my projects, I figured that the CMakeFiles.txt that is associated to the sources will always be moved around with the sources so the day you change the directory organisation you'll have to change all CMakeFiles.txt because they contain full paths to the sources instead of relative paths.

It's really a minor suggestion but I believe it will also help maintainability.

Title: Re: Some CMake remarks
Post by: Laurent on January 06, 2013, 07:10:03 pm

Quote

I was talking about setting the cmake directory as a cmake module path

Ok. I thought CMAKE_MODULE_PATH was only used by find_package.

Quote

For the sources paths, there is a lot of full path to the source files that should just be relative to the current file

I didn't test, but I'm pretty sure that relative paths are broken when you use a macro.

Title: Re: Some CMake remarks
Post by: Klaim on January 06, 2013, 07:17:50 pm

Quote

Ok. I thought CMAKE_MODULE_PATH was only used by find_package.

Yes, it is and it also makes the registered paths implicit in include.

Quote

I didn't test, but I'm pretty sure that relative paths are broken when you use a macro.

Never heard about that... In my biggest project I use tons of macros that are defined in a directory for cmake modules, then used in tons of sub projects. Never had a problem so far.
What kind of macro-related problems are you thinking about?
Macros should generate code in-place so it should be safe, but maybe I'm missing something.

Title: Re: Some CMake remarks
Post by: Laurent on January 06, 2013, 08:48:37 pm

I don't remember, but I'm sure that I did it on purpose :-\

Anyway, these paths are put in variables so it doesn't really matter if they are implicit or explicit, they are only written once. I don't think it deserves a modification.