Merge pull request #70 from brawer/fetchcontent-roaring

Use cmake FetchContent for CRoaring

Author: bdon

.gitmodules

@@ -1,6 +1,3 @@
-[submodule "vendor/CRoaring"]
-	path = vendor/CRoaring
-	url = https://github.com/RoaringBitmap/CRoaring.git
 [submodule "vendor/s2geometry"]
 	path = vendor/s2geometry
 	url = https://github.com/google/s2geometry.git

CMakeLists.txt

@@ -9,8 +9,8 @@ set(CMAKE_CXX_FLAGS "-Wno-deprecated-declarations")
 set(CMAKE_CXX_FLAGS "-pthread")
 set(OSMX_VERSION "0.2.0")
 set(BUILD_SHARED_LIBS OFF CACHE INTERNAL "")
-set(ROARING_BUILD_STATIC ON CACHE INTERNAL "")
-set(ROARING_LINK_STATIC ON CACHE INTERNAL "")
+set(ROARING_USE_CPM OFF)
+set(ENABLE_ROARING_TESTS OFF)
 
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake")
 include(FetchContent)
@@ -69,8 +69,15 @@ FetchContent_Declare(
     EXCLUDE_FROM_ALL
     FIND_PACKAGE_ARGS)
 
+FetchContent_Declare(
+    roaring
+    GIT_REPOSITORY https://github.com/RoaringBitmap/CRoaring.git
+    GIT_TAG v4.3.6
+    EXCLUDE_FROM_ALL
+    FIND_PACKAGE_ARGS)
+
 FetchContent_MakeAvailable(
-    CapnProto Catch2 cxxopts LMDB nlohmann_json Osmium Protozero)
+    CapnProto Catch2 cxxopts LMDB nlohmann_json Osmium Protozero roaring)
 
 if(NOT CapnProto_FOUND)
     add_subdirectory(${capnproto_SOURCE_DIR} EXCLUDE_FROM_ALL)
@@ -104,11 +111,8 @@ if(NOT Protozero_FOUND)
 endif()
 
 add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/vendor/s2geometry EXCLUDE_FROM_ALL)
-add_subdirectory(${CMAKE_CURRENT_SOURCE_DIR}/vendor/CRoaring EXCLUDE_FROM_ALL)
 
 include_directories(vendor/s2geometry/src)
-include_directories(vendor/CRoaring/cpp)
-include_directories(vendor/CRoaring/include)
 include_directories(include)
 
 # needed for Expat install dir

Dockerfile

@@ -1,5 +1,7 @@
 FROM alpine:3.22 AS builder
 
+# TODO: Add croaring-dev once available in Alpine Linux.
+# https://gitlab.alpinelinux.org/alpine/aports/-/merge_requests/87769
 RUN apk add --no-cache  \
     clang               \
     cmake               \
@@ -31,7 +33,7 @@ FROM alpine:3.22
 
 # cxxopts, libosmium, nlohmann-json and protozero are header-only
 # C++ libraries; catch2 is only used for testing. We do not need
-# to install these.
+# them in the production container.
 RUN apk add --no-cache  \
     libbz2              \
     libcrypto3          \

examples/bbox_wkt.cpp

@@ -5,7 +5,7 @@
 #include "s2/s2latlng.h"
 #include "s2/s2region_coverer.h"
 #include "s2/s2latlng_rect.h"
-#include "roaring64map.hh"
+#include "roaring/roaring64map.hh"
 
 using namespace std;
 

include/osmx/storage.h

@@ -7,7 +7,7 @@
 #include "osmx/messages.capnp.h"
 #include "osmx/util.h"
 #include "s2/s2cell_id.h"
-#include "roaring64map.hh"
+#include "roaring/roaring64map.hh"
 
 namespace osmx { namespace db {
 
@@ -103,7 +103,7 @@ class IndexWriter : public Noncopyable {
   int mWrites = 0;
 };
 
-void traverseCell(MDB_cursor *cursor,S2CellId cell_id,Roaring64Map &set);
-void traverseReverse(MDB_cursor *cursor,uint64_t from, Roaring64Map &set);
+void traverseCell(MDB_cursor *cursor, S2CellId cell_id, roaring::Roaring64Map &set);
+void traverseReverse(MDB_cursor *cursor, uint64_t from, roaring::Roaring64Map &set);
 
-} }
+} }

src/extract.cpp

@@ -147,9 +147,9 @@ void cmdExtract(int argc, char * argv[]) {
     cout << "Query cells: " << covering.cell_ids().size() << endl;
   }
 
-  Roaring64Map node_ids;
-  Roaring64Map way_ids;
-  Roaring64Map relation_ids;
+  roaring::Roaring64Map node_ids;
+  roaring::Roaring64Map way_ids;
+  roaring::Roaring64Map relation_ids;
 
   MDB_env* env = db::createEnv(result["osmx"].as<string>(),false);
   MDB_txn* txn;
@@ -214,8 +214,8 @@ void cmdExtract(int argc, char * argv[]) {
     MDB_cursor *cursor;
     CHECK_LMDB(mdb_dbi_open(txn, "relation_relation", MDB_INTEGERKEY | MDB_DUPSORT | MDB_DUPFIXED | MDB_INTEGERDUP, &dbi));
     CHECK_LMDB(mdb_cursor_open(txn,dbi,&cursor));
-    Roaring64Map discovered_relations;
-    Roaring64Map discovered_relations_2;
+    roaring::Roaring64Map discovered_relations;
+    roaring::Roaring64Map discovered_relations_2;
 
     for (auto const &relation_id : relation_ids) {
       db::traverseReverse(cursor,relation_id,discovered_relations);

src/storage.cpp

@@ -168,7 +168,7 @@ void IndexWriter::commit() {
   CHECK_LMDB(mdb_txn_commit(mTxn));
 }
 
-void traverseCell(MDB_cursor *cursor,S2CellId cell_id,Roaring64Map &set) {
+void traverseCell(MDB_cursor *cursor, S2CellId cell_id, roaring::Roaring64Map &set) {
   S2CellId start = cell_id.child_begin(CELL_INDEX_LEVEL);
   S2CellId end = cell_id.child_end(CELL_INDEX_LEVEL);
   MDB_val key, data;
@@ -191,7 +191,7 @@ void traverseCell(MDB_cursor *cursor,S2CellId cell_id,Roaring64Map &set) {
   }
 }
 
-void traverseReverse(MDB_cursor *cursor,uint64_t from, Roaring64Map &set) {
+void traverseReverse(MDB_cursor *cursor,uint64_t from, roaring::Roaring64Map &set) {
   MDB_val key, data;
   key.mv_size = sizeof(uint64_t);
   key.mv_data = (void *)&from;

src/update.cpp

@@ -2,13 +2,57 @@
 #include <cassert>
 #include <set>
 #include "cxxopts.hpp"
-#include "roaring.hh"
 #include "osmium/handler.hpp"
 #include "osmium/io/any_input.hpp"
 #include "osmium/visitor.hpp"
 #include "osmium/util/progress_bar.hpp"
+#include "roaring/roaring.hh"
+
+// Historically, OSMExpress had vendored its dependencies, but we move away
+// from this. At the moment, the S2 geometry library is the last remaining
+// dependency still being vendored. Our (rather ancient) bundled version
+// of S2 tests for a number of implementations of the Standard C library.
+// If S2 happens to recognize the library, it includes <byteswap.h>
+// (unless it is being compiled by a Microsoft or Apple compiler);
+// otherwise, S2 falls back to its own byteswap implementation.
+// What S2 does (or rather did, in the old version we happen
+// to bundle) isn't so great; it would have been better for S2 to use
+// a standards-conforming way of byteswapping, test for its presence,
+// and only use the fallback if that test fails. But that's how it is.
+//
+// Anyhow, the (equally ancient) version of CRoaring, another library
+// that we previously vendored into OSMExpress, was polluting the
+// C macro namespace in a way that made our bundled version of S2
+// believe to be on a C library it knew about. Therefore, at the time
+// when OSMExpress still vendored that old version of CRoaring, the
+// bundled version of S2 would always include <byteswap.h> instead
+// of (re-)defining it, even if S2 did not recognize the C library.
+//
+// As we upgraded CRoaring to a newer version, which does not pollute
+// the C macro namespace anymore, the C preprocessor would now execute
+// the fallback path in the S2 headers when compiling with a Standard C
+// library that our old version of S2 does not recognize. This caused
+// compilation errors on Alpine Linux, which uses musl, a very lightweight
+// but fully standards-conforming implementation of the Standard C library.
+//
+// The following hack prevents our vendored old version of S2 from
+// supplying its own byteswap functions. On Bionic (and also other
+// modern libc implementations, including musl), it is sufficient to
+// include <byteswap.h>. Note we cannot explicitly test for musl here,
+// because musl does not define a __MUSL__ macro. (They don't want to,
+// since such a macro would not be standards-conforming; whether it's
+// really helpful to be so puristic has been the subject of much debate).
+//
+// TODO: Remove this hack once we stop vendoring the S2 geometry library.
+// https://github.com/bdon/OSMExpress/issues/20
+#if !defined(_MSC_VER) && !defined(__APPLE__) && !defined(__GLIBC__) \
+    && !defined(__BIONIC__) && !defined(__ASYLO__)
+#define __BIONIC__ 1
+#endif
+
 #include "s2/s2latlng.h"
 #include "s2/s2cell_union.h"
+
 #include "osmx/storage.h"
 #include "osmx/util.h"