MegaGlest / megaglest-source

MegaGlest real-time strategy game engine (cross-platform, 3-d)
http://megaglest.org/
357 stars 90 forks source link

Errors Building With G++ 10 #197

Closed Jammyjamjamman closed 4 years ago

Jammyjamjamman commented 4 years ago

MegaGlest develop branch fails to compile in its current state on linux, with G++ 10 compiler.

The error is to do with globals being multiply defined in source/shared_lib/include/feathery_ftp/ftpTypes.h. I'm not actually sure why it successfully compiles with older compilers.

Here's the changes I made to fix the error:

diff --git a/source/shared_lib/include/feathery_ftp/ftpTypes.h b/source/shared_lib/include/feathery_ftp/ftpTypes.h
index 89310228..ca731066 100644
--- a/source/shared_lib/include/feathery_ftp/ftpTypes.h
+++ b/source/shared_lib/include/feathery_ftp/ftpTypes.h
@@ -69,7 +69,7 @@ typedef uint16_t port_t;
 extern "C" {
 #endif

-int VERBOSE_MODE_ENABLED;
+extern int VERBOSE_MODE_ENABLED;

 typedef ip_t (*ftpFindExternalFTPServerIpType)(ip_t clientIp);
 typedef void (*ftpAddUPNPPortForwardType)(int internalPort, int externalPort);
@@ -77,11 +77,11 @@ typedef void (*ftpRemoveUPNPPortForwardType)(int internalPort, int externalPort)
 typedef int (*ftpIsValidClientType)(ip_t clientIp);
 typedef int (*ftpIsClientAllowedToGetFileType)(ip_t clientIp, const char *username, const char *filename);

-ftpFindExternalFTPServerIpType ftpFindExternalFTPServerIp;
-ftpAddUPNPPortForwardType              ftpAddUPNPPortForward;
-ftpRemoveUPNPPortForwardType   ftpRemoveUPNPPortForward;
-ftpIsValidClientType            ftpIsValidClient;
-ftpIsClientAllowedToGetFileType ftpIsClientAllowedToGetFile;
+extern ftpFindExternalFTPServerIpType  ftpFindExternalFTPServerIp;
+extern ftpAddUPNPPortForwardType               ftpAddUPNPPortForward;
+extern ftpRemoveUPNPPortForwardType    ftpRemoveUPNPPortForward;
+extern ftpIsValidClientType            ftpIsValidClient;
+extern ftpIsClientAllowedToGetFileType ftpIsClientAllowedToGetFile;

 #ifdef __cplusplus
 }
diff --git a/source/shared_lib/sources/feathery_ftp/ftpRuntime.c b/source/shared_lib/sources/feathery_ftp/ftpRuntime.c
index f30c3a73..6e3e521f 100644
--- a/source/shared_lib/sources/feathery_ftp/ftpRuntime.c
+++ b/source/shared_lib/sources/feathery_ftp/ftpRuntime.c
@@ -30,6 +30,13 @@
 #include "ftp.h"
 #include "ftpMessages.h"

+int VERBOSE_MODE_ENABLED;
+
+ftpFindExternalFTPServerIpType ftpFindExternalFTPServerIp;
+ftpAddUPNPPortForwardType              ftpAddUPNPPortForward;
+ftpRemoveUPNPPortForwardType   ftpRemoveUPNPPortForward;
+ftpIsValidClientType            ftpIsValidClient;
+ftpIsClientAllowedToGetFileType ftpIsClientAllowedToGetFile;

 /**
  * @brief server-sockets that listens for incoming connections

A workaround is to downgrade g++.

Jammyjamjamman commented 4 years ago

@andy5995 found out why the code for rmw wouldn't compile on g++ 10, : https://www.linuxquestions.org/questions/showthread.php?p=6124441#post6124441

We have the same error here. Long story short, the code that errors is invalid C. It causes globals to be defined multiple times. The fix I've pasted above should be the correct way to fix it.

andy5995 commented 4 years ago

In the thread @Jammyjamjamman linked to above, the quick (but discouraged) workaround mentioned is to add the -fcommon flag to the compiler. That will help anyone trying to compile using g++ 10 without the patch Jammy posted above.

andy5995 commented 4 years ago

CXXFLAGS=-fcommon ./build-mg.sh will pass it to the build script. This should be mentioned to anyone who is having problems building the last release using g++10. Preferably though, a patch would be released.

titiger commented 4 years ago

does the patch work on windows too ? If yes we want it :)

andy5995 commented 4 years ago

Another option is instead of using the patch, replace the entire feathery ftp library with the most recent feathery ftp svn code. I tried compiling the last svn revision available and it doesn't have that bug. The code in the MG repo is from 2010, and the svn version is dated 2013. What do you think @titiger ? I know it's not tested, but we do enough testing with MG now that it would get testing in before MG next release.

Jammyjamjamman commented 4 years ago

Another option is instead of using the patch, replace the entire feathery ftp library with the most recent feathery ftp svn code. I tried compiling the last svn revision available and it doesn't have that bug. The code in the MG repo is from 2010, and the svn version is dated 2013. What do you think @titiger ? I know it's not tested, but we do enough testing with MG now that it would get testing in before MG next release.

I think I'd be in favour for this option.

tomreyn commented 4 years ago

Yet another option would be to replace the feathery lib and all calls to it (won't be few) code by just libcurl (HTTP/FTP client, MG already uses it), and a small HTTP server for the host part. I don't think MG ever uploads via FTP, just downloads, so that's probably a sufficient, and probably much better, alternative. I assume it would be a backwards incompatible change, though. And would likely involve an amount of work. FTP really deserves to R.I.P., though.

andy5995 commented 4 years ago

does the patch work on windows too ? If yes we want it :)

@titiger, shouldn't be any issue building on Windows, but the Appveyor Windows build has been broken for a while. Ticket

@Jammyjamjamman committed the patch, so this ticket could be closed, but a new one created for discussing what @tomreyn suggested (https://github.com/MegaGlest/megaglest-source/issues/197#issuecomment-647031738)

Jammyjamjamman commented 4 years ago

Fixed with https://github.com/MegaGlest/megaglest-source/commit/5a3520540276a6fd06f7c88e571b6462978e3eab