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 2020/11/24 14:57:48 UTC
[GitHub] [arrow] josiahyan opened a new pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
josiahyan opened a new pull request #8757:
URL: https://github.com/apache/arrow/pull/8757
I'm hoping to work on parts of the GCS filesystem implementation, and this is a prerequisite.
----------------------------------------------------------------
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] xhochy commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
xhochy commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-733745201
> @xhochy I can't seem to summon the bot. But I read what the action did, and saw that it required a very specific version of the cmake autoformatter, as you suggested. It works now! Thanks for your help!
The bot did run but didn't make any changes to your CMake files: https://github.com/apache/arrow/runs/1450988053?check_suite_focus=true#step:10:11
----------------------------------------------------------------
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] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r584312257
##########
File path: ci/conda_env_unix.yml
##########
@@ -19,6 +19,8 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
Review comment:
The currently available version in conda (1.24) is incompatible with the way I initially packaged it. I believe I saw in the CI run that `storage_client` was missing or something, as the google-cloud-cpp package has [started namespacing its exports](https://github.com/googleapis/google-cloud-cpp/blob/5bf147226e9333c5106d5fc8b8bba62657eeab97/google/cloud/storage/legacy.cmake.in#L28) (finally!).
Given the hard link against abseil libraries (as gRPC does too), to reduce packaging size, I decided that the version was safer pinned. The abseil libraries actually loaded probably depends on the matrix of abseil versions and users of it. Personally, I extract it by a bazel query (provided in comments) and a Python script to do lookup the symbols in the ar files. I believe the gRPC builds do something similar, or just pull them out from the compiler invocations (does @pitrou or @kou know how they were determined in commits like [these](https://github.com/apache/arrow/commit/4ecac056dfa88a0032fed7f36a20d2886c39ef90)?). Ideally the compiler would do it, but I didn't dig into why the build system wouldn't allow so (I'm guessing the libs have to bundled in and linked separately somehow?).
I'm hoping to upgrade the dependency when there is code using it (which would be the GCS implementation of the Arrow file interfaces).
----------------------------------------------------------------
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] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-733405694
@xhochy I can't seem summon the bot. But I read what the action did, and saw that it required a very specific version of the cmake autoformatter, as you suggested. It works now! Thanks for your help!
----------------------------------------------------------------
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] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-890440359
> What's the status of this PR?
Sorry, but I lost track of following up on this PR! I believed it was ready to merge in Feb, but assumed people had lost interest in pursuing (native) GCS support, or that this change was not quite worth making for now. Is there still a need for this? If so, I can attempt to rebase it again.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r584312257
##########
File path: ci/conda_env_unix.yml
##########
@@ -19,6 +19,8 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
Review comment:
The currently available version in conda (1.24) is incompatible with the way I initially packaged it. I believe I saw in the CI run that `storage_client` was missing or something, as the google-cloud-cpp package has [started namespacing its exports](https://github.com/googleapis/google-cloud-cpp/blob/5bf147226e9333c5106d5fc8b8bba62657eeab97/google/cloud/storage/legacy.cmake.in#L28) (finally!).
Given the hard link against abseil libraries (as gRPC does too), to reduce packaging size, I decided that the version was safer pinned. The abseil libraries actually loaded probably depends on the matrix of abseil versions and users of it (gRPC, and the proposed google-cloud-cpp inclusion). Personally, I [extract it by a bazel query](https://github.com/apache/arrow/blob/7076b4c75c6981dac7b9e11e5661260cb7ddedf5/cpp/cmake_modules/ThirdpartyToolchain.cmake#L2397) (provided in comments) and a Python script to lookup the symbols in the ar files. I believe the gRPC builds do something similar, or just pull them out from the compiler invocations (does @pitrou or @kou know how they were determined in commits like [these](https://github.com/apache/arrow/commit/4ecac056dfa88a0032fed7f36a20d2886c39ef90)?). Ideally the compiler would do the dead section elimination, but I didn't dig into why the build system wouldn't allow so (I'm guessing the libs have to bundled in and linked separately som
ehow?).
I'm hoping to upgrade the dependency when there is code using it (which would be the GCS implementation of the Arrow file interfaces).
----------------------------------------------------------------
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] xhochy commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
xhochy commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-733180367
> Running the cmake autoformatter heavily modifies the touched cmake files; should I be doing that?
Maybe you have a different version of that installed and that formats differently? You can also summon the autoformatter with `@github-actions<space>autotune` on the 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] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-929407709
I’ve discussed this PR with @coryan , and we think its better to start over, given the need to upgrade dependencies, and handle the Abseil libraries (which I believe, when used with gRPC are causing the build to fail on some platforms) with more scripting. He will be taking over this work.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r706923155
##########
File path: ci/conda_env_unix.txt
##########
@@ -19,5 +19,7 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
+nlohmann_json>=3.4.0
Review comment:
what is the new JSON dependency for?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r584312257
##########
File path: ci/conda_env_unix.yml
##########
@@ -19,6 +19,8 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
Review comment:
The currently available version in conda (1.24) is incompatible with the way I initially packaged it. I believe I saw in the CI run that `storage_client` was missing or something, as the google-cloud-cpp package has [started namespacing its exports](https://github.com/googleapis/google-cloud-cpp/blob/5bf147226e9333c5106d5fc8b8bba62657eeab97/google/cloud/storage/legacy.cmake.in#L28) (finally!).
Given the hard link against abseil libraries (as gRPC does too), to reduce packaging size, I decided that the version was safer pinned. The abseil libraries actually loaded probably depends on the matrix of abseil versions and users of it. Personally, I extract it by a bazel query (provided in comments) and a Python script to lookup the symbols in the ar files. I believe the gRPC builds do something similar, or just pull them out from the compiler invocations (does @pitrou or @kou know how they were determined in commits like [these](https://github.com/apache/arrow/commit/4ecac056dfa88a0032fed7f36a20d2886c39ef90)?). Ideally the compiler would do it, but I didn't dig into why the build system wouldn't allow so (I'm guessing the libs have to bundled in and linked separately somehow?).
I'm hoping to upgrade the dependency when there is code using it (which would be the GCS implementation of the Arrow file interfaces).
----------------------------------------------------------------
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] salqadri commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
salqadri commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-890187215
What's the status of 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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] kou commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
kou commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-890584090
Could you rebase on the master? Then I'll review 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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-903460946
> @josiahyan do you think you will have time to rebase? Otherwise I can see how difficult this might be.
Whoops sorry, I lost track of this. I’ll be trying to rebase this - I don’t see any major changes (so far) that should result in conflicts.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-733053945
https://issues.apache.org/jira/browse/ARROW-8147
----------------------------------------------------------------
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 #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r716892201
##########
File path: ci/conda_env_unix.txt
##########
@@ -19,5 +19,7 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
+nlohmann_json>=3.4.0
Review comment:
Conda has dependency handling, so there is no reason to mention it explicitly, though.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
emkornfield commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r706924551
##########
File path: ci/conda_env_unix.txt
##########
@@ -19,5 +19,7 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
+nlohmann_json>=3.4.0
Review comment:
I guess google's SDK must rely on 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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] josiahyan closed pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan closed pull request #8757:
URL: https://github.com/apache/arrow/pull/8757
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-929407709
I’ve discussed this PR with @coryan , and we think its better to start over, given the need to upgrade dependencies, and handle the Abseil libraries (which I believe, when used with gRPC are causing the build to fail on some platforms) with more scripting. He will be taking over this work.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] josiahyan edited a comment on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan edited a comment on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-733405694
@xhochy I can't seem to summon the bot. But I read what the action did, and saw that it required a very specific version of the cmake autoformatter, as you suggested. It works now! Thanks for your help!
----------------------------------------------------------------
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 #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r533241989
##########
File path: ci/docker/conda-cpp.dockerfile
##########
@@ -73,6 +73,7 @@ ENV ARROW_BUILD_TESTS=ON \
ARROW_PARQUET=ON \
ARROW_PLASMA=ON \
ARROW_S3=ON \
+ ARROW_GCS=ON \
Review comment:
Can we keep alphebetical order? Same for other changes below.
----------------------------------------------------------------
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] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r584312257
##########
File path: ci/conda_env_unix.yml
##########
@@ -19,6 +19,8 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
Review comment:
The currently available version in conda (1.24) is incompatible with the way I initially packaged it. I believe I saw in the CI run that `storage_client` was missing or something, as the google-cloud-cpp package has [started namespacing its exports](https://github.com/googleapis/google-cloud-cpp/blob/5bf147226e9333c5106d5fc8b8bba62657eeab97/google/cloud/storage/legacy.cmake.in#L28) (finally!).
Given the hard link against abseil libraries (as gRPC does too), to reduce packaging size, I decided that the version was safer pinned. The abseil libraries actually loaded probably depend on the matrix of abseil versions and users of it (gRPC, and the proposed google-cloud-cpp inclusion), making it more brittle. Personally, I [extract it by a bazel query](https://github.com/apache/arrow/blob/7076b4c75c6981dac7b9e11e5661260cb7ddedf5/cpp/cmake_modules/ThirdpartyToolchain.cmake#L2397) (provided in comments) and a Python script to lookup the symbols in the ar files. I believe the gRPC builds do something similar, or just pull them out from the compiler invocations (does @pitrou or @kou know how they were determined in commits like [these](https://github.com/apache/arrow/commit/4ecac056dfa88a0032fed7f36a20d2886c39ef90)?). Ideally the compiler would do the dead section elimination, but I didn't dig into why the build system wouldn't allow so (I'm guessing the libs have to be bundled in
and linked separately somehow?).
I'm hoping to upgrade the dependency when there is code using it (which would be the GCS implementation of the Arrow file interfaces).
----------------------------------------------------------------
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] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r584312257
##########
File path: ci/conda_env_unix.yml
##########
@@ -19,6 +19,8 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
Review comment:
The currently available version in conda (1.24) is incompatible with the way I initially packaged it. I believe I saw in the CI run that `storage_client` was missing or something, as the google-cloud-cpp package has [started namespacing its exports](https://github.com/googleapis/google-cloud-cpp/blob/5bf147226e9333c5106d5fc8b8bba62657eeab97/google/cloud/storage/legacy.cmake.in#L28) (finally!).
Given the hard link against abseil libraries (as gRPC does too), I decided that the version was safer pinned.
I'm hoping to upgrade the dependency when there is code using it (which would be the GCS implementation of the Arrow file interfaces).
----------------------------------------------------------------
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] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-733398391
@github-actions autotune
----------------------------------------------------------------
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] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-733327947
I'll take a look! Didn't know I could do 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] nealrichardson commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r533651013
##########
File path: cpp/thirdparty/versions.txt
##########
@@ -32,14 +32,17 @@ ARROW_BOOST_BUILD_VERSION=1.71.0
ARROW_BROTLI_BUILD_VERSION=v1.0.7
ARROW_BZIP2_BUILD_VERSION=1.0.8
ARROW_CARES_BUILD_VERSION=1.16.1
+ARROW_CRC32C_BUILD_VERSION=1.0.6
ARROW_GBENCHMARK_BUILD_VERSION=v1.5.2
+ARROW_GCPSDK_BUILD_VERSION=v1.20.0
ARROW_GFLAGS_BUILD_VERSION=v2.2.2
ARROW_GLOG_BUILD_VERSION=v0.4.0
ARROW_GRPC_BUILD_VERSION=v1.29.1
ARROW_GTEST_BUILD_VERSION=1.8.1
ARROW_JEMALLOC_BUILD_VERSION=5.2.1
ARROW_LZ4_BUILD_VERSION=v1.9.2
ARROW_MIMALLOC_BUILD_VERSION=v1.6.4
+ARROW_NLOHMANN_JSON_BUILD_VERSION=v3.9.0
Review comment:
Should URLs for these new dependencies also be added to `DEPENDENCIES` below? cf. ARROW-6407 ARROW-6333
----------------------------------------------------------------
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] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-769948829
Whoops no problem; I’ll rebase it tonight or over the weekend. Thanks for
picking this up again!
On Fri, Jan 29, 2021 at 12:30 PM Neal Richardson <no...@github.com>
wrote:
> Hi @josiahyan <https://github.com/josiahyan>, apologies for the delay
> here. Could you please rebase this? Hopefully we can get this reviewed and
> merged soon after.
>
> —
> You are receiving this because you were mentioned.
>
>
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/arrow/pull/8757#issuecomment-769942171>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACPH2DGRRPB7VG6QSWOYL4DS4LWCVANCNFSM4UA73NKQ>
> .
>
----------------------------------------------------------------
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] josiahyan edited a comment on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan edited a comment on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-733751240
Oh, I know why! The CI queue was full (my test runs were taking hours to go through). By the time the runner got to my PR (Wed, 25 Nov 2020 02:15:47 GMT), my final commit with the manual cmake autoformat had gone in about an hour before.
----------------------------------------------------------------
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] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r533728016
##########
File path: cpp/thirdparty/versions.txt
##########
@@ -32,14 +32,17 @@ ARROW_BOOST_BUILD_VERSION=1.71.0
ARROW_BROTLI_BUILD_VERSION=v1.0.7
ARROW_BZIP2_BUILD_VERSION=1.0.8
ARROW_CARES_BUILD_VERSION=1.16.1
+ARROW_CRC32C_BUILD_VERSION=1.0.6
ARROW_GBENCHMARK_BUILD_VERSION=v1.5.2
+ARROW_GCPSDK_BUILD_VERSION=v1.20.0
ARROW_GFLAGS_BUILD_VERSION=v2.2.2
ARROW_GLOG_BUILD_VERSION=v0.4.0
ARROW_GRPC_BUILD_VERSION=v1.29.1
ARROW_GTEST_BUILD_VERSION=1.8.1
ARROW_JEMALLOC_BUILD_VERSION=5.2.1
ARROW_LZ4_BUILD_VERSION=v1.9.2
ARROW_MIMALLOC_BUILD_VERSION=v1.6.4
+ARROW_NLOHMANN_JSON_BUILD_VERSION=v3.9.0
Review comment:
Whoops, thanks! I missed that. I've added the URLs for the dependencies I've inserted into `versions.txt`.
However, I've left out the backup URLs that use the ursa-labs archive in `ThirdPartyDependencies.cmake`, which I understand will be derived from the `versions.txt` URL eventually (if this gets merged). Let me know if its appropriate to link add these URLs that don't currently exist.
----------------------------------------------------------------
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 #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
kou commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r584358894
##########
File path: ci/conda_env_unix.yml
##########
@@ -19,6 +19,8 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
Review comment:
I determined required abseil libraries manually. CI error helped me.
----------------------------------------------------------------
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] xhochy commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
xhochy commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r584248195
##########
File path: ci/conda_env_unix.yml
##########
@@ -19,6 +19,8 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
Review comment:
What is the reason for the exact pin here?
----------------------------------------------------------------
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] nealrichardson commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r533838250
##########
File path: cpp/thirdparty/versions.txt
##########
@@ -32,14 +32,17 @@ ARROW_BOOST_BUILD_VERSION=1.71.0
ARROW_BROTLI_BUILD_VERSION=v1.0.7
ARROW_BZIP2_BUILD_VERSION=1.0.8
ARROW_CARES_BUILD_VERSION=1.16.1
+ARROW_CRC32C_BUILD_VERSION=1.0.6
ARROW_GBENCHMARK_BUILD_VERSION=v1.5.2
+ARROW_GCPSDK_BUILD_VERSION=v1.20.0
ARROW_GFLAGS_BUILD_VERSION=v2.2.2
ARROW_GLOG_BUILD_VERSION=v0.4.0
ARROW_GRPC_BUILD_VERSION=v1.29.1
ARROW_GTEST_BUILD_VERSION=1.8.1
ARROW_JEMALLOC_BUILD_VERSION=5.2.1
ARROW_LZ4_BUILD_VERSION=v1.9.2
ARROW_MIMALLOC_BUILD_VERSION=v1.6.4
+ARROW_NLOHMANN_JSON_BUILD_VERSION=v3.9.0
Review comment:
No, it's fine, don't add URLs that don't exist
----------------------------------------------------------------
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] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r584312257
##########
File path: ci/conda_env_unix.yml
##########
@@ -19,6 +19,8 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
Review comment:
The currently available version in conda (1.24) is incompatible with the way I initially packaged it. I believe I saw in the CI run that `storage_client` was missing or something, as the google-cloud-cpp package has [started namespacing its exports](https://github.com/googleapis/google-cloud-cpp/blob/5bf147226e9333c5106d5fc8b8bba62657eeab97/google/cloud/storage/legacy.cmake.in#L28) (finally!).
Given the hard link against abseil libraries (as gRPC does too), to reduce packaging size, I decided that the version was safer pinned. The abseil libraries actually loaded probably depends on the matrix of abseil versions and users of it (gRPC, and the proposed google-cloud-cpp inclusion). Personally, I extract it by a bazel query (provided in comments) and a Python script to lookup the symbols in the ar files. I believe the gRPC builds do something similar, or just pull them out from the compiler invocations (does @pitrou or @kou know how they were determined in commits like [these](https://github.com/apache/arrow/commit/4ecac056dfa88a0032fed7f36a20d2886c39ef90)?). Ideally the compiler would do it, but I didn't dig into why the build system wouldn't allow so (I'm guessing the libs have to bundled in and linked separately somehow?).
I'm hoping to upgrade the dependency when there is code using it (which would be the GCS implementation of the Arrow file interfaces).
----------------------------------------------------------------
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] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-736638562
@github-actions rebase
----------------------------------------------------------------
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] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r533728016
##########
File path: cpp/thirdparty/versions.txt
##########
@@ -32,14 +32,17 @@ ARROW_BOOST_BUILD_VERSION=1.71.0
ARROW_BROTLI_BUILD_VERSION=v1.0.7
ARROW_BZIP2_BUILD_VERSION=1.0.8
ARROW_CARES_BUILD_VERSION=1.16.1
+ARROW_CRC32C_BUILD_VERSION=1.0.6
ARROW_GBENCHMARK_BUILD_VERSION=v1.5.2
+ARROW_GCPSDK_BUILD_VERSION=v1.20.0
ARROW_GFLAGS_BUILD_VERSION=v2.2.2
ARROW_GLOG_BUILD_VERSION=v0.4.0
ARROW_GRPC_BUILD_VERSION=v1.29.1
ARROW_GTEST_BUILD_VERSION=1.8.1
ARROW_JEMALLOC_BUILD_VERSION=5.2.1
ARROW_LZ4_BUILD_VERSION=v1.9.2
ARROW_MIMALLOC_BUILD_VERSION=v1.6.4
+ARROW_NLOHMANN_JSON_BUILD_VERSION=v3.9.0
Review comment:
Whoops, thanks! I missed that. I've added the URLs for the dependencies I've inserted into `versions.txt`.
However, I've left out the backup URLs that use the ursa-labs archive in `ThirdPartyDependencies.cmake`, which I understand will be derived from the `versions.txt` URL eventually (if this gets merged). Let me know if its appropriate to link add these URLs.
----------------------------------------------------------------
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] nealrichardson commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-890512290
> > What's the status of this PR?
>
> Sorry, but I lost track of following up on this PR! I believed it was ready to merge in Feb, but assumed people had lost interest in pursuing (native) GCS support, or that this change was not quite worth making for now. Is there still a need for this? If so, I can attempt to rebase it again.
I think there's interest (I'm interested, at least). I'd like to see someone more skilled in cmake than I to review and approve: @kou @pitrou @xhochy ?
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-733035841
Running the cmake autoformatter heavily modifies the touched cmake files; should I be doing 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] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-733751240
Oh, I know why! The CI queue was full (my test runs were taking hours to go through). By the time the runner got to my PR (Wed, 25 Nov 2020 02:15:47 GMT), my final commit had gone in.
----------------------------------------------------------------
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] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r584312257
##########
File path: ci/conda_env_unix.yml
##########
@@ -19,6 +19,8 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
Review comment:
The currently available version in conda (1.24) is incompatible with the way I initially packaged it. I believe I saw in the CI run that `storage_client` was missing or something, as the google-cloud-cpp package has [started namespacing its exports](https://github.com/googleapis/google-cloud-cpp/blob/5bf147226e9333c5106d5fc8b8bba62657eeab97/google/cloud/storage/legacy.cmake.in#L28) (finally!).
Given the hard link against abseil libraries (as gRPC does too), to reduce packaging size, I decided that the version was safer pinned. The abseil libraries actually loaded probably depend on the matrix of abseil versions and users of it (gRPC, and the proposed google-cloud-cpp inclusion), making it more brittle. Personally, I [extract it by a bazel query](https://github.com/apache/arrow/blob/7076b4c75c6981dac7b9e11e5661260cb7ddedf5/cpp/cmake_modules/ThirdpartyToolchain.cmake#L2397) (provided in comments) and a Python script to lookup the symbols in the ar files. I believe the gRPC builds do something similar, or just pull them out from the compiler invocations (does @pitrou or @kou know how they were determined in commits like [these](https://github.com/apache/arrow/commit/4ecac056dfa88a0032fed7f36a20d2886c39ef90)?). Ideally the linker would do the dead section elimination, but I didn't dig into why the build system wouldn't allow so (I'm guessing the libs have to be bundled in a
nd linked separately somehow?).
I'm hoping to upgrade the dependency when there is code using it (which would be the GCS implementation of the Arrow file interfaces).
----------------------------------------------------------------
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] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r584312257
##########
File path: ci/conda_env_unix.yml
##########
@@ -19,6 +19,8 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
Review comment:
The currently available version in conda (1.24) is incompatible with the way I initially packaged it. I believe I saw in the CI run that `storage_client` was missing or something, as the google-cloud-cpp package has [started namespacing its exports](https://github.com/googleapis/google-cloud-cpp/blob/5bf147226e9333c5106d5fc8b8bba62657eeab97/google/cloud/storage/legacy.cmake.in#L28) (finally!).
Given the hard link against abseil libraries (as gRPC does too), to reduce packaging size, I decided that the version was safer pinned. The abseil libraries actually loaded probably depends on the matrix of abseil versions and users of it (gRPC, and the proposed google-cloud-cpp inclusion). Personally, I [extract it by a bazel query](https://github.com/apache/arrow/blob/7076b4c75c6981dac7b9e11e5661260cb7ddedf5/cpp/cmake_modules/ThirdpartyToolchain.cmake#L2397) (provided in comments) and a Python script to lookup the symbols in the ar files. I believe the gRPC builds do something similar, or just pull them out from the compiler invocations (does @pitrou or @kou know how they were determined in commits like [these](https://github.com/apache/arrow/commit/4ecac056dfa88a0032fed7f36a20d2886c39ef90)?). Ideally the compiler would do it, but I didn't dig into why the build system wouldn't allow so (I'm guessing the libs have to bundled in and linked separately somehow?).
I'm hoping to upgrade the dependency when there is code using it (which would be the GCS implementation of the Arrow file interfaces).
----------------------------------------------------------------
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] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-787375395
Sorry about the delay! I've rebased the commits and manually merged in the changes made in master.
----------------------------------------------------------------
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] josiahyan closed pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan closed pull request #8757:
URL: https://github.com/apache/arrow/pull/8757
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-769942171
Hi @josiahyan, apologies for the delay here. Could you please rebase this? Hopefully we can get this reviewed and merged soon after.
----------------------------------------------------------------
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] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r584312257
##########
File path: ci/conda_env_unix.yml
##########
@@ -19,6 +19,8 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
Review comment:
The currently available version in conda (1.24) is incompatible with the way I initially packaged it. I believe I saw in the CI run that `storage_client` was missing or something, as the google-cloud-cpp package has [started namespacing its exports](https://github.com/googleapis/google-cloud-cpp/blob/5bf147226e9333c5106d5fc8b8bba62657eeab97/google/cloud/storage/legacy.cmake.in#L28) (finally!).
Given the hard link against abseil libraries (as gRPC does too), to reduce packaging size, I decided that the version was safer pinned. The abseil libraries actually loaded probably depend on the matrix of abseil versions and users of it (gRPC, and the proposed google-cloud-cpp inclusion), making it more brittle. Personally, I [extract it by a bazel query](https://github.com/apache/arrow/blob/7076b4c75c6981dac7b9e11e5661260cb7ddedf5/cpp/cmake_modules/ThirdpartyToolchain.cmake#L2397) (provided in comments) and a Python script to lookup the symbols in the ar files. I believe the gRPC builds do something similar, or just pull them out from the compiler invocations (does @pitrou or @kou know how they were determined in commits like [these](https://github.com/apache/arrow/commit/4ecac056dfa88a0032fed7f36a20d2886c39ef90)?). Ideally the compiler would do the dead section elimination, but I didn't dig into why the build system wouldn't allow so (I'm guessing the libs have to bundled in an
d linked separately somehow?).
I'm hoping to upgrade the dependency when there is code using it (which would be the GCS implementation of the Arrow file interfaces).
----------------------------------------------------------------
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] josiahyan commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-890584178
Will rebase!
On Sun, Aug 1, 2021 at 1:46 PM Sutou Kouhei ***@***.***>
wrote:
> Could you rebase on the master? Then I'll review this.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/arrow/pull/8757#issuecomment-890584090>, or
> unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ACPH2DGIQYWZJBQFOK4RMNLT2WXCDANCNFSM4UA73NKQ>
> .
>
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r533463385
##########
File path: ci/docker/conda-cpp.dockerfile
##########
@@ -73,6 +73,7 @@ ENV ARROW_BUILD_TESTS=ON \
ARROW_PARQUET=ON \
ARROW_PLASMA=ON \
ARROW_S3=ON \
+ ARROW_GCS=ON \
Review comment:
Thanks for reviewing! I've reordered them.
----------------------------------------------------------------
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] josiahyan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
josiahyan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r584312257
##########
File path: ci/conda_env_unix.yml
##########
@@ -19,6 +19,8 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
Review comment:
The currently available version in conda (1.24) is incompatible with the way I initially packaged it. I believe I saw in the CI run that `storage_client` was missing or something, as the google-cloud-cpp package has [started namespacing its exports](https://github.com/googleapis/google-cloud-cpp/blob/5bf147226e9333c5106d5fc8b8bba62657eeab97/google/cloud/storage/legacy.cmake.in#L28) (finally!).
Given the hard link against abseil libraries (as gRPC does too), to reduce packaging size, I decided that the version was safer pinned. The abseil libraries actually loaded probably depends on the matrix of abseil versions and users of it (gRPC, and the proposed google-cloud-cpp inclusion). Personally, I [extract it by a bazel query](https://github.com/apache/arrow/blob/7076b4c75c6981dac7b9e11e5661260cb7ddedf5/cpp/cmake_modules/ThirdpartyToolchain.cmake#L2397) (provided in comments) and a Python script to lookup the symbols in the ar files. I believe the gRPC builds do something similar, or just pull them out from the compiler invocations (does @pitrou or @kou know how they were determined in commits like [these](https://github.com/apache/arrow/commit/4ecac056dfa88a0032fed7f36a20d2886c39ef90)?). Ideally the compiler would do the dead code elimination, but I didn't dig into why the build system wouldn't allow so (I'm guessing the libs have to bundled in and linked separately someho
w?).
I'm hoping to upgrade the dependency when there is code using it (which would be the GCS implementation of the Arrow file interfaces).
----------------------------------------------------------------
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] coryan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
coryan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r716888140
##########
File path: ci/conda_env_unix.txt
##########
@@ -19,5 +19,7 @@
autoconf
ccache
+google-cloud-cpp=1.20.0
+nlohmann_json>=3.4.0
Review comment:
I am one of the authors of google's GCS SDK. Yes, the SDK definitely depends on `nlohmann_json`.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] coryan commented on a change in pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
coryan commented on a change in pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#discussion_r716885521
##########
File path: cpp/cmake_modules/ThirdpartyToolchain.cmake
##########
@@ -2940,6 +3013,190 @@ if(ARROW_S3)
endif()
endif()
+# ----------------------------------------------------------------------
+# GCS and dependencies
+
+macro(build_google_cloud_cpp)
+ message(STATUS "Building GCPSDK from source")
+ message(STATUS "Only building storage_client component of google-cloud-cpp GCP SDK")
+
+ build_absl_once()
+
+ # List of dependencies taken from https://github.com/googleapis/google-cloud-cpp/blob/master/doc/packaging.md
+ # Base the arguments to external projects on google-cloud-cpp/super/external/*.cmake files
+ # Also check external packages list with `bazel query 'deps(//google/cloud/storage:storage_client)'`
+
+ # Build crc32c
+ set(CRC32C_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/crc32c_ep-install")
+ set(CRC32C_CMAKE_ARGS
+ -DCRC32C_BUILD_TESTS=OFF
+ -DCRC32C_BUILD_BENCHMARKS=OFF
+ -DCRC32C_USE_GLOG=OFF
+ -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
+ -DCMAKE_CXX_STANDARD=11
+ "-DCMAKE_CXX_FLAGS=${EP_CXX_FLAGS}"
+ -DCMAKE_INSTALL_LIBDIR=lib
+ "-DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>")
+ set(CRC32C_BUILD_BYPRODUCTS)
+
+ set(_CRC32C_STATIC_LIBRARY
+ "${CRC32C_PREFIX}/lib/${CMAKE_STATIC_LIBRARY_PREFIX}crc32c${CMAKE_STATIC_LIBRARY_SUFFIX}"
+ )
+ set(CRC32C_BUILD_BYPRODUCTS ${_CRC32C_STATIC_LIBRARY})
+ set(CRC32C_LIBRARIES crc32c)
+
+ externalproject_add(crc32c_ep
+ ${EP_LOG_OPTIONS}
+ INSTALL_DIR ${CRC32C_PREFIX}
+ URL ${CRC32C_SOURCE_URL}
+ CMAKE_ARGS ${CRC32C_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${CRC32C_BUILD_BYPRODUCTS})
+ add_library(Crc32c::crc32c STATIC IMPORTED)
+ set_target_properties(Crc32c::crc32c
+ PROPERTIES IMPORTED_LOCATION ${_CRC32C_STATIC_LIBRARY}
+ INTERFACE_INCLUDE_DIRECTORIES
+ "${CRC32C_INCLUDE}/include")
+ add_dependencies(Crc32c::crc32c crc32c_ep)
+
+ # "Build" nlohmann-json
+ set(NLOHMANN_JSON_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/nlohmann_json_ep-install")
+ set(NLOHMANN_JSON_CMAKE_ARGS
+ -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
+ -DCMAKE_CXX_STANDARD=11
+ "-DCMAKE_CXX_FLAGS=${EP_CXX_FLAGS}"
+ -DBUILD_TESTING=OFF
+ "-DCMAKE_INSTALL_PREFIX=<INSTALL_DIR>")
+
+ set(NLOHMANN_JSON_BUILD_BYPRODUCTS ${NLOHMANN_JSON_PREFIX}/include/json.hpp)
+
+ externalproject_add(nlohmann_json_ep
+ ${EP_LOG_OPTIONS}
+ INSTALL_DIR ${NLOHMANN_JSON_PREFIX}
+ URL ${NLOHMANN_JSON_SOURCE_URL}
+ CMAKE_ARGS ${NLOHMANN_JSON_CMAKE_ARGS}
+ BUILD_BYPRODUCTS ${NLOHMANN_JSON_BUILD_BYPRODUCTS})
+ add_library(nlohmann_json::nlohmann_json INTERFACE IMPORTED)
+ set_target_properties(nlohmann_json::nlohmann_json
+ PROPERTIES INTERFACE_INCLUDE_DIRECTORIES
+ "${NLOHMANN_JSON_PREFIX}/include")
+ add_dependencies(nlohmann_json::nlohmann_json nlohmann_json_ep)
+
+ # Curl is required on all platforms, but building it internally might also trip over S3's copy.
+ # For now, force its inclusion from the underlying system or fail.
+ # TODO: make curl a proper (bundled) third party dependency
+ # Also, curl has a dependency on zlib; see this with `bazel query 'allpaths(//google/cloud/storage:storage_client, @zlib//:zlib)'`
+ find_package(CURL REQUIRED 7.47.0)
+
+ # Build google-cloud-cpp, with only storage_client
+
+ # Inject vendored packages via CMAKE_PREFIX_PATH
+ list(APPEND GCPSDK_PREFIX_PATH_LIST ${ABSL_PREFIX})
+ list(APPEND GCPSDK_PREFIX_PATH_LIST ${CRC32C_PREFIX})
+ list(APPEND GCPSDK_PREFIX_PATH_LIST ${NLOHMANN_JSON_PREFIX})
+ if(CARES_VENDORED)
+ list(APPEND GCPSDK_PREFIX_PATH_LIST ${CARES_PREFIX})
+ endif()
+
+ set(GCPSDK_PREFIX_PATH_LIST_SEP_CHAR "|")
+ list(JOIN GCPSDK_PREFIX_PATH_LIST ${GCPSDK_PREFIX_PATH_LIST_SEP_CHAR}
+ GCPSDK_PREFIX_PATH)
+
+ set(GCPSDK_CXX_FLAGS "${EP_CXX_FLAGS}")
+ # workaround for crc32c not declaring its header interface in its Find*.cmake file
Review comment:
You can avoid this by using crc32c 1.1.1:
https://github.com/google/crc32c/commit/b377ce46f56823cf38dd7045f4997dd4b5c6d04c
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] emkornfield commented on pull request #8757: ARROW-8147: [C++] Add google-cloud-cpp to ThirdpartyToolchain
Posted by GitBox <gi...@apache.org>.
emkornfield commented on pull request #8757:
URL: https://github.com/apache/arrow/pull/8757#issuecomment-903010637
@josiahyan do you think you will have time to rebase? Otherwise I can see how difficult this might be.
--
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.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org