-
Notifications
You must be signed in to change notification settings - Fork 281
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
Add install target for Linux and Mac #206
base: master
Are you sure you want to change the base?
Conversation
Still needed: the SC2APIConfig.cmake. I'm not particularly satisfied with the way the protocol headers are installed. It might be better if they were at a subdirectory, but that means that their internal includes need to be changed during protoc generation.
All that's missing is fixing the export set problem.
…riate. This cleans up a lot of the destructors as well.
…ctored thread vector setup to construct the thread in-place within the vector.
Conflicts: contrib/civetweb src/CMakeLists.txt src/sc2api/sc2_coordinator.cc src/sc2api/sc2_replay_observer.cc
This reverts commit 90b194a.
Scratch that. I need to do some updates apparently. I'll leave this here as a promise to get it updated soon. |
Found and fixed. Hopefully there's no more hiccups. |
This is a bit to review; I'll look into it later today. Thank you! Sorry for the delayed response, things have been busy on our end. |
Honestly, it's been more delayed from my end, just sitting there for a couple months, so I can hardly blame you. It might be worth looking at #90 partly to understand some of the issues that are present in this (and would be in any similar pull request) and partly just because it's been there a while. |
@AnthonyBrunasso any progress on this? |
I'll check in on this failure soon. |
This addresses #90.
Some tldr to begin:
I've addressed all of this fairly in depth in #90 except for maybe Windows support. It does build and it does install, but I'm just not sure it does them well. I can explain more if necessary.
Civetweb needs to pull in the changes I made to allow disabling compiling the executable (PR and merge commit).
Protobuf needs to be brought up to date with at least this commit.
I haven't actively touched this in a while. I did recompile both this fork and my project that used it as a target and they both worked so I can confirm this works for at least Ubuntu. I expect it to still be working on Mac but if someone wants to confirm for me, please be my guest. I'll answer any other questions as I can.