-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[R] R package fails to install when using clang-16 as the compiler #33819
Comments
@nealrichardson I can't reproduce using Dockerfile: FROM silkeh/clang:dev-bullseye
RUN apt-get update && apt-get install -y git cmake libxml2-dev libcurl4-openssl-dev libssl-dev
RUN git clone https://github.com/apache/arrow.git /arrow
RUN mkdir /arrow-build && cd /arrow-build && cmake /arrow/cpp -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DARROW_CSV=ON -DARROW_DATASET=ON -DARROW_FILESYSTEM=ON -DARROW_COMPUTE=ON -DARROW_PARQUET=ON -DBoost_SOURCE=BUNDLED && cmake --build . && cmake --install . --prefix /arrow-dist
ENV ARROW_HOME /arrow-dist
CMD /bin/bash So I'm guessing it's either S3 or GCS. |
...and with |
Does that image have boost on it? Have you tried with dependency source BUNDLED? |
That also works; however, when I check out the 10.0.1 release and try to build that, I don't get any errors either. (Again, will try the R package install from source specifically on Monday). |
No errors with the R package (10.0.1) either: FROM silkeh/clang:dev-bullseye
RUN apt-get update && apt-get install -y git r-base cmake libcurl4-openssl-dev libssl-dev
RUN R -e "install.packages(c('lifecycle', 'purrr', 'rlang', 'tidyselect', 'vctrs', 'dplyr'))"
RUN mkdir ~/.R && \
echo 'CXX11=clang++ -std=c++11' > ~/.R/Makevars && \
echo 'CXX14=clang++ -std=c++14' >> ~/.R/Makevars && \
echo 'CXX17=clang++ -std=c++17' >> ~/.R/Makevars && \
echo 'CXX20=clang++ -std=c++20' >> ~/.R/Makevars && \
echo 'CXX=clang++ -std=c++11' >> ~/.R/Makevars && \
echo 'CC=clang' >> ~/.R/Makevars
ENV ARROW_R_DEV true
RUN R -e 'install.packages("arrow"); library(arrow)'
CMD /bin/bash |
Ok, I can email BDR and ask for more details about his setup. Can you find the clang version/commit from which it was built? |
It looks like the image uses https://apt.llvm.org/ , which has a development branch set of packages for Debian. I'm guessing BDR is on Fedora...it would be nice to know how exactly he installed it since it appears that does matter. The image I'm using was built 24 days ago ( https://hub.docker.com/layers/silkeh/clang/dev-bullseye/images/sha256-227546652b0b6d95f8acd5377ec53b32d9f417091f6fbe2190633e0cd30a837a?context=explore ). |
Right I saw that, but I don't know what version was there 24 days ago when the image was built, and that might be useful information if his clang version is different. |
Good call...here's what I get:
Probably best if I verify one more time tonight when I'm back at home (the computer that ran the check is in my basement). |
Also, if you |
I double checked and I have indeed been running the check under the clang from 23 days ago (version above). I got a little worried because the buddy building/uploading that image hasn't uploaded an aarch64 one for a long time and the version of clang on that image is 11 😬
|
A few details I just noticed in the output on https://www.stats.ox.ac.uk/pub/bdr/clang16/arrow.out:
So, (1) we're already using a newer clang build than he is, but (2) we're not testing with R-devel compiled by that clang, for what that's worth. |
Thanks...and there's also that libc++ vs the GCC difference to keep in mind. I can poke away at attempting a Fedora image too but for the purposes of submission (if we get there first) we can probably say that we compiled the package under the latest clang available to us. I updated the docker image and get:
...which is still a week old or so. I get the same result (no build problems). |
Ok, sounds good. Might be a good idea to upgrade our bundled boost anyway, just to avoid a potential I Told You So. |
#33851 for boost |
As now shown on the package check page, the clang build on Fedora now appears broken, too. We have a CI nightly that attempts to replicate the fedora clang build and that is still passing the last I checked. The clang-16 build is also on Fedora, so there's a very good chance the problem is fedora and not dev clang. I will investigate the Fedora issue tomorrow (maybe the version of Fedora got bumped?). |
The error shown is:
Which looks to me like something timed out? I tried a bunch of things around FROM fedora:36
RUN dnf install -y R git llvm clang
RUN mkdir ~/.R && \
echo 'CXX11=clang++ -std=c++11' > ~/.R/Makevars && \
echo 'CXX14=clang++ -std=c++14' >> ~/.R/Makevars && \
echo 'CXX17=clang++ -std=c++17' >> ~/.R/Makevars && \
echo 'CXX20=clang++ -std=c++20' >> ~/.R/Makevars && \
echo 'CXX=clang++ -std=c++11' >> ~/.R/Makevars && \
echo 'CC=clang' >> ~/.R/Makevars
RUN R -e 'Sys.setenv(ARROW_R_DEV="true"); install.packages(c("arrow", "testthat", "dplyr", "stringr", "lubridate", "hms"), repos = "https://cloud.r-project.org/")'
RUN git clone https://github.com/apache/arrow.git /arrow && cd /arrow && git checkout tags/apache-arrow-10.0.1
RUN cd /arrow/r/tests && R -e 'Sys.setenv(ARROW_R_DEV="true"); source("testthat.R")' ...but haven't been able to make anything hang yet. That's not quite right...it's clang-14 and the first time I did it I was missing some packages. Fedora 37 was released recently too and he may have updated without updating the "check environments" page. |
We submitted this to CRAN, hoping the error we couldn't replicate might have disappeared, but apparently not:
They tried again with
Is this familiar to anyone? |
I was able to get a build error, although I had to build R from source using clang-16 and libc++ ( https://github.com/paleolimbot/docker-images/blob/main/2023-02-03_clang16-r-devel/Dockerfile ). The error isn't the one reported by CRAN, however. The error seems related to the use of
I also have a check going that doesn't involve an R source build (faster but maybe less accurate): https://github.com/paleolimbot/docker-images/blob/main/2023-02-03_clang16/Dockerfile Some further reading: From https://www.stats.ox.ac.uk/pub/bdr/clang16/README.txt:
Also see https://www.stats.ox.ac.uk/pub/bdr/Rconfig/r-devel-linux-x86_64-fedora-clang and https://apt.llvm.org/ I will meditate on all this over the weekend...any/all ideas welcome! |
Maybe start with the rhub fedora-clang-devel job and then pull the latest clang into that? |
Ok! Reproduced thanks to Dockerfile: FROM rhub/clang16:latest
RUN R -e "install.packages(c('cpp11', 'assertthat', 'bit64', 'lifecycle', 'purrr', 'rlang', 'tidyselect', 'vctrs', 'dplyr'))"
ENV ARROW_R_DEV=true I downloaded the release candidate from https://github.com/ursacomputing/crossbow/suites/10690329002/artifacts/535225657 (the r-binary-packages run for #33952 ). And did an I can reproduce the output from CRAN there. I believe the compile error is here:
Full log:
|
Sounds like the boost upgrade is it then. Upgrading thrift won't change anything because they still include boost in that location: https://github.com/apache/thrift/blob/master/lib/cpp/src/thrift/protocol/TJSONProtocol.cpp#L22 @kou has been submitting some PRs over there to help them remove their boost dependencies but this one hasn't been handled yet. |
There is a line in https://www.stats.ox.ac.uk/pub/bdr/clang16/README.txt that says:
...so we may be out of luck until a Boost version exists that does not use it? |
Although, if we merge #33890 it's slightly easier to check. |
This particular boost header (the only one we include that used So boost upgrade should be enough for us. |
Here's another (probably bad) idea: I wonder if we could patch the thrift cmakelists so that we just don't build the TJSONProtocol, which I bet that Parquet doesn't need: https://github.com/apache/thrift/blob/master/lib/cpp/CMakeLists.txt#L43 https://github.com/apache/thrift/blob/master/lib/cpp/src/thrift/protocol/TJSONProtocol.cpp#L22 is what uses Boost.Locale, which pulls in this hash.hpp. So if we didn't build that file, we wouldn't include the boost header that uses unary_function, and the build wouldn't fail on clang 16. Is that crazy, @kou? |
In fact, if I read it correctly, if we did that, we wouldn't need boost at all for Thrift except on Windows? Of the headers listed as needed for thrift on https://github.com/apache/arrow/blob/master/cpp/build-support/trim-boost.sh#L52-L53, locale is only used there, scoped_exit is only used in code that is conditionally built on windows, and I can't find mention of the third header anymore. |
I can confirm that updating boost using the commits in #33890 fix the Thrift component of the Arrow build: FROM rhub/clang16:latest
RUN apt-get update && apt-get install -y cmake git libssl-dev
# Checkout the Arrow + switch to a branch
RUN git clone https://github.com/thisisnic/arrow.git /arrow && \
cd /arrow && \
git switch r-11.0.0
RUN git config --global user.email "you@example.com" && \
git config --global user.name "Your Name"
# pick the boost fix
RUN cd /arrow && \
git remote add paleolimbot https://github.com/paleolimbot/arrow.git && \
git fetch paleolimbot && \
git switch update-bundled-boost && \
git switch r-11.0.0 && \
git cherry-pick 964488a66fa89a5818589910be06eaa55e485370 && \
git cherry-pick 654afce44ece22b2c5a84e62bb46d4cf0f256aa3 && \
git cherry-pick ddc5c78dd722eb8068a9eae9ce5ae07c954ae2e3 && \
git cherry-pick a934a4806e3d7972cc9b42f737615369ba1599c0
RUN mkdir /arrow-build && \
cd /arrow-build && \
cmake /arrow/cpp \
-DCMAKE_C_COMPILER="clang-16" -DCMAKE_CXX_COMPILER="clang++-16" \
-DCMAKE_CXX_FLAGS="-stdlib=libc++" \
-DARROW_CSV=ON -DARROW_DATASET=ON -DARROW_FILESYSTEM=ON \
-DARROW_COMPUTE=ON -DARROW_PARQUET=ON -DBoost_SOURCE=BUNDLED
# Pass even if the build fails so that we can copy error logs from /arrow-build
ENV VERBOSE 1
RUN cd /arrow-build && cmake --build . > /build_log.txt || true
RUN cp /arrow-build/thrift_ep-prefix/src/thrift_ep-stamp/thrift_ep-build-*.log / || true The Arrow R package build also needs checking (didn't get there today an am on PTO tomorrow but can check Wednesday). |
This PR updates the bundled version of Boost, as it was suggested by a maintainer of CRAN (R packaging) that the older version of boost might be responsible for an Arrow build failure on Fedora/clang (#33819). Closes #33851. The resulting tarball also has to be uploaded to https://apache.jfrog.io/ui/native/arrow/thirdparty/7.0.0/ (right?) to kick in and almost certainly needs to be tested through a round of CI. I'm not sure how to do either of those things but perhaps @ assignUser does? * Closes: #33851 Authored-by: Dewey Dunnington <dewey@fishandwhistle.net> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
I've merged #33890. Now we don't have any problem, right? |
Thanks! I think we're good! At least from the clang16 perspective...I validated that the R package installs with clang-16 (i.e., there were no additional build errors): FROM rhub/clang16:latest
RUN apt-get update && apt-get install -y cmake git libssl-dev
# Checkout the Arrow + switch to a branch
RUN git clone https://github.com/thisisnic/arrow.git /arrow && \
cd /arrow && \
git switch r-11.0.0
RUN git config --global user.email "you@example.com" && \
git config --global user.name "Your Name"
# pick the boost fix
RUN cd /arrow && \
git remote add paleolimbot https://github.com/paleolimbot/arrow.git && \
git fetch paleolimbot && \
git switch update-bundled-boost && \
git switch r-11.0.0 && \
git cherry-pick 964488a66fa89a5818589910be06eaa55e485370 && \
git cherry-pick 654afce44ece22b2c5a84e62bb46d4cf0f256aa3 && \
git cherry-pick ddc5c78dd722eb8068a9eae9ce5ae07c954ae2e3 && \
git cherry-pick a934a4806e3d7972cc9b42f737615369ba1599c0
RUN apt-get install -y rsync
RUN cd /arrow/r && make sync-cpp && cd .. && R CMD build r
RUN R -e 'install.packages("pak")' && R -e "pak::local_install_deps('/arrow/r')"
ENV VERBOSE 1
ENV ARROW_R_DEV true
RUN R CMD INSTALL /arrow/arrow_11.0.0.tar.gz
RUN R -e 'library(arrow)' I will also run |
This PR updates the bundled version of Boost, as it was suggested by a maintainer of CRAN (R packaging) that the older version of boost might be responsible for an Arrow build failure on Fedora/clang (apache#33819). Closes apache#33851. The resulting tarball also has to be uploaded to https://apache.jfrog.io/ui/native/arrow/thirdparty/7.0.0/ (right?) to kick in and almost certainly needs to be tested through a round of CI. I'm not sure how to do either of those things but perhaps @ assignUser does? * Closes: apache#33851 Authored-by: Dewey Dunnington <dewey@fishandwhistle.net> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
This PR updates the bundled version of Boost, as it was suggested by a maintainer of CRAN (R packaging) that the older version of boost might be responsible for an Arrow build failure on Fedora/clang (apache#33819). Closes apache#33851. The resulting tarball also has to be uploaded to https://apache.jfrog.io/ui/native/arrow/thirdparty/7.0.0/ (right?) to kick in and almost certainly needs to be tested through a round of CI. I'm not sure how to do either of those things but perhaps @ assignUser does? * Closes: apache#33851 Authored-by: Dewey Dunnington <dewey@fishandwhistle.net> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
This PR updates the bundled version of Boost, as it was suggested by a maintainer of CRAN (R packaging) that the older version of boost might be responsible for an Arrow build failure on Fedora/clang (apache#33819). Closes apache#33851. The resulting tarball also has to be uploaded to https://apache.jfrog.io/ui/native/arrow/thirdparty/7.0.0/ (right?) to kick in and almost certainly needs to be tested through a round of CI. I'm not sure how to do either of those things but perhaps @ assignUser does? * Closes: apache#33851 Authored-by: Dewey Dunnington <dewey@fishandwhistle.net> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
The fix was merged right? And the cran checks look fine so I am closing this :) |
This PR updates the bundled version of Boost, as it was suggested by a maintainer of CRAN (R packaging) that the older version of boost might be responsible for an Arrow build failure on Fedora/clang (apache#33819). Closes apache#33851. The resulting tarball also has to be uploaded to https://apache.jfrog.io/ui/native/arrow/thirdparty/7.0.0/ (right?) to kick in and almost certainly needs to be tested through a round of CI. I'm not sure how to do either of those things but perhaps @ assignUser does? * Closes: apache#33851 Authored-by: Dewey Dunnington <dewey@fishandwhistle.net> Signed-off-by: Sutou Kouhei <kou@clear-code.com>
For everyone still having trouble with arrow installation and needing parquet functionality in the short term, try the nanoparquet package :) |
Describe the bug, including details regarding any error messages, version, and platform.
On the CRAN package check page ( https://cran.r-project.org/web/checks/check_results_arrow.html ) there is a new warning about clang-16. We don't get any diagnostics in the compiler output ( https://www.stats.ox.ac.uk/pub/bdr/clang16/arrow.log ) because we suppress that output by default; however, it seems as though the failure is at the Arrow C++ build step and not the R package build step.
On submission we will need to submit an explanation or we will need to patch the vendored Arrow C++ to ensure that it builds prior to submission. The first step is to reproduce...LLVM provides a Dockerfile that we can probably build from ( https://github.com/llvm/llvm-project/blob/main/llvm/utils/docker/debian10/Dockerfile +
apt-get install -y r-base
will get us close).Component(s)
R
The text was updated successfully, but these errors were encountered: