kou commented on code in PR #14273:
URL: https://github.com/apache/arrow/pull/14273#discussion_r984003581
##########
ci/scripts/python_wheel_macos_build.sh:
##########
@@ -96,25 +96,27 @@ cmake \
-DARROW_BUILD_SHARED=ON \
-DARROW_BUILD_STATIC=OFF \
-DARROW_BUILD_TESTS=OFF \
+ -DARROW_COMPUTE=ON \
+ -DARROW_CSV=ON \
-DARROW_DATASET=${ARROW_DATASET} \
Review Comment:
> I don't think we want to create wheels without dataset enabled
I think so but `${ARROW_DATASET}` is also used for `export
PYARROW_WITH_DATASET=${ARROW_DATASET}` below. So I think that we use
`${ARROW_DATASET}` here for consistency. (`ARROW_DATASET` is initialized by `:
${ARROW_DATASET:=ON}`.)
##########
ci/scripts/python_wheel_manylinux_build.sh:
##########
@@ -89,31 +89,32 @@ pushd /tmp/arrow-build
# https://github.com/aws/aws-sdk-cpp/issues/1809 is fixed and vcpkg
# ships the fix.
cmake \
- -DARROW_BROTLI_USE_SHARED=OFF \
-DARROW_BUILD_SHARED=ON \
-DARROW_BUILD_STATIC=OFF \
-DARROW_BUILD_TESTS=OFF \
+ -DARROW_COMPUTE=ON \
+ -DARROW_CSV=ON \
-DARROW_DATASET=${ARROW_DATASET} \
+ -DARROW_DATASET=ON \
Review Comment:
Thanks but I choose `-DARROW_DATASET=${ARROW_DATASET}` for consistency.
##########
ci/scripts/python_wheel_macos_build.sh:
##########
@@ -129,9 +131,9 @@ cmake \
-DCMAKE_INSTALL_PREFIX=${build_dir}/install \
-DCMAKE_OSX_ARCHITECTURES=${CMAKE_OSX_ARCHITECTURES} \
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD} \
- -DOPENSSL_USE_STATIC_LIBS=ON \
Review Comment:
Yes. This is unrelated to `ARROW_PYTHON`. Sorry for including this to this
pull request.
`OPENSSL_USE_STATIC_LIBS` is redundant here because there is
`-DARROW_DEPENDENCY_USE_SHARED=OFF` in this command line.
`OPENSSL_USE_STATIC_LIBS` is set automatically in
https://github.com/apache/arrow/blob/master/cpp/cmake_modules/FindOpenSSLAlt.cmake#L48-L52
.
I just found this by sorting CMake options. So I mix this change into this
pull request. Sorry.
##########
cpp/CMakePresets.json:
##########
@@ -117,17 +117,46 @@
"ARROW_GANDIVA": "ON"
}
},
+ {
+ "name": "features-python-minimal",
+ "inherits": [
+ "features-minimal"
+ ],
+ "hidden": true,
+ "cacheVariables": {
+ "ARROW_COMPUTE": "ON",
+ "ARROW_CSV": "ON",
+ "ARROW_FILESYSTEM": "ON",
+ "ARROW_JSON": "ON"
Review Comment:
> Maybe you can also keep the exact name to keep it working for people that
were using those presets?
It makes sense. I added no `-minimal`/`-maximal` versions.
##########
docs/source/developers/python.rst:
##########
@@ -313,24 +313,26 @@ created above (stored in ``$ARROW_HOME``):
$ mkdir arrow/cpp/build
$ pushd arrow/cpp/build
-
$ cmake -DCMAKE_INSTALL_PREFIX=$ARROW_HOME \
-DCMAKE_INSTALL_LIBDIR=lib \
-DCMAKE_BUILD_TYPE=Debug \
+ -DARROW_BUILD_TESTS=ON \
+ -DARROW_COMPUTE=ON \
+ -DARROW_CSV=ON \
-DARROW_DATASET=ON \
+ -DARROW_FILESYSTEM=ON \
+ -DARROW_HDFS=ON \
+ -DARROW_JSON=ON \
+ -DARROW_PARQUET=ON \
+ -DARROW_WITH_BROTLI=ON \
-DARROW_WITH_BZ2=ON \
- -DARROW_WITH_ZLIB=ON \
- -DARROW_WITH_ZSTD=ON \
-DARROW_WITH_LZ4=ON \
-DARROW_WITH_SNAPPY=ON \
- -DARROW_WITH_BROTLI=ON \
- -DARROW_PARQUET=ON \
+ -DARROW_WITH_ZLIB=ON \
+ -DARROW_WITH_ZSTD=ON \
-DPARQUET_REQUIRE_ENCRYPTION=ON \
- -DARROW_PYTHON=ON \
- -DARROW_BUILD_TESTS=ON \
..
- $ make -j4
- $ make install
+ $ cmake --build . --target install --config Debug
Review Comment:
Ah, sorry. I added this accidentally. I revert this.
I found `-j4` isn't suitable here. We can use `-j$(nproc)` on Linux and
`-j$(sysctl -n hw.ncpu)` on macOS. But I thought that it's better that we use
Ninja here. And I started rewriting this for Ninja but I stopped it because
this pull request isn't for the change. But this change remained.
--
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]
