kszucs commented on a change in pull request #9096:
URL: https://github.com/apache/arrow/pull/9096#discussion_r553880289



##########
File path: ci/scripts/wheel_windows_build.bat
##########
@@ -0,0 +1,92 @@
+echo "Building windows wheel..."
+
+call "C:\Program Files (x86)\Microsoft Visual 
Studio\2017\Community\VC\Auxiliary\Build\vcvars64.bat"
+
+echo "=== (%PYTHON_VERSION%) Clear output directories and leftovers ==="
+del /s /q C:\arrow-build
+del /s /q C:\arrow-dist
+del /s /q C:\arrow\python\dist
+del /s /q C:\arrow\python\build
+del /s /q C:\arrow\python\pyarrow\*.so
+del /s /q C:\arrow\python\pyarrow\*.so.*
+
+echo "=== (%PYTHON_VERSION%) Building Arrow C++ libraries ==="
+set ARROW_DATASET=ON
+set ARROW_FLIGHT=ON
+set ARROW_GANDIVA=OFF
+set ARROW_HDFS=ON
+set ARROW_ORC=OFF
+set ARROW_PARQUET=ON
+set ARROW_MIMALLOC=ON
+set ARROW_S3=ON
+set ARROW_TENSORFLOW=ON
+set ARROW_WITH_BROTLI=ON
+set ARROW_WITH_BZ2=ON
+set ARROW_WITH_LZ4=ON
+set ARROW_WITH_SNAPPY=ON
+set ARROW_WITH_ZLIB=ON
+set ARROW_WITH_ZSTD=ON
+set CMAKE_UNITY_BUILD=ON
+set CMAKE_GENERATOR=Visual Studio 15 2017 Win64
+
+mkdir C:\arrow-build
+pushd C:\arrow-build
+cmake ^
+    -DARROW_BUILD_SHARED=ON ^
+    -DARROW_BUILD_STATIC=OFF ^
+    -DARROW_BUILD_TESTS=OFF ^
+    -DARROW_CXXFLAGS="/MP" ^
+    -DARROW_DATASET=%ARROW_DATASET% ^
+    -DARROW_DEPENDENCY_SOURCE=SYSTEM ^
+    -DARROW_DEPENDENCY_USE_SHARED=OFF ^
+    -DARROW_FLIGHT=%ARROW_FLIGHT% ^
+    -DARROW_GANDIVA=%ARROW_GANDIVA% ^
+    -DARROW_HDFS=%ARROW_HDFS% ^
+    -DARROW_MIMALLOC=%ARROW_MIMALLOC% ^
+    -DARROW_ORC=%ARROW_ORC% ^
+    -DARROW_PACKAGE_KIND="wheel-windows" ^
+    -DARROW_PARQUET=%ARROW_PARQUET% ^
+    -DARROW_PYTHON=ON ^
+    -DARROW_S3=%ARROW_S3% ^
+    -DARROW_TENSORFLOW=%ARROW_TENSORFLOW% ^
+    -DARROW_WITH_BROTLI=%ARROW_WITH_BROTLI% ^
+    -DARROW_WITH_BZ2=%ARROW_WITH_BZ2% ^
+    -DARROW_WITH_LZ4=%ARROW_WITH_LZ4% ^
+    -DARROW_WITH_SNAPPY=%ARROW_WITH_SNAPPY% ^
+    -DARROW_WITH_ZLIB=%ARROW_WITH_ZLIB% ^
+    -DARROW_WITH_ZSTD=%ARROW_WITH_ZSTD% ^
+    -DCMAKE_BUILD_TYPE=%CMAKE_BUILD_TYPE% ^
+    -DLZ4_MSVC_LIB_PREFIX="" ^
+    -DLZ4_MSVC_STATIC_LIB_SUFFIX="" ^
+    -DZSTD_MSVC_LIB_PREFIX="" ^
+    -DCMAKE_CXX_COMPILER=clcache ^
+    -DCMAKE_INSTALL_PREFIX=C:\arrow-dist ^
+    -DCMAKE_TOOLCHAIN_FILE=C:\vcpkg\scripts\buildsystems\vcpkg.cmake ^
+    -DCMAKE_UNITY_BUILD=%CMAKE_UNITY_BUILD% ^
+    -DMSVC_LINK_VERBOSE=ON ^
+    -DVCPKG_TARGET_TRIPLET=x64-windows-static-md-%CMAKE_BUILD_TYPE% ^
+    -G "%CMAKE_GENERATOR%" ^
+    C:\arrow\cpp || exit /B
+cmake --build . --config %CMAKE_BUILD_TYPE% --target install || exit /B
+popd
+
+echo "=== (%PYTHON_VERSION%) Building wheel ==="
+set PYARROW_BUILD_TYPE=%CMAKE_BUILD_TYPE%
+set PYARROW_BUNDLE_ARROW_CPP=ON
+set PYARROW_BUNDLE_BOOST=OFF
+set PYARROW_CMAKE_GENERATOR=%CMAKE_GENERATOR%
+set PYARROW_INSTALL_TESTS=ON
+set PYARROW_WITH_DATASET=%ARROW_DATASET%
+set PYARROW_WITH_FLIGHT=%ARROW_FLIGHT%
+set PYARROW_WITH_GANDIVA=%ARROW_GANDIVA%
+set PYARROW_WITH_HDFS=%ARROW_HDFS%
+set PYARROW_WITH_ORC=%ARROW_ORC%
+set PYARROW_WITH_PARQUET=%ARROW_PARQUET%
+set PYARROW_WITH_S3=%ARROW_S3%
+set ARROW_HOME=C:\arrow-dist
+
+pushd C:\arrow\python
+@REM bundle the msvc runtime
+cp "C:\Program Files (x86)\Microsoft Visual 
Studio\2017\Community\VC\Redist\MSVC\14.16.27012\x64\Microsoft.VC141.CRT\msvcp140.dll"
 pyarrow\

Review comment:
       I plan to implement this bundling logic in cmake, but we can defer it 
for after the release.

##########
File path: python/setup.py
##########
@@ -385,17 +385,19 @@ def _bundle_arrow_cpp(self, build_prefix, build_lib):
                 build_prefix, build_lib,
                 "{}_regex".format(self.boost_namespace),
                 implib_required=False)
-        if sys.platform == 'win32':
-            # zlib uses zlib.dll for Windows
-            zlib_lib_name = 'zlib'
-            move_shared_libs(build_prefix, build_lib, zlib_lib_name,
-                             implib_required=False)
-            if self.with_flight:
-                # DLL dependencies for gRPC / Flight
-                for lib_name in ['libcrypto-1_1-x64',
-                                 'libssl-1_1-x64']:
-                    move_shared_libs(build_prefix, build_lib, lib_name,
-                                     implib_required=False)
+        # TODO(kszucs): consider to bundle the following shared libraries

Review comment:
       We can choose to bundle security sensitive shared libraries instead of 
linking them statically.

##########
File path: dev/tasks/tasks.yml
##########
@@ -586,29 +490,31 @@ tasks:
 
   ############################## Wheel Windows ################################
 
-  wheel-win-cp36m:
+  wheel-windows-cp36m:
     ci: github
-    template: python-wheels/github.win.yml
+    template: python-wheels/github.windows.yml
     params:
       python_version: 3.6
     artifacts:
       - pyarrow-{no_rc_version}-cp36-cp36m-win_amd64.whl
 
-  wheel-win-cp37m:
+  wheel-windows-cp37m:
     ci: github
-    template: python-wheels/github.win.yml
+    template: python-wheels/github.windows.yml
     params:
       python_version: 3.7
     artifacts:
       - pyarrow-{no_rc_version}-cp37-cp37m-win_amd64.whl
 
-  wheel-win-cp38:
+  wheel-windows-cp38m:
     ci: github
-    template: python-wheels/github.win.yml
+    template: python-wheels/github.windows.yml
     params:
       python_version: 3.8
     artifacts:
-      - pyarrow-{no_rc_version}-cp38-cp38-win_amd64.whl
+      - pyarrow-{no_rc_version}-cp38-cp38m-win_amd64.whl
+
+  # TODO(kszucs): add python 3.9

Review comment:
       `choco` fails to install python 3.9, investigating it.

##########
File path: python/setup.py
##########
@@ -385,17 +385,19 @@ def _bundle_arrow_cpp(self, build_prefix, build_lib):
                 build_prefix, build_lib,
                 "{}_regex".format(self.boost_namespace),
                 implib_required=False)
-        if sys.platform == 'win32':
-            # zlib uses zlib.dll for Windows
-            zlib_lib_name = 'zlib'
-            move_shared_libs(build_prefix, build_lib, zlib_lib_name,
-                             implib_required=False)
-            if self.with_flight:
-                # DLL dependencies for gRPC / Flight
-                for lib_name in ['libcrypto-1_1-x64',
-                                 'libssl-1_1-x64']:
-                    move_shared_libs(build_prefix, build_lib, lib_name,
-                                     implib_required=False)
+        # TODO(kszucs): consider to bundle the following shared libraries

Review comment:
       > In the case of OpenSSL, if we bundled a shared version the version in 
the main system will be preferred.
   
   Shouldn't we prefer the system one over the bundled one?
   
   > Not sure though where this is updated from.
   
   You mean the bundled or the system one?
   
   

##########
File path: python/setup.py
##########
@@ -385,17 +385,19 @@ def _bundle_arrow_cpp(self, build_prefix, build_lib):
                 build_prefix, build_lib,
                 "{}_regex".format(self.boost_namespace),
                 implib_required=False)
-        if sys.platform == 'win32':
-            # zlib uses zlib.dll for Windows
-            zlib_lib_name = 'zlib'
-            move_shared_libs(build_prefix, build_lib, zlib_lib_name,
-                             implib_required=False)
-            if self.with_flight:
-                # DLL dependencies for gRPC / Flight
-                for lib_name in ['libcrypto-1_1-x64',
-                                 'libssl-1_1-x64']:
-                    move_shared_libs(build_prefix, build_lib, lib_name,
-                                     implib_required=False)
+        # TODO(kszucs): consider to bundle the following shared libraries

Review comment:
       How about keeping it as is, linking these libraries statically?

##########
File path: dev/tasks/tasks.yml
##########
@@ -586,29 +490,31 @@ tasks:
 
   ############################## Wheel Windows ################################
 
-  wheel-win-cp36m:
+  wheel-windows-cp36m:
     ci: github
-    template: python-wheels/github.win.yml
+    template: python-wheels/github.windows.yml
     params:
       python_version: 3.6
     artifacts:
       - pyarrow-{no_rc_version}-cp36-cp36m-win_amd64.whl
 
-  wheel-win-cp37m:
+  wheel-windows-cp37m:
     ci: github
-    template: python-wheels/github.win.yml
+    template: python-wheels/github.windows.yml
     params:
       python_version: 3.7
     artifacts:
       - pyarrow-{no_rc_version}-cp37-cp37m-win_amd64.whl
 
-  wheel-win-cp38:
+  wheel-windows-cp38m:
     ci: github
-    template: python-wheels/github.win.yml
+    template: python-wheels/github.windows.yml
     params:
       python_version: 3.8
     artifacts:
-      - pyarrow-{no_rc_version}-cp38-cp38-win_amd64.whl
+      - pyarrow-{no_rc_version}-cp38-cp38m-win_amd64.whl
+
+  # TODO(kszucs): add python 3.9

Review comment:
       Fixed.

##########
File path: python/setup.py
##########
@@ -385,17 +385,19 @@ def _bundle_arrow_cpp(self, build_prefix, build_lib):
                 build_prefix, build_lib,
                 "{}_regex".format(self.boost_namespace),
                 implib_required=False)
-        if sys.platform == 'win32':
-            # zlib uses zlib.dll for Windows
-            zlib_lib_name = 'zlib'
-            move_shared_libs(build_prefix, build_lib, zlib_lib_name,
-                             implib_required=False)
-            if self.with_flight:
-                # DLL dependencies for gRPC / Flight
-                for lib_name in ['libcrypto-1_1-x64',
-                                 'libssl-1_1-x64']:
-                    move_shared_libs(build_prefix, build_lib, lib_name,
-                                     implib_required=False)
+        # TODO(kszucs): consider to bundle the following shared libraries

Review comment:
       Ok, I'm going to clean up the commented section used for bundling from 
`python/CMakeLists.txt` and `setup.py` then.

##########
File path: python/setup.py
##########
@@ -385,17 +385,19 @@ def _bundle_arrow_cpp(self, build_prefix, build_lib):
                 build_prefix, build_lib,
                 "{}_regex".format(self.boost_namespace),
                 implib_required=False)
-        if sys.platform == 'win32':
-            # zlib uses zlib.dll for Windows
-            zlib_lib_name = 'zlib'
-            move_shared_libs(build_prefix, build_lib, zlib_lib_name,
-                             implib_required=False)
-            if self.with_flight:
-                # DLL dependencies for gRPC / Flight
-                for lib_name in ['libcrypto-1_1-x64',
-                                 'libssl-1_1-x64']:
-                    move_shared_libs(build_prefix, build_lib, lib_name,
-                                     implib_required=False)
+        # TODO(kszucs): consider to bundle the following shared libraries

Review comment:
       And create a follow-up JIRA.

##########
File path: ci/scripts/wheel_windows_build.bat
##########
@@ -0,0 +1,92 @@
+echo "Building windows wheel..."
+
+call "C:\Program Files (x86)\Microsoft Visual 
Studio\2017\Community\VC\Auxiliary\Build\vcvars64.bat"
+
+echo "=== (%PYTHON_VERSION%) Clear output directories and leftovers ==="
+del /s /q C:\arrow-build
+del /s /q C:\arrow-dist
+del /s /q C:\arrow\python\dist
+del /s /q C:\arrow\python\build
+del /s /q C:\arrow\python\pyarrow\*.so
+del /s /q C:\arrow\python\pyarrow\*.so.*
+
+echo "=== (%PYTHON_VERSION%) Building Arrow C++ libraries ==="
+set ARROW_DATASET=ON
+set ARROW_FLIGHT=ON
+set ARROW_GANDIVA=OFF
+set ARROW_HDFS=ON
+set ARROW_ORC=OFF
+set ARROW_PARQUET=ON
+set ARROW_MIMALLOC=ON
+set ARROW_S3=ON
+set ARROW_TENSORFLOW=ON
+set ARROW_WITH_BROTLI=ON
+set ARROW_WITH_BZ2=ON
+set ARROW_WITH_LZ4=ON
+set ARROW_WITH_SNAPPY=ON
+set ARROW_WITH_ZLIB=ON
+set ARROW_WITH_ZSTD=ON
+set CMAKE_UNITY_BUILD=ON
+set CMAKE_GENERATOR=Visual Studio 15 2017 Win64
+
+mkdir C:\arrow-build
+pushd C:\arrow-build
+cmake ^
+    -DARROW_BUILD_SHARED=ON ^
+    -DARROW_BUILD_STATIC=OFF ^
+    -DARROW_BUILD_TESTS=OFF ^
+    -DARROW_CXXFLAGS="/MP" ^
+    -DARROW_DATASET=%ARROW_DATASET% ^
+    -DARROW_DEPENDENCY_SOURCE=SYSTEM ^
+    -DARROW_DEPENDENCY_USE_SHARED=OFF ^
+    -DARROW_FLIGHT=%ARROW_FLIGHT% ^
+    -DARROW_GANDIVA=%ARROW_GANDIVA% ^
+    -DARROW_HDFS=%ARROW_HDFS% ^
+    -DARROW_MIMALLOC=%ARROW_MIMALLOC% ^
+    -DARROW_ORC=%ARROW_ORC% ^
+    -DARROW_PACKAGE_KIND="wheel-windows" ^
+    -DARROW_PARQUET=%ARROW_PARQUET% ^
+    -DARROW_PYTHON=ON ^
+    -DARROW_S3=%ARROW_S3% ^
+    -DARROW_TENSORFLOW=%ARROW_TENSORFLOW% ^
+    -DARROW_WITH_BROTLI=%ARROW_WITH_BROTLI% ^
+    -DARROW_WITH_BZ2=%ARROW_WITH_BZ2% ^
+    -DARROW_WITH_LZ4=%ARROW_WITH_LZ4% ^
+    -DARROW_WITH_SNAPPY=%ARROW_WITH_SNAPPY% ^
+    -DARROW_WITH_ZLIB=%ARROW_WITH_ZLIB% ^
+    -DARROW_WITH_ZSTD=%ARROW_WITH_ZSTD% ^
+    -DCMAKE_BUILD_TYPE=%CMAKE_BUILD_TYPE% ^
+    -DLZ4_MSVC_LIB_PREFIX="" ^
+    -DLZ4_MSVC_STATIC_LIB_SUFFIX="" ^
+    -DZSTD_MSVC_LIB_PREFIX="" ^
+    -DCMAKE_CXX_COMPILER=clcache ^
+    -DCMAKE_INSTALL_PREFIX=C:\arrow-dist ^
+    -DCMAKE_TOOLCHAIN_FILE=C:\vcpkg\scripts\buildsystems\vcpkg.cmake ^
+    -DCMAKE_UNITY_BUILD=%CMAKE_UNITY_BUILD% ^
+    -DMSVC_LINK_VERBOSE=ON ^
+    -DVCPKG_TARGET_TRIPLET=x64-windows-static-md-%CMAKE_BUILD_TYPE% ^
+    -G "%CMAKE_GENERATOR%" ^
+    C:\arrow\cpp || exit /B
+cmake --build . --config %CMAKE_BUILD_TYPE% --target install || exit /B
+popd
+
+echo "=== (%PYTHON_VERSION%) Building wheel ==="
+set PYARROW_BUILD_TYPE=%CMAKE_BUILD_TYPE%
+set PYARROW_BUNDLE_ARROW_CPP=ON
+set PYARROW_BUNDLE_BOOST=OFF
+set PYARROW_CMAKE_GENERATOR=%CMAKE_GENERATOR%
+set PYARROW_INSTALL_TESTS=ON
+set PYARROW_WITH_DATASET=%ARROW_DATASET%
+set PYARROW_WITH_FLIGHT=%ARROW_FLIGHT%
+set PYARROW_WITH_GANDIVA=%ARROW_GANDIVA%
+set PYARROW_WITH_HDFS=%ARROW_HDFS%
+set PYARROW_WITH_ORC=%ARROW_ORC%
+set PYARROW_WITH_PARQUET=%ARROW_PARQUET%
+set PYARROW_WITH_S3=%ARROW_S3%
+set ARROW_HOME=C:\arrow-dist
+
+pushd C:\arrow\python
+@REM bundle the msvc runtime
+cp "C:\Program Files (x86)\Microsoft Visual 
Studio\2017\Community\VC\Redist\MSVC\14.16.27012\x64\Microsoft.VC141.CRT\msvcp140.dll"
 pyarrow\

Review comment:
       Created a follow-up jira 
https://issues.apache.org/jira/browse/ARROW-11185

##########
File path: python/setup.py
##########
@@ -385,17 +385,19 @@ def _bundle_arrow_cpp(self, build_prefix, build_lib):
                 build_prefix, build_lib,
                 "{}_regex".format(self.boost_namespace),
                 implib_required=False)
-        if sys.platform == 'win32':
-            # zlib uses zlib.dll for Windows
-            zlib_lib_name = 'zlib'
-            move_shared_libs(build_prefix, build_lib, zlib_lib_name,
-                             implib_required=False)
-            if self.with_flight:
-                # DLL dependencies for gRPC / Flight
-                for lib_name in ['libcrypto-1_1-x64',
-                                 'libssl-1_1-x64']:
-                    move_shared_libs(build_prefix, build_lib, lib_name,
-                                     implib_required=False)
+        # TODO(kszucs): consider to bundle the following shared libraries

Review comment:
       https://issues.apache.org/jira/browse/ARROW-11186




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to