You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/01/22 02:31:17 UTC
[GitHub] [arrow] ianmcook opened a new pull request #9287: Add vcpkg.json manifest
ianmcook opened a new pull request #9287:
URL: https://github.com/apache/arrow/pull/9287
@kszucs could you review this please? My main purpose in adding is to improve the experience for Arrow C++ devs using Windows, but I noticed it also relates to your [TODO in #9096](https://github.com/apache/arrow/pull/9096/files#diff-990134cce6657dbbcf95457cf1a56810a7efa1f6cd58ecc27557c7d6ff45b533R67-R68). vcpkg does not have any `requirements.txt`-style package enumeration mechanism, but it supports this JSON manifest as a mechanism of defining dependencies.
In the `vcpkg install` command, you specify the path to the directory containing this manifest file with `--x-manifest-root` which later will change to `--manifest-root`. See details at https://vcpkg.readthedocs.io/en/stable/specifications/manifests/.
There are some differences between the packages listed in this manifest versus the packages you listed in the `vcpkg install` commands in #9096
- This installs `gtest` and `benchmark`
- This installs `boost` instead of separate `boost-filesystem`, `boost-regex`, etc.
- This does not explicitly include the `core` feature of `aws-sdk-cpp` because explicitly including it causes an error, and it gets installed anyway
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-779880497
Revision: a1950d6fdaf69e5f34523f8fc4523f2af8b0163f
Submitted crossbow builds: [ursacomputing/crossbow @ actions-99](https://github.com/ursacomputing/crossbow/branches/all?query=actions-99)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-99-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root [WIP]
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-767166823
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565759236
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
Review comment:
Yes. Could you point me to an example showing the tests that would be most appropriate here? Thank you
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769575602
You can ignore the failure in this pull request.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-776275223
@github-actions crossbow submit wheel-manylinux2010-cp36m wheel-windows-cp36m
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kszucs commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r577788746
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,98 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+
+@rem Install build dependencies with vcpkg
+
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+
+@rem Set environment variables
+
+set ARROW_TEST_DATA=%cd%\testing\data
+set PARQUET_TEST_DATA=%cd%\cpp\submodules\parquet-testing\data
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+
+@rem Build Arrow C++ library
+
+mkdir cpp\build
+pushd cpp\build
+
+@rem TODO(ianmcook): test using --parallel %NUMBER_OF_PROCESSORS% with
+@rem cmake --build instead of specifying -DARROW_CXXFLAGS="/MP" here
+@rem (see https://gitlab.kitware.com/cmake/cmake/-/issues/20564)
+
+@rem TODO(ianmcook): Add -DARROW_BUILD_BENCHMARKS=ON after the issue described
+@rem at https://github.com/google/benchmark/issues/1046 is resolved
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
+ -DARROW_DATASET=ON ^
+ -DARROW_DEPENDENCY_SOURCE=SYSTEM ^
+ -DARROW_FLIGHT=ON ^
+ -DARROW_MIMALLOC=ON ^
+ -DARROW_PACKAGE_PREFIX="%VCPKG_INSTALLED%\x64-windows" ^
+ -DARROW_PARQUET=ON ^
+ -DARROW_PYTHON=OFF ^
+ -DARROW_WITH_BROTLI=ON ^
+ -DARROW_WITH_BZ2=ON ^
+ -DARROW_WITH_LZ4=ON ^
+ -DARROW_WITH_SNAPPY=ON ^
+ -DARROW_WITH_ZLIB=ON ^
+ -DARROW_WITH_ZSTD=ON ^
+ -DCMAKE_BUILD_TYPE=release ^
+ -DCMAKE_TOOLCHAIN_FILE="C:\vcpkg\scripts\buildsystems\vcpkg.cmake" ^
+ -DCMAKE_UNITY_BUILD=ON ^
+ -DLZ4_MSVC_LIB_PREFIX="" ^
+ -DLZ4_MSVC_STATIC_LIB_SUFFIX="" ^
+ -D_VCPKG_INSTALLED_DIR="%VCPKG_INSTALLED%" ^
+ -DVCPKG_MANIFEST_MODE=ON ^
+ -DVCPKG_TARGET_TRIPLET="x64-windows" ^
+ -DZSTD_MSVC_LIB_PREFIX="" ^
+ .. || exit /B 1
+
+cmake --build . --target INSTALL --config Release || exit /B 1
+
+
+@rem Test Arrow C++ library
+
+@rem TODO(ianmcook): Troubleshoot two test failures:
Review comment:
@ianmcook please create a follow-up jira for this
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-776276674
Revision: 58dc1db128c56ee9d00680158af7adfee22e36e5
Submitted crossbow builds: [ursacomputing/crossbow @ actions-84](https://github.com/ursacomputing/crossbow/branches/all?query=actions-84)
|Task|Status|
|----|------|
|wheel-manylinux2010-cp36m|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-84-github-wheel-manylinux2010-cp36m)|
|wheel-windows-cp36m|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-84-github-wheel-windows-cp36m)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook edited a comment on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook edited a comment on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-765740804
FYI, the version of vcpkg that is currently preinstalled on the Github Actions Windows images is 2020.11.12 (as noted [here](https://github.com/actions/virtual-environments/blob/main/images/win/Windows2019-Readme.md)). This version has a bug that causes the installation of `aws-cpp-sdk` to fail. (See details at https://github.com/awslabs/aws-c-common/issues/734 and https://github.com/microsoft/vcpkg/pull/14716.) When running vcpkg in Github Actions on Windows, remove the preinstalled vcpkg and install the newest version from source.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-768631803
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565763280
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
Review comment:
Upon further investigation, a few sources point to this `/MP` method having better compatibility with Visual Studio than `--parallel`, so I think I will keep it this way
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-779447574
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root [WIP]
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-767166823
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565685799
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
Review comment:
The CMake that's being used here is the one distributed with Visual Studio. This command adds that CMake to the path and sets some environment variables to make it work correctly. If I omit this, then the `cmake` command below gives `'cmake' is not recognized as an internal or external command,
operable program or batch file.`
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769547052
I believe the **C++ / AMD64 Windows MinGW 64 C++** test failure is random and unrelated to the changes in this PR. It passed before I pushed the last two commits, and those commits only added comments and whitespace.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-779881082
@github-actions crossbow submit wheel-manylinux2010-cp36m wheel-windows-cp36m
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769911089
> The issue may lay in the implementation, so I suggest to create follow-up issues in order to investigate it.
Ok, thank you. I added a TODO comment just above the `ctest` call, and I'll open a separate issue to investigate this. Can we merge this PR despite this Crossbow job failing? That way I can use the same Crossbow job to troubleshoot the issue in a separate PR.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r573274466
##########
File path: cpp/CMakeSettings.json
##########
@@ -0,0 +1,21 @@
+{
+ "configurations": [
+ {
+ "name": "x64-Debug (default)",
+ "generator": "Ninja",
+ "configurationType": "Debug",
+ "inheritEnvironments": [ "msvc_x64_x64" ],
+ "buildRoot": "${projectDir}\\out\\build\\${name}",
+ "installRoot": "${projectDir}\\out\\install\\${name}",
+ "cmakeCommandArgs": "",
+ "buildCommandArgs": "",
+ "ctestCommandArgs": "",
+ "variables": [
+ {
+ "name":"VCPKG_MANIFEST_INSTALL",
+ "value":"OFF"
+ }
+ ]
+ }
+ ]
+}
Review comment:
For developers who are using Visual Studio 2019 and have vcpkg installed, when the `arrow/cpp` directory is opened in the Visual Studio IDE, it automatically identifies the `vcpkg.json` manifest and begins installing the dependencies listed in it into a subdirectory within the project. This is an undesirable behavior because the developer might want to use conda to install the dependencies or they might have already used vcpkg to install the dependencies outside the project. This `CMakeSettings.json` file sets the CMake variable `VCPKG_MANIFEST_INSTALL` to `OFF`, preventing Visual Studio from performing this automatic action. In [ARROW-11336](https://issues.apache.org/jira/browse/ARROW-11336), I will document this and instruct users to change the value to `ON` if they do wish for vcpkg to automatically install dependencies.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565512759
##########
File path: dev/tasks/vcpkg-tests/test-vcpkg-win.yml
##########
@@ -0,0 +1,54 @@
+# 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: must set "Crossbow" as name to have the badge links working in the
+# github comment reports!
+name: Crossbow
+
+on:
+ push:
+ branches:
+ - "*-github-*"
+
+jobs:
+ test-vcpkg-win:
+ name: Install build deps with vcpkg and build Arrow C++
+ runs-on: windows-2019
+ steps:
+ - name: Checkout Arrow
+ run: |
+ git clone --no-checkout {{ arrow.remote }} arrow
+ git -C arrow fetch -t {{ arrow.remote }} {{ arrow.branch }}
+ git -C arrow checkout FETCH_HEAD
+ git -C arrow submodule update --init --recursive
+ - name: Remove and Reinstall vcpkg
Review comment:
Would you like to add a comment explaining why?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-779447817
Revision: bb55c0d9b5041132baadb342e90ff6afbbe4212b
Submitted crossbow builds: [ursacomputing/crossbow @ actions-96](https://github.com/ursacomputing/crossbow/branches/all?query=actions-96)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-96-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-768782410
Revision: 7703a3830825c66ae878250fd99c624edadb7a22
Submitted crossbow builds: [ursacomputing/crossbow @ actions-53](https://github.com/ursacomputing/crossbow/branches/all?query=actions-53)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-53-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565816686
##########
File path: dev/tasks/tasks.yml
##########
@@ -1788,6 +1791,12 @@ tasks:
UBUNTU: 18.04
run: ubuntu-docs
+ ############################## vcpkg tests ##################################
+
+ test-build-vcpkg-win:
+ ci: github
+ template: vcpkg-tests/test-vcpkg-win.yml
Review comment:
We use `${CI_PLATFORM}.${PLATFORM}.yml` format for template file name:
```suggestion
template: vcpkg-tests/github.windows.yml
```
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,73 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build and Test Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_BENCHMARKS=ON ^
+ -DARROW_BUILD_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
+ -DARROW_DATASET=ON ^
+ -DARROW_DEPENDENCY_SOURCE=SYSTEM ^
+ -DARROW_FLIGHT=ON ^
+ -DARROW_MIMALLOC=ON ^
+ -DARROW_PACKAGE_PREFIX="%VCPKG_INSTALLED%\x64-windows" ^
+ -DARROW_PARQUET=ON ^
+ -DARROW_PYTHON=OFF ^
+ -DARROW_WITH_BROTLI=ON ^
+ -DARROW_WITH_BZ2=ON ^
+ -DARROW_WITH_LZ4=ON ^
+ -DARROW_WITH_SNAPPY=ON ^
+ -DARROW_WITH_ZLIB=ON ^
+ -DARROW_WITH_ZSTD=ON ^
+ -DCMAKE_BUILD_TYPE=release ^
+ -DCMAKE_TOOLCHAIN_FILE="C:\vcpkg\scripts\buildsystems\vcpkg.cmake" ^
+ -DCMAKE_UNITY_BUILD=ON ^
+ -DLZ4_MSVC_LIB_PREFIX="" ^
+ -DLZ4_MSVC_STATIC_LIB_SUFFIX="" ^
+ -D_VCPKG_INSTALLED_DIR="%VCPKG_INSTALLED%" ^
+ -DVCPKG_TARGET_TRIPLET="x64-windows" ^
+ -DZSTD_MSVC_LIB_PREFIX="" ^
+ .. || exit /B
+
+cmake --build . --target INSTALL --config Release || exit /B 1
Review comment:
Do we need explicit `1` for `exit /B`?
Other `exit /B`s don't specify `1`.
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
Review comment:
Thanks. I see.
Could you note https://gitlab.kitware.com/cmake/cmake/-/issues/20564 as a comment? We will reconsider this when CMake improves about this.
##########
File path: dev/tasks/tasks.yml
##########
@@ -1788,6 +1791,12 @@ tasks:
UBUNTU: 18.04
run: ubuntu-docs
+ ############################## vcpkg tests ##################################
+
+ test-build-vcpkg-win:
Review comment:
OK.
But I don't want to increase tasks that are ran only when release candidate verification. Normally, these tasks are outdated and they increase release cost. I hope that we can go to the next step by the next release.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769318455
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565758202
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
Review comment:
Sure, I'll change it to that. FYI this `cmake` command was mostly copied from [ci/appveyor-cpp-build.bat](https://github.com/apache/arrow/blob/master/ci/appveyor-cpp-build.bat#L91-L121) with changes only as needed.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kszucs commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769875913
The issue may lay in the implementation, so I suggest to create follow-up issues in order to investigate it.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769225593
Revision: a4c4247be629ef9e0af7ded66fe8d854a8effcbf
Submitted crossbow builds: [ursacomputing/crossbow @ actions-54](https://github.com/ursacomputing/crossbow/branches/all?query=actions-54)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-54-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kszucs commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-765260248
Thanks Ian for working on this!
I have a couple of questions:
- could we define multiple manifest files for different use cases (e.g. for building wheels or having a minimal build)?
- how can one utilize this manifest file (I assume we need to document this in the developer guide)?
- why do we need to add a version string to the manifest (if it is mandatory than we need to update the version bumping scripts used during the release procedure)?
Could you setup a github actions build to test it?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-767241021
@kszucs I added a Crossbow test. It runs a GHA job on a Windows instance. The job installs the current version of vcpkg, installs all the dependencies listed in `vcpkg.json`, runs CMake with many Arrow features enabled, and builds Arrow.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-765071768
https://issues.apache.org/jira/browse/ARROW-11340
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565778646
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
Review comment:
- https://gitlab.kitware.com/cmake/cmake/-/issues/20564
- https://blog.kitware.com/cmake-building-with-all-your-cores/ (recommends `/MP`, but it's from 2013)
- https://gitlab.kitware.com/cmake/cmake/-/merge_requests/1962 (seems to show `--parallel` support is somewhat new, and many Windows users run Visual Studio 2015 or 2017 which I'm not sure supports it)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565689340
##########
File path: dev/tasks/vcpkg-tests/test-vcpkg-win.yml
##########
@@ -0,0 +1,54 @@
+# 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: must set "Crossbow" as name to have the badge links working in the
+# github comment reports!
+name: Crossbow
+
+on:
+ push:
+ branches:
+ - "*-github-*"
+
+jobs:
+ test-vcpkg-win:
+ name: Install build deps with vcpkg and build Arrow C++
+ runs-on: windows-2019
+ steps:
+ - name: Checkout Arrow
+ run: |
+ git clone --no-checkout {{ arrow.remote }} arrow
+ git -C arrow fetch -t {{ arrow.remote }} {{ arrow.branch }}
+ git -C arrow checkout FETCH_HEAD
+ git -C arrow submodule update --init --recursive
+ - name: Remove and Reinstall vcpkg
Review comment:
Sure; it's explained in [this comment](https://github.com/apache/arrow/pull/9287#issuecomment-765740804) but I'll add that in a comment here in the code also
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-779446680
Revision: bb55c0d9b5041132baadb342e90ff6afbbe4212b
Submitted crossbow builds: [ursacomputing/crossbow @ actions-95](https://github.com/ursacomputing/crossbow/branches/all?query=actions-95)
|Task|Status|
|----|------|
|wheel-manylinux2010-cp36m|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-95-github-wheel-manylinux2010-cp36m)|
|wheel-windows-cp36m|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-95-github-wheel-windows-cp36m)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769348439
I rebased to resolve a seemingly unrelated test failure
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-765071688
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kszucs commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kszucs commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-775932559
@github-actions crossbow submit wheel-manylinux2010-cp36m wheel-windows-cp36m
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-779879894
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-776279813
I expect the *test-build-vcpkg-win* test to show a failure, for reasons described above
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565745442
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
Review comment:
If we enable this, how about running tests not just building?
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
Review comment:
How about using `--parallel %NUMBER_OF_PROCESSOR%` with `cmake --build` instead of specifying `/MP` here?
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
+ -DARROW_DATASET=ON ^
+ -DARROW_FLIGHT=ON ^
+ -DARROW_MIMALLOC=ON ^
+ -DARROW_PARQUET=ON ^
+ -DARROW_PYTHON=OFF ^
+ -DARROW_WITH_BROTLI=ON ^
+ -DARROW_WITH_BZ2=ON ^
+ -DARROW_WITH_LZ4=ON ^
+ -DARROW_WITH_SNAPPY=ON ^
+ -DARROW_WITH_ZLIB=ON ^
+ -DARROW_WITH_ZSTD=ON ^
+ -DARROW_DEPENDENCY_SOURCE=SYSTEM ^
+ -DCMAKE_BUILD_TYPE=release ^
+ -DCMAKE_INSTALL_PREFIX="" ^
+ -DCMAKE_UNITY_BUILD=ON ^
+ -DCMAKE_TOOLCHAIN_FILE="C:\vcpkg\scripts\buildsystems\vcpkg.cmake" ^
+ -DLZ4_MSVC_LIB_PREFIX="" ^
+ -DLZ4_MSVC_STATIC_LIB_SUFFIX="" ^
+ -DZSTD_MSVC_LIB_PREFIX="" ^
+ -DARROW_PACKAGE_PREFIX="%VCPKG_INSTALLED%\x64-windows" ^
+ -DVCPKG_TARGET_TRIPLET="x64-windows" ^
+ -D_VCPKG_INSTALLED_DIR="%VCPKG_INSTALLED%" ^
+ -DARROW_BUILD_SHARED=ON ^
Review comment:
Could you sort this list in alphabetical order?
##########
File path: cpp/vcpkg.json
##########
@@ -0,0 +1,40 @@
+{
+ "name": "arrow",
+ "version-string": "3.0.0-SNAPSHOT",
Review comment:
I'll push a commit to update this automatically in release process.
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
+ -DARROW_DATASET=ON ^
+ -DARROW_FLIGHT=ON ^
+ -DARROW_MIMALLOC=ON ^
+ -DARROW_PARQUET=ON ^
+ -DARROW_PYTHON=OFF ^
+ -DARROW_WITH_BROTLI=ON ^
+ -DARROW_WITH_BZ2=ON ^
+ -DARROW_WITH_LZ4=ON ^
+ -DARROW_WITH_SNAPPY=ON ^
+ -DARROW_WITH_ZLIB=ON ^
+ -DARROW_WITH_ZSTD=ON ^
+ -DARROW_DEPENDENCY_SOURCE=SYSTEM ^
+ -DCMAKE_BUILD_TYPE=release ^
+ -DCMAKE_INSTALL_PREFIX="" ^
Review comment:
Why should we specify empty prefix?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook edited a comment on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook edited a comment on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-767241021
@kszucs I added a Crossbow test. It runs a GHA job on a Windows instance. The job installs the current version of vcpkg, installs all the dependencies listed in `vcpkg.json`, runs CMake with many Arrow features enabled, and builds Arrow. I added this task to the `test` group. It takes about 2 hours to run.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-765740804
FYI, the version of vcpkg that is currently preinstalled on the Github Actions Windows images is 2020.11.12 (as noted [here](https://github.com/actions/virtual-environments/blob/main/images/win/Windows2019-Readme.md)). This version has a bug (https://github.com/awslabs/aws-c-common/issues/734) that causes the installation of `aws-cpp-sdk` to fail. When running vcpkg in Github Actions on Windows, remove the preinstalled vcpkg and install the newest version from source.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565769285
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
Review comment:
Could you share the sources?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565689525
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
Review comment:
Ow, I forgot that the vcpkg model is to rebuild everything from source. So this is gonna build OpenSSL, gRPC, Boost... everytime.
I understand why you didn't enable Gandiva!
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769581604
Revision: 3c3d139be7fe1922f521f1fefe900e4af3d6e047
Submitted crossbow builds: [ursacomputing/crossbow @ actions-58](https://github.com/ursacomputing/crossbow/branches/all?query=actions-58)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-58-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769327534
Revision: f7b57c6ac738ef6bd660506fd5cc4675d5057302
Submitted crossbow builds: [ursacomputing/crossbow @ actions-55](https://github.com/ursacomputing/crossbow/branches/all?query=actions-55)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-55-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565773451
##########
File path: dev/tasks/tasks.yml
##########
@@ -1788,6 +1791,12 @@ tasks:
UBUNTU: 18.04
run: ubuntu-docs
+ ############################## vcpkg tests ##################################
+
+ test-build-vcpkg-win:
Review comment:
For now it might be better to run it only when we do release candidate verification.
A next step will be to get an Arrow 3.0.0 "port" (that's the term they use) added to vcpkg, so users can simply `vcpkg install arrow` in their projects that use the Arrow C++ library as a dependency. Currently there is an [old port of Arrow on vcpkg](https://github.com/microsoft/vcpkg/tree/master/ports/arrow) and to date we have not treated vcpkg like a formal release channel. Once that is done, we will want to run nightly tests that install Arrow with vcpkg onto one or more supported platforms.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-776280261
Revision: 58dc1db128c56ee9d00680158af7adfee22e36e5
Submitted crossbow builds: [ursacomputing/crossbow @ actions-85](https://github.com/ursacomputing/crossbow/branches/all?query=actions-85)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-85-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root [WIP]
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-767167113
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565689327
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
Review comment:
Ow, I forgot that the vcpkg model is to build everything from source. So this is gonna rebuilt OpenSSL, Boost, gRPC... everytime.
I understand why you didn't enable Gandiva!
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root [WIP]
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-767170700
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565746322
##########
File path: dev/tasks/tasks.yml
##########
@@ -1788,6 +1791,12 @@ tasks:
UBUNTU: 18.04
run: ubuntu-docs
+ ############################## vcpkg tests ##################################
+
+ test-build-vcpkg-win:
Review comment:
Should we run this job nightly?
We can run this job nightly by adding this to `nightly:` group.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769580046
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769531741
Revision: 82a4c9301a8790ba249f6ef374193d1256aa90cb
Submitted crossbow builds: [ursacomputing/crossbow @ actions-57](https://github.com/ursacomputing/crossbow/branches/all?query=actions-57)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-57-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r577806537
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,98 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+
+@rem Install build dependencies with vcpkg
+
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+
+@rem Set environment variables
+
+set ARROW_TEST_DATA=%cd%\testing\data
+set PARQUET_TEST_DATA=%cd%\cpp\submodules\parquet-testing\data
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+
+@rem Build Arrow C++ library
+
+mkdir cpp\build
+pushd cpp\build
+
+@rem TODO(ianmcook): test using --parallel %NUMBER_OF_PROCESSORS% with
+@rem cmake --build instead of specifying -DARROW_CXXFLAGS="/MP" here
+@rem (see https://gitlab.kitware.com/cmake/cmake/-/issues/20564)
+
+@rem TODO(ianmcook): Add -DARROW_BUILD_BENCHMARKS=ON after the issue described
+@rem at https://github.com/google/benchmark/issues/1046 is resolved
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
+ -DARROW_DATASET=ON ^
+ -DARROW_DEPENDENCY_SOURCE=SYSTEM ^
+ -DARROW_FLIGHT=ON ^
+ -DARROW_MIMALLOC=ON ^
+ -DARROW_PACKAGE_PREFIX="%VCPKG_INSTALLED%\x64-windows" ^
+ -DARROW_PARQUET=ON ^
+ -DARROW_PYTHON=OFF ^
+ -DARROW_WITH_BROTLI=ON ^
+ -DARROW_WITH_BZ2=ON ^
+ -DARROW_WITH_LZ4=ON ^
+ -DARROW_WITH_SNAPPY=ON ^
+ -DARROW_WITH_ZLIB=ON ^
+ -DARROW_WITH_ZSTD=ON ^
+ -DCMAKE_BUILD_TYPE=release ^
+ -DCMAKE_TOOLCHAIN_FILE="C:\vcpkg\scripts\buildsystems\vcpkg.cmake" ^
+ -DCMAKE_UNITY_BUILD=ON ^
+ -DLZ4_MSVC_LIB_PREFIX="" ^
+ -DLZ4_MSVC_STATIC_LIB_SUFFIX="" ^
+ -D_VCPKG_INSTALLED_DIR="%VCPKG_INSTALLED%" ^
+ -DVCPKG_MANIFEST_MODE=ON ^
+ -DVCPKG_TARGET_TRIPLET="x64-windows" ^
+ -DZSTD_MSVC_LIB_PREFIX="" ^
+ .. || exit /B 1
+
+cmake --build . --target INSTALL --config Release || exit /B 1
+
+
+@rem Test Arrow C++ library
+
+@rem TODO(ianmcook): Troubleshoot two test failures:
Review comment:
[ARROW-11675](https://issues.apache.org/jira/browse/ARROW-11675)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-768764237
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook edited a comment on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook edited a comment on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-776279813
I expect the **test-build-vcpkg-win** test to show a failure, for reasons described above. I will check the output to confirm the failure is happening in the expected place, as before, not in an earlier unexpected place
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-777793474
Revision: 4f93799cce332bc002e19e0f216ab9bad57d290e
Submitted crossbow builds: [ursacomputing/crossbow @ actions-86](https://github.com/ursacomputing/crossbow/branches/all?query=actions-86)
|Task|Status|
|----|------|
|wheel-manylinux2010-cp36m|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-86-github-wheel-manylinux2010-cp36m)|
|wheel-windows-cp36m|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-86-github-wheel-windows-cp36m)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-776353907
The Crossbow wheel builds are failing for different reasons than before. I rebased this fork to apache:master so that might have pulled in some other recent changes that are now causing them to fail.
The **Integration / AMD64 Conda Integration Test** check is also failing with the same error as here in other recent C++ PRs so that seems unrelated to the changes in this PR.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-768657638
Revision: 5408df1d51c6f710f51b82e6a959928a760379eb
Submitted crossbow builds: [ursacomputing/crossbow @ actions-51](https://github.com/ursacomputing/crossbow/branches/all?query=actions-51)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-51-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565757146
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
+ -DARROW_DATASET=ON ^
+ -DARROW_FLIGHT=ON ^
+ -DARROW_MIMALLOC=ON ^
+ -DARROW_PARQUET=ON ^
+ -DARROW_PYTHON=OFF ^
+ -DARROW_WITH_BROTLI=ON ^
+ -DARROW_WITH_BZ2=ON ^
+ -DARROW_WITH_LZ4=ON ^
+ -DARROW_WITH_SNAPPY=ON ^
+ -DARROW_WITH_ZLIB=ON ^
+ -DARROW_WITH_ZSTD=ON ^
+ -DARROW_DEPENDENCY_SOURCE=SYSTEM ^
+ -DCMAKE_BUILD_TYPE=release ^
+ -DCMAKE_INSTALL_PREFIX="" ^
Review comment:
Thanks for catching that; I'll remove it
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769527283
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565513338
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
Review comment:
I'm curious whether this is necessary. Isn't CMake able to detect the VS install? Or is this for vcpkg?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769932647
@kszucs After this is merged, there is a slight possibility that we could see unexpected behavior in `ci/docker/python-wheel-windows-vs2017.dockerfile` and `ci/docker/python-wheel-manylinux-201x.dockerfile`. I do not think this will happen, but there's a chance that Visual Studio's CMake will automatically identify the `vcpkg.json` manifest file and attempt to install all the packages according to that manifest instead of using the ones installed by the prior `vcpkg` command. This should not happen because `vcpkg integrate install` has not been run, but in the event that it does happen, the solution is to run `vcpkg integrate remove` and/or to delete `vcpkg.json` before running `cmake`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-765071688
<!--
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.
-->
Thanks for opening a pull request!
Could you open an issue for this pull request on JIRA?
https://issues.apache.org/jira/browse/ARROW
Then could you also rename pull request title in the following format?
ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}
See also:
* [Other pull requests](https://github.com/apache/arrow/pulls/)
* [Contribution Guidelines - How to contribute patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root [WIP]
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-767197362
Revision: 94f35d7da5312eb572360a7c983b080b4c5cc48d
Submitted crossbow builds: [ursacomputing/crossbow @ actions-49](https://github.com/ursacomputing/crossbow/branches/all?query=actions-49)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-49-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r566238986
##########
File path: dev/tasks/tasks.yml
##########
@@ -1788,6 +1791,12 @@ tasks:
UBUNTU: 18.04
run: ubuntu-docs
+ ############################## vcpkg tests ##################################
+
+ test-build-vcpkg-win:
+ ci: github
+ template: vcpkg-tests/test-vcpkg-win.yml
Review comment:
Renamed in 4c4247
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r573274466
##########
File path: cpp/CMakeSettings.json
##########
@@ -0,0 +1,21 @@
+{
+ "configurations": [
+ {
+ "name": "x64-Debug (default)",
+ "generator": "Ninja",
+ "configurationType": "Debug",
+ "inheritEnvironments": [ "msvc_x64_x64" ],
+ "buildRoot": "${projectDir}\\out\\build\\${name}",
+ "installRoot": "${projectDir}\\out\\install\\${name}",
+ "cmakeCommandArgs": "",
+ "buildCommandArgs": "",
+ "ctestCommandArgs": "",
+ "variables": [
+ {
+ "name":"VCPKG_MANIFEST_INSTALL",
+ "value":"OFF"
+ }
+ ]
+ }
+ ]
+}
Review comment:
For developers who are using Visual Studio 2019 and have vcpkg installed, when the `arrow/cpp` directory is opened in the Visual Studio IDE, it automatically identifies the `vcpkg.json` manifest and begins installing the dependencies listed in it into a subdirectory within the project. This is an undesirable behavior because the developer might want to use conda to install the dependencies or they might have already used vcpkg to install the dependencies outside the project. This `CMakeSettings.json` file sets the CMake variable `VCPKG_MANIFEST_MODE` to `OFF`, preventing Visual Studio from performing this automatic action. In [ARROW-11336](https://issues.apache.org/jira/browse/ARROW-11336), I will document this and instruct users to change the value to `ON` if they do wish for vcpkg to automatically install dependencies.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root [WIP]
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-767171243
Revision: 1ddb30d7e5fe4f15178261e3d90cbe7f71b71078
Submitted crossbow builds: [ursacomputing/crossbow @ actions-48](https://github.com/ursacomputing/crossbow/branches/all?query=actions-48)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-48-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565773057
##########
File path: cpp/vcpkg.json
##########
@@ -0,0 +1,40 @@
+{
+ "name": "arrow",
+ "version-string": "4.0.0-SNAPSHOT",
+ "dependencies": [
+ "abseil",
+ {
+ "name": "aws-sdk-cpp",
+ "features": [
+ "config",
+ "cognito-identity",
+ "identity-management",
+ "s3",
+ "sts",
+ "transfer"
+ ]
+ },
+ "benchmark",
Review comment:
OK. Then how about adding `-DARROW_BUILD_BENCHMARKS=ON`?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769213179
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-776351534
The **test-build-vcpkg-win** failure is for the same reasons as noted above in https://github.com/apache/arrow/pull/9287#issuecomment-769843489 and should not block this from being merged.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565778646
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
Review comment:
- https://gitlab.kitware.com/cmake/cmake/-/issues/20564
- https://blog.kitware.com/cmake-building-with-all-your-cores/ (recommends `/MP`, but it's from 2013)
- https://gitlab.kitware.com/cmake/cmake/-/merge_requests/1962 (seems to show `--parallel` support is somewhat new, and many Windows users run Visual Studio 2017 or newer which I'm not sure supports it)
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-780045754
@kszucs My latest commits in this PR fix the unintended side effects that were occurring because of the addition of the manifest file `cpp/vcpkg.json`:
- The Python wheel builds (which use vcpkg to install dependencies but are _not_ supposed to use the manifest) were failing, because the existence of the manifest was causing the Visual Studio CMake toolchain to behave in some unexpected ways. I have set several variables and options that resolve this.
- When a developer opened the `cpp` directory in the Visual Studio 2019 IDE, it would automatically recognize the manifest and initiate a `vcpkg install` regardless of whether the developer intended to use vcpkg to install dependencies. I added a `CMakeSettings.json` file to disable this behavior.
The **wheel-manylinux2010-cp36m** and **wheel-windows-cp36m** Crossbow builds are now passing
The **test-build-vcpkg-win** failure is for the same reasons as noted above in https://github.com/apache/arrow/pull/9287#issuecomment-769843489 and should not block this from being merged.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook edited a comment on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook edited a comment on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-767241021
@kszucs I added a Crossbow test. It runs a GHA job on a Windows instance. The job installs the current version of vcpkg, installs all the dependencies listed in `vcpkg.json`, runs CMake with many Arrow features enabled, and builds Arrow. I added this task to the `test` group. It takes about 2 hours to run.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769348563
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565511772
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
Review comment:
Ninja generally builds much faster than msbuild, unless the latter improved recently.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r566235009
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
+ -DARROW_DATASET=ON ^
+ -DARROW_FLIGHT=ON ^
+ -DARROW_MIMALLOC=ON ^
+ -DARROW_PARQUET=ON ^
+ -DARROW_PYTHON=OFF ^
+ -DARROW_WITH_BROTLI=ON ^
+ -DARROW_WITH_BZ2=ON ^
+ -DARROW_WITH_LZ4=ON ^
+ -DARROW_WITH_SNAPPY=ON ^
+ -DARROW_WITH_ZLIB=ON ^
+ -DARROW_WITH_ZSTD=ON ^
+ -DARROW_DEPENDENCY_SOURCE=SYSTEM ^
+ -DCMAKE_BUILD_TYPE=release ^
+ -DCMAKE_INSTALL_PREFIX="" ^
Review comment:
I suspect that removing this caused the failure on Crossbow: https://github.com/ursacomputing/crossbow/runs/1781750297?check_suite_focus=true#step:4:3807
I'll restore it and see if that fixes the failure
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r566221390
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,73 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build and Test Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_BENCHMARKS=ON ^
+ -DARROW_BUILD_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
+ -DARROW_CXXFLAGS="/MP" ^
+ -DARROW_DATASET=ON ^
+ -DARROW_DEPENDENCY_SOURCE=SYSTEM ^
+ -DARROW_FLIGHT=ON ^
+ -DARROW_MIMALLOC=ON ^
+ -DARROW_PACKAGE_PREFIX="%VCPKG_INSTALLED%\x64-windows" ^
+ -DARROW_PARQUET=ON ^
+ -DARROW_PYTHON=OFF ^
+ -DARROW_WITH_BROTLI=ON ^
+ -DARROW_WITH_BZ2=ON ^
+ -DARROW_WITH_LZ4=ON ^
+ -DARROW_WITH_SNAPPY=ON ^
+ -DARROW_WITH_ZLIB=ON ^
+ -DARROW_WITH_ZSTD=ON ^
+ -DCMAKE_BUILD_TYPE=release ^
+ -DCMAKE_TOOLCHAIN_FILE="C:\vcpkg\scripts\buildsystems\vcpkg.cmake" ^
+ -DCMAKE_UNITY_BUILD=ON ^
+ -DLZ4_MSVC_LIB_PREFIX="" ^
+ -DLZ4_MSVC_STATIC_LIB_SUFFIX="" ^
+ -D_VCPKG_INSTALLED_DIR="%VCPKG_INSTALLED%" ^
+ -DVCPKG_TARGET_TRIPLET="x64-windows" ^
+ -DZSTD_MSVC_LIB_PREFIX="" ^
+ .. || exit /B
+
+cmake --build . --target INSTALL --config Release || exit /B 1
Review comment:
I think we do (per https://docs.github.com/en/actions/creating-actions/setting-exit-codes-for-actions). Thanks for catching that. I'll add the `1` everywhere.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565760742
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
+ -DARROW_BOOST_USE_SHARED=ON ^
+ -DARROW_BUILD_STATIC=OFF ^
+ -DARROW_BUILD_TESTS=ON ^
Review comment:
`ctest --output-on-failure --parallel %NUMBER_OF_PROCESSORS --timeout 300` will work.
https://github.com/apache/arrow/blob/master/ci/appveyor-cpp-build.bat#L127 also uses similar command line.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-765515335
Replies inline
> Thanks Ian for working on this!
>
> I have a couple of questions:
>
> * could we define multiple manifest files for different use cases (e.g. for building wheels or having a minimal build)?
Yes, definitely. Or would it be better to use this one as the master copy and apply patches to it to create the copies for other use cases?
For safety, we should never name the copies `vcpkg.json` because Visual Studio automatically detects manifests with that file name.
> * how can one utilize this manifest file (I assume we need to document this in the developer guide)?
I'm working on that in [ARROW-11336](https://issues.apache.org/jira/browse/ARROW-11336)
> * why do we need to add a version string to the manifest (if it is mandatory than we need to update the version bumping scripts used during the release procedure)?
It's mandatory. (The vcpkg docs say so and I confirmed by testing.)
>
> Could you setup a github actions build to test it?
Sure, I'll work on that
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kszucs closed pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kszucs closed pull request #9287:
URL: https://github.com/apache/arrow/pull/9287
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769317092
The Crossbow build is failing because of https://github.com/google/benchmark/issues/1046. I will remove Benchmark from the build for now.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565689525
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
Review comment:
I see.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook edited a comment on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook edited a comment on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769932647
@kszucs After this is merged, there is a slight possibility that we could see unexpected behavior in `ci/docker/python-wheel-windows-vs2017.dockerfile` and `ci/docker/python-wheel-manylinux-201x.dockerfile`. I do not think this will happen, but there's a chance that Visual Studio's CMake will automatically identify the `vcpkg.json` manifest file and attempt to install all the packages according to that manifest instead of using the ones installed by the prior `vcpkg install` command. This should not happen because `vcpkg integrate install` has not been run, but in the event that it does happen, the solution is to run `vcpkg integrate remove` and/or to delete `vcpkg.json` before running `cmake`.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r566238986
##########
File path: dev/tasks/tasks.yml
##########
@@ -1788,6 +1791,12 @@ tasks:
UBUNTU: 18.04
run: ubuntu-docs
+ ############################## vcpkg tests ##################################
+
+ test-build-vcpkg-win:
+ ci: github
+ template: vcpkg-tests/test-vcpkg-win.yml
Review comment:
Renamed in a4c4247
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-776279752
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root [WIP]
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-767197072
@github-actions crossbow submit test-build-vcpkg-win
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769352306
Revision: 4a883c974e5fd4951e8e1f3cdd2e74ec1f653f39
Submitted crossbow builds: [ursacomputing/crossbow @ actions-56](https://github.com/ursacomputing/crossbow/branches/all?query=actions-56)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-56-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-777792954
@github-actions crossbow submit wheel-manylinux2010-cp36m wheel-windows-cp36m
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-775943963
Revision: 23d7ea5d751885ac06b8c14a5c353513f0baae02
Submitted crossbow builds: [ursacomputing/crossbow @ actions-82](https://github.com/ursacomputing/crossbow/branches/all?query=actions-82)
|Task|Status|
|----|------|
|wheel-manylinux2010-cp36m|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-82-github-wheel-manylinux2010-cp36m)|
|wheel-windows-cp36m|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-82-github-wheel-windows-cp36m)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565693581
##########
File path: dev/tasks/vcpkg-tests/test-vcpkg-win.yml
##########
@@ -0,0 +1,54 @@
+# 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: must set "Crossbow" as name to have the badge links working in the
+# github comment reports!
+name: Crossbow
+
+on:
+ push:
+ branches:
+ - "*-github-*"
+
+jobs:
+ test-vcpkg-win:
+ name: Install build deps with vcpkg and build Arrow C++
+ runs-on: windows-2019
+ steps:
+ - name: Checkout Arrow
+ run: |
+ git clone --no-checkout {{ arrow.remote }} arrow
+ git -C arrow fetch -t {{ arrow.remote }} {{ arrow.branch }}
+ git -C arrow checkout FETCH_HEAD
+ git -C arrow submodule update --init --recursive
+ - name: Remove and Reinstall vcpkg
Review comment:
Added in 5408df1
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-779446443
@github-actions crossbow submit wheel-manylinux2010-cp36m wheel-windows-cp36m
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook edited a comment on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook edited a comment on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769932647
@kszucs After this is merged, there is a ~~slight~~ possibility that we could see unexpected behavior in `ci/scripts/python_wheel_manylinux_build.sh` and `ci/scripts/python_wheel_windows_build.bat`. I do not think this will happen, but there's a chance that Visual Studio's CMake will automatically identify the `vcpkg.json` manifest file and attempt to install all the packages according to that manifest instead of using the ones installed by the prior `vcpkg install` command. ~~This should not happen because `vcpkg integrate install` has not been run,~~ but in the event that it does happen, the solution is to ~~run `vcpkg integrate remove`~~ use the `cmake` flag `-DVCPKG_MANIFEST_INSTALL=OFF` and/or to delete `vcpkg.json` before running `cmake`. **[Update: This did happen, and I added `-DVCPKG_MANIFEST_INSTALL=OFF` in 77fc842]**
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook edited a comment on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook edited a comment on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769317092
Some builds are failing apparently because of https://github.com/google/benchmark/issues/1046. I will remove Benchmark from the build for now.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-776047253
Revision: 77fc8421372a896ddef55cc05b7c8c5066e34ee9
Submitted crossbow builds: [ursacomputing/crossbow @ actions-83](https://github.com/ursacomputing/crossbow/branches/all?query=actions-83)
|Task|Status|
|----|------|
|wheel-manylinux2010-cp36m|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-83-github-wheel-manylinux2010-cp36m)|
|wheel-windows-cp36m|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-83-github-wheel-windows-cp36m)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565688258
##########
File path: dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat
##########
@@ -0,0 +1,69 @@
+@rem Licensed to the Apache Software Foundation (ASF) under one
+@rem or more contributor license agreements. See the NOTICE file
+@rem distributed with this work for additional information
+@rem regarding copyright ownership. The ASF licenses this file
+@rem to you under the Apache License, Version 2.0 (the
+@rem "License"); you may not use this file except in compliance
+@rem with the License. You may obtain a copy of the License at
+@rem
+@rem http://www.apache.org/licenses/LICENSE-2.0
+@rem
+@rem Unless required by applicable law or agreed to in writing,
+@rem software distributed under the License is distributed on an
+@rem "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+@rem KIND, either express or implied. See the License for the
+@rem specific language governing permissions and limitations
+@rem under the License.
+
+@rem Run VsDevCmd.bat to set Visual Studio environment variables for building
+@rem on the command line. This is the path for Visual Studio Enterprise 2019
+call "C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\Common7\Tools\VsDevCmd.bat" -arch=amd64
+
+@rem Install build dependencies with vcpkg
+@rem TODO(ianmcook): change --x-manifest-root to --manifest-root after it
+@rem changes in vcpkg
+vcpkg install ^
+ --triplet x64-windows ^
+ --x-manifest-root cpp ^
+ --clean-after-build ^
+ || exit /B 1
+
+set VCPKG_INSTALLED=%cd%\cpp\vcpkg_installed
+
+@rem Build Arrow C++ library
+mkdir cpp\build
+pushd cpp\build
+
+cmake -G "Visual Studio 16 2019" -A x64 ^
Review comment:
My intention here was to keep the build as vanilla as possible. The amount time it takes to generate and build is very small compared to the time vcpkg takes to build all the packages, so it won't make much of a proportional difference either way.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769395206
> Some builds are failing apparently because of [google/benchmark#1046](https://github.com/google/benchmark/issues/1046). I will remove Benchmark from the build for now.
Could you note this as a comment?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565773451
##########
File path: dev/tasks/tasks.yml
##########
@@ -1788,6 +1791,12 @@ tasks:
UBUNTU: 18.04
run: ubuntu-docs
+ ############################## vcpkg tests ##################################
+
+ test-build-vcpkg-win:
Review comment:
For now it might be better to run it only manually, and maybe also when we do release candidate verification.
A next step will be to get an Arrow 3.0.0 "port" (that's the term they use) added to vcpkg, so users can simply `vcpkg install arrow` in their projects that use the Arrow C++ library as a dependency. Currently there is an [old port of Arrow on vcpkg](https://github.com/microsoft/vcpkg/tree/master/ports/arrow) and to date we have not treated vcpkg like a formal release channel. Once that is done, we will want to run nightly tests that install Arrow with vcpkg onto one or more supported platforms.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-776046225
@github-actions crossbow submit wheel-manylinux2010-cp36m wheel-windows-cp36m
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-769843489
@kszucs we're getting two failures in `ctest` when the dependencies are installed with vcpkg:
```
[ RUN ] TestStatisticsSortOrder/0.MinMax
D:/a/crossbow/crossbow/arrow/cpp/src/parquet/statistics_test.cc(667): error: Expected equality of these values:
stats_[i].min()
Which is: "\0\0\0\0"
cc_metadata->statistics()->EncodeMin()
Which is: "\x3\0\0\0"
D:/a/crossbow/crossbow/arrow/cpp/src/parquet/statistics_test.cc(835): error: Expected: this->VerifyParquetStats() doesn't generate new fatal failures in the current thread.
Actual: it does.
[ FAILED ] TestStatisticsSortOrder/0.MinMax, where TypeParam = struct parquet::PhysicalType<1> (0 ms)
```
```
[ RUN ] TestStatistic.Int32Extremums
D:/a/crossbow/crossbow/arrow/cpp/src/parquet/statistics_test.cc(897): error: Expected equality of these values:
stats->min()
Which is: -2147483648
expected_min
Which is: 0
[ FAILED ] TestStatistic.Int32Extremums (0 ms)
```
Do we need to apply a patch to one or more vcpkg ports to fix this? Is this why you apply [ci/vcpkg/ports.patch](https://github.com/apache/arrow/blob/master/ci/vcpkg/ports.patch) to the vcpkg sources when building wheels?
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-779881795
Revision: a1950d6fdaf69e5f34523f8fc4523f2af8b0163f
Submitted crossbow builds: [ursacomputing/crossbow @ actions-100](https://github.com/ursacomputing/crossbow/branches/all?query=actions-100)
|Task|Status|
|----|------|
|wheel-manylinux2010-cp36m|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-100-github-wheel-manylinux2010-cp36m)|
|wheel-windows-cp36m|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-100-github-wheel-windows-cp36m)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565759016
##########
File path: cpp/vcpkg.json
##########
@@ -0,0 +1,40 @@
+{
+ "name": "arrow",
+ "version-string": "4.0.0-SNAPSHOT",
+ "dependencies": [
+ "abseil",
+ {
+ "name": "aws-sdk-cpp",
+ "features": [
+ "config",
+ "cognito-identity",
+ "identity-management",
+ "s3",
+ "sts",
+ "transfer"
+ ]
+ },
+ "benchmark",
Review comment:
Can we remove this?
It seems that we don't run benchmark with the new job in this pull request.
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] kou commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r566581474
##########
File path: dev/tasks/vcpkg-tests/github.windows.yml
##########
@@ -0,0 +1,63 @@
+# 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: must set "Crossbow" as name to have the badge links working in the
+# github comment reports!
+name: Crossbow
+
+on:
+ push:
+ branches:
+ - "*-github-*"
+
+jobs:
+ test-vcpkg-win:
+ name: Install build deps with vcpkg and build Arrow C++
+ runs-on: windows-2019
+ steps:
+ - name: Checkout Arrow
+ run: |
+ git clone --no-checkout {{ arrow.remote }} arrow
+ git -C arrow fetch -t {{ arrow.remote }} {{ arrow.branch }}
+ git -C arrow checkout FETCH_HEAD
+ git -C arrow submodule update --init --recursive
+ - name: Remove and Reinstall vcpkg
+ # As of January 2021, the version of vcpkg that is preinstalled on the
+ # Github Actions windows-2019 image is 2020.11.12, as noted at
+ # https://github.com/actions/virtual-environments/blob/main/images/win/Windows2019-Readme.md
+ # This version of vcpkg has a bug that causes the installation of
+ # aws-cpp-sdk to fail. See details at
+ # https://github.com/awslabs/aws-c-common/issues/734
+ # and https://github.com/microsoft/vcpkg/pull/14716.
+ # When running vcpkg in Github Actions on Windows, remove the
+ # preinstalled vcpkg and install the newest version from source.
+ shell: cmd
+ run: |
+ CALL vcpkg integrate remove 2>NUL
+ CALL C:
+ CALL cd \
+ CALL rmdir /s /q vcpkg 2>NUL
+ CALL git clone https://github.com/microsoft/vcpkg.git vcpkg
+ CALL cd vcpkg
+ CALL bootstrap-vcpkg.bat -win64 -disableMetrics
+ CALL vcpkg integrate install
+ CALL setx PATH "%PATH%;C:\vcpkg"
+ - name: Install Dependencies with vcpkg and Build Arrow C++
+ shell: cmd
+ run: |
+ CALL cd arrow
+ CALL dev\tasks\vcpkg-tests\cpp-build-vcpkg.bat
Review comment:
We need to set `ARROW_TEST_DATA` and `PARQUET_TEST_DATA` environment variables for test. Sorry for not mentioning them. The following will work:
```yaml
env:
ARROW_TEST_DATA: ${{ env.GITHUB_WORKSPACE }}\arrow\testing\data
PARQUET_TEST_DATA: ${{ env.GITHUB_WORKSPACE }}\arrow\cpp\submodules\parquet-testing\data
```
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root [WIP]
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#issuecomment-767167113
Revision: af7b3cf8ce99c5f19969629c7c6c312668cfd223
Submitted crossbow builds: [ursacomputing/crossbow @ actions-47](https://github.com/ursacomputing/crossbow/branches/all?query=actions-47)
|Task|Status|
|----|------|
|test-build-vcpkg-win|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-47-github-test-build-vcpkg-win)|
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r566582921
##########
File path: dev/tasks/vcpkg-tests/github.windows.yml
##########
@@ -0,0 +1,63 @@
+# 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: must set "Crossbow" as name to have the badge links working in the
+# github comment reports!
+name: Crossbow
+
+on:
+ push:
+ branches:
+ - "*-github-*"
+
+jobs:
+ test-vcpkg-win:
+ name: Install build deps with vcpkg and build Arrow C++
+ runs-on: windows-2019
+ steps:
+ - name: Checkout Arrow
+ run: |
+ git clone --no-checkout {{ arrow.remote }} arrow
+ git -C arrow fetch -t {{ arrow.remote }} {{ arrow.branch }}
+ git -C arrow checkout FETCH_HEAD
+ git -C arrow submodule update --init --recursive
+ - name: Remove and Reinstall vcpkg
+ # As of January 2021, the version of vcpkg that is preinstalled on the
+ # Github Actions windows-2019 image is 2020.11.12, as noted at
+ # https://github.com/actions/virtual-environments/blob/main/images/win/Windows2019-Readme.md
+ # This version of vcpkg has a bug that causes the installation of
+ # aws-cpp-sdk to fail. See details at
+ # https://github.com/awslabs/aws-c-common/issues/734
+ # and https://github.com/microsoft/vcpkg/pull/14716.
+ # When running vcpkg in Github Actions on Windows, remove the
+ # preinstalled vcpkg and install the newest version from source.
+ shell: cmd
+ run: |
+ CALL vcpkg integrate remove 2>NUL
+ CALL C:
+ CALL cd \
+ CALL rmdir /s /q vcpkg 2>NUL
+ CALL git clone https://github.com/microsoft/vcpkg.git vcpkg
+ CALL cd vcpkg
+ CALL bootstrap-vcpkg.bat -win64 -disableMetrics
+ CALL vcpkg integrate install
+ CALL setx PATH "%PATH%;C:\vcpkg"
+ - name: Install Dependencies with vcpkg and Build Arrow C++
+ shell: cmd
+ run: |
+ CALL cd arrow
+ CALL dev\tasks\vcpkg-tests\cpp-build-vcpkg.bat
Review comment:
Thanks. Added in the `.bat` file in 3c3d139
----------------------------------------------------------------
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:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #9287: ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #9287:
URL: https://github.com/apache/arrow/pull/9287#discussion_r565769598
##########
File path: cpp/vcpkg.json
##########
@@ -0,0 +1,40 @@
+{
+ "name": "arrow",
+ "version-string": "4.0.0-SNAPSHOT",
+ "dependencies": [
+ "abseil",
+ {
+ "name": "aws-sdk-cpp",
+ "features": [
+ "config",
+ "cognito-identity",
+ "identity-management",
+ "s3",
+ "sts",
+ "transfer"
+ ]
+ },
+ "benchmark",
Review comment:
The larger purpose of adding this `vcpkg.json` manifest file is to improve the experience for Windows-based Arrow C++ developers who prefer vcpkg over conda ([ARROW-11336](https://issues.apache.org/jira/browse/ARROW-11336)). We want to give Windows devs two alternative ways to get the same build dependencies: vcpkg or conda. Therefore I want to keep what's here as similar as possible to what's in [ci/conda_env_cpp.yml](https://github.com/apache/arrow/blob/master/ci/conda_env_cpp.yml), which is what we [tell Windows devs to use with conda](https://arrow.apache.org/docs/developers/cpp/windows.html#using-conda-forge-for-build-dependencies). That contains benchmark and gtest, so I think this should too. That way all the latter instructions can be consistent regardless of whether vcpkg or conda was used.
P.S. I can provide the mapping of each conda-forge package to the analogous vcpkg package if you'd like to see that.
----------------------------------------------------------------
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:
users@infra.apache.org