From 2bca6bdfb9d073c09aed5a3cbd12968aeadf4083 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Fri, 8 Jul 2022 18:15:17 +0200 Subject: [PATCH 01/12] Enable Cython tests on windows wheels --- ci/scripts/python_wheel_windows_test.bat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/scripts/python_wheel_windows_test.bat b/ci/scripts/python_wheel_windows_test.bat index 2b7aad3abe9..2abf8ca50fe 100755 --- a/ci/scripts/python_wheel_windows_test.bat +++ b/ci/scripts/python_wheel_windows_test.bat @@ -17,7 +17,7 @@ @echo on -set PYARROW_TEST_CYTHON=OFF +set PYARROW_TEST_CYTHON=ON set PYARROW_TEST_DATASET=ON set PYARROW_TEST_FLIGHT=ON set PYARROW_TEST_GANDIVA=OFF From ba9046ffe5f8047c7fdd1afde059f9d7bf8f036b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 6 Oct 2022 12:47:05 +0200 Subject: [PATCH 02/12] Add some temporary debugging --- ci/scripts/python_wheel_windows_test.bat | 3 +++ python/pyarrow/tests/test_cython.py | 1 + 2 files changed, 4 insertions(+) diff --git a/ci/scripts/python_wheel_windows_test.bat b/ci/scripts/python_wheel_windows_test.bat index 2abf8ca50fe..0bca28e7a3f 100755 --- a/ci/scripts/python_wheel_windows_test.bat +++ b/ci/scripts/python_wheel_windows_test.bat @@ -35,6 +35,9 @@ set PYARROW_TEST_TENSORFLOW=ON set ARROW_TEST_DATA=C:\arrow\testing\data set PARQUET_TEST_DATA=C:\arrow\submodules\parquet-testing\data +@REM Upgrade setuptools to fix issue with Microsoft Visual C++ 14.0 not found +pip install --upgrade setuptools + @REM Install testing dependencies pip install -r C:\arrow\python\requirements-wheel-test.txt || exit /B 1 diff --git a/python/pyarrow/tests/test_cython.py b/python/pyarrow/tests/test_cython.py index 7152f3f1d44..ef01bec6924 100644 --- a/python/pyarrow/tests/test_cython.py +++ b/python/pyarrow/tests/test_cython.py @@ -45,6 +45,7 @@ import pyarrow as pa ext_modules = cythonize({pyx_file!r}) + print("Finished cythonize") compiler_opts = {compiler_opts!r} custom_ld_path = {test_ld_path!r} From 97c4532c2c3d7c31d85d83fe8830240f3e09ea21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 6 Oct 2022 15:42:27 +0200 Subject: [PATCH 03/12] Change docker image to contain vs2017 to be able to run cython tests --- docker-compose.yml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 1c3813757fa..03fc415f902 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -983,8 +983,15 @@ services: # doesn't exit properly on fail python-wheel-windows-test: - image: python:${PYTHON}-windowsservercore-1809 + image: ${REPO}:python-${PYTHON}-wheel-windows-test-vs2017-vcpkg-${VCPKG}-${PYTHON_WHEEL_WINDOWS_IMAGE_REVISION} + build: + args: + vcpkg: ${VCPKG} + python: ${PYTHON} + context: . + dockerfile: ci/docker/python-wheel-windows-vs2017.dockerfile volumes: + - "${DOCKER_VOLUME_PREFIX}python-wheel-windows-test-clcache:C:/clcache" - type: bind source: . target: "C:/arrow" From 8e38b3677c82bfe0fd62dc95cd0f67e1541ce672 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 6 Oct 2022 16:18:05 +0200 Subject: [PATCH 04/12] Update volume cache name --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 03fc415f902..13c67554cdb 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -991,7 +991,7 @@ services: context: . dockerfile: ci/docker/python-wheel-windows-vs2017.dockerfile volumes: - - "${DOCKER_VOLUME_PREFIX}python-wheel-windows-test-clcache:C:/clcache" + - "${DOCKER_VOLUME_PREFIX}python-wheel-windows-clcache:C:/clcache" - type: bind source: . target: "C:/arrow" From 81193edb04786f0764b7236897128b6ec8a98e89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Fri, 7 Oct 2022 11:34:33 +0200 Subject: [PATCH 05/12] Add new dockerfile with vs2017 to be able to build cython on tests --- ...ython-wheel-windows-test-vs2017.dockerfile | 40 +++++++++++++++++++ docker-compose.yml | 6 +-- 2 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 ci/docker/python-wheel-windows-test-vs2017.dockerfile diff --git a/ci/docker/python-wheel-windows-test-vs2017.dockerfile b/ci/docker/python-wheel-windows-test-vs2017.dockerfile new file mode 100644 index 00000000000..6cf95231730 --- /dev/null +++ b/ci/docker/python-wheel-windows-test-vs2017.dockerfile @@ -0,0 +1,40 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# NOTE: You must update PYTHON_WHEEL_WINDOWS_IMAGE_REVISION in .env +# when you update this file. + +# based on mcr.microsoft.com/windows/servercore:ltsc2019 +# contains choco and vs2017 preinstalled +FROM abrarov/msvc-2017:2.11.0 + +# Remove previous installations of python from the base image +# NOTE: a more recent base image (tried with 2.12.1) comes with python 3.9.7 +# and the msi installers are failing to remove pip and tcl/tk "products" making +# the subsequent choco python installation step failing for installing python +# version 3.9.* due to existing python version +RUN wmic product where "name like 'python%%'" call uninstall /nointeractive && \ + rm -rf Python* + +# Define the full version number otherwise choco falls back to patch number 0 (3.7 => 3.7.0) +ARG python=3.8 +RUN (if "%python%"=="3.7" setx PYTHON_VERSION "3.7.9" && setx PATH "%PATH%;C:\Python37;C:\Python37\Scripts") & \ + (if "%python%"=="3.8" setx PYTHON_VERSION "3.8.10" && setx PATH "%PATH%;C:\Python38;C:\Python38\Scripts") & \ + (if "%python%"=="3.9" setx PYTHON_VERSION "3.9.7" && setx PATH "%PATH%;C:\Python39;C:\Python39\Scripts") & \ + (if "%python%"=="3.10" setx PYTHON_VERSION "3.10.2" && setx PATH "%PATH%;C:\Python310;C:\Python310\Scripts") +RUN choco install -r -y --no-progress python --version=%PYTHON_VERSION% +RUN python -m pip install -U pip setuptools diff --git a/docker-compose.yml b/docker-compose.yml index 13c67554cdb..57cd03d11d8 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -981,15 +981,13 @@ services: target: "C:/arrow" command: arrow\\ci\\scripts\\python_wheel_windows_build.bat - # doesn't exit properly on fail python-wheel-windows-test: - image: ${REPO}:python-${PYTHON}-wheel-windows-test-vs2017-vcpkg-${VCPKG}-${PYTHON_WHEEL_WINDOWS_IMAGE_REVISION} + image: ${REPO}:python-${PYTHON}-wheel-windows-test-vs2017-${PYTHON_WHEEL_WINDOWS_IMAGE_REVISION} build: args: - vcpkg: ${VCPKG} python: ${PYTHON} context: . - dockerfile: ci/docker/python-wheel-windows-vs2017.dockerfile + dockerfile: ci/docker/python-wheel-windows-test-vs2017.dockerfile volumes: - "${DOCKER_VOLUME_PREFIX}python-wheel-windows-clcache:C:/clcache" - type: bind From 93efa30282471835109555444aa4099ccd6b6efe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Fri, 7 Oct 2022 12:50:01 +0200 Subject: [PATCH 06/12] Add unix tools to path to be able to use rm --- ci/docker/python-wheel-windows-test-vs2017.dockerfile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ci/docker/python-wheel-windows-test-vs2017.dockerfile b/ci/docker/python-wheel-windows-test-vs2017.dockerfile index 6cf95231730..6013efcd465 100644 --- a/ci/docker/python-wheel-windows-test-vs2017.dockerfile +++ b/ci/docker/python-wheel-windows-test-vs2017.dockerfile @@ -22,6 +22,9 @@ # contains choco and vs2017 preinstalled FROM abrarov/msvc-2017:2.11.0 +# Add unix tools to path +RUN setx path "%path%;C:\Program Files\Git\usr\bin" + # Remove previous installations of python from the base image # NOTE: a more recent base image (tried with 2.12.1) comes with python 3.9.7 # and the msi installers are failing to remove pip and tcl/tk "products" making From 785078fa4ff7f362dfa0ec2233fc4ec21f8ad490 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Fri, 7 Oct 2022 15:15:19 +0200 Subject: [PATCH 07/12] Add some debugging to understand where the failures are coming from --- python/pyarrow/tests/test_cython.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/python/pyarrow/tests/test_cython.py b/python/pyarrow/tests/test_cython.py index ef01bec6924..67bdcb551a6 100644 --- a/python/pyarrow/tests/test_cython.py +++ b/python/pyarrow/tests/test_cython.py @@ -114,9 +114,12 @@ def test_cython_api(tmpdir): # Check basic functionality orig_path = sys.path[:] sys.path.insert(0, str(tmpdir)) + #TODO: Remove DEBUGs try: + print(f"DEBUG - Current sys.path: {sys.path}") mod = __import__('pyarrow_cython_example') check_cython_example_module(mod) + print("DEBUG - After checking pyarrow_cython_example") finally: sys.path = orig_path @@ -135,13 +138,14 @@ def test_cython_api(tmpdir): delim, var = ';', 'PATH' else: delim, var = ':', 'LD_LIBRARY_PATH' - + print(f"DEBUG - Current sys.path: {sys.path}") subprocess_env[var] = delim.join( pa.get_library_dirs() + [subprocess_env.get(var, '')] ) subprocess.check_call([sys.executable, '-c', code], stdout=subprocess.PIPE, env=subprocess_env) + print(f"DEBUG - Last check call") @pytest.mark.cython From 469961c43cf63964de853dda1c0e0cd735ae5a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 13 Oct 2022 12:52:21 +0200 Subject: [PATCH 08/12] Add os.add_dll_directory for cython test on acse of python > 3.8 --- python/pyarrow/tests/test_cython.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/python/pyarrow/tests/test_cython.py b/python/pyarrow/tests/test_cython.py index 67bdcb551a6..133a3561197 100644 --- a/python/pyarrow/tests/test_cython.py +++ b/python/pyarrow/tests/test_cython.py @@ -114,7 +114,7 @@ def test_cython_api(tmpdir): # Check basic functionality orig_path = sys.path[:] sys.path.insert(0, str(tmpdir)) - #TODO: Remove DEBUGs + # TODO: Remove DEBUGs try: print(f"DEBUG - Current sys.path: {sys.path}") mod = __import__('pyarrow_cython_example') @@ -127,14 +127,26 @@ def test_cython_api(tmpdir): # pyarrow imported first. code = """if 1: import sys + import os + + if getattr(os, 'add_dll_directory'): + # Add dll directory was added on python 3.8 + # and is required in order to find extra DLLs + # only for win32 + for dir in {library_dirs}: + os.add_dll_directory(dir) mod = __import__({mod_name!r}) arr = mod.make_null_array(5) assert mod.get_array_length(arr) == 5 assert arr.null_count == 5 - """.format(mod_name='pyarrow_cython_example') + """.format(mod_name='pyarrow_cython_example', + library_dirs=pa.get_library_dirs()) - if sys.platform == 'win32': + if sys.platform == 'win32' and not \ + getattr(os, 'add_dll_directory'): + # Python 3.8 onwards don't check extension module DLLs on path + # use os.add_dll_directory instead. delim, var = ';', 'PATH' else: delim, var = ':', 'LD_LIBRARY_PATH' @@ -145,7 +157,7 @@ def test_cython_api(tmpdir): subprocess.check_call([sys.executable, '-c', code], stdout=subprocess.PIPE, env=subprocess_env) - print(f"DEBUG - Last check call") + print("DEBUG - Last check call") @pytest.mark.cython From 422831d17ee8cf8c2f5a5ad307f3f0486a88b2d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Thu, 13 Oct 2022 15:12:16 +0200 Subject: [PATCH 09/12] Fix non-windows tests --- python/pyarrow/tests/test_cython.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/python/pyarrow/tests/test_cython.py b/python/pyarrow/tests/test_cython.py index 133a3561197..a2370fec08f 100644 --- a/python/pyarrow/tests/test_cython.py +++ b/python/pyarrow/tests/test_cython.py @@ -129,12 +129,14 @@ def test_cython_api(tmpdir): import sys import os - if getattr(os, 'add_dll_directory'): + try: # Add dll directory was added on python 3.8 # and is required in order to find extra DLLs # only for win32 for dir in {library_dirs}: os.add_dll_directory(dir) + except AttributeError: + pass mod = __import__({mod_name!r}) arr = mod.make_null_array(5) @@ -144,9 +146,9 @@ def test_cython_api(tmpdir): library_dirs=pa.get_library_dirs()) if sys.platform == 'win32' and not \ - getattr(os, 'add_dll_directory'): + getattr(os, 'add_dll_directory', False): # Python 3.8 onwards don't check extension module DLLs on path - # use os.add_dll_directory instead. + # we have to use os.add_dll_directory instead. delim, var = ';', 'PATH' else: delim, var = ':', 'LD_LIBRARY_PATH' From d5fe64c6ef9f3270fe7b936edd5d02c21302c43f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Fri, 14 Oct 2022 11:08:58 +0200 Subject: [PATCH 10/12] Clean up some debugging --- ci/scripts/python_wheel_windows_test.bat | 3 --- python/pyarrow/tests/test_cython.py | 6 ------ 2 files changed, 9 deletions(-) diff --git a/ci/scripts/python_wheel_windows_test.bat b/ci/scripts/python_wheel_windows_test.bat index 0bca28e7a3f..2abf8ca50fe 100755 --- a/ci/scripts/python_wheel_windows_test.bat +++ b/ci/scripts/python_wheel_windows_test.bat @@ -35,9 +35,6 @@ set PYARROW_TEST_TENSORFLOW=ON set ARROW_TEST_DATA=C:\arrow\testing\data set PARQUET_TEST_DATA=C:\arrow\submodules\parquet-testing\data -@REM Upgrade setuptools to fix issue with Microsoft Visual C++ 14.0 not found -pip install --upgrade setuptools - @REM Install testing dependencies pip install -r C:\arrow\python\requirements-wheel-test.txt || exit /B 1 diff --git a/python/pyarrow/tests/test_cython.py b/python/pyarrow/tests/test_cython.py index a2370fec08f..92b061813f4 100644 --- a/python/pyarrow/tests/test_cython.py +++ b/python/pyarrow/tests/test_cython.py @@ -45,7 +45,6 @@ import pyarrow as pa ext_modules = cythonize({pyx_file!r}) - print("Finished cythonize") compiler_opts = {compiler_opts!r} custom_ld_path = {test_ld_path!r} @@ -114,12 +113,9 @@ def test_cython_api(tmpdir): # Check basic functionality orig_path = sys.path[:] sys.path.insert(0, str(tmpdir)) - # TODO: Remove DEBUGs try: - print(f"DEBUG - Current sys.path: {sys.path}") mod = __import__('pyarrow_cython_example') check_cython_example_module(mod) - print("DEBUG - After checking pyarrow_cython_example") finally: sys.path = orig_path @@ -152,14 +148,12 @@ def test_cython_api(tmpdir): delim, var = ';', 'PATH' else: delim, var = ':', 'LD_LIBRARY_PATH' - print(f"DEBUG - Current sys.path: {sys.path}") subprocess_env[var] = delim.join( pa.get_library_dirs() + [subprocess_env.get(var, '')] ) subprocess.check_call([sys.executable, '-c', code], stdout=subprocess.PIPE, env=subprocess_env) - print("DEBUG - Last check call") @pytest.mark.cython From 0c05f7fe630670eb52389e213b040d0eb9d2e7bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Wed, 19 Oct 2022 10:27:02 +0200 Subject: [PATCH 11/12] Fix not setting LD_LIBRARY_PATH on windows if os.add_dll_directory is present --- python/pyarrow/tests/test_cython.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/python/pyarrow/tests/test_cython.py b/python/pyarrow/tests/test_cython.py index 92b061813f4..56c07c96c51 100644 --- a/python/pyarrow/tests/test_cython.py +++ b/python/pyarrow/tests/test_cython.py @@ -141,11 +141,11 @@ def test_cython_api(tmpdir): """.format(mod_name='pyarrow_cython_example', library_dirs=pa.get_library_dirs()) - if sys.platform == 'win32' and not \ - getattr(os, 'add_dll_directory', False): - # Python 3.8 onwards don't check extension module DLLs on path - # we have to use os.add_dll_directory instead. - delim, var = ';', 'PATH' + if sys.platform == 'win32': + if not hasattr(os, 'add_dll_directory'): + # Python 3.8 onwards don't check extension module DLLs on path + # we have to use os.add_dll_directory instead. + delim, var = ';', 'PATH' else: delim, var = ':', 'LD_LIBRARY_PATH' subprocess_env[var] = delim.join( From 3ff78d0209202df17a2405986c88f4c53dcd875f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ra=C3=BAl=20Cumplido?= Date: Wed, 19 Oct 2022 12:35:22 +0200 Subject: [PATCH 12/12] Only update PATH/LD_LIBRARY_PATH if it is necessary --- python/pyarrow/tests/test_cython.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/python/pyarrow/tests/test_cython.py b/python/pyarrow/tests/test_cython.py index 56c07c96c51..42d8302afab 100644 --- a/python/pyarrow/tests/test_cython.py +++ b/python/pyarrow/tests/test_cython.py @@ -141,6 +141,7 @@ def test_cython_api(tmpdir): """.format(mod_name='pyarrow_cython_example', library_dirs=pa.get_library_dirs()) + var = None if sys.platform == 'win32': if not hasattr(os, 'add_dll_directory'): # Python 3.8 onwards don't check extension module DLLs on path @@ -148,9 +149,11 @@ def test_cython_api(tmpdir): delim, var = ';', 'PATH' else: delim, var = ':', 'LD_LIBRARY_PATH' - subprocess_env[var] = delim.join( - pa.get_library_dirs() + [subprocess_env.get(var, '')] - ) + + if var: + subprocess_env[var] = delim.join( + pa.get_library_dirs() + [subprocess_env.get(var, '')] + ) subprocess.check_call([sys.executable, '-c', code], stdout=subprocess.PIPE, env=subprocess_env)