Add filesystem related lib #1940
Conversation
CMakeLists.txt
Outdated
| @@ -169,6 +169,7 @@ list(APPEND EXTERNAL_LIBS Threads::Threads) | |||
| list(APPEND EXTERNAL_LIBS ${Backtrace_LIBRARY}) | |||
| list(APPEND EXTERNAL_LIBS xxhash) | |||
| list(APPEND EXTERNAL_LIBS span-lite) | |||
| list(APPEND EXTERNAL_LIBS stdc++fs) | |||
There was a problem hiding this comment.
Is this test only, or we may using it outside testing?
There was a problem hiding this comment.
I am fine to link stdc++fs into kvrocks_objs rather than just tests, but here are some points need to be considered:
- it seems in higher-version (>= 9) gcc (or, libstdc++) we don't need to explicitly link this library any more, so I wonder if it (
stdc++fs) will be duplicated. I think we'd better to add a gcc version check here. BTW I encourage everyone to use new version of compiler as much as possible, e.g. gcc 12 or 13 - what will happen if we use libc++ rather than libstdc++? Does libc++ also provide
stdc++fs?
|
CI failed on macOS, which is related to the case about libc++ rather than libstdc++ that I mentioned before. We need to add a check about the std lib impl that are currently using. |
|
Passed CI tests, PTAL @PragmaTwice @mapleFU |
|
Please retry analysis of this Pull-Request directly on SonarCloud |
| @@ -148,7 +148,6 @@ endif() | |||
| find_package(Threads REQUIRED) | |||
|
|
|||
| list(APPEND EXTERNAL_LIBS glog) | |||
| list(APPEND EXTERNAL_LIBS snappy) | |||
There was a problem hiding this comment.
ld: warning: ignoring duplicate libraries: '_deps/snappy-build/libsnappy.a', 'clang_17.0_cxx17_64_relwithdebinfo/libtbb.a'
I found a warning when compiling with clang 17. It appears that deleting this line does not affect the compilation process.
There was a problem hiding this comment.
I'd prefer to explicit list this dep instead of depend on transitive dep.
|
close: #1939