ARROW-10541: [C++] Add re2 library to core arrow / ARROW_WITH_RE2 #8756
Conversation
|
@github-actions autotune everything |
| define_option(ARROW_WITH_UTF8PROC | ||
| "Build with support for Unicode properties using the utf8proc library" ON) | ||
| define_option(ARROW_WITH_RE2 | ||
| "Build with support for regular expressions using the re2 library" ON) |
There was a problem hiding this comment.
I find this confusing. I understand how it works--if ARROW_COMPUTE is on, these are on too unless you set them to OFF--but it looks like they're always built by default, unlike the compression libs listed above.
Maybe it's irrelevant that it looks confusing here. Or maybe at least we should add a comment here explaining how the default is really ON iff ARROW_COMPUTE is ON too. Or is it possible to define the default option here to be literally whatever ARROW_COMPUTE is, like define_option(ARROW_WITH_RE2 "..." ARROW_COMPUTE)?
There was a problem hiding this comment.
This is sadly the prettiest solution I can think of at the moment. You can define conditional options in CMake but they only make sense based on attributes that are not set by the user but the environment (e.g. which operating system).
I'll add a comment to this and the above option explaining that they are only used in certain situations.
There was a problem hiding this comment.
Instead of a comment could that explanation be a parenthesis of the description for this option?
|
Everything failing in #8759 is past the CMake additions I did here, so this should be good to go. |
|
#8459 builds on Travis, looking good 👍 |
|
|
||
| if(ARROW_GANDIVA) | ||
| set(ARROW_WITH_RE2 ON) | ||
| endif() |
There was a problem hiding this comment.
How about forcing to enable/disable ARROW_WITH_RE2 in ThridpartyToolchain.cmake like ARROW_WITH_THRIFT?
| "Build with support for Unicode properties using the utf8proc library" ON) | ||
| "Build with support for Unicode properties using the utf8proc library (only used if ARROW_COMPUTE is ON)" ON) | ||
| define_option(ARROW_WITH_RE2 | ||
| "Build with support for regular expressions using the re2 library (only used if ARROW_COMPUTE or ARROW_GANDIVA is ON)" ON) |
There was a problem hiding this comment.
We can remove ARROW_WITH_UTF8PROC and ARROW_WITH_RE2 if we set them in ThirdpartyToolchain.cmake internally like ARROW_WITH_THRIFT.
There was a problem hiding this comment.
No, the difference to ARROW_WITH_THRIFT is that you can still build ARROW_COMPUTE even if ARROW_WITH_UTF8PROC=OFF. In that case, only some features of compute won't be build but the majority will work.
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
|
@github-actions autotune |
Because we require RE2 with ARROW_COMPUTE by default. See also: ARROW-10541 and apache#8756
Because we require RE2 with ARROW_COMPUTE by default. See also: ARROW-10541 and #8756 Closes #8786 from kou/cpp-example-static Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Antoine Pitrou <antoine@python.org>
5 participants
No description provided.