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

Port to using CMake. #3012

Closed
wants to merge 5 commits into from
Closed

Conversation

Symbitic
Copy link

@Symbitic Symbitic commented Aug 16, 2020

An initial working version of porting the build system to CMake.

CMake includes many advantages over hand-written Makefiles, including:

  • Cross-platform support.
  • Better IDE integration.

Instructions on everything are included in the updated Readme.

Now, there are some things to be aware of:

  1. I started this fork about 1 week ago, so it won't include any changes since then. That said, there were no changes to the source code, so everything should be able to easily merge there.
  2. I wasn't able to port the make net command. I might need some help with that.
  3. Profiling no longer completes with running just a single command. See README for more information. Basically, it boils down to building once with -DENABLE_PROFILE=generate (which will automatically run the finished binary when done), then running again with -DENABLE_PROFILE=use.
  4. The updated Travis file is completely untested.
  5. Appveyor wasn't ported at all.

One final thing to note: It works by building a libstockfish static library then linking that to the main stockfish binary. There are no changes to the code; the static library just compiles all the files except main.cpp then the binary target compiles just that file.

The reason I did this is because I'm hoping Stockfish can be made into a library (aka can have strings passed directly instead of using getline). I'm working on a cross-platform Chess GUI, and one of the most frustrating things was the fact that no major chess engine includes a library - they just expect frontends to run them as an external process and then forward them messages. That doesn't work well when dealing with Windows and iOS especially.

That's my take. If the official Stockfish developers are uninterested in that, I can remove that part of the build script very easily.

@MichaelB7
Copy link
Contributor

MichaelB7 commented Aug 16, 2020

How well will this work with all the existing chess GUIs today? That is one of the beauties of Stockfish today, it is cross platform and easily accessible by many and different GUIs today. The approach has worked well this far. I also do not believe we should necessarily kowtow to Apple and iOS - they are the ones who set their own rules , not us. Apple has made enough money off the backs of others.

Edit/. I'm also assuming you will be making your GUI free to all - and if so , you should state that. That may help your cause.

@snicolet
Copy link
Member

snicolet commented Aug 16, 2020

Making Stockfish as a library: sure, you could consider this as a home project on your personal git branch. Be careful to read the GNU licence carefully if you want to distribute your cross-platform GUI, however, as it may very quickly require that you distribute your GUI source code and the modified Stockfish source code along with the binaries.

About cmake: probably not an option, because each build tool we add needs extra maintenance, and we prefer to keep SF toolchain as simple as possible.

@Symbitic
Copy link
Author

@MichaelB7 Stockfish works well when a platform supports calling external processes. But on iOS or even Android (where it's possible but still a pain) or any other embedded platform, it's just not practical. For my case specifically, I was working on GUI in Qt. Since I'm using the open-source version, GPL was already a given. But my example was just a use case. There are plenty of other use cases where something like this would be useful:

Stockfish stockfish;
while(true) {
    std::string move = convertGuiActionToString();
    stockfish.process(move);
    if (stockfish.isCheck()) { ... }
}

That's just pseudo-code by the way.


@snicolet If the official Stockfish developers aren't interested in Stockfish as a library, I will remove that code from CMake. I don't have enough time to maintain my own branch. This is just about contributing a CMake port to use as a replacement for GNU Make.

I think using CMake would actually be far simpler than using Make. It builds natively on Windows (removing the need for MinGW unless desired). Try comparing CMakeLists.txt and EnableFlags.cmake to Makefile. See if CMake would require more maintenance.

Some other advantages of CMake:

  • Supports CPack for automatically bundling into tar/zip/NSIS installer for easy distribution. Once done, Travis/Appveyor can upload them to GitHub releases.
  • Automatically detects hardware (no need for make build x86-64-ssse3, it just detects if the compiler supports ssse3).

If the Stockfish developers are not interested in using CMake as a replacement for GNU Make, then it's probably best to just close this.

@Fanael
Copy link
Contributor

Fanael commented Aug 16, 2020

Automatically detects hardware (no need for make build x86-64-ssse3, it just detects if the compiler supports ssse3).

It checks if the compiler supports it, but not if the hardware does, so the binary compiled with default settings crashed with SIGILL on the machine I tried to build it on.

@Symbitic
Copy link
Author

Fair enough. I already have avx2 and avx512 disabled by default (enabled by using -DUSE_AVX2=ON). I can change it to disable other flags by default.

@syzygy1
Copy link
Contributor

syzygy1 commented Aug 16, 2020

$ cmake
bash: cmake: command not found...
Similar command is: 'make'

This now behaves similarly to `make x86-64-modern`.
Also, port Appveyor.
@Symbitic
Copy link
Author

@syzygy1 do you have cmake installed (which cmake)?

I fixed the issue with the compiler flags. By default, it behaves similarly to make build x86-64-modern, and all compiler flags can be enabled/disabled as needed (see README).

I also changed Appveyor. That was surprisingly easy, since it already used CMake.

@syzygy1
Copy link
Contributor

syzygy1 commented Aug 16, 2020

@Symbitic I don't, which is a bit my point ;-)
We have a Makefile that works. Why should we switch to another tool?
Apparently it makes creating a PGO build more difficult. Why would we want to make that more difficult?

No longer need to run CMake with 2 different commands.
Just use `-DENABLE_PROFILING=generate`, and CMake handles the rest.
Also add script to download NNUE.
@Symbitic
Copy link
Author

@syzygy1 Makefile works... when run under very specific circumstances (Unix-like platform with compilers, tools, and libraries installed). CMake fits a lot more use case scenarios. It's also just a bit more natural if you're not a Unix-geeks like us ;-)

As for the PGO build: I just fixed that. All it takes now is just running cmake with -DENABLE_PROFILE=generate, and CMake will handle the rest.

I also rewrote the script for downloading the NNUE file in pure CMake, so it doesn't require Wget or cURL (or even bash). Profiling should work without problems even on Windows as soon as I add the MSVC compiler flags.

@syzygy1
Copy link
Contributor

syzygy1 commented Aug 17, 2020

"Not a Unix-geeks like us".......
How many of you are there? Just fork this repository and stick to the license. Thanks.

@gonzalezjo
Copy link

For what it’s worth, I strongly prefer CMake to traditional Makefiles.

@vondele
Copy link
Member

vondele commented Aug 17, 2020

BTW, I'm open to look into this, the current Makefile and build process and CI is getting stretched to its limits, cmake is a pretty common tool these days, and might be able to solve some to the issues.

I can even sympathize with the idea of a (static) library being part of the build results, which could facilitate integrating stockfish in other (GPLed, obviously) tools. It is somewhat orthogonal to this patch, but this patch facilitates this, of course.

I would approach this PR in a conservative way, and I'd like to see it pass CI before considering (so needs rebasing, probably after I merge the Android ndk patch to the Makefile). I would also need some fishtest worker changes, obviously. So neither accept nor reject right now.

@bftjoe
Copy link
Contributor

bftjoe commented Sep 14, 2020

"About cmake: probably not an option, because each build tool we add needs extra maintenance, and we prefer to keep SF toolchain as simple as possible."

Hard to believe this when Stockfish already uses CMake for Appveyor builds. Since CMake supports more platforms than makefile this doesn't seem to make sense.

@Sopel97
Copy link
Member

Sopel97 commented Sep 21, 2020

It's also just a bit more natural if you're not a Unix-geeks like us ;-)

Ironically my experience is that make is easier on windows than cmake to get working.

Automatically detects hardware (no need for make build x86-64-ssse3, it just detects if the compiler supports ssse3).

This can very well be done in make too. But ARCH is a feature - it allows crosscompilation

@vondele
Copy link
Member

vondele commented Oct 14, 2020

I'll close this as this is now outdated quite a bit. In case the PR is updated we can reopen the discussion

@vondele vondele closed this Oct 14, 2020
@Symbitic
Copy link
Author

Symbitic commented Nov 1, 2020

I never got a clear answer on if people were really interested enough in this. I can update it easy enough since there are no changes to the code itself, but keeping on top of the frequent updates to Stockfish requires some effort.

If the devs are willing and ready to accept a switch to CMake, I will update this. If not, I'll leave this fork up in case someone wants to use it as a reference someday.

@bftjoe bftjoe mentioned this pull request Mar 5, 2022
@Greendogo
Copy link

Pulled your branch and built on Windows 11 w/ cmake 3.28.1 with no alterations to your code.

Used Microsoft Visual Studio 17 2022 without any errors. Very cool.

Here you can see me running the baby's first bench and the compiler command:

PS C:\Users\Paulito\git\Stockfish-master\build\Debug> .\stockfish.exe
Stockfish 040124 by the Stockfish developers (see AUTHORS file)
position startpos
bench 16 1 1 current depth NNUE
info string Hash table allocation: Windows large pages not used.
info string Hash table allocation: Windows large pages not used.

Position: 1/1
info string classical evaluation enabled.
info depth 1 seldepth 1 multipv 1 score cp 109 nodes 20 nps 10000 tbhits 0 time 2 pv e2e3
bestmove e2e3

===========================
Total time (ms) : 2
Nodes searched  : 21
Nodes/second    : 10500
compiler

Compiled by MSVC (version 193833133.0) on Microsoft Windows 64-bit
Compilation settings include:  64bit AVX2 POPCNT
__VERSION__ macro expands to: (undefined macro)

After successfully building your branch with CMake on Windows 11, I've become a staunch advocate for adopting CMake as the primary build system for Stockfish. The seamless integration with Microsoft Visual Studio 17 2022 and the absence of errors during the build process speak volumes about CMake's robustness and efficiency. Make on Windows is just not as accessible as some here believe it to be.

CMake's cross-platform support is unparalleled, making it an ideal choice for a globally collaborated project like Stockfish. Unlike GNU Make, which often presents hurdles on Windows systems, CMake facilitates a more inclusive and frictionless development environment. This inclusion is especially critical for a project with diverse contributions like Stockfish.

Moreover, the flexibility CMake offers is impressive. It accommodates various build systems like Make, MSVC, and Ninja, providing developers the freedom to choose their preferred tools without compromising the build process's integrity. This adaptability is crucial for a project as dynamic and complex as Stockfish.

In light of these advantages, I strongly believe that migrating Stockfish to CMake and ending maintenance of the makefile would be a strategic move, enhancing the project's accessibility and scalability while streamlining the development workflow.

I know this is woefully out of date, but are there any others here that would be interested in this development in a new PR?

@Symbitic
Copy link
Author

Symbitic commented Jan 5, 2024

@Greendogo I'd be more than happy to restart this, especially since porting GitHub workflows will be a lot easier than Travis, but I never got an answer about whether the devs are willing to stop using Make and use CMake from now on. @syzygy1 seemed resistant to the idea.

@Greendogo
Copy link

@syzygy1 hasn't really been active in Conan for 3 years. I don't know if they're still around or not.

@Greendogo
Copy link

@Greendogo I'd be more than happy to restart this, especially since porting GitHub workflows will be a lot easier than Travis, but I never got an answer about whether the devs are willing to stop using Make and use CMake from now on. @syzygy1 seemed resistant to the idea.

One suggestion I'll make is that you should definitely remove the library separation stuff. It's a great idea, but it's more important to have success with cmake first and not to make this important migration polluted with separate concerns. That library-ization task can be tackled in a separate PR after cmake is accepted.

@Symbitic
Copy link
Author

Symbitic commented Jan 5, 2024

I'd prefer to get an acknowledgement that the devs are interested before I start working on this again.

@Disservin
Copy link
Member

@Symbitic To give you some kind of answer, vondele was always relatively open to this, and I guess his main concern was if everything can be ported over to CMake easily, which would be good to know before.
Our current Makefile has grown quite large and in my opinion perhaps even a bit difficult/annoying to maintain.
The fact is such a change will not just happen overnight once the PR is written, fishtest needs to adjusted and we need to come up with a plan on how to upgrade the workers to be able to execute CMake.
Also Mingw is not just for make, but also for g++ which has been our main compiler for quite some time now.

Regarding #4626 there also have been some concerncs raised as to support a newer build tool like CMake. Regarding the library support, we actually have been thinking about going in that direction, but a lot needs to be done in that direction.
Like we need to remove all global variables in Stockfish (I actually already have a branch for that but it's WIP).

Stuff like the net download can probably be more easier implemented in a script which is then just called? So my question is: Is porting everything from the Makefile to CMake feasible, without having to rely on some hacks to make things work?

@Disservin
Copy link
Member

I don't want to start a long discussion about all possible build tools but Meson might be an alternative, and it looks a bit more readable than cmake and looks similar to make. Lc0 also uses it.

@Symbitic
Copy link
Author

Symbitic commented Jan 5, 2024

@Disservin It is absolutely feasible. I already have the net download implemented in pure CMake. Doesn't even need curl or wget. So to answer your question: yes, everything can be ported from Makefile to CMake.

@Disservin
Copy link
Member

@Symbitic Ok, while I cant give you a certain answer if we will switch to CMake, I think having a working up to date version of this would be interesting.
Given that @vondele has been open to this in the past, I could imagine that he likes this even more today. Unfortunately he is currently quite busy so a response from him will take some time, but we can have a draft pr in the meantime if you like.

@Disservin
Copy link
Member

Now I understand if you dont want to invest time into this if the answer to the adoption is uncertain..

@Greendogo
Copy link

What would make you certain about switching to CMake?

Also, should we open this as a new Issue for discussion instead of doing this discussion in the old closed PR?

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.

None yet