eli-schwartz commented on code in PR #45854:
URL: https://github.com/apache/arrow/pull/45854#discussion_r2023011646


##########
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:
   You can stick all arguments in a toolchain file and then load the toolchain 
file by calling `-Csetup-args=--native-file=$TOOLCHAIN_FILE`.
   
   In general I don't think it's a great idea to allow environment variables to 
override every possible build option as it becomes difficult to juggle and 
pushes complexity into the build files to do option parsing and evaluate 
precedence etc. what happens if you have:
   
   ```
   export PYARROW_USE_FOO=true
   
   meson setup builddir/ -D use_foo=false
   ```
   
   Which value wins?
   
   Meson does support environment variables, particularly, standard stuff like 
CC, CFLAGS, LLVM_CONFIG, etc. Those will populate the environment object in 
meson's interpreter core, since meson knows whether an option was specifically 
defined across various data sources. But there's no 
`get_option('use_foo').was_set_on_cli()` so I don't really see how you could 
correctly implement an environment variable.
   
   More interesting than a general "read environment variables" facility would 
be a proposal to have meson support `$MESON_SETUP_ARGS` and have meson parse 
them as a preface for `sys.argv` (so that says.argv overrides it when the two 
different).
   
   Alternatively perhaps load a toolchain file via an environment variable?



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