karldw commented on a change in pull request #11001:
URL: https://github.com/apache/arrow/pull/11001#discussion_r697644910



##########
File path: r/tools/nixlibs.R
##########
@@ -329,24 +290,34 @@ build_libarrow <- function(src_dir, dst_dir) {
   env_vars <- paste0(names(env_var_list), '="', env_var_list, '"', collapse = 
" ")
   env_vars <- with_s3_support(env_vars)
   env_vars <- with_mimalloc(env_vars)
-  if (tolower(Sys.info()[["sysname"]]) %in% "sunos") {
-    # jemalloc doesn't seem to build on Solaris
-    # nor does thrift, so turn off parquet,
-    # and arrowExports.cpp requires parquet for dataset (ARROW-11994), so turn 
that off
-    # xsimd doesn't compile, so set SIMD level to NONE to skip it
-    # re2 and utf8proc do compile,
-    # but `ar` fails to build libarrow_bundled_dependencies, so turn them off
-    # so that there are no bundled deps
-    env_vars <- paste(env_vars, "ARROW_JEMALLOC=OFF ARROW_PARQUET=OFF 
ARROW_DATASET=OFF ARROW_WITH_RE2=OFF ARROW_WITH_UTF8PROC=OFF 
EXTRA_CMAKE_FLAGS=-DARROW_SIMD_LEVEL=NONE")
+  # turn_off_thirdparty_features() needs to happen after with_mimalloc() and
+  # with_s3_support(), since those might turn features ON.

Review comment:
       Working with `env_var_list` is definitely easier. 
   
   Rather than have `with_mimalloc` and `with_s3_support` each check for 
download status, it strikes me as cleaner to have  do their own specific 
compiler/dependency checks before the `thirdparty_deps_unavailable` check. Then 
if those features (and everything else) ends up not being downloadable, they're 
set to OFF again.




-- 
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: github-unsubscr...@arrow.apache.org

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


Reply via email to