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

Enables SSL for MySQL in Mac OS X builds #6730

Merged
merged 3 commits into from Feb 19, 2016

Conversation

louis89
Copy link
Contributor

@louis89 louis89 commented Mar 15, 2015

MySQL connections using SSL do not work for Mac OS X builds. This PR makes the necessary changes to the MySQL Makefile to enable SSL in Mac OS X builds.

I have built and tested this on Mac OS X 10.10, 10.9, Windows 7 x86_64, and Fedora 20 x86_64.

@Memphiz
Copy link
Member

Memphiz commented Mar 15, 2015

this change doesn't affect windows (and fedora only if you use unified depends for compiling) - also this change affects ios and android aswell. Do you happen to know which ssl library it uses? (gnutls or openssl?)

@Paxxi
Copy link
Member

Paxxi commented Mar 15, 2015

mysql uses cyassl for it's ssl functions, at least that's the case on Windows https://github.com/cyassl/cyassl

@Memphiz
Copy link
Member

Memphiz commented Mar 15, 2015

Then i wonder how this PR could do anything good - unified depends doesn't have cyassl - pointing --with-ssl to our depends prefix won't find any cyassl library ...

@Paxxi
Copy link
Member

Paxxi commented Mar 16, 2015

@Memphiz on windows it downloads it itself or includes it in the tarball, i havent looked at if this is the case on linux as well but i would guess so, at least if building with cmake

@Memphiz
Copy link
Member

Memphiz commented Mar 16, 2015

unified depends is different - it only builds what we explicitly prepare to build. And there is no rule to build cyassl - lets wait on @louis89 to comment

@louis89
Copy link
Contributor Author

louis89 commented Mar 16, 2015

Thanks for looking at this PR so quickly. Indeed this change does not seem necessary for Windows or Linux. I was just making clear that this change does not break Windows or Linux builds. However, for Mac OS X, MySQL connections over SSL do not work without this.

MySQL can be configured to use yaSSL or OpenSSL, by specifying a path with the --with-ssl switch MySQL searches for and tries to link to OpenSSL. See here: http://dev.mysql.com/doc/refman/5.1/en/building-with-ssl-support.html

I haven't tried building and testing this on Android or iOS so it is good to bring those things up. I don't have build environments setup for those things so testing will take some time for me, but I can do that if you guys would like me to.

I had created a PR quite a while ago to add the initial support for MySQL connections over SSL, but never tested it on Mac OS X because I never had any Mac machines on my network until a few months ago. From that PR the following comment by @theuni pointed me in the right direction to get this working on Mac OS X. #2566 (comment) It is unfortunate that change didn't make it into the first PR related to this.

@Memphiz
Copy link
Member

Memphiz commented Mar 16, 2015

jenkins build this please (lets see if it compiles on all platforms...)

@louis89
Copy link
Contributor Author

louis89 commented Mar 16, 2015

So the Linux builds failed trying to compile MySQL, but other platforms built ok... I ran the Linux build through my Jenkins to make sure I wasn't crazy and it builds for me. Not sure what the difference is between our environments/build procedure yet...

@Memphiz
Copy link
Member

Memphiz commented Mar 16, 2015

The difference that we build the depends and you use system libs on jenkins maybe ;)

@Memphiz
Copy link
Member

Memphiz commented Mar 16, 2015

I think you might need to add zlib here (as its now a dependency of mysql):

https://github.com/xbmc/xbmc/blob/master/tools/depends/target/Makefile#L89

but not sure if this is the issue here...

@Memphiz
Copy link
Member

Memphiz commented Mar 16, 2015

@louis89 as you have the needed environment to test this - are you able to test ios with this build? (i fetched it from the workspace just before the next jenkins build was run):

https://dl.dropboxusercontent.com/u/30371861/kodi-20150316-ba9481c--ios.deb

@louis89
Copy link
Contributor Author

louis89 commented Mar 16, 2015

Indeed using system libs instead of building the dependencies myself seems to be the difference. I am looking into how to change my build procedure now to include building the dependencies so that I can test this until it is truly fixed.

I could try testing the iOS build with the xcode simulator, but I do not have an iOS device with me to test that build on. I can also try testing the Android build I do have Android devices for testing.

@Memphiz
Copy link
Member

Memphiz commented Mar 16, 2015

just look into tools/buildsteps/linux64 to get an idea what is needed - the call order is prepare-depends, configure-depends, make-depends, prepare-xbmc, configure-xbmc, make-xbmc

Our ios builds don't run in the simulator - i try to do an android build for testing for you ...

@@ -13,7 +13,8 @@ CONFIGURE=cp -f $(CONFIG_SUB) $(CONFIG_GUESS) .; \
--enable-thread-safe-client --with-extra-charsets=complex \
--with-named-thread-libs=-lc --with-named-curses-libs=-lncurses \
--with-libedit \
--without-server --without-bench --without-docs --without-man --disable-shared
--without-server --without-bench --without-docs --without-man --disable-shared \
-with-ssl=$(PREFIX) LIBS=-lz

This comment was marked as spam.

This comment was marked as spam.

@louis89
Copy link
Contributor Author

louis89 commented Mar 17, 2015

Thanks @Memphiz I will test when I can. Also I am able to reproduce this build problem in my environment now. I'll keep looking into this when I can today.

@louis89
Copy link
Contributor Author

louis89 commented Mar 21, 2015

@Memphiz The Android build is good. MySQL connections over SSL are working, tested on Android 4.4.

I am still looking into the Linux build problem though. Hopefully I will have a solution before the weekend is over because I will probably be too busy during the week to look at this again until next weekend...

@MartijnKaijser
Copy link
Member

@Memphiz small nudge if you want this in

@Memphiz
Copy link
Member

Memphiz commented Jul 8, 2015

Its not ready. It still misses an extra "-" in the added configure flag and acts up on linux.

@tranaconda
Copy link

Any chance this will make it into Jarvis?

@wsnipex
Copy link
Member

wsnipex commented Nov 16, 2015

nope, too late, even if the issues were fixed.

@louis89 louis89 force-pushed the add_ssl_to_mysql_for_mac_os_x_builds branch from 901ec3f to cedd610 Compare December 12, 2015 22:02
@louis89
Copy link
Contributor Author

louis89 commented Dec 12, 2015

Wow! Sorry for taking such a long hiatus from this work... I have just updated the PR with the correct number of hyphens and added "-ldl" to LIBS. I squashed down the commit and rebased this onto master to freshen things up a bit. This built well and worked well for me on OSX 10.11 and Fedora 22.

@tranaconda
Copy link

So... Can this be merged now?

@Memphiz
Copy link
Member

Memphiz commented Feb 14, 2016

jenkins build this please

@Memphiz
Copy link
Member

Memphiz commented Feb 14, 2016

nope

rbpi:
configure: error: Could not link with SSL libs at /home/jenkins/workspace/LINUX-RBPI/tools/depends/xbmc-depends/arm-linux-gnueabihf

linux:
configure: error: Could not link with SSL libs at /home/jenkins/workspace/LINUX-64/tools/depends/xbmc-depends/x86_64-linux-gnu

I have rebased your pr to master and hopefully added a fix for this here:

https://github.com/Memphiz/xbmc/commits/add_ssl_to_mysql_for_mac_os_x_builds

Kicked a jenkins testbuild here:

http://jenkins.kodi.tv/view/Helpers/job/BuildMulti-All/1196/

@Memphiz
Copy link
Member

Memphiz commented Feb 14, 2016

@louis89 you would need to pick the top 2 commits for this to work

https://github.com/Memphiz/xbmc/commits/add_ssl_to_mysql_for_mac_os_x_builds

@louis89
Copy link
Contributor Author

louis89 commented Feb 15, 2016

Thanks @Memphiz! I will pick the top two commits off of your branch as soon as I get a chance, hopefully today. Obviously my build environment for Linux is still not quite the same as the one you guys use to build Kodi. My build must still be linking some depends against system libs...

@louis89 louis89 force-pushed the add_ssl_to_mysql_for_mac_os_x_builds branch from cedd610 to e63d71b Compare February 18, 2016 12:26
@louis89
Copy link
Contributor Author

louis89 commented Feb 18, 2016

@Memphiz I've moved your two commits over and rebased this onto master again. Wasn't sure if you would also like me to squash your commits into one. Wanted to keep credit where credit is due with the two commits you authored. Not sure if that history will be maintained if I squash them.

@Memphiz
Copy link
Member

Memphiz commented Feb 18, 2016

jenkins build and merge

Memphiz added a commit that referenced this pull request Feb 19, 2016
…uilds

Enables SSL for MySQL in Mac OS X builds
@Memphiz Memphiz merged commit d2273d6 into xbmc:master Feb 19, 2016
@tranaconda
Copy link

Is it possible to put this into 16.1 since it's more of a fix rather than a new feature? Or would it mess something up?

@Memphiz
Copy link
Member

Memphiz commented Mar 7, 2016

i have no opinion on that as i don't care about that feature... ehrm ... bugfix...

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.

None yet

7 participants