-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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-17545: [C++][CI] Mandate C++17 instead of C++11 #13991
Conversation
|
This comment was marked as outdated.
This comment was marked as outdated.
@kou It seems the centos-7-amd64 job uses gcc 4.8.5, should we switch to the devtoolset instead? |
The packaging jobs fail at the "upload artifacts" step with a weird error:
https://github.com/ursacomputing/crossbow/runs/8075587016?check_suite_focus=true#step:11:47 @raulcd Do you know where that might come from? |
I am not entirely sure why the remote is different but on a successful nightly job https://github.com/ursacomputing/crossbow/runs/8066802928?check_suite_focus=true#step:11:4 the archery command uses:
Did you use |
I did not use anything, I let |
7cd1e51
to
7736565
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Sure. The following patch will work: diff --git a/dev/tasks/linux-packages/apache-arrow/yum/centos-7/Dockerfile b/dev/tasks/linux-packages/apache-arrow/yum/centos-7/Dockerfile
index 04e74012f9..1da8e0fb79 100644
--- a/dev/tasks/linux-packages/apache-arrow/yum/centos-7/Dockerfile
+++ b/dev/tasks/linux-packages/apache-arrow/yum/centos-7/Dockerfile
@@ -18,13 +18,20 @@
ARG FROM=centos:7
FROM ${FROM}
+ENV \
+ SCL=devtoolset-11
+
ARG DEBUG
RUN \
quiet=$([ "${DEBUG}" = "yes" ] || echo "--quiet") && \
yum update -y ${quiet} && \
- yum install -y ${quiet} epel-release && \
yum install -y ${quiet} \
+ centos-release-scl-rh \
+ epel-release && \
+ yum install -y ${quiet} \
+ ${SCL}-gcc-c++ \
+ ${SCL}-make \
bison \
boost169-devel \
brotli-devel \
@@ -33,7 +40,6 @@ RUN \
cmake3 \
curl-devel \
flex \
- gcc-c++ \
gflags-devel \
git \
glog-devel \
@@ -42,7 +48,6 @@ RUN \
json-devel \
libzstd-devel \
lz4-devel \
- make \
ninja-build \
openssl-devel \
pkg-config \ |
1cc350f
to
657b4cd
Compare
This comment was marked as outdated.
This comment was marked as outdated.
@kou The CentOS 7 patched build now fails with:
https://github.com/ursacomputing/crossbow/runs/8088068865?check_suite_focus=true#step:6:2097 Would you like to suggest or push a fix? |
Oh, sorry. I'll push a fix to this branch later. |
ce0b494
to
2e8620b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Weeeeeeell... Clang wasn't in the pinning I commented on - and there's no incompatibility from the POV of conda(-forge). In other words, this is not going to be fixed by synching the pinnings. It's due to the combination of:
|
This is probably due to Gandiva which compiles some files to LLVM bitcode. In any case, I won't debug this any further. @xhochy is the primary maintainer of conda files and he can fix them when/if desired :-) |
5bc1c8d
to
27f586e
Compare
This comment was marked as outdated.
This comment was marked as outdated.
27f586e
to
ee0805b
Compare
This comment was marked as outdated.
This comment was marked as outdated.
(rebased) |
Hmm, I think I need to investigate the "Sphinx & Numpydoc" CI failure to see if it's caused by these changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
ee0805b
to
964be6f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
964be6f
to
fb53ff7
Compare
Revision: fb53ff7 Submitted crossbow builds: ursacomputing/crossbow @ cxx17-20 |
It seems like this was fixed in the meantime (on git master?). |
The Travis-CI s390x build times out. I don't think there's anything we can do against it, though (short of making the build optional). |
Revision: 24a9099 Submitted crossbow builds: ursacomputing/crossbow @ cxx17-21 |
I will merge if CI turns out as green as can be hoped given the current failures on git master. |
Benchmark runs are scheduled for baseline = c9844f0 and contender = 9d65981. 9d65981 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
['Python', 'R'] benchmarks have high level of regressions. |
This PR switches our build system to require C++17 instead of C++11. Because the conda packaging jobs are out of sync with the conda-forge files, the Windows conda packaging jobs are broken with this change. The related task (sync conda packaging files with conda-forge) is tracked in ARROW-17635. Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: Antoine Pitrou <antoine@python.org>
This PR switches our build system to require C++17 instead of C++11. Because the conda packaging jobs are out of sync with the conda-forge files, the Windows conda packaging jobs are broken with this change. The related task (sync conda packaging files with conda-forge) is tracked in ARROW-17635. Authored-by: Antoine Pitrou <antoine@python.org> Signed-off-by: Antoine Pitrou <antoine@python.org>
This PR switches our build system to require C++17 instead of C++11.
Because the conda packaging jobs are out of sync with the conda-forge files, the Windows conda packaging jobs are broken with this change. The related task (sync conda packaging files with conda-forge) is tracked in ARROW-17635.