Skip to content

ARROW-4611: [C++] Rework CMake logic #3688

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
xhochy wants to merge 1 commit into from
Closed

ARROW-4611: [C++] Rework CMake logic #3688

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
107 changes: 37 additions & 70 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,16 +56,15 @@ matrix:
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
script:
- $TRAVIS_BUILD_DIR/ci/travis_lint.sh
- name: "C++ unit tests w/ Valgrind, clang 7.0"
- name: "C++ unit tests w/ clang 7.0, system packages"
os: linux
env:
- ARROW_TRAVIS_VALGRIND=1
- ARROW_TRAVIS_USE_TOOLCHAIN=1
- ARROW_TRAVIS_FLIGHT=1
- ARROW_TRAVIS_USE_SYSTEM=1
- ARROW_TRAVIS_PLASMA=1
- ARROW_TRAVIS_ORC=1
- ARROW_TRAVIS_PARQUET=1
- ARROW_TRAVIS_GANDIVA=1
- ARROW_TRAVIS_VERBOSE=1
- ARROW_TRAVIS_USE_SYSTEM_JAVA=1
- ARROW_BUILD_WARNING_LEVEL=CHECKIN
- CC="clang-7"
Expand All @@ -80,56 +79,25 @@ matrix:
script:
- $TRAVIS_BUILD_DIR/ci/travis_script_cpp.sh || travis_terminate 1
# Separating Valgrind and C++ coverage makes individual jobs shorter
- name: "C++ unit tests w/ gcc 5.4, coverage"
- name: "C++ unit tests w/ conda-forge toolchain, coverage"
compiler: gcc
language: cpp
os: linux
jdk: openjdk8
env:
- ARROW_BUILD_WARNING_LEVEL=CHECKIN
- ARROW_TRAVIS_COVERAGE=1
- ARROW_TRAVIS_USE_TOOLCHAIN=1
- ARROW_TRAVIS_FLIGHT=1
- ARROW_TRAVIS_PLASMA=1
- ARROW_TRAVIS_ORC=1
- ARROW_TRAVIS_PARQUET=1
- ARROW_TRAVIS_GANDIVA=1
- ARROW_TRAVIS_GANDIVA_JAVA=1
- ARROW_TRAVIS_USE_SYSTEM_JAVA=1
- ARROW_BUILD_WARNING_LEVEL=CHECKIN
before_script:
- if [ $ARROW_CI_CPP_AFFECTED != "1" ] && [ $ARROW_CI_JAVA_AFFECTED != "1" ]; then exit; fi
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
# If either C++ or Python changed, we must install the C++ libraries
- git submodule update --init
- $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh
script:
- $TRAVIS_BUILD_DIR/ci/travis_script_cpp.sh || travis_terminate 1
- $TRAVIS_BUILD_DIR/ci/travis_script_gandiva_java.sh || travis_terminate 1
- $TRAVIS_BUILD_DIR/ci/travis_upload_cpp_coverage.sh || travis_terminate 1
- name: "C++ unit tests w/ gcc 4.8, trusty"
dist: trusty
compiler: gcc
language: cpp
os: linux
jdk: openjdk8
env:
- ARROW_TRAVIS_USE_TOOLCHAIN=1
- ARROW_TRAVIS_PLASMA=1
- ARROW_TRAVIS_ORC=1
- ARROW_TRAVIS_PARQUET=1
- ARROW_TRAVIS_GANDIVA=1
- ARROW_TRAVIS_GANDIVA_JAVA=1
- ARROW_BUILD_WARNING_LEVEL=CHECKIN
before_install:
- ulimit -c unlimited -S
- |
if [ $TRAVIS_OS_NAME == "linux" ]; then
sudo bash -c "echo -e 'Acquire::Retries 10; Acquire::http::Timeout \"20\";' > /etc/apt/apt.conf.d/99-travis-retry"
sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test
sudo apt-get update -qq
fi
- eval `python $TRAVIS_BUILD_DIR/ci/detect-changes.py`
- ARROW_TRAVIS_PLASMA=1
- ARROW_TRAVIS_USE_SYSTEM_JAVA=1
- ARROW_TRAVIS_USE_TOOLCHAIN=1
# TODO: This fails in re2 code
# - ARROW_TRAVIS_VALGRIND=1
- ARROW_TRAVIS_VERBOSE=1
before_script:
- if [ $ARROW_CI_CPP_AFFECTED != "1" ] && [ $ARROW_CI_JAVA_AFFECTED != "1" ]; then exit; fi
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
Expand All @@ -140,21 +108,23 @@ matrix:
script:
- $TRAVIS_BUILD_DIR/ci/travis_script_cpp.sh || travis_terminate 1
- $TRAVIS_BUILD_DIR/ci/travis_script_gandiva_java.sh || travis_terminate 1
- name: "Python 2.7 and 3.6 unit tests w/ Valgrind, gcc 5.4, coverage"
- $TRAVIS_BUILD_DIR/ci/travis_upload_cpp_coverage.sh || travis_terminate 1
- name: "Python 2.7 and 3.6 unit tests w/ Valgrind, conda-forge toolchain, coverage"
compiler: gcc
language: cpp
os: linux
jdk: openjdk8
env:
# Valgrind is needed for the Plasma store tests
- ARROW_TRAVIS_VALGRIND=1
- ARROW_TRAVIS_USE_TOOLCHAIN=1
- ARROW_BUILD_WARNING_LEVEL=CHECKIN
- ARROW_TRAVIS_COVERAGE=1
- ARROW_TRAVIS_FLIGHT=1
- ARROW_TRAVIS_OPTIONAL_INSTALL=1
- ARROW_TRAVIS_PYTHON_DOCS=1
- ARROW_TRAVIS_PYTHON_JVM=1
- ARROW_TRAVIS_OPTIONAL_INSTALL=1
- ARROW_BUILD_WARNING_LEVEL=CHECKIN
- ARROW_TRAVIS_USE_SYSTEM_JAVA=1
- ARROW_TRAVIS_USE_TOOLCHAIN=1
- ARROW_TRAVIS_VALGRIND=1
# TODO(wesm): Run the benchmarks outside of Travis
# - ARROW_TRAVIS_PYTHON_BENCHMARKS=1
before_script:
Expand All @@ -172,10 +142,9 @@ matrix:
- export PLASMA_VALGRIND=1
- $TRAVIS_BUILD_DIR/ci/travis_script_python.sh 3.6 || travis_terminate 1
- $TRAVIS_BUILD_DIR/ci/travis_upload_cpp_coverage.sh
- name: "[OS X] C++ w/ XCode 8.3"
- name: "[OS X] C++ w/ XCode 9.3"
compiler: clang
language: cpp
osx_image: xcode8.3
osx_image: xcode9.3
os: osx
cache:
addons:
Expand All @@ -185,41 +154,38 @@ matrix:
- ARROW_TRAVIS_FLIGHT=1
- ARROW_TRAVIS_ORC=1
- ARROW_TRAVIS_PARQUET=1
- ARROW_TRAVIS_GANDIVA=1
- ARROW_TRAVIS_GANDIVA_JAVA=1
# TODO(ARROW-4763): llvm and llvmdev packages are in conflict:
# https://github.com/conda-forge/llvmdev-feedstock/issues/60

@wesm wesm Mar 5, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oof

# - ARROW_TRAVIS_GANDIVA=1
# - ARROW_TRAVIS_GANDIVA_JAVA=1
- ARROW_TRAVIS_OPTIONAL_INSTALL=1
- ARROW_TRAVIS_VERBOSE=0
- ARROW_TRAVIS_VERBOSE=1
- ARROW_BUILD_WARNING_LEVEL=CHECKIN
# ARROW-3803: The Xcode 8.3 image has Boost libraries in /usr/local/lib
# which can get loaded before the toolchain Boost libraries. These seem to
# get loaded even though we are modifying LD_LIBRARY_PATH. We build our own
# Boost and statically link to get around the issue until this can be
# investigated further
- ARROW_TRAVIS_VENDORED_BOOST=1
before_script:
- if [ $ARROW_CI_CPP_AFFECTED != "1" ] && [ $ARROW_CI_JAVA_AFFECTED != "1" ]; then exit; fi
# If either C++ or Python changed, we must install the C++ libraries
- git submodule update --init
- $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh
script:
- $TRAVIS_BUILD_DIR/ci/travis_script_cpp.sh || travis_terminate 1
- $TRAVIS_BUILD_DIR/ci/travis_script_gandiva_java.sh
- name: "[OS X] Python w/ XCode 7.3"
# Disabled because of ARROW-4763
# - $TRAVIS_BUILD_DIR/ci/travis_script_gandiva_java.sh
- name: "[OS X] Python w/ XCode 9.3"
compiler: clang
language: cpp
osx_image: xcode7.3
osx_image: xcode9.3
os: osx
cache:
addons:
env:
- ARROW_TRAVIS_PLASMA=1
- ARROW_TRAVIS_USE_TOOLCHAIN=1
- ARROW_BUILD_WARNING_LEVEL=CHECKIN
- ARROW_TRAVIS_OPTIONAL_INSTALL=1
- ARROW_TRAVIS_VERBOSE=1
- MACOSX_DEPLOYMENT_TARGET="10.9"
before_script:
script:
- if [ $ARROW_CI_PYTHON_AFFECTED != "1" ]; then exit; fi
- $TRAVIS_BUILD_DIR/ci/travis_install_toolchain.sh || travis_terminate 1
- $TRAVIS_BUILD_DIR/ci/travis_script_python.sh 2.7 || travis_terminate 1
- $TRAVIS_BUILD_DIR/ci/travis_script_python.sh 3.6
- name: "[manylinux1] Python"
Expand Down Expand Up @@ -256,16 +222,15 @@ matrix:
- if [ $ARROW_CI_JAVA_AFFECTED != "1" ]; then exit; fi
script:
- $TRAVIS_BUILD_DIR/ci/travis_script_java.sh
- name: "Integration w/ OpenJDK 8, clang 7"

@wesm wesm Mar 5, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

maybe say what gcc?

@xhochy xhochy Mar 9, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll add conda-forge toolchain as this may change over time

- name: "Integration w/ OpenJDK 8, conda-forge toolchain"
language: java
os: linux
env: ARROW_TEST_GROUP=integration
jdk: openjdk8
env:
- ARROW_TRAVIS_PLASMA=1
- ARROW_TRAVIS_PLASMA_JAVA_CLIENT=1
- CC="clang-7"
- CXX="clang++-7"
- ARROW_TRAVIS_USE_TOOLCHAIN=1
before_script:
- if [ $ARROW_CI_INTEGRATION_AFFECTED != "1" ]; then exit; fi
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
Expand All @@ -275,7 +240,8 @@ matrix:
- $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh
script:
- $TRAVIS_BUILD_DIR/ci/travis_script_integration.sh
- $TRAVIS_BUILD_DIR/ci/travis_script_plasma_java_client.sh
# TODO(ARROW-4764): Using system java and plasma built with conda doesn't work
# - $TRAVIS_BUILD_DIR/ci/travis_script_plasma_java_client.sh

@wesm wesm Mar 5, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems like this should not be tested here

- name: "NodeJS"
language: node_js
os: linux
Expand All @@ -289,7 +255,6 @@ matrix:
- $TRAVIS_BUILD_DIR/ci/travis_script_js.sh
- name: "C++ & GLib & Ruby w/ gcc 5.4"
compiler: gcc
language: cpp
os: linux
env:
- ARROW_TRAVIS_ORC=1
Expand Down Expand Up @@ -369,6 +334,7 @@ matrix:
dist: trusty
env:
- ARROW_TRAVIS_PARQUET=1
- ARROW_TRAVIS_USE_SYSTEM=1
before_install:
# Have to copy-paste this here because of how R's build steps work
- eval `python $TRAVIS_BUILD_DIR/ci/detect-changes.py`
Expand All @@ -379,6 +345,7 @@ matrix:
sudo add-apt-repository -y ppa:ubuntu-toolchain-r/test
sudo apt-get update -qq
fi
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
- $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh --only-library
- export LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$TRAVIS_BUILD_DIR/cpp-install/lib
Expand Down
10 changes: 10 additions & 0 deletions LICENSE.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -837,3 +837,13 @@ This project includes code from the manylinux project.
Copyright: 2016 manylinux
Homepage: https://github.com/pypa/manylinux
License: The MIT License (MIT)

--------------------------------------------------------------------------------

This project include code from CMake.

* cpp/cmake_modules/FindGTest.cmake is based on code from CMake.

Copyright: Copyright 2000-2019 Kitware, Inc. and Contributors
Homepage: https://gitlab.kitware.com/cmake/cmake
License: 3-clause BSD
4 changes: 4 additions & 0 deletions NOTICE.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ This product includes software from Google Guava (Apache 2.0)
* Copyright (C) 2007 The Guava Authors
* https://github.com/google/guava

This product include software from CMake (BSD 3-Clause)
* CMake - Cross Platform Makefile Generator
* Copyright 2000-2019 Kitware, Inc. and Contributors

@wesm wesm Mar 5, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this is needed here. In fact, most of NOTICE.txt can be emptied I think

@xhochy xhochy Mar 9, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

So what should then actually go into NOTICE?

@wesm wesm Mar 10, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We only need to put in there was is legally required by the license for a project with code that is included.

contrast Apache Kudu's NOTICE with its LICENSE


The web site includes files generated by Jekyll.

--------------------------------------------------------------------------------
Expand Down
8 changes: 2 additions & 6 deletions appveyor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ cache:
- C:\Users\Appveyor\clcache
- C:\Users\Appveyor\.cargo\registry

matrix:
fast_finish: true
#matrix:

@fsaintjacques fsaintjacques Mar 6, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For debug?

@xhochy xhochy Mar 9, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, will remove before merge.

# fast_finish: true

environment:
global:
Expand All @@ -61,10 +61,6 @@ environment:
- JOB: "Build_Debug"
GENERATOR: Ninja
CONFIGURATION: "Debug"
- JOB: "Cmake_Script_Tests"
GENERATOR: Ninja
CONFIGURATION: "Release"
BUILD_SCRIPT: "CMake_Build_Script"
- JOB: "MinGW32"
MINGW_PACKAGE_PREFIX: mingw-w64-i686
MINGW_PREFIX: c:\msys64\mingw32
Expand Down
2 changes: 0 additions & 2 deletions ci/appveyor-build.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,6 @@ git config core.symlinks true
git reset --hard
if "%JOB:~,5%" == "MinGW" (
call ci\appveyor-cpp-build-mingw.bat
) else if "%JOB%" == "Cmake_Script_Tests" (
call ci\appveyor-cpp-test-cmake-script.bat
) else (
call ci\appveyor-cpp-build.bat
)
Expand Down
7 changes: 1 addition & 6 deletions ci/appveyor-cpp-build-mingw.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,12 @@ set CPP_BUILD_DIR=cpp\build
mkdir %CPP_BUILD_DIR%
pushd %CPP_BUILD_DIR%

set BOOST_ROOT=%MINGW_PREFIX%
set LZ4_HOME=%MINGW_PREFIX%
set ZSTD_HOME=%MINGW_PREFIX%
set SNAPPY_HOME=%MINGW_PREFIX%
set BROTLI_HOME=%MINGW_PREFIX%
set FLATBUFFERS_HOME=%MINGW_PREFIX%
cmake ^
-G "MSYS Makefiles" ^
-DCMAKE_INSTALL_PREFIX=%INSTALL_DIR% ^
-DCMAKE_BUILD_TYPE=%CMAKE_BUILD_TYPE% ^
-DARROW_VERBOSE_THIRDPARTY_BUILD=OFF ^
-DARROW_PACKAGE_PREFIX=%MINGW_PREFIX% ^
-DARROW_JEMALLOC=OFF ^
-DARROW_USE_GLOG=OFF ^
-DARROW_PYTHON=ON ^
Expand Down
4 changes: 2 additions & 2 deletions ci/appveyor-cpp-build.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ if "%JOB%" == "Static_Crt_Build" (
pushd cpp\build-debug

cmake -G "%GENERATOR%" ^
-DARROW_VERBOSE_THIRDPARTY_BUILD=OFF ^
-DARROW_VERBOSE_THIRDPARTY_BUILD=ON ^
-DARROW_USE_STATIC_CRT=ON ^
-DARROW_BOOST_USE_SHARED=OFF ^
-DARROW_BUILD_SHARED=OFF ^
Expand Down Expand Up @@ -108,7 +108,7 @@ if "%JOB%" == "Toolchain" (
set CONDA_PACKAGES=%CONDA_PACKAGES% --file=ci\conda_env_cpp.yml
)

conda create -n arrow -q -y %CONDA_PACKAGES% -c conda-forge
conda create -n arrow -q -y %CONDA_PACKAGES% -c conda-forge || exit /B
Comment thread
xhochy marked this conversation as resolved.
Outdated

call activate arrow

Expand Down
2 changes: 2 additions & 0 deletions ci/appveyor-cpp-setup-mingw.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ pacman -S --noconfirm ^
"%MINGW_PACKAGE_PREFIX%-boost" ^
"%MINGW_PACKAGE_PREFIX%-brotli" ^
"%MINGW_PACKAGE_PREFIX%-cmake" ^
"%MINGW_PACKAGE_PREFIX%-double-conversion" ^
"%MINGW_PACKAGE_PREFIX%-flatbuffers" ^
"%MINGW_PACKAGE_PREFIX%-gcc" ^
"%MINGW_PACKAGE_PREFIX%-gflags" ^
Expand All @@ -32,6 +33,7 @@ pacman -S --noconfirm ^
"%MINGW_PACKAGE_PREFIX%-meson" ^
"%MINGW_PACKAGE_PREFIX%-protobuf" ^
"%MINGW_PACKAGE_PREFIX%-python3-numpy" ^
"%MINGW_PACKAGE_PREFIX%-rapidjson" ^
"%MINGW_PACKAGE_PREFIX%-snappy" ^
"%MINGW_PACKAGE_PREFIX%-zlib" ^
"%MINGW_PACKAGE_PREFIX%-zstd" || exit /B
Loading