[GitHub] [arrow] pitrou commented on a change in pull request #10865: ARROW-3699: [C++] Dockerfile for testing 32-bit C++ build

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

pitrou commented on a change in pull request #10865:
URL: https://github.com/apache/arrow/pull/10865#discussion_r758566511



##########
File path: cpp/src/arrow/compute/kernels/aggregate_test.cc
##########
@@ -3451,7 +3451,7 @@ TEST_F(TestInt64QuantileKernel, Int64) {
 #undef DOUBLE
 #undef O
 
-#ifndef __MINGW32__
+#if ARROW_BITNESS != 32

Review comment:
       Instead of skipping this on all 32-bit platforms, can you skip only on x86?

##########
File path: dev/tasks/tasks.yml
##########
@@ -984,14 +984,18 @@ tasks:
       flags: -e ARROW_SKYHOOK=ON
       image: ubuntu-cpp
 
-{% for debian_version in ["10", "11"] %}
-  test-debian-{{ debian_version }}-cpp:
+{% for arch, flags in [("amd64", ""), ("i386", "-e ARROW_S3=OFF -e ARROW_GANDIVA=OFF")] %}

Review comment:
       Nit, but for readability and maintainability I'd favour separate snippets for amd64 and i386.

##########
File path: ci/docker/debian-10-cpp.dockerfile
##########
@@ -73,7 +73,9 @@ RUN apt-get update -y -q && \
 
 COPY ci/scripts/install_minio.sh \
      /arrow/ci/scripts/
-RUN /arrow/ci/scripts/install_minio.sh ${arch} linux latest /usr/local
+RUN if [ "${arch}" != "i386" ]; then \
+      /arrow/ci/scripts/install_minio.sh ${arch} linux latest /usr/local; \

Review comment:
       Instead of putting the `if` here, put it in the download script?




-- 
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-unsubscribe@arrow.apache.org

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