cmake improvements

Everything about development and the OpenMW source code.
User avatar
Ace (SWE)
Posts: 887
Joined: 15 Aug 2011, 14:56

Re: cmake improvements

Post by Ace (SWE) »

I'm guessing that the components are supposed to be in their own project, if so then it seems to be working.
User avatar
Zini
Posts: 5538
Joined: 06 Aug 2011, 15:16

Re: cmake improvements

Post by Zini »

Okay. Then I consider the cmake clean up task concluded.

I did not rewrite the scripts for libs, apps/esmtool (not worth it, since we are essentially finished with these) and apps/launcher (don't know enough about Qt to not mess up the scripts).

Both components and apps/openmw have new cmake scripts that will make adding/removing/renaming source files much easier in the future. The change in components should also improve build speed a bit in some cases.
User avatar
pvdk
Posts: 528
Joined: 12 Aug 2011, 16:34

Re: cmake improvements

Post by pvdk »

Tried to compile OpenMW today after being away for a while, but it fails for me.

Linking Boost for the launcher fails with the following error:

Code: Select all

[...]In function `Cfg::ConfigurationManager::loadConfig(boost::filesystem3::path const&, boost::program_options::variables_map&, boost::program_options::options_description&)':
configurationmanager.cpp:(.text+0xf98): undefined reference to `boost::program_options::basic_parsed_options<char> boost::program_options::parse_config_file<char>(std::basic_istream<char, std::char_traits<char> >&, boost::program_options::options_description const&, bool)'
[...]
Note: I'm using Boost 1.47.
I'm trying to fix this bug myself but haven't had any luck yet.

UPDATE: Problem seems related to my PKGBUILD as it compiles in my normal dev env.
User avatar
Zini
Posts: 5538
Joined: 06 Aug 2011, 15:16

Re: cmake improvements

Post by Zini »

Sorry, can't help you with that. But I am pretty sure that I touched neither the launcher nor boost during the cmake clean up. Are you sure, that the cmake changes triggered the problem? Might be another unrelated change or simply a hiccup in your build system.
User avatar
pvdk
Posts: 528
Joined: 12 Aug 2011, 16:34

Re: cmake improvements

Post by pvdk »

Yeah something's wrong with the PGKBUILD script. It calls cmake with -DCMAKE_INSTALL_PREFIX=/usr. It did work before, I don't know what's wrong.
User avatar
pvdk
Posts: 528
Joined: 12 Aug 2011, 16:34

Re: cmake improvements

Post by pvdk »

@Zini: I've sent you a pull request for some fixes I made to the CMake files.

Please be aware that the approach in this commit means that CMake won't be able to track individual files anymore. Not a big problem but it would mean people should do a make clean after each pull, as CMake can't figure out if a file is changed. People reporting issues would always have to do a complete build before we look into the issue reported.

The variable for the component files used in the /apps/openmw CMakeLists.txt contained a typo and it was out of scope (thus empty), since it got defined in a subdirectory. I fixed that with a redefine as PARENT_SCOPE.

Also, your macro includes every file it finds in the subdirectories, even non-source files. I have some backup files and testing files in my source tree which got added. For that I would like to suggest a different approach.
It currently works for the /apps/openmw CMakeLists.txt but the components CMakeLists.txt excludes some directories, so I couldn't use it for that. (When I included the directories it excludes the build breaks, which we should look into because the tests are the the things that break.)
Please have a look at my suggestion here.
This would mean the OpenMW part in the macro could be removed.

Let me know what you think.

Regards,
pvdk.
corristo
Posts: 495
Joined: 12 Aug 2011, 08:29

Re: cmake improvements

Post by corristo »

Does it means that after every tiny change in sources one should recompile entire project? Would'n it be a huge pain for developers?
User avatar
Zini
Posts: 5538
Joined: 06 Aug 2011, 15:16

Re: cmake improvements

Post by Zini »

Please be aware that the approach in this commit means that CMake won't be able to track individual files anymore. Not a big problem but it would mean people should do a make clean after each pull, as CMake can't figure out if a file is change
Is that so? I never experienced this problem. cmake usually recognises file changes just fine, even after a merge or switching branches. The only problem I noticed is, that when a header that did not had a implementation file before suddenly gets one, it won't be compiled. Running cmake again fixes this problem.
Also, your macro includes every file it finds in the subdirectories, even non-source files. I have some backup files and testing files in my source tree which got added. For that I would like to suggest a different approach.
I agree that the GLOBs are not a good idea (they certainly weren't mine). But my original approach didn't work with Visual Studio and so someone came up with this idea. I guess we could replace them by explicitly checking for hpp and cpp files in the macro instead (or limiting the GLOBs to these patterns).

Your changes in the pull request look good. The suggestion you made in this thread doesn't look good since it would indeed exhibit the problems mentioned above.
User avatar
pvdk
Posts: 528
Joined: 12 Aug 2011, 16:34

Re: cmake improvements

Post by pvdk »

Yeah I did not encounter the issue myself but the CMake documentation explicitly warns for this problem, and several mailinglist posts do so too. You are right that a re-run of CMake fixes this problem, as such it isn't a big issue, people only have to be aware of it.

The approach I suggested in the gist only includes *.c, *.cpp, *.h and *.hpp files. Essentially it does the same as the macro with less code, as stuff like list (APPEND files "${f}") list (APPEND OPENMW_FILES "${f}") is not necessary at all.
User avatar
Zini
Posts: 5538
Joined: 06 Aug 2011, 15:16

Re: cmake improvements

Post by Zini »

Yes, but unless I misunderstand this code snippet, you are not specifying the source files explicitly. And that would cause exactly the kind of trouble you mentioned, essentially requiring people to re-run cmake every time after adding/removing/moving a file and not just only in some rare edge cases.
Post Reply