Add install target for Linux and Mac#206
Open
kuzi117 wants to merge 26 commits intoBlizzard:masterfrom
kuzi117:master
Open
Add install target for Linux and Mac#206kuzi117 wants to merge 26 commits intoBlizzard:masterfrom kuzi117:master
kuzi117 wants to merge 26 commits intoBlizzard:masterfrom
kuzi117:master
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.
Contributor
Author
|
Scratch that. I need to do some updates apparently. I'll leave this here as a promise to get it updated soon. |
Contributor
Author
|
Found and fixed. Hopefully there's no more hiccups. |
Contributor
|
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. |
Contributor
Author
|
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. |
Contributor
|
@AnthonyBrunasso any progress on this? |
Contributor
Author
|
I'll check in on this failure soon. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.