search

rustls / rustls-ffi

Use Rustls from any language
Other
128 stars 30 forks source link

Simplify build systems, consider removing cmake #390

Open cpu opened 7 months ago

cpu commented 7 months ago

In https://github.com/rustls/rustls-ffi/pull/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 https://github.com/rustls/rustls-ffi/pull/274 (my personal preference, and the preference of multiple downstream packagers).

jsha commented 7 months ago

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 commented 6 months ago

Thanks for the extra historical context :+1: 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 :tada:

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 :+1: