ARROW-12876: [R] Fix build flags on Raspberry Pi #10404
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
While this is cool (thanks for creating this) it seems to me that it might be more appropriate to put this check into the C++ layer. That's where we express other runtime dependencies (like the compression libs). Then (I think, I'm not an expert on the build process) you don't need to add it here. |
|
@thisisnic Can we build Apache Arrow C++ without additional C++ flag ( |
|
Interesting. I found similar reports (e.g. redis/redis#6275) in other places. Is this only R buid problem? |
|
According to the discussion and logs on ARROW-12860, where this was originally reported, the Arrow C++ (static) library builds but the shared library that R builds with it fails to load due to missing symbols. IIUC, a workaround in the R package is appropriate for now, and the "right" solution is to do ARROW-6312 and in that code include Also somewhat related: ARROW-8041 |
|
It appears that it is essentially a private dependency of libc which is not listed in libc.so. It's very similar to Line 879 in 98f352e @nealrichardson 's solution is mostly correct and would work for R (and other consumers of the Arrow static library) but would fail for anyone consuming Arrow as a shared library on a raspberry pi. So I think, in addition to listing it as a private lib, Arrow will still need to link it when building the shared library. The Arrow shared library would build without |
thisisnic
force-pushed
the
ARROW-12876_raspberry_pi
branch
from
a3e70b8
to
710ff09
Compare
|
@westonpace |
|
@thisisnic Not quite. We don't want to link to libatomic on all non-windows non-apple architectures. Most Linux architectures will have atomics provided by gcc itself. I googled a bit and it seems there are several options.
Sorry for so many options. I just don't really know which one would be the best to choose. |
|
@kou @westonpace any objection to merging the R-only fix for this and ticketing a C++ followup to do it right? Or can one of you push a fix to cmake that does this the way you prefer? |
|
Can we see the build error log without this change? |
The original failure log is on ARROW-12860. |
Sorry, had not done anything on this myself as it's beyond my skillset, and was focussing on other things ahead of the release. If we do merge the R-only fix, we should end up dropping the last commit and merging up to the previous one. |
|
@thisisnic Could you try the following change with #10626 on Raspberry Pi? diff --git a/r/configure b/r/configure
index aa7e7a8d0..899a4a9f8 100755
--- a/r/configure
+++ b/r/configure
@@ -82,9 +82,8 @@ else
pkg-config --version >/dev/null 2>&1
if [ $? -eq 0 ] && [ "$ARROW_USE_PKG_CONFIG" != "false" ]; then
PKGCONFIG_CFLAGS=`pkg-config --cflags --silence-errors ${PKG_CONFIG_NAME}`
- PKGCONFIG_LIBS=`pkg-config --libs-only-l --silence-errors ${PKG_CONFIG_NAME}`
- PKGCONFIG_DIRS=`pkg-config --libs-only-L --silence-errors ${PKG_CONFIG_NAME}`
- # TODO: what about --libs-only-other?
+ PKGCONFIG_LIBS=`pkg-config --libs-only-l --libs-only-other --static --silence-errors ${PKG_CONFIG_NAME}`
+ PKGCONFIG_DIRS=`pkg-config --libs-only-L --static --silence-errors ${PKG_CONFIG_NAME}`
fi
if [ "$PKGCONFIG_CFLAGS" ] && [ "$PKGCONFIG_LIBS" ]; then |
thisisnic
force-pushed
the
ARROW-12876_raspberry_pi
branch
2 times, most recently
from
04a993b
to
1254c20
Compare
eerhardt
reviewed
Jun 30, 2021
| @@ -51,7 +51,7 @@ public static async Task Main(string[] args) | |||
| using (var writer = new ArrowFileWriter(stream, recordBatch.Schema)) | |||
| { | |||
| await writer.WriteRecordBatchAsync(recordBatch); | |||
| await writer.WriteFooterAsync(); | |||
| await writer.WriteEndAsync(); | |||
There was a problem hiding this comment.
This looks like a bad merge. Were you expecting to make a change here? Can this be reverted?
There was a problem hiding this comment.
I was planning to revert this, will mark this PR as a draft to flag this, sorry!
|
I'm super lost trying to work out what to do with this. I've tried installing a version of the R package which has the build flags changed in the case of Raspberry Pi for the R package. I've created a .tar.gz file and the installed it via The lack of successful C++ installation is due to the code in I've tried taking the released version of Arrow 4.0.1 and just updating the I have no idea how I'd add in the change from #10626 given the current setup, unless I suppose I was manually installing the C++ libraries myself and then setting up the R install to look at these. It's been a while since I looked at it, but I'm pretty sure the change I made to add the I'm a bit lost in the above conversation, but it sounds like what is going on here is:
|
I think that is a very good summary. Yesterday I spent a little bit of time trying to setup a docker environment where the -latomic flag is needed but it seems that it is less "what software is present" (which docker can control) and more "what hardware is present" (which docker doesn't control as much) so I wasn't able to produce a "raspberry-pi-like environment" to test in. A friend of mine agreed to mail me a spare Pi that he wasn't using so I can take a look a week or so. |
|
I wiped my Pi and started again just to make sure I hadn't installed things which aren't mentioned in the dev guides which may need adding, etc. I got an install error this time, but I'm a bit unsure as to which bit points to the relevant missing components. Is it re2, or are there other relevant bits in the output here? |
|
We will need to do more work to wire up the changes that kou made, I think. I can help next week. |
|
OK, thanks @nealrichardson , will give you a shout then then |
thisisnic
force-pushed
the
ARROW-12876_raspberry_pi
branch
from
1254c20
to
a3e70b8
Compare
Co-authored-by: Neal Richardson <neal.p.richardson@gmail.com>
Co-authored-by: Neal Richardson <neal.p.richardson@gmail.com>
thisisnic
force-pushed
the
ARROW-12876_raspberry_pi
branch
from
a3e70b8
to
0b09589
Compare
nealrichardson
approved these changes
Jul 26, 2021
This was referenced Aug 19, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checks if installing on Raspberry PI OS, and if so, adds an additional build flag so that the compiler can link to one of the necessary libraries.