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

Simplify build systems, consider removing cmake #390

Open
cpu opened this issue Feb 28, 2024 · 2 comments
Open

Simplify build systems, consider removing cmake #390

cpu opened this issue Feb 28, 2024 · 2 comments

Comments

@cpu
Copy link
Member

cpu commented Feb 28, 2024

In #253 there was Cmake support integrated, but I'm not sure I understand the motivation. It's described as being a cross-platform solution, but we only test it for Windows (in addition to the hand-rolled Makefile that already supports Windows). Was there downstream demand for this that I'm not aware of? Is it used by any integrating projects?

This repository doesn't have an abundance of maintainer activity and I think it would be better to simplify, either removing the existing Makefile and embracing cmake, removing cmake and embracing the manual Makefile, or making progress with cargo-c via #274 (my personal preference, and the preference of multiple downstream packagers).

@jsha
Copy link
Collaborator

jsha commented Mar 7, 2024

I'm definitely in favor of simplifying our build system. The original goal for integrating CMake was that we were having trouble maintaining our hand-rolled GNU-style Makefile, and in particular we would make changes that were causing tests to break on Windows. What I learned from folks wiser in Windows than myself is that most Windows projects use MSVC, it's hard to make MSVC work well with GNU Make (and many folks won't have it), and CMake, as complex and confusing as it can be, is the best and most popular option there.

Our Windows support is pretty thin and is thanks largely to great contributions from volunteers. But it's probably no surprise that I don't regularly develop on Windows so haven't had the expertise to really make it shine. For instance you can see the README doesn't even have specific instructions for a Windows build.

I think we can straightforwardly delete Makefile.Windows in preference for our CMakeLists.txt.

I'm convinced by the preference of downstream maintainers that we should support cargo-c. Can we get it to build the client/server binaries on Windows, using MSVC, and then run the integration test? Thanks to your work bringing that test into Rust, that part can be done with Cargo alone (cargo test --locked --test client_server client_server_integration -- --ignored --exact).

@cpu
Copy link
Member Author

cpu commented Mar 11, 2024

Thanks for the extra historical context 👍 I also have no significant experience with Windows, but trust the assessment that MSVC + cmake is the paved road for that platform.

I think we can straightforwardly delete Makefile.Windows in preference for our CMakeLists.txt.

That sounds good to me and seems like an immediate improvement 🎉

Do you have thoughts on the future of the Makefile used on Linux/MacOS? I think we should decide on converting it to cmake or removing it if cargo-c can fill the need.

Can we get it to build the client/server binaries on Windows, using MSVC, and then run the integration test?

I will try to tilt at that 👍

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

No branches or pull requests

2 participants