ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root #9287

ianmcook · 2021-01-22T02:31:02Z

@kszucs could you review this please? My main purpose in adding this is to improve the experience for Arrow C++ devs using Windows, but I noticed it also relates to your TODO in #9096. 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 can 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

github-actions · 2021-01-22T02:31:22Z

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:

github-actions · 2021-01-22T02:31:37Z

https://issues.apache.org/jira/browse/ARROW-11340

kszucs · 2021-01-22T09:18:02Z

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?

ianmcook · 2021-01-22T16:09:35Z

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

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

ianmcook · 2021-01-22T23:24:35Z

FYI, the version of vcpkg that is currently preinstalled on the Github Actions Windows images is 2020.11.12 (as noted here). This version has a bug that causes the installation of aws-cpp-sdk to fail. (See details at awslabs/aws-c-common#734 and microsoft/vcpkg#14716.) When running vcpkg in Github Actions on Windows, remove the preinstalled vcpkg and install the newest version from source.

ianmcook · 2021-01-25T22:57:47Z

@github-actions crossbow submit test-build-vcpkg-win

github-actions · 2021-01-25T22:58:33Z

Revision: af7b3cf

Submitted crossbow builds: ursacomputing/crossbow @ actions-47

Task	Status
test-build-vcpkg-win

ianmcook · 2021-01-25T23:07:01Z

@github-actions crossbow submit test-build-vcpkg-win

github-actions · 2021-01-25T23:08:32Z

Revision: 1ddb30d

Submitted crossbow builds: ursacomputing/crossbow @ actions-48

Task	Status
test-build-vcpkg-win

ianmcook · 2021-01-26T00:19:58Z

@github-actions crossbow submit test-build-vcpkg-win

github-actions · 2021-01-26T00:20:46Z

Revision: 94f35d7

Submitted crossbow builds: ursacomputing/crossbow @ actions-49

Task	Status
test-build-vcpkg-win

ianmcook · 2021-01-26T02:22:35Z

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

dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat

dev/tasks/vcpkg-tests/test-vcpkg-win.yml

dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat

ianmcook · 2021-01-27T22:53:22Z

@github-actions crossbow submit test-build-vcpkg-win

github-actions · 2021-01-27T23:58:17Z

Revision: 5408df1

Submitted crossbow builds: ursacomputing/crossbow @ actions-51

Task	Status
test-build-vcpkg-win

dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat

cpp/vcpkg.json

dev/tasks/tasks.yml

cpp/vcpkg.json

ianmcook · 2021-01-28T03:01:29Z

@github-actions crossbow submit test-build-vcpkg-win

github-actions · 2021-01-28T03:56:34Z

Revision: 7703a38

Submitted crossbow builds: ursacomputing/crossbow @ actions-53

Task	Status
test-build-vcpkg-win

kou

+1

We can merge this with some minor improvements and green CI result.

dev/tasks/tasks.yml

dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat

ianmcook · 2021-02-16T14:41:41Z

@github-actions crossbow submit test-build-vcpkg-win

github-actions · 2021-02-16T14:42:32Z

Revision: a1950d6

Submitted crossbow builds: ursacomputing/crossbow @ actions-99

Task	Status
test-build-vcpkg-win

ianmcook · 2021-02-16T14:43:27Z

@github-actions crossbow submit wheel-manylinux2010-cp36m wheel-windows-cp36m

github-actions · 2021-02-16T14:44:32Z

Revision: a1950d6

Submitted crossbow builds: ursacomputing/crossbow @ actions-100

Task	Status
wheel-manylinux2010-cp36m
wheel-windows-cp36m

ianmcook · 2021-02-16T18:51:36Z

@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 #9287 (comment) and should not block this from being merged.

dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat

kszucs

LGTM, thanks Ian!

@kszucs

@kszucs could you review this please? My main purpose in adding this is to improve the experience for Arrow C++ devs using Windows, but I noticed it also relates to your [TODO in apache#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 can 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 apache#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 Closes apache#9287 from ianmcook/ARROW-11340 Lead-authored-by: Ian Cook <ianmcook@gmail.com> Co-authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Krisztián Szűcs <szucs.krisztian@gmail.com>

@kszucs

@kszucs could you review this please? My main purpose in adding this is to improve the experience for Arrow C++ devs using Windows, but I noticed it also relates to your [TODO in apache#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 can 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 apache#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 Closes apache#9287 from ianmcook/ARROW-11340 Lead-authored-by: Ian Cook <ianmcook@gmail.com> Co-authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Krisztián Szűcs <szucs.krisztian@gmail.com>

ianmcook changed the title ~~Add vcpkg.json manifest~~ ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root Jan 22, 2021

github-actions bot added the Component: C++ label Jan 22, 2021

ianmcook changed the title ~~ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root~~ ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root [WIP] Jan 25, 2021

kszucs force-pushed the master branch from d238279 to 437c8c9 Compare January 25, 2021 21:53

ianmcook force-pushed the ARROW-11340 branch from 4ddb364 to e0ae8c2 Compare January 25, 2021 22:25

ianmcook changed the title ~~ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root [WIP]~~ ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root Jan 26, 2021

pitrou requested a review from kou January 27, 2021 17:48

pitrou reviewed Jan 27, 2021

View reviewed changes

dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat Show resolved Hide resolved

pitrou reviewed Jan 27, 2021

View reviewed changes

dev/tasks/vcpkg-tests/test-vcpkg-win.yml Outdated Show resolved Hide resolved

pitrou reviewed Jan 27, 2021

View reviewed changes

dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat Show resolved Hide resolved

kou reviewed Jan 28, 2021

View reviewed changes

dev/tasks/tasks.yml Show resolved Hide resolved

kou reviewed Jan 28, 2021

View reviewed changes

cpp/vcpkg.json Show resolved Hide resolved

kou approved these changes Jan 28, 2021

View reviewed changes

dev/tasks/tasks.yml Show resolved Hide resolved

dev/tasks/tasks.yml Outdated Show resolved Hide resolved

dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat Show resolved Hide resolved

dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat Show resolved Hide resolved

ianmcook added 18 commits February 15, 2021 17:48

Add note to test --parallel instead of /MP

f6a7777

Specify exit code 1

04ea0c1

Restore empty install prefix

339d706

Rename YAML per convention

f462e63

Build without benchmarks

80ffa6b

Add comment explaining why building without benchmarks

ed45f82

Add line breaks for clarity

8a3241b

Set test data vars

d2f711f

Improve comments in bat file

19e6fdb

Add TODO re test failures

aa98702

Turn off vcpkg manifest install for Python wheel builds

7adada4

Explicitly enable vcpkg manifest install in test

456838b

Add CMakeSettings.json

a059cea

Fix indentation

45f5842

Fix cmake option typo

4d52ffe

Use VCPKG_MANIFEST_MODE not VCPKG_MANIFEST_INSTALL

f64e33b

Set VCPKG_FEATURE_FLAGS=-manifests

ce68bd3

Set cmake option _VCPKG_INSTALLED_DIR

a1950d6

ianmcook force-pushed the ARROW-11340 branch from bb55c0d to a1950d6 Compare February 15, 2021 22:48

kszucs reviewed Feb 17, 2021

View reviewed changes

dev/tasks/vcpkg-tests/cpp-build-vcpkg.bat Show resolved Hide resolved

kszucs approved these changes Feb 17, 2021

View reviewed changes

kszucs closed this in ad4504e Feb 17, 2021

asfimport mentioned this pull request May 26, 2021

[C++] Add vcpkg.json manifest to cpp project root #27237

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root #9287

ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root #9287

ianmcook commented Jan 22, 2021 •

edited

Loading

github-actions bot commented Jan 22, 2021

github-actions bot commented Jan 22, 2021

kszucs commented Jan 22, 2021

ianmcook commented Jan 22, 2021

ianmcook commented Jan 22, 2021 •

edited

Loading

ianmcook commented Jan 25, 2021

github-actions bot commented Jan 25, 2021

ianmcook commented Jan 25, 2021

github-actions bot commented Jan 25, 2021

ianmcook commented Jan 26, 2021

github-actions bot commented Jan 26, 2021

ianmcook commented Jan 26, 2021 •

edited

Loading

ianmcook commented Jan 27, 2021

github-actions bot commented Jan 27, 2021

ianmcook commented Jan 28, 2021

github-actions bot commented Jan 28, 2021

kou left a comment

ianmcook commented Feb 16, 2021

github-actions bot commented Feb 16, 2021

ianmcook commented Feb 16, 2021

github-actions bot commented Feb 16, 2021

ianmcook commented Feb 16, 2021

kszucs left a comment

ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root #9287

ARROW-11340: [C++] Add vcpkg.json manifest to cpp project root #9287

Conversation

ianmcook commented Jan 22, 2021 • edited Loading

github-actions bot commented Jan 22, 2021

github-actions bot commented Jan 22, 2021

kszucs commented Jan 22, 2021

ianmcook commented Jan 22, 2021

ianmcook commented Jan 22, 2021 • edited Loading

ianmcook commented Jan 25, 2021

github-actions bot commented Jan 25, 2021

ianmcook commented Jan 25, 2021

github-actions bot commented Jan 25, 2021

ianmcook commented Jan 26, 2021

github-actions bot commented Jan 26, 2021

ianmcook commented Jan 26, 2021 • edited Loading

ianmcook commented Jan 27, 2021

github-actions bot commented Jan 27, 2021

ianmcook commented Jan 28, 2021

github-actions bot commented Jan 28, 2021

kou left a comment

Choose a reason for hiding this comment

ianmcook commented Feb 16, 2021

github-actions bot commented Feb 16, 2021

ianmcook commented Feb 16, 2021

github-actions bot commented Feb 16, 2021

ianmcook commented Feb 16, 2021

kszucs left a comment

Choose a reason for hiding this comment

ianmcook commented Jan 22, 2021 •

edited

Loading

ianmcook commented Jan 22, 2021 •

edited

Loading

ianmcook commented Jan 26, 2021 •

edited

Loading