[GitHub] [rocketmq-client-cpp] AGSaidi commented on a diff in pull request #338: [ISSUE #194] Fix build error on aarch64/arm platform

[GitBox <gi...@apache.org>]

AGSaidi commented on code in PR #338:
URL: https://github.com/apache/rocketmq-client-cpp/pull/338#discussion_r922616018


##########
CMakeLists.txt:
##########
@@ -183,11 +183,14 @@ ELSE ()
             # -rdynamic
             )
 
-    if (CMAKE_BUILD_BITS EQUAL 32)
-        list(APPEND CXX_FLAGS "-m32")
-    else () #not-condition
-        list(APPEND CXX_FLAGS "-m64")
-    endif ()
+    option(BUILD_FOR_ARM "Build for arm64 platform" OFF)

Review Comment:
   Why not detect the Arm platform and if it's Arm then skip this code instead of having a build option that means people building for Arm need to know to set this?



##########
build.sh:
##########
@@ -372,9 +373,18 @@ BuildBoost() {
   pwd
   if [ $verbose -eq 0 ]; then
     echo "build boost without detail log."
-    ./b2 -j$cpu_num cflags=-fPIC cxxflags=-fPIC --with-atomic --with-thread --with-system --with-chrono --with-date_time --with-log --with-regex --with-serialization --with-filesystem --with-locale --with-iostreams threading=multi link=static release install --prefix=${install_lib_dir} &> boostbuild.txt
+    if [ "$(uname -m)" = "aarch64" ]; then
+      ./b2 -j$cpu_num cflags=-fPIC cxxflags=-fPIC --with-atomic --with-thread --with-system --with-chrono --with-date_time --with-log --with-regex --with-serialization --with-filesystem --with-locale --with-iostreams threading=multi link=static release install --prefix=${install_lib_dir} architecture=arm &> boostbuild.txt
+    else
+      ./b2 -j$cpu_num cflags=-fPIC cxxflags=-fPIC --with-atomic --with-thread --with-system --with-chrono --with-date_time --with-log --with-regex --with-serialization --with-filesystem --with-locale --with-iostreams threading=multi link=static release install --prefix=${install_lib_dir} &> boostbuild.txt
+    fi
   else
-    ./b2 -j$cpu_num cflags=-fPIC cxxflags=-fPIC --with-atomic --with-thread --with-system --with-chrono --with-date_time --with-log --with-regex --with-serialization --with-filesystem --with-locale --with-iostreams threading=multi link=static release install --prefix=${install_lib_dir}
+    if [ "$(uname -m)" = "aarch64" ]; then
+      ./b2 -j$cpu_num cflags=-fPIC cxxflags=-fPIC --with-atomic --with-thread --with-system --with-chrono --with-date_time --with-log --with-regex --with-serialization --with-filesystem --with-locale --with-iostreams threading=multi link=static release install --prefix=${install_lib_dir} architecture=arm

Review Comment:
   why is it necessary to pass architecture=arm when building boost natively?



##########
build.sh:
##########
@@ -29,7 +29,7 @@ declare fname_openssl="openssl*.tar.gz"
 declare fname_libevent="libevent*.zip"
 declare fname_jsoncpp="jsoncpp*.zip"
 declare fname_boost="boost*.tar.gz"
-declare fname_openssl_down="openssl-1.1.1d.tar.gz"
+declare fname_openssl_down="openssl-1.1.1i.tar.gz"

Review Comment:
   There are many existing [cves in 1.1.1d](https://nvd.nist.gov/vuln/search/results?form_type=Advanced&results_type=overview&search_type=all&cpe_vendor=cpe%3A%2F%3Aopenssl&cpe_product=cpe%3A%2F%3A%3Aopenssl&cpe_version=cpe%3A%2F%3Aopenssl%3Aopenssl%3A1.1.1d) 



-- 
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: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org