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
Add Experimental CMake support #1059
base: master
Are you sure you want to change the base?
Conversation
Wow! Nice work! How does build time compare? What does cmake do better than make? Would this make reproducible builds more possible? |
@alexjurkiewicz see #903 for some background.
Is there a problem with reproducible builds? 4d52153 removed the only issue I'm aware of. |
Doesn't seem to work for me: https://gist.github.com/alexjurkiewicz/6a25453c1d1acbf20bb99dc4e23fa656 |
Thanks for testing and providing the build output! 2 unrelated issues I can already see:
I did some last minute target moving before committing and I might not have tested properly. I'll get back to it after work. CEST here, so it'll be some time :) |
I reread the output: those are all warnings last two lines say build files have been written correctly. What happens if you do a subsequent make? Can you post the build output as well? Just in case there is some confusion: cmake is not compiling the sources but creating a makefile/ninja/xcode (untested) setup that then can be used to compile |
About this, I am not sure what you mean. Should I be testing with a different |
Build output:
|
Have a look into
|
@alexjurkiewicz W.r.t how to run the CMake build I tried to update the documentation to make some things more explicit and clearer. Please have a loot at the README-CMAKE.md your feedback is welcome (and everyone else's as well). It is important to me that the README is correct and actually helpful! W.r.t ncurses: I am expecting to be able to find ncurses with pgk-conf. I.e. for ncurses installed via homebrew you need to Since your issue report prompted me to redo all builds an retest them locally I did find 2 more things:
And ofc, checkwhite has to be pacified :) Oh and you asked about builds times: |
This might already be apparent, but the OS X builds currently don't ever use pkg-config, so nothing is tested with pkg-config packages at all on OS X. I think the reason for this is just historical (and maybe it's a bit less reliable overall, I've had problems with pkg-config installed via anaconda), but it does mean that trying to enable this adds an extra layer of potential problems. As to ncurses, it currently builds with system ncurses, so there shouldn't be anything too complicated about that. On my system, the link libraries are:
In the compile flags there's just |
@rawlins Thanks for the info, Ill see that I fixup this PR so that it handles MacOS system ncurses |
I think I have now addressed what was raised so far:
|
Sorry for the slow response. But nice! It works perfectly on my system too :) |
Note: If you have checked out this branch be aware that I try to rebase it on top of master once a week to detect breaking changes early. |
What's left for this to be merged? Looking forward to the change! |
Thanks for continuing to work on this! I tried it out and was able to get up to the point of linking where it seemed to fail to link with lua. This is OS X, with lua installed via macports.
Link errors:
Any hints? |
Other possibly superficial comments / questions while taking what is really my first look at this:
|
Oh, sorry—I'm not the original author, if that's what you thought. I was just wondering what needed to be done still. I can probably answer a few of those questions though, but I'm sure they won't be perfectly accurate.
CMake should be perfectly good for using builds under contribs. It doesn't look like it's currently set up to use contribs programmatically, but you might be able to comment Again, take the above with a grain of salt—I'm just answering what I can in case the original author isn't available for a while or something. /shrug |
@perryprog yes, sorry for any confusion, I do realize that you're not the PR author. |
With a bit more investigation it looks like this step is incorrect: I have both lua 5.1 and lua 5.3 installed via macports and this path is actually to the lua 5.3 lib. Lua 5.3 is in Edit: I upgraded cmake (from 3.13.1 to 3.14.5) to no effect, but from some googling I do suspect it may be an issue in one of the cmake FindLua scripts that I'll need to work around (I just don't know how). |
@rawlins ill look into it, probably not before Friday tho |
I am on lunch brake so I can only comment to some of the things @rawlins and @perryprog wrote, some of the stuff I have to look into proper. 1.) About Lua link errors, actually no idea. I have tested on MacOS with home-brew. Ill red to investigate this a bit. 2.) About README. I just wanted to have some readme, I will move that where ever you prefer. 3.) Switching build arguments most of the time means you need to re-run cmake. After you called cmake a 'CMakeCache.txt' is added to your build folder (and respectively updated on each cmake invocation). Here you can always check what variables are set to. NOTE: if you re-run cmake it will not clear variables that you did not define on a second invocation, e.g. you call cmake -DMY_VARIABLE=5 -DMY_OTHER_VARIABLE=6
... hack hack
cmake -DMY_VARIABLE=10 CMakeCache.txt will still list Having several build folders is probably the way to go, id say probably because there are quite a few 'nice' things we can do later, like e.g. making WebTiles, LocalTiles, Console top level build targets so that they can be build from a single invocation of make. (But that requires some work on all the code generation tools) Does that answer your question? 4.) About crawl_ref: @perryprog pretty much nailed it :) 5.) About Contribs: I did start with building from contribs only which you can see from the commented out code. I can change the MacOS build to use contribs only. Will update the PR accordingly. Since this PR is only supposed to get cmake started in crawl I would like to keep it limited for now to in that case then build linux from external deps and macOS from contrib. OK? 6.) About out of source builds: Cmake does support in source builds but for this project I explicitly added a check to disallow them. Out of source builds allow to maintain multiple build trees on a single source folder, i.e. I can quickly switch configs and only rebuild changed parts of my build tree. I think this is very useful for crawls atm since it allows me to quickly compile all three frontends (web, local, console) with a bit of bash, e.g. cd WebTiles-build && make -j$(nproc) && \
cd ../LocalTiles-build && make -j$(nproc) && \
cd ../NoTiles-build && make -j$(nproc) && cd .. Also one more abstract property of out of source tree builds is that they help to keep dependencies on generated code more explicit since they usually have to be modelled. EDIT: typos |
@rawlins & @perryprog I an currently making all contribs available for Linux and MacOS, this will take a bit since testing each change takes a lot of time. @rawlins In the meantime you could help me with the lua5.3 dynlib issue. If the paths you mentioned above are correct this almost looks like an issue in the FindLua module from cmake. Which would really suck. To understand whats happening I would like to get some trace output from cmake. To do that I would like to ask you to create a new clean build folder and run cmake with |
Support for building with CMake added for Linux/OSX native builds. Build output can be controlled by supplying TILE_MODE to the build. WEB_TILES for building the web version. LOCAL_TILES for native tiles and NO_TILES for ascii mode. E.g. cmake -DTILE_MODE=WEB_TILES <path to source> Use of "prebuilt" level compiler sources can be controlled by -DUSE_PREBUILT_LEVCOMP=ON/OFF Mergebase can be set with -DMERGE_BASE=<commit id> Arbitray defines can be passed to the build by -DADDITIONAL_DEFINES=FOO;BAR=1;DEBUG Build output is generated to 'stage' and crawl can be run diretly from its build location. All dependent files are copied to stage. Note that starting the webserver from stage can be done by ./webserver/server.py Note: Since there is unfortunately still in-tree generation hapening it is not advised to have different build types (localtiles/webtiles/notiles) running side by side w.o. EXPLICITLY cleaning the source folder. Limitations: * No special Travis support * Limited compflags.h (no cflags, no ldflags) * No crossbuild (linux -> Windows) atm * No Windows native build atm (not tested) * Only builds from external deps * Only support out-of-tree builds * Still does generate some files in source tree Fixes: #903
Using contribs defaults to ON on MacOS and OFF on Linux.
So this should address your concerns @rawlins. One thing i noticed today is that PCRE support is yet missing in this PR. It this is immediately required I can add this soon (TM) |
Thanks for working on this! Unfortunately I currently cannot build:
This is on an up-to-date arch linux x86-64 system. NO_TILES builds work fine. Haven't tried WEB_TILES. |
@aidanholm i also ran into that issue. i fixed it and a few other things i found and made a PR against @Ozaq 's branch (https://github.com/Ozaq/crawl/pull/1). are people still interested in this? i can help if there's interest, i'd really like to see this happen. |
@brianlheim yes, there is absolutely still strong interest in moving away from the horrible makefile that we are currently stuck with, but it would need to be a rather feature-complete conversion; cross-compiling with mingw and using vendored dependencies are hard requirements. There are also people building on msys and probably some on WSL as well. |
ok! just want to make sure i understand:
regarding CI, Travis has macOS and Windows images available now, and the Windows image, although still in 'early adoption' stage, can use MSYS2. i would recommend adding jobs for those while the newer build system settles. the coverage doesn't have to be as thorough as the linux jobs you already have. |
is it this build (https://travis-ci.org/github/crawl/crawl/jobs/564379148)? so linux compiling a binary to be used inside a windows mingw environment? |
@brianlheim No, those binaries are windows-native and don't require a special environment to run. The mingw toolchain description from the homepage:
|
okay... so just cross compiling a binary to be used on windows then. am i correct in thinking that's what is meant by "cross-compiling with mingw" above though? |
Yes, that's what aidanholm was referring to, and that's very important for us as it's how we produce native windows builds, including for release. The MSYS2 option is what we sort of recommend for windows people wanting to do development, but I'm not sure if that's truly our recommended windows dev environment any more. Typical windows compilation using MSVC was fixed somewhat recently and still works, to my knowledge. |
okay! are MSVC and MSYS2 options hard requirements for CMake support in this PR? or merely aspirational? |
I will leave a full answer to that question to people more qualified to answer, but I think it's fair to say that we need to support at least one method of building crawl natively from windows. I guess MSVC would be the best if we had to choose one, since it works across recent versions of windows? I can't comment on the current state of compilation under windows, though. Other devs probably can do so more easily. |
sounds good! i've set up a few simple travis jobs that use cmake on my own fork, one is an MSVC build. will tweak around with it and see if there are any major issues. |
As someone who uses MSYS2 to compile crawl, I have to say I vastly prefer it to MSVC, would be sad to have to move to MSVC. MSYS2 is very easy to work with, especially for people who work in Windows but are also familiar with Unix. It's self-contained, doesn't 'invade' the OS unlike MS tools (especially MSVC) do, and things like reinstalling the MSYS2 environment from scratch are really easy. Also, MSYS2 itself is an envirnoment, i.e. a shell, package manager, tools, basically a 'fake Unix' within windows, but most of that is not that relevant here. The actual complier toolchain is Mingw-w64. Basically MSYS2 is a full 'development' platform, and Mingw-w64 (contained within MSYS2) is a 'distribution' platform. And that's how it is being used here, for easy compliation of windows binaries with minimal overhead. |
If msys is a fully featured alternative to MSVC that is reasonably easy for windows users to compile with, I'd probably prefer to remove MSVC support: crawl already has an immense maintenance burden, and we already use msys for official binaries. If there are no devteam members using it and few enough users, I don't think the added burden is worth it (from e.g. binaries built in weird ways). I'd say the same for Xcode: it looks like the project file for that is already broken, so presumably noone is using it. I'd be interested in knowing what the benefits are of MSVC; presumably a more familiar dev environment for windows users? |
I'd say it's definitely easier to compile in MSYS2 (and generally easier to install, maintain, get working without issues), but harder to do do full-fledged development (including debugging, etc...). Also thanks to MSYS2 being POSIX-compliant, you can just directly use a ton of Unix stuff and knowledge and approach. The sort of full development that would be easier in MSVC probably doesn't matter much for non-devteam contributors, people like me who just want to compile and fix small bugs here and there... I mean, my steps to get crawl compiled after a recent reinstall were to install MSYS2 and then do this sequence: That's it, it took a few minutes. And all of it just exists in its own folder without touching the rest of the Windows OS in any way. If I think about installing MSVC (which in itself is quite a burden), and then look at this: https://github.com/crawl/crawl/blob/master/crawl-ref/INSTALL.md#maintenance-notes it seems like a nightmare. But maybe people who use MSVC could chime in here. |
Re MSVC vs MSYS2: dropping MSYS2 isn't on the table (no devteam member uses MSVC and we can't even support it -- there's not even any currently active devteam member with a windows machine AFAIK, we just use VMs). Part of the appeal of cmake was that it would hopefully make handling MSVC easier exactly given that we can't directly support it. Nothing is a hard requirement for cmake in the sense that the plan has been to aim to merge some form of cmake building without removing make support (because of the complexity of the task we don't have any plans to convert online servers to cmake any time soon after this PR gets merged), but MSYS2 is definitely a hard requirement for a complete dcss build system. re travis, we are still running travis jobs but have begun moving to github actions for most CI improvements, I think. There is a mac build via github actions. See https://github.com/crawl/crawl/tree/master/.github/workflows for the setup, though this is still evolving. |
This PR hasn't seen any changes in a couple years. I definitely think resurrecting the project is possible but I may soon close the PR as stale if it doesn't get picked up soon. |
Support for building with CMake added for Linux/OSX native builds.
Build output can be controlled by supplying TILE_MODE to the build.
WEB_TILES for building the web version. LOCAL_TILES for native tiles and
NO_TILES for ascii mode.
E.g.
Use of "prebuilt" level compiler sources can be controlled by
-DUSE_PREBUILT_LEVCOMP=ON/OFF
Mergebase can be set with
-DMERGE_BASE=
Arbitray defines can be passed to the build by
-DADDITIONAL_DEFINES=FOO;BAR=1;DEBUG
Build output is generated to 'stage' and crawl can be run diretly from
its build location. All dependent files are copied to stage. Note that
starting the webserver from stage can be done by
./webserver/server.py
Note:
Since there is unfortunately still in-tree generation hapening it is not
advised to have different build types (localtiles/webtiles/notiles)
running side by side w.o. EXPLICITLY cleaning the source folder.
Limitations: