WillAyd commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2021363161


##########
ci/appveyor-cpp-build.bat:
##########
@@ -118,25 +118,59 @@ pushd python
 @rem Build and install pyarrow
 @rem
 
-set PYARROW_CMAKE_GENERATOR=%GENERATOR%
-set PYARROW_CXXFLAGS=%ARROW_CXXFLAGS%
-set PYARROW_PARALLEL=2
-set PYARROW_WITH_ACERO=ON
-set PYARROW_WITH_DATASET=ON
-set PYARROW_WITH_FLIGHT=%ARROW_BUILD_FLIGHT%
-set PYARROW_WITH_GANDIVA=%ARROW_BUILD_GANDIVA%
-set PYARROW_WITH_GCS=%ARROW_GCS%
-set PYARROW_WITH_ORC=%ARROW_ORC%
-set PYARROW_WITH_PARQUET=ON
-set PYARROW_WITH_PARQUET_ENCRYPTION=ON
-set PYARROW_WITH_S3=%ARROW_S3%
-set PYARROW_WITH_SUBSTRAIT=ON
+set PYARROW_WITH_ACERO=enabled
+set PYARROW_WITH_DATASET=enabled
+
+if /i "%ARROW_BUILD_FLIGHT%" == "ON" (
+    set PYARROW_WITH_FLIGHT=enabled
+) else (
+    set PYARROW_WITH_FLIGHT=auto
+)
+
+if /i "%ARROW_BUILD_GANDIVA%" == "ON" (
+    set PYARROW_WITH_GANDIVA=enabled
+) else (
+    set PYARROW_WITH_GANDIVA=auto
+)
+
+if /i "%ARROW_BUILD_GCS%" == "ON" (
+    set PYARROW_WITH_GCS=enabled
+) else (
+    set PYARROW_WITH_GCS=auto
+)
+
+if /i "%ARROW_BUILD_ORC%" == "ON" (
+    set PYARROW_WITH_ORC=enabled
+) else (
+    set PYARROW_WITH_ORC=auto
+)
+
+set PYARROW_WITH_PARQUET=enabled
+set PYARROW_WITH_PARQUET_ENCRYPTION=enabled
+
+if /i "%ARROW_BUILD_S3%" == "ON" (
+    set PYARROW_WITH_S3=enabled
+) else (
+    set PYARROW_WITH_S3=auto
+)
+
+set PYARROW_WITH_SUBSTRAIT=enabled
 
 set ARROW_HOME=%CONDA_PREFIX%\Library
 @rem ARROW-3075; pkgconfig is broken for Parquet for now
 set PARQUET_HOME=%CONDA_PREFIX%\Library
 
-pip install --no-deps --no-build-isolation -vv --editable .
+pip install --no-deps --no-build-isolation -vv . ^
+    -Csetup-args="-Dacero=%PYARROW_WITH_ACERO%" ^

Review Comment:
   I think Meson can handle this better through use of the 
`-Dauto_features=...` setting. I've essentially tried in this PR to follow the 
CMake pattern of specifying ON/OFF for each option, with minor tweaks to the 
syntax. However, with Meson, you can simplify that by opting in/out, depending 
on what is simpler.
   
   For example, if we had 5 options for acero, compute, csv, flight, and 
substrait, AFAIK with CMake you'd have to provide:
   
   ```
   -DARROW_ACERO=ON -DARROW_COMPUTE=ON -DARROW_CSV=ON -DARROW_FLIGHT=ON 
-DARROW_SUBSTRAIT=ON
   ```
   
   If you wanted to enable all of them. With Meson, you could more simply just 
write:
   
   ```
   -Dauto_features=enabled
   ``` 
   
   to opt into all of these. 
   
   If you wanted all but flight, you would then do:
   
   ```
   -Dauto_features=enabled -Dflight=disabled
   ```
   
   So generally I think that would be a bit easier, but I wanted to avoid the 
larger diff for this initial PR. Happy to look further into refactoring towards 
that approach though if you agree it is easier and think it is a blocker



-- 
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