Skip to content

ARROW-6757: [Release] Use same CMake generator for C++ and Python when verifying RC, remove Python 3.5 from wheel verification #6350

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wesm wants to merge 1 commit into from
Closed

ARROW-6757: [Release] Use same CMake generator for C++ and Python when verifying RC, remove Python 3.5 from wheel verification #6350

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions dev/release/verify-release-candidate-wheels.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,17 +25,14 @@
@echo on

set _CURRENT_DIR=%CD%
set _VERIFICATION_DIR=C:\tmp\arrow-verify-release
set _VERIFICATION_DIR=C:\tmp\arrow-verify-release-wheels

if not exist "C:\tmp\" mkdir C:\tmp
if exist %_VERIFICATION_DIR% rd %_VERIFICATION_DIR% /s /q
if not exist %_VERIFICATION_DIR% mkdir %_VERIFICATION_DIR%

cd %_VERIFICATION_DIR%

CALL :verify_wheel 3.5 %1 %2
if errorlevel 1 GOTO error

CALL :verify_wheel 3.6 %1 %2
if errorlevel 1 GOTO error

Expand All @@ -62,7 +59,7 @@ set ARROW_VERSION=%2
set RC_NUMBER=%3
set PY_VERSION_NO_PERIOD=%PY_VERSION:.=%

set CONDA_ENV_PATH=C:\tmp\arrow-verify-release\_verify-wheel-%PY_VERSION%
set CONDA_ENV_PATH=%_VERIFICATION_DIR%\_verify-wheel-%PY_VERSION%
call conda create -p %CONDA_ENV_PATH% ^
--no-shortcuts -f -q -y python=%PY_VERSION% ^
|| EXIT /B 1
Expand Down
12 changes: 9 additions & 3 deletions dev/release/verify-release-candidate.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ pushd %ARROW_SOURCE%\cpp\build
@rem This is the path for Visual Studio Community 2017
call "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\Common7\Tools\VsDevCmd.bat" -arch=amd64

cmake -G "Ninja" ^
Copy link
Member Author

@wesm wesm Feb 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If anyone knows how to control the MSVC version used by Ninja let me know

Copy link
Member

@pitrou pitrou Feb 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is usually done by calling the vcvarsall.bat script for the particular MSVC install (or apparently VsDevCmd in more recent versions).

Copy link
Member Author

@wesm wesm Feb 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had this conversation in the past, I think. The MSVC "environment setup" scripts provided by VS are more crude than what can be obtained using the -G flag.

Using an explicit generator it's possible to choose the exact toolchain you want, but you must use msbuild, not Ninja. It would be interesting to know if there's a way to both use Ninja and select the target toolchain. The answer may be no but anyway it would be nice to know

Copy link
Member

@kou kou Feb 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't try it but we may be able to use -DMSVC_TOOLSET_VERSION=142 for Visual C++ 2019 or something: https://cmake.org/cmake/help/latest/variable/MSVC_TOOLSET_VERSION.html#variable:MSVC_TOOLSET_VERSION

@rem NOTE(wesm): not using Ninja for now to be able to more easily control the
@rem generator used

cmake -G "%GENERATOR%" ^
-DCMAKE_INSTALL_PREFIX=%ARROW_HOME% ^
-DARROW_BUILD_STATIC=OFF ^
-DARROW_BOOST_USE_SHARED=ON ^
Expand All @@ -90,11 +93,13 @@ cmake -G "Ninja" ^
-DARROW_PARQUET=ON ^
.. || exit /B

cmake --build . --target INSTALL --config Release

@rem NOTE(wesm): Building googletest is flaky for me with ninja. Building it
@rem first fixes the problem
ninja googletest_ep || exit /B

ninja install || exit /B
@rem ninja googletest_ep || exit /B
@rem ninja install || exit /B

@rem Get testing datasets for Parquet unit tests
git clone https://github.com/apache/parquet-testing.git %_VERIFICATION_DIR%\parquet-testing
Expand All @@ -112,6 +117,7 @@ popd
@rem Build and import pyarrow
pushd %ARROW_SOURCE%\python

set PYARROW_CMAKE_GENERATOR=%GENERATOR%
set PYARROW_WITH_FLIGHT=1
set PYARROW_WITH_PARQUET=1
python setup.py build_ext --inplace --bundle-arrow-cpp bdist_wheel || exit /B
Expand Down