search

ekg / seqwish

alignment to variation graph inducer
MIT License
143 stars 18 forks source link

Fix the build for Linux ARM64 #99

Closed julien-faye closed 2 years ago

julien-faye commented 2 years ago

Pass '-mcx16' as a CMake option only for x86_64. Run the build and tests in a Docker container for ARM64. Do not use sanitizers because they are not usable without ptrace (in container).

julien-faye commented 2 years ago

Any feedback on the suggested changes ?

ekg commented 2 years ago

First, it's awesome. Second, I'm not sure that -mcx16 is still a requirement for any build, on newer systems. Anyone have any insight?

I need to test before merging, but it looks good.

martin-g commented 2 years ago

Good job, @julien-faye ! I just needed to build Seqwish on Linux ARM64 and your PR helped me to do it quickly! I hope it get merged soon!

ekg commented 2 years ago

Just did some quick tests and this looks good to go! Thank you!

julien-faye commented 2 years ago

Awesome! Thank you, @ekg !

ekg commented 2 years ago

Unfortunately, I now can't build (apparently due to the drop of the compare and swap compiler option):

transclosure.cpp:(.text+0x1051): undefined reference to `__sync_bool_compare_and_swap_16'

How should we resolve this?

ekg commented 2 years ago

Weirdly, I can build on one system, but not another. On both, I'm using GCC 11 and have x86_64. On one I'm building in guix.

julien-faye commented 2 years ago

Hi @ekg !

You will need to pass -mcx16 to Cmake as it is done at https://github.com/ekg/seqwish/pull/99/files#diff-793f8d3760fa3fb06eb7cc5f7c3f34381da8b1952889b26aa09450bc05bbceadR35

I can send a new PR to make CMakeLists.txt more x86_64 friendly, i.e. this flag to be added by default if the user hasn't provided explicitly -DCMAKE_C_FLAGS=... and -DCMAKE_CXX_FLAGS=... Linux ARM64 users will have to provide those options explicitly but I think this is fine!

ekg commented 2 years ago

It looks like the problem was with guix! So no issues here.