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

Build on OSX #72

Closed
wants to merge 2 commits into from
Closed

Build on OSX #72

wants to merge 2 commits into from

Conversation

guysherman
Copy link

This aims to fix #27 and #71

Borrowed a lot from the cpprestsdk CMAKE, the changes revolved around:

  • using the correct OpenSSL, because homebrew installs it "Keg Only"
  • injecting a missing -L directive for gettext (libintl)
  • adding -DBOOST_LOG_DYN_LINK to make boost link properly
  • adding the same warnings/suppressions/cflags as Casablanca

In resources.cpp changed _RECOURSES to _RESOURCES

Guy Sherman added 2 commits June 12, 2016 21:45
Borrowed a lot from the cpprestsdk CMAKE, the changes revolved around:
* using the correct OpenSSL, because homebrew installs it "Keg Only"
* injecting a missing -L directive for gettext (libintl)
* adding -DBOOST_LOG_DYN_LINK to make boost link properly
* adding the same warnings/suppressions/cflags as Casablanca
Presumably this was preventing all builds from working, as
the resources from the dat file were not being included because
the #ifdef was failing.
@azurecla
Copy link

Hi @guysherman, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.azure.com.

TTYL, AZPRBOT;

@guysherman guysherman changed the title Cmake osx over dev Build on OSX Jun 12, 2016
@azurecla
Copy link

@guysherman, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, AZPRBOT;


```
brew install glibmm libxml++ libsigc++ ossp-uuid gettext openssl
```
Copy link
Member

@hanzhumsft hanzhumsft Jun 16, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

glibmm is no longer used in our project, so is libsigc++.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you let me know why gettext is needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hanzhumsft it seems the cmake was putting -lintl into the linker commands, which is part of gettext. I'll remove the references to glibmm and libsigc++.

@hanzhumsft
Copy link
Member

Thanks a lot @guysherman for the PR.

I gave some feedback inline. Please check.

@guysherman
Copy link
Author

@hanzhumsft I will close this PR and raise two new ones, separating the two fixes in this one.

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