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/05/27 09:17:41 UTC
[GitHub] [arrow] liyafan82 opened a new pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
liyafan82 opened a new pull request #7287:
URL: https://github.com/apache/arrow/pull/7287
Some of our test source code requires the process.hpp file (and its dependent libraries). Our current build support does not include these files, causing build failures like:
`fatal error: boost/process.hpp: No such file or directory`
In this PR, we add required header files which are sufficient to make flight build successful.
The added header files are verified in my local machine. However, it seems the script is supposed to run from the server side, so we cannot 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] nealrichardson edited a comment on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
nealrichardson edited a comment on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-682000384
> > Is there a JIRA open to upgrade to thrift 0.13.0?
>
> @emkornfield AFAIK, there is no such a JIRA, and I am not sure if we have a plan to upgrade to thrift 0.13.0 recently.
https://issues.apache.org/jira/browse/ARROW-8049 is the JIRA to upgrade thrift. [It looks like the obstacle we ran into before](https://github.com/apache/arrow/pull/6572) was that it requires cmake >= 3.10, and we only require 3.2. It seems we could apply a simple patch to resolve that. So it's doable, if it's worth our time to bump the version.
----------------------------------------------------------------
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 #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-636384923
Apologies if I'm slow to iterate on this; since this is a test dependency, it doesn't seem urgent, and I've got a few other things I'm juggling at the moment.
----------------------------------------------------------------
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 #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#discussion_r441176530
##########
File path: cpp/build-support/trim-boost.sh
##########
@@ -32,12 +32,12 @@ set -eu
: ${BOOST_URL:=https://dl.bintray.com/boostorg/release/${BOOST_VERSION}/source/${BOOST_FILE}.tar.gz}
# Arrow tests require these
-BOOST_LIBS="system.hpp filesystem.hpp"
+BOOST_LIBS="process.hpp filesystem.hpp"
Review comment:
I wasn't sure if it was safe to remove system.hpp because it is named explicitly in cmake and don't have time to track this down right now, so leaving a note to future self.
```suggestion
# TODO: is system still required? We declare `--with-libraries=filesystem,regex,system`
# in cmake, but there is no #include <boost/system.hpp> in our source anymore
BOOST_LIBS="system.hpp process.hpp filesystem.hpp"
```
----------------------------------------------------------------
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] liyafan82 commented on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-645216956
@nealrichardson
I am sorry we still have missing files: boost/utility/enable_if.hpp.
```
In file included from arrow-cpp-test-debug/thrift_ep-prefix/src/thrift_ep/lib/cpp/src/thrift/TApplicationException.h:23:0,
from arrow-cpp-test-debug/thrift_ep-prefix/src/thrift_ep/lib/cpp/src/thrift/TApplicationException.cpp:20:
arrow-cpp-test-debug/thrift_ep-prefix/src/thrift_ep/lib/cpp/src/thrift/Thrift.h:45:10: fatal error: boost/utility/enable_if.hpp: No such file or directory
#include <boost/utility/enable_if.hpp>
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
```
----------------------------------------------------------------
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] wesm commented on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
wesm commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-652142725
Could this be picked up 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-636384787
I just tweaked trim-boost.sh, will need to test that this does the right thing locally, then I can update bintray and we can try all of the builds.
----------------------------------------------------------------
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 #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-682000384
> > Is there a JIRA open to upgrade to thrift 0.13.0?
>
> @emkornfield AFAIK, there is no such a JIRA, and I am not sure if we have a plan to upgrade to thrift 0.13.0 recently.
https://issues.apache.org/jira/browse/ARROW-8049 is the JIRA to upgrade thrift
----------------------------------------------------------------
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] liyafan82 commented on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-652145927
> Could this be picked up again?
Maybe we can pick it up after upgrading to Thrift 0.13.
----------------------------------------------------------------
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] liyafan82 commented on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-636589577
> Apologies if I'm slow to iterate on this; since this is a test dependency, it doesn't seem urgent, and I've got a few other things I'm juggling at the moment.
@nealrichardson Thanks a lot for your effort, and please take your time
----------------------------------------------------------------
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] wesm commented on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
wesm commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-739990180
Closing for new since it doesn't seem there is an action possible right 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] liyafan82 commented on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-645762806
> That's odd. I wonder what's different about your build setup from the jobs we run on CI because I haven't seen that before. Do you think you could add a crossbow job that captures this build setup (is it just bundled boost and thrift, plus ARROW_FLIGHT=ON and tests on?)
>
> FWIW this boost include has been removed in Thrift 0.13: [apache/thrift@1f34504#diff-73a92ed6f7bb65d6e8f29f74ae6c94bf](https://github.com/apache/thrift/commit/1f34504f43a7a409364d4114f180762bf2679e57#diff-73a92ed6f7bb65d6e8f29f74ae6c94bf)
>
> So if/whenever we can [upgrade to 0.13](https://issues.apache.org/jira/browse/ARROW-8049), this particular header won't ever be invoked.
I agree with you that a crossbow job for this build setup is extremely useful, as even if this problem is fixed, there are other problems. I will open an issue to track it.
I was using the following flags:
-DCMAKE_BUILD_TYPE=Debug -DARROW_BUILD_TESTS=ON -DARROW_BUILD_BENCHMARKS=ON -DARROW_FLIGHT=ON -DARROW_DATASET=ON -DBOOST_SOURCE=BUNDLED -DARROW_PARQUET=ON -DARROW_JNI=ON -DARROW_BUILD_EXAMPLES=ON -DCMAKE_CXX_COMPILER=.../gcc-7.2.0/bin/c++ -DCMAKE_C_COMPILER=.../gcc-7.2.0/bin/gcc
----------------------------------------------------------------
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] emkornfield commented on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
emkornfield commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-681346310
Is there a JIRA open to upgrade to thrift 0.13.0?
----------------------------------------------------------------
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] wesm edited a comment on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
wesm edited a comment on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-739990180
Closing for now since it doesn't seem there is an action possible right 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] liyafan82 commented on a change in pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on a change in pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#discussion_r431540394
##########
File path: cpp/build-support/trim-boost.sh
##########
@@ -41,6 +41,8 @@ BOOST_LIBS="$BOOST_LIBS regex.hpp"
BOOST_LIBS="$BOOST_LIBS multiprecision/cpp_int.hpp"
# These are for Thrift when Thrift_SOURCE=BUNDLED
BOOST_LIBS="$BOOST_LIBS algorithm/string.hpp locale.hpp noncopyable.hpp numeric/conversion/cast.hpp scope_exit.hpp scoped_array.hpp shared_array.hpp tokenizer.hpp version.hpp"
+#These are for flight
+BOOST_LIBS="$BOOST_LIBS process.hpp process asio fusion"
Review comment:
Thanks for your comments.
Some other header files are referenced (directly or indirectly) from process.hpp.
I tried to refine sub-directories in process, asio and fusion, but gave up because too many individual individual header files were required.
----------------------------------------------------------------
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 #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-634544583
https://issues.apache.org/jira/browse/ARROW-8771
----------------------------------------------------------------
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] fsaintjacques commented on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
fsaintjacques commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-634777086
@nealrichardson we might need a re-upload.
----------------------------------------------------------------
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 #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-645432605
That's odd. I wonder what's different about your build setup from the jobs we run on CI because I haven't seen that before. Do you think you could add a crossbow job that captures this build setup (is it just bundled boost and thrift, plus ARROW_FLIGHT=ON and tests on?)
FWIW this boost include has been removed in Thrift 0.13: https://github.com/apache/thrift/commit/1f34504f43a7a409364d4114f180762bf2679e57#diff-73a92ed6f7bb65d6e8f29f74ae6c94bf
So if/whenever we can [upgrade to 0.13](https://issues.apache.org/jira/browse/ARROW-8049), this particular header won't ever be invoked.
----------------------------------------------------------------
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 #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-645042129
@liyafan82 I rebuilt the boost bundle and uploaded to bintray. Can you re-run whichever tests you have that failed because of this before and see if they work 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] liyafan82 commented on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-645103962
> @liyafan82 I rebuilt the boost bundle and uploaded to bintray. Can you re-run whichever tests you have that failed because of this before and see if they work now? If they're good now, then we can merge this.
@nealrichardson Thanks for your effort, I will re-run the build today.
----------------------------------------------------------------
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] wesm closed pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
wesm closed pull request #7287:
URL: https://github.com/apache/arrow/pull/7287
----------------------------------------------------------------
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 #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-634786539
Yes, we will. Good test of the process we (hopefully) set up 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] liyafan82 commented on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
liyafan82 commented on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-681623530
> Is there a JIRA open to upgrade to thrift 0.13.0?
@emkornfield AFAIK, there is no such a JIRA, and I am not sure if we have a plan to upgrade to thrift 0.13.0 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] nealrichardson commented on a change in pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#discussion_r431282877
##########
File path: cpp/build-support/trim-boost.sh
##########
@@ -41,6 +41,8 @@ BOOST_LIBS="$BOOST_LIBS regex.hpp"
BOOST_LIBS="$BOOST_LIBS multiprecision/cpp_int.hpp"
# These are for Thrift when Thrift_SOURCE=BUNDLED
BOOST_LIBS="$BOOST_LIBS algorithm/string.hpp locale.hpp noncopyable.hpp numeric/conversion/cast.hpp scope_exit.hpp scoped_array.hpp shared_array.hpp tokenizer.hpp version.hpp"
+#These are for flight
+BOOST_LIBS="$BOOST_LIBS process.hpp process asio fusion"
Review comment:
I thought you only needed process.hpp? Why also `process asio fusion`?
----------------------------------------------------------------
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 edited a comment on pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
nealrichardson edited a comment on pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#issuecomment-645042129
@liyafan82 I rebuilt the boost bundle and uploaded to bintray. Can you re-run whichever tests you have that failed because of this before and see if they work now? If they're good now, then we can merge 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] nealrichardson commented on a change in pull request #7287: ARROW-8771: [C++] Add boost/process library to build support
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #7287:
URL: https://github.com/apache/arrow/pull/7287#discussion_r431923571
##########
File path: cpp/build-support/trim-boost.sh
##########
@@ -41,6 +41,8 @@ BOOST_LIBS="$BOOST_LIBS regex.hpp"
BOOST_LIBS="$BOOST_LIBS multiprecision/cpp_int.hpp"
# These are for Thrift when Thrift_SOURCE=BUNDLED
BOOST_LIBS="$BOOST_LIBS algorithm/string.hpp locale.hpp noncopyable.hpp numeric/conversion/cast.hpp scope_exit.hpp scoped_array.hpp shared_array.hpp tokenizer.hpp version.hpp"
+#These are for flight
+BOOST_LIBS="$BOOST_LIBS process.hpp process asio fusion"
Review comment:
`bcp` should figure that out and pull whatever dependencies are needed into the trimmed boost bundle we build, so if `process.hpp` is all that Flight references, let's just include 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