Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARROW-8771: [C++] Add boost/process library to build support #7287

Closed
wants to merge 3 commits into from

Conversation

liyafan82
Copy link
Contributor

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.

@github-actions
Copy link

@fsaintjacques
Copy link
Contributor

@nealrichardson we might need a re-upload.

@nealrichardson
Copy link
Member

Yes, we will. Good test of the process we (hopefully) set up before.

cpp/build-support/trim-boost.sh Outdated Show resolved Hide resolved
@nealrichardson
Copy link
Member

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.

@nealrichardson
Copy link
Member

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.

@liyafan82
Copy link
Contributor Author

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

@nealrichardson
Copy link
Member

nealrichardson commented Jun 16, 2020

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

@liyafan82
Copy link
Contributor Author

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

@liyafan82
Copy link
Contributor Author

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

@nealrichardson
Copy link
Member

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

So if/whenever we can upgrade to 0.13, this particular header won't ever be invoked.

@liyafan82
Copy link
Contributor Author

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

So if/whenever we can upgrade to 0.13, 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

@wesm
Copy link
Member

wesm commented Jul 1, 2020

Could this be picked up again?

@liyafan82
Copy link
Contributor Author

Could this be picked up again?

Maybe we can pick it up after upgrading to Thrift 0.13.

@emkornfield
Copy link
Contributor

Is there a JIRA open to upgrade to thrift 0.13.0?

@liyafan82
Copy link
Contributor Author

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.

@nealrichardson
Copy link
Member

nealrichardson commented Aug 27, 2020

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

@wesm
Copy link
Member

wesm commented Dec 7, 2020

Closing for now since it doesn't seem there is an action possible right now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants