search

Tectu / malloy

A cross-platform C++20 library providing embeddable server & client components for HTTP and WebSocket.
BSD 3-Clause "New" or "Revised" License
66 stars 8 forks source link

Support MacOS #46

Closed 0x00002a closed 2 years ago

0x00002a commented 3 years ago

This is mostly for tracking.

MacOS should probably be supported, as clang catches up to msvc and gcc it should be easier/feasible to support clang and by extension mac. I'm currently working on getting to clang to compile malloy, but even then I doubt it will work straight off as libc++ has very different (and generally cleaner) transitive includes to libstdc++ (and microsofts version too I guess). I can setup clang to use libc++ on my linux box, but theres still differences. Therefore we at least need the CI to build mac and preferably also someone who actually has a mac box who can contribute.

Tectu commented 3 years ago

I have access to a MacMini which we use internally for CD purposes.

Tectu commented 3 years ago

Seems like clang 12.0 doesn't support concepts at all? o.O If that's truly the case I guess this library won't be supporting clang until 13.0 is out. I really feel like getting rid of the concepts would be a step backwards.

0x00002a commented 3 years ago

It does support concepts, I've got it building in the CI (link), just chasing down a segfault which only occures on windows, only with clang, and gives a different error code each time (and which ends in a different position in the call stack/different dll entirely). What might be a bit more of an issue is coroutines, msvc has the TS implemented apparently and GCC has had the full since 10, clang though has had "partial" support since 8 and apparently nothing more on it yet. It doesn't like the "lambda in unevaluated expression" though, so I've had to move those to seperate structs.

It doesn't seem to support TLS with boost< 1.76.0 though, gives a ~12mb log at the end of which are some redefinition errors about boost::system::error_code::category::something

Tectu commented 3 years ago

What might be a bit more of an issue is coroutines,

Well, as discussed here, coroutines usage will be something for the future anyway, right? As far as I know there is currently no code depending on coroutines.

It doesn't like the "lambda in unevaluated expression" though, so I've had to move those to seperate structs.

If you change those, I'd say keep the current code but just commented out, add a comment as to why that is and add the workaround implementation below that.

It doesn't seem to support TLS with boost< 1.76.0 though, gives a ~12mb log at the end of which are some redefinition errors about boost::system::error_code::category::something

As I'm only a few minutes away from a pizza, all I can say to this at the moment is: lol 😛🍕

0x00002a commented 3 years ago

Update on this, I think the segfault is due to a mismatch between the libstdc++ version and clang, on the bright side we now have some mocks for http::connection. I'm going to see if I can get it to compile with the clang msys env, which uses libc++, so we will have a way of testing that in the CI too. Although I'm not sure if libc++ supports enough 20 🤔

0x00002a commented 3 years ago

Ok so libc++ 12 does not support concepts, I'm guessing thats what you ran into above. 13 does but msys doesn't have it yet... this is insane, I haven't had this much trouble with getting a toolchain working since I first started learning C++.

My feat-clang-build branch builds fine on linux/clang, and also in msys (but segfaults when running the tests), should I just make the CI build with clang on linux and hold off on msys till it gets libc++13?

Tectu commented 3 years ago

Ok so libc++ 12 does not support concepts, I'm guessing thats what you ran into above.

Exactly.

My feat-clang-build branch builds fine on linux/clang, and also in msys (but segfaults when running the tests), should I just make the CI build with clang on linux and hold off on msys till it gets libc++13?

Yes, I do think that it's very valuable to add the CI tests for clang13 on the corresponding platforms and ignore msys2 for now.

13 does but msys doesn't have it yet... this is insane, I haven't had this much trouble with getting a toolchain working since I first started learning C++.

Trust me - I feel you there... At this point I'm wondering whether it's time to ditch MSYS2. A couple of years ago this used to be the best of all the options I evaluated (very personal opinion ofc) but yeah... lately I am spending more time working around MSYS2 issues than anything else. Before MSYS2 I was a heavy user of cygwin. Maybe there are also a few new options these days that I should check out :)

0x00002a commented 3 years ago

Before MSYS2 I was a heavy user of cygwin. Maybe there are also a few new options these days that I should check out :)

I like msys2 personally, after getting back into it for this with more linux experience it really is quite nice. My issues so far are with the toolchains, mingw has only ever been slow and/or buggy for me. Although I guess the libstdc++ issue is with msys (although I'm suprised it isn't well known about if so, so I may be wrong about that one). msys does have a cygwin env under /usr apparently. Theres also WSL which I've never tried but heard good things about, that might be a viable option if it can produce windows binaries.

Yes, I do think that it's very valuable to add the CI tests for clang13 on the corresponding platforms and ignore msys2 for now.

afaik nothing has clang13 yet, its the trunk version. arch doesn't for sure so I doubt anything else will.

Is there a particular reason why we can't only support MSVC on windows? I know its historically been quite slow with change but its now neck-and-neck with gcc for implemenation of 20 and ahead of clang

Tectu commented 3 years ago

As you mentioned the main problem arises from the toolchains provided by MSYS2. The devel is in the detail tho: PROVIDED by MSYS2. I ran into several conflicts (especially in the early days of MSYS2 tho) where environments were colliding if not carefully manually setting everything up). As far as I can tell it's also very difficult - if not impossible - to have several toolchains (or other packages, eg boost) with different versions under MSYS2. While it might work technically under the hood (doubtful) the way the packages are built & distributed doesn't allow choosing versions via pacman. WSL is certainly interesting but as far as I understood back when it was all the rage one cannot compile binaries which run directly on Windows (eg. not within WSL). The beauty of something like MinGW/Cygwin/MSYS2 is that one has "all" the fun Unix tools available but you're still able to produce native executables.

Is there a particular reason why we can't only support MSVC on windows? I know its historically been quite slow with change but its now neck-and-neck with gcc for implemenation of 20 and ahead of clang

Exactly because of historical reasons 🙈 I have plenty of projects (both personal hobby stuff & company/customer/proprietary/$$$) projects which were unable to be built with MSVC back in the day. Some of these projects are either already or going to use malloy eventually and it would take weeks to months to migrate everything to MSVC. One thought tho... C++11 brought some "standard(s)" for memory models. Are MSVC binaries compatible with MSVC binaries as of today? Then we could build (shared) libraries of malloy with MSVC and simply pull them into the other projects which are currently unable to build under MSVC (not necessarily for C++ support reasons - I understand that MSVC caught up a lot the past few years(?)).

afaik nothing has clang13 yet, its the trunk version. arch doesn't for sure so I doubt anything else will.

Communication problems. I'm good with your initial idea to provide clang CI for Linux for now.

0x00002a commented 3 years ago

One thought tho... C++11 brought some "standard(s)" for memory models. Are MSVC binaries compatible with MSVC binaries as of today? Then we could build (shared) libraries of malloy with MSVC and simply pull them into the other projects which are currently unable to build under MSVC (not necessarily for C++ support reasons - I understand that MSVC caught up a lot the past few years(?)).

If you mean could we link stuff compiled with older MSVC to this library compiled with modern MSVC, the docs say "yes as along as its one of 2015, 2017, or 2019". If its whether we can link msvc-dlls to non-msvc compiled applications I'm guessing no since C++ has no standard ABI but I'm not sure.

Tectu commented 3 years ago

Yeah... I think I won't be able to migrate some of the projects away from MinGW any time soon. So unfortunately I am stuck with the requirement that this library builds with MinGW on Windows :/

Tectu commented 3 years ago

I quickly tried to compile this on a Mac:

Joels-Mac-mini:.build jbo$ cmake -DBOOST_ROOT=~/Downloads/boost_1_76_0 -DMALLOY_FEATUYRE_TLS=OFF ..
-- The CXX compiler identification is AppleClang 12.0.5.12050022
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Library/Developer/CommandLineTools/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found Boost: /Users/jbo/Downloads/boost_1_76_0 (found suitable version "1.76.0", minimum required is "1.74.0")  
-- Build spdlog: 1.8.3
-- Looking for C++ include pthread.h
-- Looking for C++ include pthread.h - found
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE  
-- Build type: Release

---------------------
Malloy configuration:
  Build:
    Examples : ON
    Tests    : ON
  Features:
    Client   : ON
    Server   : ON
    HTML     : ON
    TLS      : OFF
---------------------

-- Configuring done
-- Generating done
CMake Warning:
  Manually-specified variables were not used by the project:

    MALLOY_FEATUYRE_TLS

-- Build files have been written to: /Users/jbo/projects/malloy/.build
Joels-Mac-mini:.build jbo$ make
Scanning dependencies of target spdlog
[  1%] Building CXX object _deps/spdlog-build/CMakeFiles/spdlog.dir/src/spdlog.cpp.o
[  3%] Building CXX object _deps/spdlog-build/CMakeFiles/spdlog.dir/src/stdout_sinks.cpp.o
[  5%] Building CXX object _deps/spdlog-build/CMakeFiles/spdlog.dir/src/color_sinks.cpp.o
[  7%] Building CXX object _deps/spdlog-build/CMakeFiles/spdlog.dir/src/file_sinks.cpp.o
[  9%] Building CXX object _deps/spdlog-build/CMakeFiles/spdlog.dir/src/async.cpp.o
[ 11%] Building CXX object _deps/spdlog-build/CMakeFiles/spdlog.dir/src/cfg.cpp.o
[ 13%] Building CXX object _deps/spdlog-build/CMakeFiles/spdlog.dir/src/fmt.cpp.o
[ 15%] Linking CXX static library libspdlog.a
[ 15%] Built target spdlog
Scanning dependencies of target malloy-objs
[ 17%] Building CXX object lib/malloy/CMakeFiles/malloy-objs.dir/client/controller.cpp.o
In file included from /Users/jbo/projects/malloy/lib/malloy/client/controller.cpp:1:
In file included from /Users/jbo/projects/malloy/lib/malloy/client/controller.hpp:8:
In file included from /Users/jbo/projects/malloy/lib/malloy/../malloy/client/type_traits.hpp:8:
/Users/jbo/projects/malloy/lib/malloy/../malloy/type_traits.hpp:25:45: error: 'Func' does not refer to a value
    concept accept_handler = std::invocable<Func, malloy::error_code>;
                                            ^
/Users/jbo/projects/malloy/lib/malloy/../malloy/type_traits.hpp:24:23: note: declared here
    template<typename Func>
                      ^
/Users/jbo/projects/malloy/lib/malloy/../malloy/type_traits.hpp:25:35: error: no member named 'invocable' in namespace 'std'
    concept accept_handler = std::invocable<Func, malloy::error_code>;
                             ~~~~~^
/Users/jbo/projects/malloy/lib/malloy/../malloy/type_traits.hpp:31:49: error: 'Func' does not refer to a value
    concept async_read_handler = std::invocable<Func, boost::beast::error_code, std::size_t>;
                                                ^
/Users/jbo/projects/malloy/lib/malloy/../malloy/type_traits.hpp:30:23: note: declared here
    template<typename Func>
                      ^
/Users/jbo/projects/malloy/lib/malloy/../malloy/type_traits.hpp:31:39: error: no member named 'invocable' in namespace 'std'
    concept async_read_handler = std::invocable<Func, boost::beast::error_code, std::size_t>;
                                 ~~~~~^
In file included from /Users/jbo/projects/malloy/lib/malloy/client/controller.cpp:1:
In file included from /Users/jbo/projects/malloy/lib/malloy/client/controller.hpp:8:
/Users/jbo/projects/malloy/lib/malloy/../malloy/client/type_traits.hpp:52:36: error: no template named 'move_constructible' in namespace 'std'; did you mean 'is_move_constructible'?
    concept response_filter = std::move_constructible<F> && requires(const F& f, const typename F::header_type& h)
                              ~~~~~^~~~~~~~~~~~~~~~~~
                                   is_move_constructible
/Library/Developer/CommandLineTools/SDKs/MacOSX11.3.sdk/usr/include/c++/v1/type_traits:3072:29: note: 'is_move_constructible' declared here
struct _LIBCPP_TEMPLATE_VIS is_move_constructible
                            ^
In file included from /Users/jbo/projects/malloy/lib/malloy/client/controller.cpp:1:
In file included from /Users/jbo/projects/malloy/lib/malloy/client/controller.hpp:8:
/Users/jbo/projects/malloy/lib/malloy/../malloy/client/type_traits.hpp:52:58: error: expected '(' for function-style cast or type construction
    concept response_filter = std::move_constructible<F> && requires(const F& f, const typename F::header_type& h)
                              ~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
/Users/jbo/projects/malloy/lib/malloy/../malloy/client/type_traits.hpp:62:60: error: no template named 'move_constructible' in namespace 'std'; did you mean 'is_move_constructible'?
    concept http_callback = response_filter<Filter>&& std::move_constructible<F>&& requires(F cb, const Filter& f, const typename Filter::header_type& h){
                                                      ~~~~~^~~~~~~~~~~~~~~~~~
                                                           is_move_constructible
/Library/Developer/CommandLineTools/SDKs/MacOSX11.3.sdk/usr/include/c++/v1/type_traits:3072:29: note: 'is_move_constructible' declared here
struct _LIBCPP_TEMPLATE_VIS is_move_constructible
                            ^
In file included from /Users/jbo/projects/malloy/lib/malloy/client/controller.cpp:1:
In file included from /Users/jbo/projects/malloy/lib/malloy/client/controller.hpp:8:
/Users/jbo/projects/malloy/lib/malloy/../malloy/client/type_traits.hpp:62:81: error: expected '(' for function-style cast or type construction
    concept http_callback = response_filter<Filter>&& std::move_constructible<F>&& requires(F cb, const Filter& f, const typename Filter::header_type& h){
                                                      ~~~~~~~~~~~~~~~~~~~~~~~~~~^
/Users/jbo/projects/malloy/lib/malloy/../malloy/client/type_traits.hpp:62:29: error: use of undeclared identifier 'response_filter'
    concept http_callback = response_filter<Filter>&& std::move_constructible<F>&& requires(F cb, const Filter& f, const typename Filter::header_type& h){
                            ^
In file included from /Users/jbo/projects/malloy/lib/malloy/client/controller.cpp:1:
In file included from /Users/jbo/projects/malloy/lib/malloy/client/controller.hpp:9:
In file included from /Users/jbo/projects/malloy/lib/malloy/../malloy/client/websocket/connection.hpp:3:
In file included from /Users/jbo/projects/malloy/lib/malloy/../malloy/websocket/connection.hpp:13:
/Users/jbo/projects/malloy/lib/malloy/../malloy/websocket/stream.hpp:78:60: error: no type named 'async_read_handler' in namespace 'malloy::concepts'
                template<concepts::const_buffer_sequence Buff, concepts::async_read_handler Callback>
                                                               ~~~~~~~~~~^
/Users/jbo/projects/malloy/lib/malloy/../malloy/websocket/stream.hpp:79:41: error: unknown type name 'Callback'
                void async_write(const Buff& buffers, Callback&& done)
                                                      ^
/Users/jbo/projects/malloy/lib/malloy/../malloy/websocket/stream.hpp:90:53: error: no type named 'async_read_handler' in namespace 'malloy::concepts'
                template<concepts::dynamic_buffer Buff, concepts::async_read_handler Callback>
                                                        ~~~~~~~~~~^
/Users/jbo/projects/malloy/lib/malloy/../malloy/websocket/stream.hpp:91:31: error: unknown type name 'Callback'
                void async_read(Buff& buff, Callback&& done)
                                            ^
/Users/jbo/projects/malloy/lib/malloy/../malloy/websocket/stream.hpp:101:28: error: no type named 'accept_handler' in namespace 'malloy::concepts'
        template<concepts::accept_handler Callback>
                 ~~~~~~~~~~^
/Users/jbo/projects/malloy/lib/malloy/../malloy/websocket/stream.hpp:102:63: error: unknown type name 'Callback'
                void async_close(boost::beast::websocket::close_reason why, Callback&& done) {
                                                                            ^
/Users/jbo/projects/malloy/lib/malloy/../malloy/websocket/stream.hpp:114:85: error: no type named 'accept_handler' in namespace 'malloy::concepts'
                auto async_accept(const boost::beast::http::request<Body, Fields>& req, concepts::accept_handler auto&& done)
                                                                                        ~~~~~~~~~~^
/Users/jbo/projects/malloy/lib/malloy/../malloy/websocket/stream.hpp:125:22: error: no type named 'accept_handler' in namespace 'malloy::concepts'
                template<concepts::accept_handler Callback>
                         ~~~~~~~~~~^
/Users/jbo/projects/malloy/lib/malloy/../malloy/websocket/stream.hpp:126:62: error: unknown type name 'Callback'
                void async_handshake(std::string host, std::string target, Callback&& done)
                                                                           ^
In file included from /Users/jbo/projects/malloy/lib/malloy/client/controller.cpp:1:
In file included from /Users/jbo/projects/malloy/lib/malloy/client/controller.hpp:9:
In file included from /Users/jbo/projects/malloy/lib/malloy/../malloy/client/websocket/connection.hpp:3:
/Users/jbo/projects/malloy/lib/malloy/../malloy/websocket/connection.hpp:99:28: error: no type named 'accept_handler' in namespace 'malloy::concepts'
        template<concepts::accept_handler Callback>
                 ~~~~~~~~~~^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
make[2]: *** [lib/malloy/CMakeFiles/malloy-objs.dir/client/controller.cpp.o] Error 1
make[1]: *** [lib/malloy/CMakeFiles/malloy-objs.dir/all] Error 2
make: *** [all] Error 2
0x00002a commented 3 years ago

mhmm, libc++12 has no concepts library support. Need 13 for that unfortunately.

Tectu commented 3 years ago

Ah yeah - this was discussed lengthy before - brain fart 🤦

Time to figure out how to use libc++13 on MacOS with clang 12.0.5 then. I take it that this will take a couple of days as I am really not familiar with MacOS and I (maybe incorrectly) expect this going to be a PITA.

0x00002a commented 3 years ago

Just an update on this, I tried compiling malloy with clang trunk + libc++ which I built from source (on linux). It doesn't work, looks like libc++ removed result_of (which was removed in 20) and asio doesn't use the replacement for it yet (at least not when using libc++)

Tectu commented 2 years ago

So, lets get back to this...

I'm currently on FreeBSD 13.0-RELEASE. I tried to compile malloy using clang version 12, 13 and 14. As discussed prior 12 is not something we have to look at anyway. With version 13 I run into another problem: std::invocable is not available. I'm not yet sure why I am hitting this case as godbolt doesn't seem to have a problem with this (even explicitly setting -stdlib=libc++.

Tectu commented 2 years ago

I'm able to compile malloy successfully using clang 13.0.0. One caveat is that one must use an spdlog version newer than the last 1.9.2 release due to an issue with libc++ and fmt: https://github.com/fmtlib/fmt/issues/2455 which leads to https://github.com/gabime/spdlog/issues/2142

I've simply used the v1.x branch of spdlog and that works well.

The next step from my side is to test this on an actual MacOS system.

Tectu commented 2 years ago

I had another go at this today. I got malloy to compile, link & run successfully on an x86 MacOS 12.4 machine.

Using homebrew, the following packages were installed:

After that, I ran CMake like this:

cmake \
  -G Ninja \
  -B build \
  -DCMAKE_CXX_COMPILER=/usr/local/opt/llvm/bin/clang++ \
  -DMALLOY_DEPENDENCY_FMT_DOWNLOAD=OFF \
  -DMALLOY_DEPENDENCY_SPDLOG_DOWNLOAD=OFF

Manually specifying the C++ compiler was necessary as CMake would otherwise default to the system's compiler.

However, at this point CMake complained about the compiler test not succeeding. This was due to the fact that CMake/LLVM was still picking up the incorrect libc++ version from the base system. This hurdle can be overcome by manually setting the LDFLAGS environment variable prior to launching CMake:

export LDFLAGS=-L/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/lib

Once MacOS ships with a decent version of Clang this should no longer be necessary.

arms-spageddie commented 2 years ago

I tested this today and yesterday on two separate MacBooks.

I manually downloaded boost v1.79 and manually compiled/installed the libraries on the first MacBook. After that I ran CMake and it was able to successfully locate boost and I was able to compile the Malloy program without issue. This was tested on a machine running clang v13.1.6.

On the second MacBook I installed boost v1.79 using brew and was able to compile the Malloy program without issue. This was tested on a machine running clang v13.1.6.

On both tests I did NOT have to manually set LDFLAGS as mentioned by @Tectu above.

Tectu commented 2 years ago

@astrobatman That is great news!

Which version(s) of MacOS are those two machines running?

arms-spageddie commented 2 years ago

Both MacBooks were running macOS 12.4.

Tectu commented 2 years ago

Awesome, thanks for the info. I think hereby this issue can finally be closed - Thanks for the help!