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/15 04:11:17 UTC
[GitHub] [arrow] ilijapuaca opened a new pull request #7189: ARROW-8795: [C++] Limited iOS support
ilijapuaca opened a new pull request #7189:
URL: https://github.com/apache/arrow/pull/7189
Per suggestion from [JIRA](https://issues.apache.org/jira/browse/ARROW-8795), opening up a pull request to potentially get the ball rolling on having iOS as one of the supported platforms.
Per writeup in [this doc](https://github.com/UnfoldedInc/deck.gl-native-dependencies/blob/master/docs/iOS-BUILD.md#arrow-v0170), there are a few issues with cmake's XCode generator that require a few workarounds even when using their most recent versions.
The writeup also describes issues that we've encountered building the library, as well as issues that were not resolved as they use optional features we are currently not making use of.
----------------------------------------------------------------
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 pull request #7189: ARROW-8795: [C++] Limited iOS support
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #7189:
URL: https://github.com/apache/arrow/pull/7189#issuecomment-638981861
Thank you @ilijapuaca !
----------------------------------------------------------------
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] ilijapuaca commented on pull request #7189: ARROW-8795: [C++] Limited iOS support
Posted by GitBox <gi...@apache.org>.
ilijapuaca commented on pull request #7189:
URL: https://github.com/apache/arrow/pull/7189#issuecomment-638546575
> @ilijapuaca Ah, sorry for that. You need a specific `cmake-format` version:
>
> ```
> pip install cmake-format==0.5.2
> ```
>
> should do the trick.
@pitrou Yep, that worked much better. Pushed the formatting update
----------------------------------------------------------------
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 #7189: ARROW-8795: [C++] Limited iOS support
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7189:
URL: https://github.com/apache/arrow/pull/7189#issuecomment-629016109
https://issues.apache.org/jira/browse/ARROW-8795
----------------------------------------------------------------
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 closed pull request #7189: ARROW-8795: [C++] Limited iOS support
Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #7189:
URL: https://github.com/apache/arrow/pull/7189
----------------------------------------------------------------
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] ilijapuaca commented on pull request #7189: ARROW-8795: [C++] Limited iOS support
Posted by GitBox <gi...@apache.org>.
ilijapuaca commented on pull request #7189:
URL: https://github.com/apache/arrow/pull/7189#issuecomment-638261371
@pitrou this now seems to build on all the platforms. Tried running `run-cmake-format.py` to fix the lint issue, but for some reason it produces a much different different format across all the cmake files on my machine, which I assume are not formatted correctly.
Any idea why that might be the case? If this seems mergeable now, perhaps someone else can fix the linter issue if this isn't a known issue that I can easily solve on my machine
----------------------------------------------------------------
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] ilijapuaca commented on a change in pull request #7189: ARROW-8795: [C++] Limited iOS support
Posted by GitBox <gi...@apache.org>.
ilijapuaca commented on a change in pull request #7189:
URL: https://github.com/apache/arrow/pull/7189#discussion_r433950238
##########
File path: cpp/src/arrow/CMakeLists.txt
##########
@@ -189,6 +189,7 @@ set(ARROW_SRCS
util/utf8.cc
util/value_parsing.cc
vendored/base64.cpp
+ vendored/datetime/ios.mm
Review comment:
I wasn't sure about usage of these files in general -- `ios.h` is included from `tz.cpp` under `#ifdef __APPLE__`, which implies it's being used on macOS as well?
I could make this file iOS-specific, but perhaps some additional updates should be made in order to consolidate `datetime` module and its usage. If that is the case, I'd need some pointers as I'm not familiar with this portion of the codebase and its usage
----------------------------------------------------------------
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 #7189: ARROW-8795: [C++] Limited iOS support
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #7189:
URL: https://github.com/apache/arrow/pull/7189#discussion_r433820499
##########
File path: cpp/src/arrow/CMakeLists.txt
##########
@@ -189,6 +189,7 @@ set(ARROW_SRCS
util/utf8.cc
util/value_parsing.cc
vendored/base64.cpp
+ vendored/datetime/ios.mm
Review comment:
Shouldn't this file only be compiled on iOS targets? At least it seems it doesn't compile properly on our CI builds.
##########
File path: cpp/CMakeLists.txt
##########
@@ -340,6 +340,11 @@ if(ARROW_ORC)
set(ARROW_WITH_ZSTD ON)
endif()
+# datetime code used by iOS requries zlib support
Review comment:
Typo: "requires"
----------------------------------------------------------------
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 #7189: ARROW-8795: [C++] Limited iOS support
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #7189:
URL: https://github.com/apache/arrow/pull/7189#discussion_r433966628
##########
File path: cpp/src/arrow/CMakeLists.txt
##########
@@ -189,6 +189,7 @@ set(ARROW_SRCS
util/utf8.cc
util/value_parsing.cc
vendored/base64.cpp
+ vendored/datetime/ios.mm
Review comment:
Hmm, then at least make this Apple-specific. `ios.mm` needs an Objective-C++ compiler, which won't be found on all platforms.
----------------------------------------------------------------
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 pull request #7189: ARROW-8795: [C++] Limited iOS support
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #7189:
URL: https://github.com/apache/arrow/pull/7189#issuecomment-638411782
@ilijapuaca Ah, sorry for that. You need a specific `cmake-format` version:
```
pip install cmake-format==0.5.2
```
should do the trick.
----------------------------------------------------------------
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