Differential D72
Migrate to premake5 Authored by Itms on Jan 15 2017, 7:44 PM.
Details This is a revision for testing https://github.com/na-Itms/0ad/tree/premake5. This revision is mainly aimed at automatically testing that the premake5 support doesn't break anything with premake4. It is also a small proof-of-concept of how we can integrate build system changes in the new Phabricator workflow. Test on *nix, Mac OSX, Windows (I put one reviewer by system ?)
Diff Detail
Event TimelineComment Actions I get this error calling update-workspaces.sh --premake5 ==== Building lua-lib (release) ====
==== Building zip-lib (release) ====
==== Building zlib-lib (release) ====
==== Building curl-lib (release) ====
asyn-thread.c
base64.c
In file included from ../../contrib/curl/lib/base64.c:26:
In file included from ../../contrib/curl/lib/urldata.h:82:
In file included from ../../contrib/curl/lib/cookie.h:26:
../../contrib/curl/include/curl/curl.h:34:10: fatal error: 'curlbuild.h' file not found
#include "curlbuild.h" /* libcurl build definitions */
^
In file included from ../../contrib/curl/lib/asyn-thread.c:60:
In file included from ../../contrib/curl/lib/urldata.h:82:
In file included from ../../contrib/curl/lib/cookie.h:26:
../../contrib/curl/include/curl/curl.h:34:10: fatal error: 'curlbuild.h' file not found
#include "curlbuild.h" /* libcurl build definitions */
^
1 error generated.
1 error generated.
make[1]: *** [obj/Release/curl-lib/asyn-thread.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make[1]: *** [obj/Release/curl-lib/base64.o] Error 1
make: *** [curl-lib] Error 2
ERROR: Premake build failedDo we need to recompile curl though? we already do from the build scripts.
Comment Actions Build is green Updating workspaces. Build (release)... Build (debug)... Running debug tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK! http://jw:8080/job/phabricator/179/ for more details. Comment Actions I fixed a number of issues I stumbled upon on Unix (including big issues with parallel builds that are more hacked away than fixed). That makes me more convinced that premake5 shouldn't become the default just now on non-Windows. However, as we want to move to VS2015 as soon as possible, I would agree on making it the default on Windows (which is the platform with the higher support and the least amount of bugs in premake5 AFAICS). I didn't test my changes still work on Windows but it should, I'll check that tonight. For reviewing, the git branch has been updated with new commits without a squashing, so you can review the novelties. Comment Actions Build is green Updating workspaces. Build (release)... Build (debug)... Running debug tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK! http://jw:8080/job/phabricator/245/ for more details. Comment Actions I'd gladly take the NASM removal before the rest of this diff. Any work being done on upstreaming some of the modules (cxxtest, ...)? During the 4.4betaSomething and 5.0alpha (or maybe even beta) something days upstream seemed quite responsible and happy about contributions. Comment Actions Sure! I created D97 (and I could play with dependent revisions this way). I'll rebase this one tomorrow.
As far as I understand https://github.com/premake/premake-core/wiki/Sharing-Your-Module, the way to go would be having a GitHub repository (0ad/premake-cxxtest) and sharing it with the community, but it wouldn't be included in premake releases. It would be nice to do that I think, at least for getting feedback from premake devs, but I don't think our current implementation is generic enough to serve as a full-fledged CxxTest premake module. The first things I'd rather do would be sending PRs upstream to address the hack I made in the cxxtest module (and other little things I spotted). Comment Actions
Thanks, that feature might sound useful for a few things I do have lying around. Does this result in the dependency patches being applied first, or just to link diffs?
Ah, I guess it might still be useful to get it into a state where others could use it (and we could get distros to include that so those don't have to use a bundled binary (or compile it) just to build the game. (Sort of a long-term plan would be to require premake as a build dependency on non-win32 instead of bundling it.) Comment Actions Here is an updated version! I rebased, fixed a couple issues and made premake5 the default on Windows (support for VS2015 itself requires some work but I already did the most difficult part of it on a branch of mine). I also fixed wraitii's issue, which comes from a gitignore problem for which I warned upstream: https://github.com/premake/premake-core/issues/693 @wraitii : I get an issue with Objective-C when building on my Hackintosh after a successful premake run. Can you see if it's easily fixable? If it isn't I can take a look. I don't think it comes from clang, but from my dev environment itself, so I'd like to have this tested on a real Mac. Comment Actions Build is green Updating workspaces. Build (release)... Build (debug)... Running debug tests... Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK! http://jw:8080/job/phabricator/349/ for more details. Comment Actions Works under OSX with the following changes: I think the only include that needs to remain an includedirs instead of a sysincludedirs is the include to the PCH header and the include to source_root. Edit: what's your use of pig-config exactly? I notice it links against my Brew-SDL2 instead of the currently linked one for example. Edit: oh yeah and Atlas fails to compile, seems to be a linking issue, will check.
Comment Actions Found the atlas issue: it's related to linking system frameworks which has changed under premake5.
Comment Actions Would be nice if we could move to VS2015 soon and use the full range of C++11 features, good patch!
Comment Actions No, we don't, we still support 4.8.1. There was a fix, because this version doesn't support default/delete ctors/dtors in headers. Comment Actions Import the changes from the github branch. I also convinced myself we should switch to premake5 and keep an option to use the legacy premake4, instead of supporting premake5 and making it the default later. I'd still be in favor of using the --premake4 option in CI scripts so that developers can discover possible bugs without having our review process disrupted. @wraitii, can you summarize what are the remaining issues on OSX? Make sure you don't use your brew hacks at all, just the regular build system ? Comment Actions Build has FAILED Updating workspaces. Link to build: http://jw:8080/job/phabricator/1282/ Comment Actions
Sounds reasonable. Could you check what that build failure was, and also look into why those files aren't detected as mostly copies? (They were detected as such in some previous iteration.)
Comment Actions The failure was obviously because I don't attach premake5 to that differential, so making premake5 the default in it has a downside ? And I can't pass --premake4 to the CI scripts yet because the argument isn't implemented ?
Comment Actions I'll check the OSX version sometimes next week.
Comment Actions Created a PR on your GitHub for the required OSX changes, which are few. However wx-widgets won't link because wx-config returns broken linker flags (tested with brew AND with the provided libs, which BTW we need to update to 3.0.3). Specifically the "-framework" flags don't really work anymore, so we need to figure out something here, maybe a regex to remove them. Comment Actions Update to the latest version and include the planned change to the Jenkins script which should theoretically work. Comment Actions Build is green Updating workspaces...
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimation]’:
FCollada/FCDocument/FCDLibrary.cpp:149:30: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
const T* cptr = ((const FCDLibrary<T>*)l1)->GetEntity(0);
^
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDAnimationClip]’:
FCollada/FCDocument/FCDLibrary.cpp:150:34: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDCamera]’:
FCollada/FCDocument/FCDLibrary.cpp:151:27: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDController]’:
FCollada/FCDocument/FCDLibrary.cpp:152:31: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEffect]’:
FCollada/FCDocument/FCDLibrary.cpp:153:27: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDEmitter]’:
FCollada/FCDocument/FCDLibrary.cpp:154:28: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDForceField]’:
FCollada/FCDocument/FCDLibrary.cpp:155:31: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDGeometry]’:
FCollada/FCDocument/FCDLibrary.cpp:156:29: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDImage]’:
FCollada/FCDocument/FCDLibrary.cpp:157:26: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDLight]’:
FCollada/FCDocument/FCDLibrary.cpp:158:26: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:159:29: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDSceneNode]’:
FCollada/FCDocument/FCDLibrary.cpp:160:30: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsModel]’:
FCollada/FCDocument/FCDLibrary.cpp:161:33: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ set but not used [-Wunused-but-set-variable]
FCollada/FCDocument/FCDLibrary.cpp: In instantiation of ‘void LibraryExport() [with T = FCDPhysicsMaterial]’:
FCollada/FCDocument/FCDLibrary.cpp:162:36: required from here
FCollada/FCDocument/FCDLibrary.cpp:141:11: warning: variable ‘cptr’ shttp://jenkins-master:8080/job/phabricator/2176/ for more details. Comment Actions Executing section Default... Executing section Source... Executing section JS... http://jenkins-master:8080/job/phabricator_lint/640/ for more details.
Comment Actions Follow leper's suggestion.
Comment Actions Build is green Updating workspaces...
../../contrib/luashim/luashim.c: In function ‘shimInitialize’:
../../contrib/luashim/luashim.c:854:32: warning: multi-character character constant [-Wmultichar]
const Node* n = findNode(reg, 'SHIM');
^
In file included from ../../contrib/curl/include/curl/curl.h:2523:0,
from ../../src/host/curl_utils.h:15,
from ../../src/host/curl_utils.c:8:
../../src/host/curl_utils.c: In function ‘curlRequest’:
../../contrib/curl/include/curl/typecheck-gcc.h:56:9: warning: call to ‘_curl_easy_setopt_err_write_callback’ declared with attribute warning: curl_easy_setopt expects a curl_write_callback argument for this option
_curl_easy_setopt_err_write_callback(); \
^
../../src/host/curl_utils.c:175:2: note: in expansion of macro ‘curl_easy_setopt’
curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, curlWriteCallback);
^
../../contrib/curl/include/curl/typecheck-gcc.h:71:9: warning: call to ‘_curl_easy_setopt_err_progress_cb’ declared with attribute warning: curl_easy_setopt expects a curl_progress_callback argument for this option
_curl_easy_setopt_err_progress_cb(); \
^
../../src/host/curl_utils.c:181:3: note: in expansion of macro ‘curl_easy_setopt’
curl_easy_setopt(curl, CURLOPT_PROGRESSFUNCTION, curlProgressCallback);
^
../../src/host/premake.c: In function ‘premake_init’:
../../src/host/premake.c:194:36: warning: multi-character character constant [-Wmultichar]
lua_rawseti(L, LUA_REGISTRYINDEX, 'SHIM');
^
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (307 tests)...................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...http://jenkins-master:8080/job/phabricator/2185/ for more details. Comment Actions Executing section Default... Executing section Source... Executing section JS... http://jenkins-master:8080/job/phabricator_lint/642/ for more details.
| ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||