Skip to content

Conversation

@aheck
Copy link
Contributor

@aheck aheck commented Jul 21, 2022

No description provided.

@Aetf
Copy link
Owner

Aetf commented Jul 25, 2022

Thanks for the PR!

Ideally, this should coordinate with windows build fixes to have compiler/OS specific in a single place.

CI needs to be extended to test on macOS (and windows considering #25). Otherwise, I may accidentally break builds on those platforms in the future as I can't test them locally. A quick search suggests that Github Action has windows and macOS environments. If you could also set up those that'd be great. Otherwise, I need to find a time to sit down and look into these...

@aheck
Copy link
Contributor Author

aheck commented Aug 7, 2022

Didn't use GitHub Actions before but I will look into it.

@aheck
Copy link
Contributor Author

aheck commented Aug 7, 2022

I got it running as a matrix build in https://github.com/aheck/libtsm/tree/macos-github-action. When I have time again I will clean it up a bit and add the final result to this PR.

From what I have seen so far it should be no problem to add Windows as the third build platform later on. But since Windows is probably a bit more work than macOS I think it is better to add Windows in a separate PR.

One question: When testing on my machines I only build the static version of the library because I prefer it over dynamic linking. This workflow builds the dynamic library and so I stumbled over the linker script that I never took notice of before: src/tsm/libtsm.sym. clang doesn't know the option --version-script that gets set in src/tsm/CMakeLists.txt and so the build failed. I disabled this option for compilers other than gcc. But do we really need this linker script? We already have the SHL_EXPORT macro to control the visibility of functions.

@aheck aheck force-pushed the macos-build-fix branch 2 times, most recently from 14870a8 to 09018c2 Compare August 9, 2022 19:48
@aheck aheck force-pushed the macos-build-fix branch from 09018c2 to 37ab668 Compare August 9, 2022 19:57
@aheck
Copy link
Contributor Author

aheck commented Aug 9, 2022

I updated it. The PR now includes the extended GitHub Actions workflow that builds on Ubuntu as well as on macOS.

Let me know what you think.

@Aetf Aetf merged commit 4af5183 into Aetf:develop Sep 18, 2022
@Aetf
Copy link
Owner

Aetf commented Sep 18, 2022

Thanks!

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