kou commented on code in PR #37821:
URL: https://github.com/apache/arrow/pull/37821#discussion_r1379718264
##########
cpp/CMakePresets.json:
##########
@@ -46,6 +46,37 @@
"CMAKE_BUILD_TYPE": "RelWithDebInfo"
}
},
+ {
+ "name": "features-emscripten",
+ "hidden": true,
+ "cacheVariables": {
+ "ARROW_ACERO": "ON",
+ "ARROW_BUILD_SHARED": "OFF",
+ "ARROW_BUILD_STATIC": "ON",
+ "ARROW_BUILD_TESTS": "ON",
Review Comment:
Could you remove this because we have it in `base-debug`?
```suggestion
```
##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -702,3 +714,47 @@ if(MSVC)
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}
${MSVC_LINKER_FLAGS}")
endif()
endif()
+
+if(CMAKE_SYSTEM_NAME STREQUAL "Emscripten")
+ # flags are:
+ # 1) We're using zlib from Emscripten ports
+ # 2) We are building library code
+ # 3) We force *everything* to build as position independent
+ # 4) And with support for C++ exceptions
+ set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -sUSE_ZLIB=1 -fPIC -fexceptions")
+ # size_t is 32 bit in emscripten wasm32 - ignore conversion errors
+ # deprecated-literal-operator error is thrown in datetime (vendored lib in
arrow)
+ set(CMAKE_CXX_FLAGS
+ "${CMAKE_CXX_FLAGS} -sUSE_ZLIB=1 -fPIC -fexceptions
-Wno-error=shorten-64-to-32 -Wno-error=deprecated-literal-operator"
+ )
+
+ # flags for creating shared libraries (only used in pyarrow, because
+ # Emscripten builds libarrow as static)
+ # flags are:
+ # 1) Tell it to use zlib from Emscripten ports
+ # 2) Tell it to use javascript / webassembly 64 bit number support.
+ # 3) Tell it to build with support for C++ exceptions
+ # 4) Skip linker flags error which happens with -soname parameter
+ set(ARROW_EMSCRIPTEN_LINKER_FLAGS "-sWASM_BIGINT=1 -fexceptions
-Wno-error=linkflags")
+ set(CMAKE_SHARED_LIBRARY_CREATE_C_FLAGS
+ "-sSIDE_MODULE=1 ${ARROW_EMSCRIPTEN_LINKER_FLAGS}")
+ set(CMAKE_SHARED_LIBRARY_CREATE_CXX_FLAGS
+ "-sSIDE_MODULE=1 ${ARROW_EMSCRIPTEN_LINKER_FLAGS}")
+ set(CMAKE_SHARED_LINKER_FLAGS "-sSIDE_MODULE=1
${ARROW_EMSCRIPTEN_LINKER_FLAGS}")
+ if(ARROW_TESTING)
+ # flags for building test executables for use in node
+ set(CMAKE_EXE_LINKER_FLAGS
+ "${ARROW_EMSCRIPTEN_LINKER_FLAGS}
-sERROR_ON_WASM_CHANGES_AFTER_LINK=1 -sALLOW_MEMORY_GROWTH -lnodefs.js
-lnoderawfs.js --pre-js ${BUILD_SUPPORT_DIR}/emscripten-test-init.js"
Review Comment:
Why do we need to use "linker" flags to set environment variables?
Test executables built with Emscripten can't read environment variables at
runtime?
##########
cpp/cmake_modules/SetupCxxFlags.cmake:
##########
@@ -700,3 +702,30 @@ if(MSVC)
set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}
${MSVC_LINKER_FLAGS}")
endif()
endif()
+
+if(CMAKE_SYSTEM_NAME STREQUAL "Emscripten")
+ # flags are:
+ # 1) We're using zlib from Emscripten ports
+ # 2) We are building library code
+ # 3) We force *everything* to build as position independent
+ # 4) And with support for C++ exceptions
+ set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -sUSE_ZLIB=1 -fPIC -fexceptions")
+ # size_t is 32 bit in emscripten wasm32 - ignore conversion errors
+ # deprecated-literal-operator error is thrown in datetime (vendored lib in
arrow)
+ set(CMAKE_CXX_FLAGS
+ "${CMAKE_CXX_FLAGS} -sUSE_ZLIB=1 -fPIC -fexceptions
-Wno-error=shorten-64-to-32 -Wno-error=deprecated-literal-operator"
Review Comment:
@joemarshall Could you check this? Or can I push some commits including this
to this branch?
##########
cpp/src/arrow/io/file_test.cc:
##########
@@ -540,6 +544,9 @@ class TestPipeIO : public ::testing::Test {
};
TEST_F(TestPipeIO, TestWrite) {
+#ifdef __EMSCRIPTEN__
Review Comment:
Which should we use? `EMSCRIPTEN`? `__EMSCRIPTEN__`?
##########
cpp/src/arrow/util/rle_encoding_test.cc:
##########
@@ -214,7 +216,12 @@ TEST(BitUtil, RoundTripIntValues) {
void ValidateRle(const std::vector<int>& values, int bit_width,
uint8_t* expected_encoding, int expected_len) {
const int len = 64 * 1024;
+#ifdef EMSCRIPTEN
+ // on Emscripten, this buffer won't fit in the stack
+ static uint8_t buffer[len];
+#else
uint8_t buffer[len];
+#endif
Review Comment:
How about using `std::vector buffer(len)` to allocate memory dynamically?
##########
cpp/src/arrow/util/rle_encoding_test.cc:
##########
@@ -227,10 +234,9 @@ void ValidateRle(const std::vector<int>& values, int
bit_width,
if (expected_len != -1) {
EXPECT_EQ(encoded_len, expected_len);
}
- if (expected_encoding != NULL) {
+ if (expected_encoding != NULL && encoded_len == expected_len) {
Review Comment:
Why do we need this?
##########
cpp/src/arrow/util/rle_encoding_test.cc:
##########
@@ -33,6 +33,8 @@
#include "arrow/util/io_util.h"
#include "arrow/util/rle_encoding.h"
+#include <stdio.h>
Review Comment:
Could you revert needles changes such as this `include` and other empty line
changes?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
