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

Fix the build for Linux ARM64 #99

Merged
merged 13 commits into from
Sep 26, 2022
Merged

Fix the build for Linux ARM64 #99

merged 13 commits into from
Sep 26, 2022

Conversation

julien-faye
Copy link
Contributor

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).

This should fix the failing tests:

```
ok 1 - seqwish prints its help
  ==5145==LeakSanitizer has encountered a fatal error.
  ==5145==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1
  ==5145==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc)
  not ok 2 - unnamed test
  #          got: 'f82bea6331f62e86cce543c36fb4c1f6'
  #     expected: 'seqwish correctly builds the graph for A-3105'
  ==5170==LeakSanitizer has encountered a fatal error.
  ==5170==HINT: For debugging, try setting environment variable LSAN_OPTIONS=verbosity=1:log_threads=1
  ==5170==HINT: LeakSanitizer does not work under ptrace (strace, gdb, etc)
```
@julien-faye
Copy link
Contributor Author

Any feedback on the suggested changes ?

@ekg
Copy link
Owner

ekg commented Sep 12, 2022

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
Copy link
Contributor

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
Copy link
Owner

ekg commented Sep 26, 2022

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

@ekg ekg merged commit bea906e into ekg:master Sep 26, 2022
@julien-faye julien-faye deleted the linux-arm64 branch September 27, 2022 06:25
@julien-faye
Copy link
Contributor Author

Awesome!
Thank you, @ekg !

@ekg
Copy link
Owner

ekg commented Oct 5, 2022

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
Copy link
Owner

ekg commented Oct 5, 2022

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
Copy link
Contributor Author

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
Copy link
Owner

ekg commented Oct 5, 2022

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

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.

3 participants