Skip to content
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

Bump arrow 14 -> 17 #5439

Merged
merged 5 commits into from
Aug 9, 2024
Merged

Bump arrow 14 -> 17 #5439

merged 5 commits into from
Aug 9, 2024

Conversation

aalkin
Copy link
Collaborator

@aalkin aalkin commented Apr 25, 2024

No description provided.

@aalkin aalkin requested a review from a team as a code owner April 25, 2024 13:28
@davidrohr
Copy link
Contributor

@aalkin : Are you checking the build errors in the CI? Is it perhaps that we use the incorrect c++ std?

@aalkin
Copy link
Collaborator Author

aalkin commented May 2, 2024

@davidrohr outdated system headers on slc7 seem to be the issue. Do we still need slc7 at all?

@davidrohr
Copy link
Contributor

I'd better let @ktf comment, if this is still needed on the grid. For EPNs / FLPs, we need SLC8 only.

@ktf
Copy link
Member

ktf commented May 2, 2024

We still need slc7 for the AliPhysics builds (which share the same root).

Moreover @chiarazampolli and @noferini should comment wether slc9 is working fine for async and we can drop slc7 support there.

@aalkin
Copy link
Collaborator Author

aalkin commented May 3, 2024

@ktf I thought for aliphysics it was a slightly different ROOT, with some patches? It is also possible to just fix arrow 14 in defaults-ali, since it is not used there anyway.

@ktf
Copy link
Member

ktf commented May 3, 2024

Ok, let's see if it works like this.

@aalkin
Copy link
Collaborator Author

aalkin commented May 4, 2024

Why not explicitly disable it like in defaults-generators?

@ktf
Copy link
Member

ktf commented May 4, 2024

I thought that's what i did, actually. Not sure why it doesn't work though.

@aalkin
Copy link
Collaborator Author

aalkin commented May 7, 2024

@ktf ROOT has arrow as an explicit dependency, can it override the defaults?

@aalkin aalkin changed the title Bump arrow 14 -> 16 Bump arrow 14 -> 17 Jul 31, 2024
@davidrohr
Copy link
Contributor

@singiamtel : could you check why the build/O2/alidist CI is failing with

Git command for package 'bookkeeping-api' failed.
Command: git clone -n https://github.com/AliceO2Group/Bookkeeping /build/nomad/alloc/9cc434e0-6c67-f267-c6e7-4bce6209e5a3/ci/local/o2-alidist/sw/SOURCES/bookkeeping-api/v0.93.0/@aliceo2/[email protected] --dissociate --reference /build/nomad/alloc/9cc434e0-6c67-f267-c6e7-4bce6209e5a3/ci/local/o2-alidist/sw/MIRROR/bookkeeping-api --filter=tree:0
In directory: .
Exit code: 128

@singiamtel
Copy link
Collaborator

@singiamtel : could you check why the build/O2/alidist CI is failing with

Git command for package 'bookkeeping-api' failed.
Command: git clone -n https://github.com/AliceO2Group/Bookkeeping /build/nomad/alloc/9cc434e0-6c67-f267-c6e7-4bce6209e5a3/ci/local/o2-alidist/sw/SOURCES/bookkeeping-api/v0.93.0/@aliceo2/[email protected] --dissociate --reference /build/nomad/alloc/9cc434e0-6c67-f267-c6e7-4bce6209e5a3/ci/local/o2-alidist/sw/MIRROR/bookkeeping-api --filter=tree:0
In directory: .
Exit code: 128

Hi @davidrohr,

Sometimes the checkout takes too long and times out. I'm not sure why it happens, I think it's related to the changes to how aliBuild clones repositories (--dissociate, --filter=tree:0 etc). Usually fixes after the CI does another run

With clang it was happening so often we had to fork it... (https://github.com/alisw/llvm-project-reduced)

We'll keep investigating, sorry for the disturbance

@davidrohr
Copy link
Contributor

davidrohr commented Aug 5, 2024 via email

@ktf
Copy link
Member

ktf commented Aug 6, 2024

I think somehow dissociate still breaks under certain conditions and checkouts hang. Is this using the latest version of alibuild.

@singiamtel
Copy link
Collaborator

I think somehow dissociate still breaks under certain conditions and checkouts hang. Is this using the latest version of alibuild.

It's on [email protected], but I think that version has the latest git flags. Probably a good idea to bump to v1.17.10 anyway

@ktf
Copy link
Member

ktf commented Aug 7, 2024

@singiamtel yes, it sounds like a good idea.

@davidrohr
Copy link
Contributor

I checked what happens here on slc7, and it seems when clang is used to compile some arrow files to LLVM bc, it picks up system GCC headers instead of the GCC headers of the alidist GCC, and those lack >C++14 support needed for enable_if_t, while it works for newer OS, when system GCC knows about C++14.
But I think this is a general problem. Clang should not use system GCC headers, but those of alidist GCC.
Failing command is:

cd /home/qon/docker/sw/BUILD/b9f858afbc4fe8e08ffc4331d164db910fc6d271/arrow/src/gandiva/precompiled && /home/qon/docker/sw/slc7_x86-64/Clang/v15.0.7-local1/bin-safe/clang -std=c++17 -DGANDIVA_IR -DNDEBUG -DARROW_STATIC -DGANDIVA_STATIC -fno-use-cxa-atexit -emit-llvm -O3 -c /home/qon/docker/sw/BUILD/b9f858afbc4fe8e08ffc4331d164db910fc6d271/arrow/src_tmp/cpp/src/arrow/util/basic_decimal.cc -o /home/qon/docker/sw/BUILD/b9f858afbc4fe8e08ffc4331d164db910fc6d271/arrow/src/gandiva/precompiled/basic_decimal.bc -I/home/qon/docker/sw/BUILD/b9f858afbc4fe8e08ffc4331d164db910fc6d271/arrow/src -I/home/qon/docker/sw/BUILD/b9f858afbc4fe8e08ffc4331d164db910fc6d271/arrow/src_tmp/cpp/src

Failing with error message

In file included from /home/qon/docker/sw/BUILD/b9f858afbc4fe8e08ffc4331d164db910fc6d271/arrow/src_tmp/cpp/src/arrow/util/endian.h:59:
/home/qon/docker/sw/BUILD/b9f858afbc4fe8e08ffc4331d164db910fc6d271/arrow/src_tmp/cpp/src/arrow/util/ubsan.h:56:13: error: no template named 'enable_if_t' in namespace 'std'; did you mean 'enable_if'?
inline std::enable_if_t<std::is_trivially_copyable_v<T>, T> SafeLoadAs(
       ~~~~~^~~~~~~~~~~
            enable_if
/usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/type_traits:1766:12: note: 'enable_if' declared here
    struct enable_if 
           ^

i.e. it picks up /usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../include/c++/4.8.5/type_traits

@davidrohr
Copy link
Contributor

Compiling a random test cpp file with our clang, it uses wrong GCC headers:

[root@926bd8ee78a7 precompiled]# /home/qon/docker/sw/slc7_x86-64/Clang/v15.0.7-local1/bin-safe/clang -v -c foo.cpp -o foo.o
clang version 15.0.7 (https://github.com/alisw/llvm-project-reduced e1644b141c9221d6fb546cefff2f7c801face5e8)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /home/qon/docker/sw/slc7_x86-64/Clang/v15.0.7-local1/bin-safe
Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/4.8.2
Found candidate GCC installation: /usr/lib/gcc/x86_64-redhat-linux/4.8.5
Selected GCC installation: /usr/lib/gcc/x86_64-redhat-linux/4.8.5

@davidrohr
Copy link
Contributor

Fix is here #5569

@davidrohr davidrohr merged commit bafbb2f into alisw:master Aug 9, 2024
12 of 14 checks passed
@aalkin aalkin deleted the bump-arrow branch August 10, 2024 06:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants