Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CR] CMake: update rules; add Linux install targets; add support for dev-builds; #11970

Merged
merged 97 commits into from
May 9, 2015
Merged

[CR] CMake: update rules; add Linux install targets; add support for dev-builds; #11970

merged 97 commits into from
May 9, 2015

Conversation

brezerk
Copy link
Contributor

@brezerk brezerk commented Apr 10, 2015

Hi there.

I'd like to use QtCreator as dev env. But It seems like cmake scripts are outdated and require some updates.

However, currently cmake build is not upstream build system for the project...

So. Do you guys have any plans for build system migration? Does this activity will be helpful for you?

@Psimage
Copy link

Psimage commented Apr 10, 2015

Hi. Thanks for updating cmake build!
It would be definitely helpful for those who used to different development environment (e.g. I'm used to IntelliJ IDEA IDE since i'm from Java world so i want to try CLion as an IDE for C++ but it only supports CMake).

ADD_CUSTOM_TARGET(
lua_bindings
COMMAND ${LUA_BINARY} generate_bindings.lua
BYPRODUCTS ${CMAKE_SOURCE_DIR}/src/lua/catabindings.cpp
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BYPRODUCTS argument in ADD_CUSTOM_TARGET command requires CMake 3.2 but required version of CMake is set to 2.8

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed, thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quoting the documentation:

The target has no output file and is ALWAYS CONSIDERED OUT OF DATE even if the commands try to create a file with the name of the target. Use ADD_CUSTOM_COMMAND to generate a file with dependencies.

And indeed, every time I run make, it recreates catabindings.cpp and recompiles and relinks it, which is at least a waste of time.

The standard Makefile has "$(LUA_DIR)/class_definitions.lua" and "$(LUASRC_DIR)/generate_bindings.lua" as explicit dependencies for regenerating the bindings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah. ADD_CUSTOM_COMMAND seems to work just fine. thanks!


# Required library linkage
if(TILES)
IF(TILES)
target_include_directories(cataclysm PUBLIC ${SDL2_INCLUDE_DIR})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not your fault, but maybe it's time to increase the required cmake version? target_include_directories was added in cmake 2.8.11, required version is "only" 2.8.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heah. We need to do something with this :) Thanks.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the build was broken before and nobody noticed in 15 days its probably safe to upgrade.

INCLUDE_DIRECTORIES( ${CMAKE_SOURCE_DIR}/src ${CMAKE_SOURCE_DIR}/src/chkjson )

# Add the actual executable
ADD_EXECUTABLE(chkjson ${CHKJSON_SOURCES} ${CHKJSON_HEADERS})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adds the chkjson file to the "all" target and it's therefor made every time somebody runs make all. It's not a big thing (only 2 source files + linking), but the original Makefile has this as a separate target that has to be invoked explicitly.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. It seems like it is used only for debug/test purposes.
I'll move it to 'test' chain when will start it's implementation :)

@BevapDin
Copy link
Contributor

make install does not install the gfx folder. According to the Makefile, it should be installed (only if TILES is defined) into $DATA_PREFIX, same place where font, json etc. goes.

@brezerk
Copy link
Contributor Author

brezerk commented Apr 10, 2015

Fixed in: https://github.com/brezerk/Cataclysm-DDA/commit/b21dd954dd414bdac793e6d0dc50dd4443513695 :P

Btw: it seems like current source code does not able to run binary from arbitrary PREFIX_PATH directory? Or am I missing something?

@brezerk
Copy link
Contributor Author

brezerk commented Apr 10, 2015

ah. I see:

398     DEFINES += -DPREFIX="$(PREFIX)"

@BevapDin
Copy link
Contributor

There is another thing, cmake installs the content of the data folder to "./share/cataclysm-dda/data/", but it belongs into "./share/cataclysm-dda/" (there should be no data folder at all).

@BevapDin
Copy link
Contributor

Btw: it seems like current source code does not able to run binary from arbitrary PREFIX_PATH directory? Or am I missing something?

I have CMAKE_INSTALL_PREFIX set to "/tmp/cataclysm" and can run the binary from there with ./bin/cataclysm --basepath . (after moving the contents of share/cataclysm-dda/data/ to share/cataclysm-dda/).

Edit: and now it works with the parameter, just ./bin/cataclysm is enough.

@brezerk
Copy link
Contributor Author

brezerk commented Apr 10, 2015

Good catch, thanks! Updating build rules...

@brezerk
Copy link
Contributor Author

brezerk commented Apr 10, 2015

OK. It seems I will need to modify the source to be able to run binary from the cmake build directory:

[ himera ] brezerk@pts/4:1028 :( 1 ~/develop/Cataclysm-DDA-Fork/build $
cmake .. -DRELESE=OFF
--  build options --
-- Build  in development mode (RELEASE=ON) --
-- CMAKE_INSTALL_PREFIX          : /home/brezerk/develop/Cataclysm-DDA-Fork
-- Configuring done
-- Generating done
-- Build files have been written to: /home/brezerk/develop/Cataclysm-DDA-Fork/build

However trying to run it:

[ himera ] brezerk@pts/4:1029  ~/develop/Cataclysm-DDA-Fork/build $
./src/cataclysm
[ himera ] brezerk@pts/4:1030 :( 1 ~/develop/Cataclysm-DDA-Fork/build $

strace output:

close(4)                                = 0
open("/home/brezerk/.cataclysm-dda/config/fonts.json", O_RDONLY) = -1 ENOENT (No such file or directory)
open("\"/home/brezerk/develop/Cataclysm-DDA-Fork\"/share/cataclysm-dda/fontdata.json", O_RDONLY) = -1 ENOENT (No such file or directory)
write(3, "16:48:50.732 WARNING : opendir ["..., 142) = 142
stat("/etc/localtime", {st_mode=S_IFREG|0644, st_size=2097, ...}) = 0

So it tries to look into 'share/cataclysm-dda'.
Will try to use -DRELEASE to control this.

@brezerk
Copy link
Contributor Author

brezerk commented May 5, 2015

Ok. I was able to test both MSYS2: MinGW32 and MinGW64 bit using:

For MinGW32:

        $ cmake -G "MSYS Makefiles" \
                -DCMAKE_MAKE_PROGRAM=mingw32-make\
                -DCMAKE_C_COMPILER=i686-w64-mingw32-gcc\
                -DCMAKE_CXX_COMPILER=i686-w64-mingw32-g++\
                -DCMAKE_SYSTEM_PREFIX_PATH=/c/msys32/mingw32\
                ..
        $ mingw32-make

For MinGW64:

        $ cmake -G "MSYS Makefiles" \
                -DCMAKE_MAKE_PROGRAM=mingw32-make\
                -DCMAKE_C_COMPILER=x86_64-w64-mingw32-gcc\
                -DCMAKE_CXX_COMPILER=x86_64-w64-mingw32-g++\
                -DCMAKE_SYSTEM_PREFIX_PATH=/c/msys64/mingw64\
                ..
        $ mingw32-make

Compiles ok. ncurses version works as expected. The tiles one fails as expected :D

Small investigation showes that it do segfault on:

WinCreate () at Y:/Cataclysm-DDA-Fork/src/sdltiles.cpp:309

 308         renderer = SDL_CreateRenderer( window, -1, SDL_RENDERER_ACCELERATED |
 309                                        SDL_RENDERER_PRESENTVSYNC | SDL_RENDERER_TARGETTEXTURE );
SDL_RENDERER_ACCELERATED the renderer uses hardware acceleration

Not sure if my KVM has any hw accel... :D

@LunaTheArcticFox
Copy link
Contributor

Not sure if my KVM has any hw accel... :D

I can test this out again when I get home for you.

@brezerk
Copy link
Contributor Author

brezerk commented May 5, 2015

@KrazyTheFox It will be helpfull. thanks.

@brezerk
Copy link
Contributor Author

brezerk commented May 5, 2015

Yeah. It seems like my assumption was correct.

I have modified config/options:SOFTWARE_RENDERING true

And now it works using sf renderer instead of hw one :)
ddatiles

@kevingranade
Copy link
Member

kevingranade commented May 5, 2015 via email

@brezerk
Copy link
Contributor Author

brezerk commented May 6, 2015

In case, if some one wish to test Cataclysm-DDA cmake build scripts, here is doc COMPILING-CMAKE.md

@brezerk brezerk changed the title [CR][WIP] CMake: update rules; add Linux install targets; add support for dev-builds; [CR] CMake: update rules; add Linux install targets; add support for dev-builds; May 6, 2015
@LunaTheArcticFox
Copy link
Contributor

Using a fresh install of msys and a clean copy of Cataclysm, I was unable to compile a working version on Windows. I still needed to specify the libraries as in my previous compilation post when using cmake. It would successfully generate the makefile and when running make, it would build without errors. Upon trying to run either cataclysm.exe or cataclysm-tiles.exe, nothing would happen and I was unable to get either of the exes to generate any error messages.

@macrosblackd
Copy link
Contributor

Did you install the deps with msys or manually?
On May 7, 2015 8:30 AM, "Mitchell Matthews" notifications@github.com
wrote:

Using a fresh install of msys and a clean copy of Cataclysm, I was unable
to compile a working version on Windows. I still needed to specify the
libraries as in my previous compilation post when using cmake. It would
successfully generate the makefile and when running make, it would build
without errors. Upon trying to run either cataclysm.exe or
cataclysm-tiles.exe, nothing would happen and I was unable to get either of
the exes to generate any error messages.


Reply to this email directly or view it on GitHub
#11970 (comment)
.

@LunaTheArcticFox
Copy link
Contributor

Did you install the deps with msys or manually?

With msys.

@macrosblackd
Copy link
Contributor

That's strange, I was able to build with cmake and it detected the
installed dependencies.

On 5/7/15, Mitchell Matthews notifications@github.com wrote:

Did you install the deps with msys or manually?

With msys.


Reply to this email directly or view it on GitHub:
#11970 (comment)

@brezerk
Copy link
Contributor Author

brezerk commented May 8, 2015

@KrazyTheFox You are specifying archive libraries for !static build. I guess, this is the reason, why you are getting broken executables.

It is strange, b/c I had no need to specify the libraries manually...

Are you using mingw shell or msys2 shell to build the project?

msys2

@LunaTheArcticFox
Copy link
Contributor

Msys 2 shell. I'll spin up a clean Windows vm today and try it in there to make sure I don't have something external messing it up.

@brezerk
Copy link
Contributor Author

brezerk commented May 8, 2015

@KrazyTheFox If I understand it correctly, default "MSYS2 Shell" is used for pacman stuff.
I just checked, and it seems like it does not setup PATH for libraries/cmake/whatever for me too.

Can you try to build using "MinGW Win64 Shell" or "MinGW Win32 Shell" please?

@LunaTheArcticFox
Copy link
Contributor

Using the MinGW32 shell did make it easier to build, but the problem wasn't that. I was building (or attempting to build) release versions with "-DRELEASE=1". When I switched that to "-DRELEASE=0" to build a debug build, it succeeded in building a working tiles build.

Edit: I'm able to build a release build that launches, however, when opened I get only a blank, black screen.

@brezerk
Copy link
Contributor Author

brezerk commented May 8, 2015

@KrazyTheFox Yes, -DRELEASE=1 require you to place binary into game folder manually make install currently will now work for Windows (will fix this in the future).

Does ncurses version works? afik: the first CataclysmDDA start can take some time, while it read game data and create an cache?

Not sure, but can you try to build with -DUSE_HOME_DIR=OFF option set.

@LunaTheArcticFox
Copy link
Contributor

@brezerk
I've moved the exe manually and placed all the required DLLs in the folder as needed. I'll give the curses version a try later as well as the home dir option, but for now I must go spend a large sum of money on an air conditioner since my HOA sucks. :( Should be able to test this out this evening.

@LunaTheArcticFox
Copy link
Contributor

I've figured out the problems with the release builds. The .exe itself can be in any folder, but it expects the data/ folder to actually be located at C:\Program Files (x86)\CataclysmDDA\share\cataclysm-dda\. This is problematic, since the game can't access any of those files without running it as an Administrator and it also mean you'll need to copy those files manually with every update. The configuration directory it generates is in the correct place, ./config/. The debug build uses all relative paths as expected.

The exact cmake command run:

cmake -G "MSYS Makefiles" -DUSE_HOME_DIR=0 -DTILES=1 -DLOCALIZE=0 -DRELEASE=1 -DCMAKE_MAKE_PROGRAM=mingw32-make -DCMAKE_C_COMPILER=i686-w64-mingw32-gcc -DCMAKE_CXX_COMPILER=i686-w64-mingw32-g++ -DCMAKE_SYSTEM_PREFIX_PATH=/c/msys32/mingw32 ..

I will say thanks for helping me out with all this and getting a simpler build system up on Windows. It'll be very nice to not have to run Cataclysm in a VM!

@brezerk
Copy link
Contributor Author

brezerk commented May 8, 2015

@KrazyTheFox thank you for your feedback.

-DPREFIX has no scene in case of WIN32 build, since it will hardcore data patch to CMAKE_INSTALL_PREFIX which is defaults to C:\Program Files (x86)\CataclysmDDA\share\cataclysm-dda\.

Fixed in: https://github.com/brezerk/Cataclysm-DDA/commit/6d0678c184c1793389e5a16fca5bc7af7eb48116

@LunaTheArcticFox
Copy link
Contributor

@brezerk
Perfect! Just went through the whole set of instructions again to double check that things work. Built a release version that runs on Windows with almost no effort. Very nice!

@kevingranade kevingranade merged commit 2cd2c05 into CleverRaven:master May 9, 2015
@brezerk brezerk deleted the improved_build_system branch May 9, 2015 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants