Code style guidelines and cleanup/work

Re: Code style guidelines and cleanup/work

Postby StefanP.MUC » 18 Sep 2011, 19:16

Can it be that cmake does a version check between Boost and MingW? I upgraded to MingW 4.6, but Ogre and the boost components that come with Ogre, are built with MingW 4.5.
StefanP.MUC
 

Re: Code style guidelines and cleanup/work

Postby oln » 18 Sep 2011, 19:41

I suppose updating mingw could be related, though wouldn't that make other libraries fail as well?
User avatar
oln
 
Posts: 1020
Joined: 26 Oct 2010, 22:16
Location: Norway

Re: Code style guidelines and cleanup/work

Postby StefanP.MUC » 19 Sep 2011, 10:05

I'm not sure how cmake detects the libs (if it's by name or some binary analysis or something else). The Boost dlls from Ogre are the only ones with "mgw45" in their name.

But I just tried compiling the Ogre Example Browser along with the examples (also with a cmakelists). This works just fines, using the Boost version that comes with Ogre.

So, maybe something in our cmake file about boost isn't right (I think I left it untouched when I did some cleaning up, however). I'm not sure what's the best thing there, but the current boost finding looks very complex. Wouldn't it be enough to set the BOOST_ROOT to "OgreSDK/boost_1_44" and do "find_library(boost)"?
Or alternativle, we could make boost an "official" dependency for OD (if we are planning to use it more intensly anyways). And then simply use "find_library(Boost)", then we could also use the newest official Boost version.

edit: Yes, renaming the Boost dlls to include "mgw46" does hack around the problem. This way I can build again, but we need to find a solution for this problem. We can't expect from our devs to have all dependencies build from source to fit their specific compiler version. I'll try to find out what'S different in the Ogre cmakelists, why it worked there.

edit: The Ogre examples link Boost statically with "set(Boost_USE_STATIC_LIBS TRUE)" and "add_definitions(-DBOOST_ALL_NO_LIB)". Maybe that's the inportant difference here.

edit2: Why are we linking to boost thread anyways? I thought we are using pthreads?
StefanP.MUC
 

Re: Code style guidelines and cleanup/work

Postby svenskmand » 19 Sep 2011, 14:39

StefanP.MUC {l Wrote}:edit2: Why are we linking to boost thread anyways? I thought we are using pthreads?

Why use pthreads if we already depend on Boost, then we might as well just depend on Boost and remove the dependency for pthread entirely.
Jamendo.com - The best music store on the net, uses CC licenses.
User avatar
svenskmand
OD Moderator
 
Posts: 1850
Joined: 09 Dec 2009, 00:07
Location: Denmark

Re: Code style guidelines and cleanup/work

Postby StefanP.MUC » 19 Sep 2011, 16:55

Yes, we should make a clear decission here. Either we switch to boost thread now and remove pthread or alternatively we remove the boost thread dependency until we switch to it in the far future. Keeping both "just in case" makes the build process unneccessarily complex and bug prone.

Maybe we should use also use static linking for everything if the licences allow us to. This seems to prevent possible conflicts as we just saw with Ogre/Boost/MingW (and also Angelscript - or other dependencies that don't provide pre-built libs for every possible compiler). And it makes the Windows shipping easier because we would have to ship less DLLs.
StefanP.MUC
 

Re: Code style guidelines and cleanup/work

Postby oln » 19 Sep 2011, 20:01

We are linking to boost::threads because ogre requires it. So if cmake fails with mingw46 I guess we have to either add boost as a dep, (if we do I guess it's smart to try to move to boost::threads already.) or link statically if that works. Unless you can give parameters to the findBoost script (part of cmake), maybe we should check that first.
User avatar
oln
 
Posts: 1020
Joined: 26 Oct 2010, 22:16
Location: Norway

Re: Code style guidelines and cleanup/work

Postby StefanP.MUC » 20 Sep 2011, 09:54

Why do we have to link OD against boost::thread if only Ogre needs it? Aren't the Ogre libs (e.g. OgreMain.Dll) linked against it already? When I did the renaming hack, the cmake script did run again, but there was nothing else to change - to start the game it still needs the DLL with "mgw45" in its name, not the one with "mgw46" (so even if we link to boost thread it seems that it is not required to run the game currently).

edit: About the size of boost: We could put all required parts of boost into our dependencies folder. The boost license allows to distribute it along with other projects, also splitted up. And if we only ship the header files used by OD then it will only be some kilobytes, I think. Then nobody would have to download the full 600 MB package.

edit2: I just noticed: The Ogre examples also use the CMake attribute QUIET when looking for boost. This ignores errors, so it could possibly be that also Ogre doesn't find boost (but doesn't tell us), but the examples work anyways.

edit3: Ah, just found it out. "set(Boost_COMPILER -mgw45) " solves it without hacking around. I'll update the cmake script accordingly.
StefanP.MUC
 

Re: Code style guidelines and cleanup/work

Postby StefanP.MUC » 20 Sep 2011, 11:08

OK, pushed the updated cmake file. I also updated some other horribly outdated stuff from the bosst part (versions and some vars were unneccessarily overridden with their default values).
StefanP.MUC
 

Re: Code style guidelines and cleanup/work

Postby oln » 20 Sep 2011, 11:09

StefanP.MUC {l Wrote}:Why do we have to link OD against boost::thread if only Ogre needs it? Aren't the Ogre libs (e.g. OgreMain.Dll) linked against it already?


I got errors if I didn't from what I remember.

And yes, you can use http://www.boost.org/doc/libs/1_47_0/to ... index.html to only copy what we would be using.
User avatar
oln
 
Posts: 1020
Joined: 26 Oct 2010, 22:16
Location: Norway

Re: Code style guidelines and cleanup/work

Postby oln » 20 Sep 2011, 13:26

The extra boost 1.42 folders were there as the boost bundled with the ogre library for visual studio 2010 had that version instead of 1.44 for some reason. (Though there were some other problems with it anyway that I can't remember.)
The "set(Boost_COMPILER -mgw45)" shouldn't be set when using msvc, will fix that.
User avatar
oln
 
Posts: 1020
Joined: 26 Oct 2010, 22:16
Location: Norway

Re: Code style guidelines and cleanup/work

Postby StefanP.MUC » 20 Sep 2011, 13:59

Ah, I see. I thought the comment meant that the Ogre package from 2010 uses boost 1.44.
StefanP.MUC
 

Re: Code style guidelines and cleanup/work

Postby paul424 » 19 May 2012, 10:08

I am so stuck , I just want to properly re- indent the code , but there is no single agreed tool or style ... I will use the emacs (setq c-default-style "linux" c-basic-offset 4) then.
BTW: I tested many from there http://emacswiki.org/emacs/IndentingC ,and ellemtel seems to be the most resonable. They say it conforms the populaErik Nyquist and Mats Henricson, Ellemtel : http://www.ktverkko.fi/~msmakela/software/Ellemtel-rules-mm.html. I fought there could be some popular formating tool on the git repo side, that autointends whenever you do the git push :D.
User avatar
paul424
OD Moderator
 
Posts: 660
Joined: 24 Jan 2012, 13:54

Re: Code style guidelines and cleanup/work

Postby oln » 19 May 2012, 14:21

I don't think it's possible to attach anything on the git side. I set up a config for artistic style earlier, that almost matches what we've set up on the wiki, but I can't find the file right now.
User avatar
oln
 
Posts: 1020
Joined: 26 Oct 2010, 22:16
Location: Norway

Who is online

Users browsing this forum: No registered users and 1 guest