Page MenuHomeWildfire Games

Build gloox with GnuTLS on macOS
ClosedPublic

Authored by Itms on Oct 19 2018, 8:26 PM.

Details

Summary

The multiplayer lobby uses TLS as of rP21875, so the statically linked gloox should be compiled with gnutls.

Test Plan

Tested by @trompetin17 on Mojave (working and bundled), by several users (trompetin's bundle), and by @Stan on Mavericks (trompetin's bundle, after rP21932).

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Lint Skipped
Unit
Unit Tests Skipped
Build Status
Buildable 6457
Build 10695: Vulcan BuildJenkins

Event Timeline

elexis created this revision.Oct 19 2018, 8:26 PM
Vulcan added a subscriber: Vulcan.Oct 19 2018, 8:29 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/748/display/redirect

Itms requested changes to this revision.Oct 20 2018, 10:47 PM
Itms added a subscriber: Itms.

The build fails on OSX: gnutls needs libnettle. ?

libraries/osx/build-osx-libs.sh
488

We could add --disable-tests.

520

There are two occurrences of --with-gnutls now, and I'm not sure removing zlib was done on purpose.

This revision now requires changes to proceed.Oct 20 2018, 10:47 PM

If you can test it, feel free to commandeer and commit.

elexis updated this revision to Diff 6944.Oct 21 2018, 4:40 PM

I have no idea what I'm doing here.

lyv added a subscriber: lyv.EditedOct 21 2018, 4:56 PM

(Travis would help out a lot in times like these)

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/750/display/redirect

Stan added a subscriber: Stan.Oct 21 2018, 7:30 PM

For the record. D1617 does this :)

Itms commandeered this revision.Oct 22 2018, 10:31 PM
Itms edited reviewers, added: elexis; removed: Itms.

I am progressing on this but not there yet. Hopefully tomorrow!

lyv added a comment.Oct 23 2018, 5:40 PM
In D1654#65495, @Stan wrote:

For the record. D1617 does this :)

Sure, but that won't help much in cases like this. It would only be triggered after a commit when it's pushed to github. Travis could only be used to the fullest in a PR based workforce I suppose.

Itms updated this revision to Diff 6947.Oct 23 2018, 8:53 PM
Itms edited the test plan for this revision. (Show Details)

This builds and works!

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/753/display/redirect

Tobbi will try on the weekend. When he tried last time, he used this diff to compile:

Index: build/workspaces/build-osx-bundle.sh
===================================================================
--- build/workspaces/build-osx-bundle.sh    (revision 21914)
+++ build/workspaces/build-osx-bundle.sh    (working copy)
@@ -22,13 +22,15 @@
 # Choices are "x86_64" or  "i386" (ppc and ppc64 not supported)
 export ARCH=${ARCH:="x86_64"}
 
-OSX_VERSION=`sw_vers -productVersion | grep -Eo "^\d+.\d+"`
+# OSX_VERSION=`sw_vers -productVersion | grep -Eo "^\d+.\d+"`
+OSX_VERSION=10.14
+
 # Set SDK and mimimum required OS X version
 export SYSROOT=${SYSROOT:="/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX$OSX_VERSION.sdk"}
-export MIN_OSX_VERSION=${MIN_OSX_VERSION:="10.7"}
+export MIN_OSX_VERSION=${MIN_OSX_VERSION:="10.8"}
 
 # 0 A.D. release version, e.g. Alpha 21 is 0.0.21
-BUNDLE_VERSION=${BUNDLE_VERSION:="0.0.X"}
+BUNDLE_VERSION=${BUNDLE_VERSION:="0.0.24"}
 
 # Define compiler as "clang", this is all Mavericks supports.
 # gcc symlinks may still exist, but they are simply clang with
Index: libraries/osx/build-osx-libs.sh
===================================================================
--- libraries/osx/build-osx-libs.sh (revision 21914)
+++ libraries/osx/build-osx-libs.sh (working copy)
@@ -516,7 +516,7 @@
   tar -xf $LIB_ARCHIVE
   pushd $LIB_DIRECTORY/nspr
 
-  (CFLAGS="$CFLAGS" CXXFLAGS="$CXXFLAGS" LDFLAGS="$LDFLAGS" ./configure --prefix="$NSPR_DIR" && make ${JOBS} && make install) || die "NSPR build failed"
+  (CFLAGS="$CFLAGS" CXXFLAGS="$CXXFLAGS" LDFLAGS="$LDFLAGS" ./configure --prefix="$NSPR_DIR" --enable-64bit && make ${JOBS} && make install) || die "NSPR build failed"
   popd
   # TODO: how can we not build the dylibs?
   rm -f lib/*.dylib

sw_vers -productVersion yields 10.13.6

Should we commit the 10.7 drop and perhaps add a comment stating that there can be version conflicts as premake uses the one version for the libraries but it's actually configured and compiled with the other version?

Itms added a comment.Oct 24 2018, 11:27 AM
In D1654#65539, @elexis wrote:

sw_vers -productVersion yields 10.13.6

Should we commit the 10.7 drop

I really don't have a clue about the share of our users (both players and people who build the game) who use a given version of macOS. If Tobbi needs to drop 10.7 to build on his machine, I think we have to do that. If somebody has 10.7 and wants to play, we can help them build it themselves.

+BUNDLE_VERSION=${BUNDLE_VERSION:="0.0.24"}

Just using the opportunity to tell him that it should be "0.0.23b", or there will be issues when we release A24.

and perhaps add a comment stating that there can be version conflicts as premake uses the one version for the libraries but it's actually configured and compiled with the other version?

I'm sorry I'm confused here: there are no different versions of the libraries at play. Are you referring to my patching of extern_libs5.lua? That is a consequence of us using static linking.

In D1654#65540, @Itms wrote:

Should we commit the 10.7 drop

I really don't have a clue about the share of our users (both players and people who build the game) who use a given version of macOS. If Tobbi needs to drop 10.7 to build on his machine, I think we have to do that. If somebody has 10.7 and wants to play, we can help them build it themselves.
I'm sorry I'm confused here: there are no different versions of the libraries at play. Are you referring to my patching of extern_libs5.lua? That is a consequence of us using static linking.

Please check the comment in https://code.wildfiregames.com/rP21683#31432

As I said I think our build scripts are broken and it's only one way how it manifests.
I think you have used the same 10.7 drop, it was praised as the official fix when a23 was built, you even have it in your own macOS branch:
https://github.com/0ad/0ad/compare/master...na-Itms:osx-fix.diff

Please look at the commit error, the macro says 10.8 exists, but the 10.8 API function does not exist. Dropping 10.7 is only a workaround, the real problem is that premake uses one version, while configure and make use the other version, that's why the macro says the one thing and the API says the other thing.

libCURL code where it fails to compile:

#if CURL_BUILD_MAC_10_8 || CURL_BUILD_IOS
if(SSLSetProtocolVersionMax != NULL) {

compile error:

vtls/darwinssl.c:1308:6: error: 'SSLSetProtocolVersionMax' is only available on macOS 10.8 or newer [-Werror,-Wunguarded-availability]

Itms added a comment.Oct 24 2018, 12:15 PM
In rP21913#31478, @Stan wrote:

I'm pretty sure we can't have letters in mod versions.

Wrong place? ? This is unrelated to mods, but if the bundle version cannot have letters, it should still be 23, not 24.

In D1654#65541, @elexis wrote:

Ah I get it now! You're talking about build-osx-bundle.sh, not build-osx-libs.sh. Yes, I think we should have 10.9 in the bundle generation script, and we should add a comment "keep in sync with build-libs" in build-bundle.

Itms added a comment.Oct 24 2018, 12:18 PM
In D1654#65543, @Itms wrote:

Yes, I think we should have 10.9 in the bundle generation script, and we should add a comment "keep in sync with build-libs" in build-bundle.

(Or 10.8, if it is properly tested - 10.9 is almost guaranteed to work on the other hand)

Stan added a comment.Oct 24 2018, 12:48 PM

Wrong place? ? This is unrelated to mods, but if the bundle version cannot have letters, it should still be 23, not 24.

I just mentioned it to keep things consistent. Another idea I proposed was to use 231 as It's very unlikely (I hope ) that we will have 231 alphas.

In D1654#65544, @Itms wrote:
In D1654#65543, @Itms wrote:

Yes, I think we should have 10.9 in the bundle generation script, and we should add a comment "keep in sync with build-libs" in build-bundle.

If the numbers must be kept in sync, then at the very least there should only be one unique variable, to prevent people from bumping only one of the numbers.
But I'm not sure if keeping the numbers in sync is actually a requirement, while it seems like a requirement that premake configure and make stage all use the same reference. No?
My concern is that I suppose there is a logic bug in the OSX build scripts that is not solved but hidden by keeping the numbers in sync (and hiding bugs is the opposite direction of fixing bug).

How is it possible that these two lines in build/premake/premake5/contrib/curl/lib/vtls/darwinssl.c

#if CURL_BUILD_MAC_10_8 || CURL_BUILD_IOS
if(SSLSetProtocolVersionMax != NULL) {

trigger this error:

vtls/darwinssl.c:1308:6: error: 'SSLSetProtocolVersionMax' is only available on macOS 10.8 or newer [-Werror,-Wunguarded-availability]

The first line (macro) says 10.8, the second line (code) says 10.7 -> crux

Current behavior: macOS library macros use version of MIN_OSX_VERSION from build-osx-libs.sh (premake stage?), but configure and make use MIN_OSX_VERSION from build-osx-bundle.sh.
Expected behavior: libraries are built using the same MIN_OSX_VERSION from the same file in all cases.

Also:
Current behavior: Compile error
Expected behavior: Compiles without SSL support (since the macro detects that 10.8 is not present).

So it's just a coincidence that it happened with one library and one version, it could happen with every library and every version.
If this deduction is correct, a "keep in sync" comment would still conceal that the script written there has a bug unrelated to that (possibly misleading readers of the script to think that the code is correct as is).

elexis added a comment.Nov 9 2018, 2:31 PM

10.7 drop issue from a23 first release: D1487

Itms added a comment.Nov 9 2018, 8:37 PM

@elexis: I have looked around about this issue, and I might have misunderstood a few remarks from you that were unclear to me, but I think I know what is bugging you. I also know why certain version numbers are 8, others 9, and why the reason for dropping 7 was unclear (thanks to some attempts by trompetin).

  • SpiderMonkey needs 10.9 to be built (not to be linked).
  • curl needs 10.8 to be built (not to be linked).
  • In build-osx-bundle.sh, the minimum version is not passed to build-osx-libs.sh (it should be
MIN_OSX_VERSION="$MIN_OSX_VERSION" ./build-osx-libs.sh $JOBS --force-rebuild >> $build_log 2>&1

with the version passed down). So even when the min is set to 10.8 in the parent script, SM builds correctly.

  • Most of the time, premake is called by directly running update-workspaces.sh and the default value for the min version is the one in premake scripts. It should be bumped.

Even when this is fixed, we still need the version number in all files (and kept in sync), because sometimes one only builds the libs, sometimes only pyrogenesis, sometimes both, sometimes the whole bundle. So if the version number is in only one place there is no way to make sure this place gets executed.

And since SM needs 10.9, and nothing else needs a higher version, 10.9 should be the minimum version.
Do you agree with my findings?

Itms added a comment.Nov 9 2018, 8:38 PM

Apart from that, I cannot reproduce my mod.io bug, so this patch should be fine (tested on a clean repo).

trompetin17 added a subscriber: trompetin17.EditedNov 10 2018, 4:26 PM

Hello, I tested this patch over osx mojave, I made the bundle and play over it.

I was able to join to lobby.
I hosted a game and play like 5 min with nani
I joined in a game
I rejoined after kick

I tested to download a mod

I only notice a 1 sec delay between actions.

Form side everythings works,

PD: one question my bundle is more bigger now, from 900m to 2.7 gb why?

It was just that it shouldn't be possible to obtain the stacktrace that Tobbi received, where the macro says 10.8 and the compiler says 10.7:

#if CURL_BUILD_MAC_10_8 || CURL_BUILD_IOS
if(SSLSetProtocolVersionMax != NULL) {

A script change making this impossible sounds good in theory.

SpiderMonkey needs 10.9 to be built (not to be linked).
curl needs 10.8 to be built (not to be linked).

Do you mean the one compiling SM needs macOS 10.9+, even when he configured the SDK for macOS versions prior to 10.9?
If the user also needs at least macOS 10.9 because of SM, then why did the first a23 release macOS work with 10.8?
(equally, I couldn't find any online resource stating which macOS version curl needs.)

I can't really figure out what's going on without succeeding to execute the script.
You are free to commandeer D1487 if you want to propose an improvement to this issue (I can commandeer back in case of doubt/new information)

Stan added a comment.Nov 10 2018, 5:46 PM

I guess you forgot to package the public mod.

Tobbi compiled with this patch and bundled an image and could join the lobby. https://mega.nz/#!Jcd2EaqB!uX5NiOYuaIzfdliy10nOZx9T2vsQK8AKltStDbE5IQs

Itms added a comment.Nov 13 2018, 6:01 PM
In D1654#66152, @elexis wrote:

It was just that it shouldn't be possible to obtain the stacktrace that Tobbi received, where the macro says 10.8 and the compiler says 10.7:

Couldn't it come from what @trompetin17 has discovered, that premake5 itself has a hardcoded 10.4 minimum version? See this line and others, all coming from this code in p5 a10.

"clock_gettime" isnt implemented in osx until 10.12, so we need a way to emulate this function from 10.9 to 10.11

https://www.gnutls.org/manual/gnutls.html
GnuTLS requires the following system calls to be available for its proper operation.

nanosleep
time
gettimeofday
clock_gettime
getrusage
getpid
send
recv
sendmsg
read (to read from /dev/urandom)
getrandom (this is Linux-kernel specific)
poll

Alternatively gloox also supports OpenSSL and LibreSSL (https://camaya.net/gloox/download/). Dunno if that's more feasible to use. (gloox ought to support the native Secure Transport like curl does. https://developer.apple.com/documentation/security/secure_transport)

Itms updated this revision to Diff 6991.Nov 17 2018, 1:06 PM

@trompetin17 Could you test this? I added -UCLOCK_REALTIME, this should work (see this piece of the GnuTLS code).

It won't work if the define is set by something else after I undef it, so it needs to be tested; unfortunately I don't have my mac VM as I am at the Capitole du Libre event.

In D1654#66251, @Itms wrote:
In D1654#66152, @elexis wrote:

It was just that it shouldn't be possible to obtain the stacktrace that Tobbi received, where the macro says 10.8 and the compiler says 10.7:

Couldn't it come from what @trompetin17 has discovered, that premake5 itself has a hardcoded 10.4 minimum version? See this line and others, all coming from this code in p5 a10.

Yikes @ hardcoding.
Not sure if the CURL compiler/linker error can come from that 10.4 hardcoding. But if that 10.4 hardcoding would be the issue, would have Tobbis 10.7 -> 10.8 change actually been possible to disable/workaround the issue?

In D1654#66248, @elexis wrote:

Tobbi compiled with this patch and bundled an image and could join the lobby. https://mega.nz/#!Jcd2EaqB!uX5NiOYuaIzfdliy10nOZx9T2vsQK8AKltStDbE5IQs

This was tested by a number of lobby players and it seems to be working on their platforms (just that they probably all have 10.12 or 10.13 which happen to not be affected by the issues you described).
Test results here: https://wildfiregames.com/forum/index.php?/topic/24799-frozen-development/&page=3&tab=comments#comment-364644
And more results in a PM session https://wildfiregames.com/forum/index.php?/messenger/9438/

(If we we're absolutely unable to fix it or fix it quickly enough, we could set gloox to disable TLS encryption only for macOS and remov the one encryption line of the macOS Privacy Policy . Currently certificate validation is disabled because gloox has this bug https://bugs.camaya.net/ticket/?id=280 , so the encryption already isn't waterproof)

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/791/

Itms updated this revision to Diff 6999.Nov 22 2018, 11:41 PM
Itms added a reviewer: trompetin17.
Itms removed a subscriber: trompetin17.

With this fix, the detection from gnutls' configure script will be overridden.

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/795/

Itms updated this revision to Diff 7013.Dec 2 2018, 2:28 PM
Itms edited the test plan for this revision. (Show Details)

Final version.

Vulcan added a comment.Dec 2 2018, 2:37 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/807/

This revision was not accepted when it landed; it landed in state Needs Review.Dec 2 2018, 9:52 PM
This revision was automatically updated to reflect the committed changes.

This patch worked with one modification:

Change

LIB_URL="https://www.gnupg.org/ftp/gcrypt/gnutls/v3.5/"

to

LIB_URL="https://www.gnupg.org/ftp/gcrypt/gnutls/v3.6/"

Otherwise, it downloads a 404 or 500 response and can't open the archive.

Also posted here: https://trac.wildfiregames.com/ticket/5453#comment:11

Also, I ran into this error after running make

Linking test
ld: warning: PIE disabled. Absolute addressing (perhaps -mdynamic-no-pic) not allowed in code signed PIE, but used in ___gmpn_divexact_1_x86_64 from ../../../libraries/osx/gmp/lib/libgmp.a(x86_64_dive_1.o). To fix this warning, don't compile with -mdynamic-no-pic or link with -Wl,-no_pie

Full logs here: https://trac.wildfiregames.com/ticket/5489