Hi <#C01UZEXF5FY|>! I'd like to contribute to the ...
# contributions
l
Hi #C01UZEXF5FY! I'd like to contribute to the source code and have been having a difficult time getting everything setup to work with Mac. Namely I've had a number of issues linking to the braft and brpc libraries. I can build the two libraries okay but cannot link to them. I was wondering, why doesn't the make list file include braft and brpc like it does for the other deps? Poking around I'm assuming that its because when the make list was created those two libraries did not support Mac builds. Now that they do, would you accept a PR from me adding them to the make list?
I would basically mimic what's here using these files as a template
j
CC: @Kishore Nallan ^
k
@Lane Goolsby The Cmake file does refer to the brpc and braft dependencies, for e.g. see here: https://github.com/typesense/typesense/blob/main/CMakeLists.txt#L103 What linking error are you getting?
The Cmake file expects the dependencies in those specific directories, so that's probably why it's not finding those libraries to link?
l
The CMakeList.txt file includes the braft and brpc deps but there is not /cmake/*.make file for them.
k
Yeah thats not part of the cmake file. For release builds we use Docker where the dependency is directly build in the environment, like this: https://github.com/typesense/typesense/blob/main/docker/development.Dockerfile#L77
So if you install it locally on /usr/local it will work.
l
What I am proposing is to add make files to the cmake folder for braft and brpc deps like there are for For, S2, Jemalloc, etc. https://github.com/typesense/typesense/blob/ccb05375b743f4618f09bec05f41f140a7dfbda4/cmake/For.cmake#L7-L10 I could manually copy the built files into the typesense folder (which is what I suspect you are doing), but using the make files to download the dep and build them statically would make things much easier for people who just want to quickly contribute to the project. The Dockerfile does the download and build process, but that only works for Docker builds. It doesn't help for people who want to build the code directly. https://github.com/typesense/typesense/blob/ccb05375b743f4618f09bec05f41f140a7dfbda4/docker/development.Dockerfile#L77-L95 As for the build error, this is what I see
LGoolsby@MyMachine typesense % ./build.sh
++ dirname ./build.sh ++ read a ++ cd . ++ pwd ++ break + PROJECT_DIR=/my/repo/dir/typesense ++ uname -s + SYSTEM_NAME=Darwin + BUILD_DIR=build-Darwin + '[' -z '' ']' + TYPESENSE_VERSION=nightly + [[ '' == \-\-\c\l\e\a\n ]] + [[ '' == \-\-\d\e\p\c\l\e\a\n ]] + cmake -DTYPESENSE_VERSION=nightly -DCMAKE_BUILD_TYPE=Release -H/my/repo/dir/typesense -B/my/repo/dir/typesense/build-Darwin -- Found ICU header files in /usr/local/opt/icu4c/include -- Found ICU libraries: /usr/local/opt/icu4c/lib/libicuuc.a -- Found LevelDB (include: /usr/local/include, library: /usr/local/lib/libleveldb.a) OpenSSL library: /usr/local/opt/openssl@1.1/lib/libssl.a;/usr/local/opt/openssl@1.1/lib/libcrypto.a -- Configuring done -- Generating done -- Build files have been written to: /my/repo/dir/typesense/build-Darwin + make typesense-server typesense-test -C /my/repo/dir/typesense/build-Darwin Consolidate compiler generated dependencies of target typesense-server [ 0%] Building CXX object CMakeFiles/typesense-server.dir/src/app_metrics.cpp.o In file included from /my/repo/dir/typesense/src/app_metrics.cpp2 In file included from /my/repo/dir/typesense/include/core_api.h3 In file included from /my/repo/dir/typesense/include/http_server.h6 In file included from /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o.h41 In file included from /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o/hostinfo.h32 In file included from /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o/multithread.h27 In file included from /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o/socket.h33 /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o/picotls.h144232: warning: missing field 'capacity' initializer [-Wmissing-field-initializers] *buf = (ptls_buffer_t){NULL}; ^ In file included from /my/repo/dir/typesense/src/app_metrics.cpp2 In file included from /my/repo/dir/typesense/include/core_api.h3 In file included from /my/repo/dir/typesense/include/http_server.h6 In file included from /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o.h45 In file included from /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o/httpclient.h29 In file included from /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o/quicly.h37 /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o/quicly/frame.h51093: warning: performing pointer subtraction with a null pointer may have undefined behavior [-Wnull-pointer-subtraction] return quicly_encode_close_frame(NULL, error_code, offending_frame_type, reason_phrase) - (uint8_t *)NULL; ^ ~~~~~~~~~~~~~~~ In file included from /my/repo/dir/typesense/src/app_metrics.cpp2 In file included from /my/repo/dir/typesense/include/core_api.h3 In file included from /my/repo/dir/typesense/include/http_server.h6 In file included from /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o.h45 In file included from /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o/httpclient.h29 In file included from /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o/quicly.h40 In file included from /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o/quicly/loss.h33 /my/repo/dir/typesense/external-Darwin/h2o-6dda7d6f21610ecd5256543384fa4b4b345a88ac/include/h2o/quicly/sentmap.h25835: warning: missing field 'tail' initializer [-Wmissing-field-initializers] *map = (quicly_sentmap_t){NULL}; ^ In file included from /my/repo/dir/typesense/src/app_metrics.cpp2 In file included from /my/repo/dir/typesense/include/core_api.h4 In file included from /my/repo/dir/typesense/include/auth_manager.h11 /my/repo/dir/typesense/include/store.h1610: fatal error: 'butil/file_util.h' file not found #include <butil/file_util.h> ^~~~~~~~~~~~~~~~~~~ 3 warnings and 1 error generated. make[3]: * [CMakeFiles/typesense-server.dir/src/app_metrics.cpp.o] Error 1 make[2]: * [CMakeFiles/typesense-server.dir/all] Error 2 make[1]: * [CMakeFiles/typesense-server.dir/rule] Error 2 make: * [typesense-server] Error 2
k
Yes I'm fine with making those part of cmake file. I agree that it will make things easier.
👍 1
l
So if you install it locally on /usr/local it will work.
I am working on a company-issued laptop and we have a ton of security software that plays merry havoc with things that try to copy to /usr/local. :)
k
Got it 👍