widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Thanks for reviewing, Shevonar! I am very grateful that you took up reviewing changes.

> It seems that the WL_SRC set contains most of the source files twice: once with indentation, once without.

Every file was only there once, but the indent was not consistent. Fixed.

[minizip]
I much rather always use a upstream version, but most distributions do not offer one. Instead of sometimes doing this or sometimes doing anything else, I much prefer we just use the bundled one and be done with. If some upstream decides to change the shipped headers of zlib and this breaks us, then we need extra patches for those systems - or we need to bring back the conditional if everything else fails.

[dependencies]
First, SDL is currently linked into everything, so it is strange that it complains about missing SDL dependency there.

Here is what I think is happening: we have a bunch of cyclic dependencies in the system (this is one of the motivations of this change: make them explicit, so they can be broken. this and deterministic builds.). sound for example includes files from logic/, so it depends on that. However, linking sound itself into a STATIC library does not seem to have any problems with missing dependencies - 'ninja sound' just works, though it must contain unresolved symbols.

Only when you want to build a .exe or a shared library will the system start complaining about missing dependencies. Right now, we only do that with tests and widelands.exe. Then, the order of the libraries on the commandline suddenly matters - and we might need to include some library more than once to get all symbols in one linker pass. I think this is where it fails right now as cmake does not seem to be doing that. 

If this problem can be solved by really mentioning all dependencies for each static library, I am unsure. There will be cycles there and I do not know what cmake makes of that then. All I know is that it compiles and links on my system (using the gold linker which seems to do multiple passes), so I cannot really move this problem forward.

-- 
https://code.launchpad.net/~widelands-dev/widelands/cmake-reworked/+merge/222455
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/cmake-reworked.