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/Feat clang build #51

Merged
merged 17 commits into from
Jul 8, 2021
Merged

Fix/Feat clang build #51

merged 17 commits into from
Jul 8, 2021

Conversation

0x00002a
Copy link
Contributor

@0x00002a 0x00002a commented Jul 8, 2021

Alright, so this should have been a pretty basic fixup, but due to toolchain issues it ended up being a right pain. Sorry it took so long, got rather frustrated and didn't wanna get burned out :p.

Most of the problems are documented here: #46. The test stuff was added to try and chase down a segfault I think is due to incompatibilities between the libstdc++, shipped with msys, and clang, just a guess tho. I kept it in since I think it will be useful.

The CI will now build with clang on ubuntu, and the mingw CI will now only built with boost 1.76.0 due to #43 (which this was originally intended to address). Issues with msys aside, the code builds with clang now so clang-tidy should work and all that remains for #46 is for libc++ to catch up with their 20 support (coming in next release (13)).

Ref: #22 #46 #43

@0x00002a
Copy link
Contributor Author

0x00002a commented Jul 8, 2021

hmm, getting a link error locally on client::controller::run(), not sure why

@Tectu
Copy link
Owner

Tectu commented Jul 8, 2021

Good work - Thanks a lot!

Sorry it took so long, got rather frustrated and didn't wanna get burned out :p.

I don't think that any apologies are in order. This is an open source project and you're free to contribute - or not to :p
There are no expectations from my side other than attitude. Contributions are highly appreciated & welcomed but certainly not expected.
I do get your point about not burning yourself out - dealing with toolchain pain is... painful :p

@0x00002a
Copy link
Contributor Author

0x00002a commented Jul 8, 2021

Ah, a clean build fixed the link error 🤦‍♀️ whoops. On another note, clang did a clean build of the tests in a few seconds flat which, is impressive considering gcc almost two minutes. Not sure what thats about

@Tectu
Copy link
Owner

Tectu commented Jul 8, 2021

You don't want to know how long building the test suite on MinGW takes... man... it's a pain. Almost feels like an embarrassment.

@Tectu
Copy link
Owner

Tectu commented Jul 8, 2021

A question regarding the naming of the CI tests: Is LLVM the correct term here? Wouldn't clang be the way to go? As I understood LLVM is the backend (which can - and does - get user by other compilers too).

@0x00002a
Copy link
Contributor Author

0x00002a commented Jul 8, 2021

A question regarding the naming of the CI tests: Is LLVM the correct term here? Wouldn't clang be the way to go? As I understood LLVM is the backend (which can - and does - get user by other compilers too).

Good point. I'll change it

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.

2 participants