As noticed by fabio, system is not needed anymore to build the game on OSX.
Tested on OSX 10.9.5 Mavericks
Differential D1691
Remove boost "system" from Mac OS build system. Stan on Dec 9 2018, 3:21 PM. Authored by
Details
As noticed by fabio, system is not needed anymore to build the game on OSX. Tested on OSX 10.9.5 Mavericks Test it on other Mac versions, and verify everything still works fine.
Diff Detail
Event TimelineComment Actions Successful build - Chance fights ever on the side of the prudent. Link to build: https://jenkins.wildfiregames.com/job/differential/821/ Comment Actions This does seem to compile, but the change isn't complete, you need to remove the line in extern_libs5.lua in build/premake. Comment Actions
@fabio, seems that boost-system is still linked in the extern_libs5.lua for Unix. Maybe another patch ? Comment Actions Successful build - Chance fights ever on the side of the prudent. Link to build: https://jenkins.wildfiregames.com/job/differential/918/
Comment Actions @fabio @elexis @vladislavbelov can you try on linux ? I removed boost-system from there as well ? Comment Actions Successful build - Chance fights ever on the side of the prudent. Link to build: https://jenkins.wildfiregames.com/job/differential/942/ Comment Actions This doesn't work for me. Linking of pyrogenesis fails with undefined references to boost::system stuff. I will retry tomorrow with a clean checkout. Comment Actions Doesn't work for me either (Arch Linux, Boost 1.68). Looks like Linux systems need the boost_system library added explicitly. Comment Actions I think we do not need to include boost::system because we don't use anything from it, but we need to link it because boost::filesystem depends on it. I think only the build-osx change should be kept, as bootstrap is clever enough to build the boost::system lib while now excluding it from distributed includes. Comment Actions Thanks for testing @Itms and @s0600204 ! I will restore the patch to only touch Macos then. Would be nice if @trompetin17 and @vladislavbelov could test it. Comment Actions Successful build - Chance fights ever on the side of the prudent. Link to build: https://jenkins.wildfiregames.com/job/differential/947/ Comment Actions I'm not sure how useful this is. We're linking statically anyways and this is OSX only so... Comment Actions I though Mac OS had a tendency to link dynamically when it could ? It isn't a big change, it more of a cleanup. If you have a good argument against committing it, I can just abandon the revision :) Comment Actions I feel extremely "meh" about this :p Comment Actions Why not? system was already removed from Linux since it is no longer used, OS X should follow... Comment Actions I said system, which got removed from Linux times ago, as I wrote here: Comment Actions But when we removed system_mt from premake5.lua it failed to build on @Itms' computer, didn't it, It's in the comment above Comment Actions Both hunks are Mac OS :) I was just stating that since we didn't remove it for Linux it wasn't an argument for doing it on Mac ;) I don't see a problem with committing this but since @wraitii seems to think this is not worth it I wanted to know why. Comment Actions I tried to test this on macOS 10.14 Mojave, but couldn't get it to work.
==== Building pyrogenesis (release) ==== Linking pyrogenesis ld: warning: directory not found for option '-L/lib' Undefined symbols for architecture x86_64: "boost::system::system_category()", referenced from: boost::filesystem::detail::canonical(boost::filesystem::path const&, boost::filesystem::path const&, boost::system::error_code*) in libboost_filesystem-mt.a(operations.o) boost::filesystem::detail::symlink_status(boost::filesystem::path const&, boost::system::error_code*) in libboost_filesystem-mt.a(operations.o) boost::filesystem::detail::read_symlink(boost::filesystem::path const&, boost::system::error_code*) in libboost_filesystem-mt.a(operations.o) boost::filesystem::detail::copy(boost::filesystem::path const&, boost::filesystem::path const&, boost::system::error_code*) in libboost_filesystem-mt.a(operations.o) (anonymous namespace)::error(int, boost::filesystem::path const&, boost::filesystem::path const&, boost::system::error_code*, char const*) in libboost_filesystem-mt.a(operations.o) boost::filesystem::detail::create_directories(boost::filesystem::path const&, boost::system::error_code*) in libboost_filesystem-mt.a(operations.o) boost::filesystem::detail::create_directory(boost::filesystem::path const&, boost::system::error_code*) in libboost_filesystem-mt.a(operations.o) ... "boost::system::generic_category()", referenced from: boost::filesystem::detail::canonical(boost::filesystem::path const&, boost::filesystem::path const&, boost::system::error_code*) in libboost_filesystem-mt.a(operations.o) boost::filesystem::detail::create_directories(boost::filesystem::path const&, boost::system::error_code*) in libboost_filesystem-mt.a(operations.o) boost::filesystem::detail::permissions(boost::filesystem::path const&, boost::filesystem::perms, boost::system::error_code*) in libboost_filesystem-mt.a(operations.o) ld: symbol(s) not found for architecture x86_64 clang: error: linker command failed with exit code 1 (use -v to see invocation) make[1]: *** [../../../binaries/system/pyrogenesis] Error 1 make: *** [pyrogenesis] Error 2 There are also two compiler warnings that I don't recall seeing when building the game without this patch: <snip> glooxwrapper.cpp precompiled.cpp In file included from ../../../source/lobby/glooxwrapper/glooxwrapper.cpp:20: ../../../source/lobby/glooxwrapper/glooxwrapper.h:642:10: warning: private field 'm_Owned' is not used [-Wunused-private-field] bool m_Owned; ^ <snip> CCmpUnitMotion.cpp CCmpUnitRenderer.cpp ../../../source/simulation2/components/CCmpUnitMotion.cpp:1365:129: warning: unused parameter 'target' [-Wunused-parameter] bool CCmpUnitMotion::MoveToPointRange(entity_pos_t x, entity_pos_t z, entity_pos_t minRange, entity_pos_t maxRange, entity_id_t target) ^ <snip> Comment Actions Weird that it compiled when I tried above. Guessing my compilation setup is a bit too customised for these kind of patches.
Yup, it's there since rP22352. The parameter only ever was INVALID_ENTITY. I did notice the warning at some point, just need to make sure I don't forget about it. Concern can be raised so I don't (but I can't raise it myself). Comment Actions I'm abandoning this :) It probably can be reopened when we actually remove these references. |