Wargus / wargus

Importer and scripts for Warcraft II: Tides of Darkness, the expansion Beyond the Dark Portal, and Aleonas Tales
GNU General Public License v2.0
355 stars 55 forks source link

Issue building StormLib due to issue in LibTomCrypt using Clang 13 on macOS Monterey #396

Closed ghost closed 2 years ago

ghost commented 2 years ago

Describe the bug When building Wargus on macOS Monterey with clang 13 there is a compilation error during the build of StormLib.

alexbenedict@Inanna2 build % cmake .. -DSTRATAGUS_INCLUDE_DIR=../../stratagus/gameheaders -DSTRATAGUS=/usr/local/games/stratagus
-- Could not find StormLib
-- Attempting to build StormLib submodule
src/libtomcrypt/src/hashes/hash_memory.c:45:10: error: implicit declaration of function 'LibTomMalloc' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    md = XMALLOC(sizeof(hash_state));
         ^
src/libtomcrypt/src/hashes/../headers/tomcrypt_custom.h:27:18: note: expanded from macro 'XMALLOC'
#define XMALLOC  LibTomMalloc
                 ^
src/libtomcrypt/src/hashes/hash_memory.c:45:8: warning: incompatible integer to pointer conversion assigning to 'hash_state *' (aka 'union Hash_state *') from 'int' [-Wint-conversion]
    md = XMALLOC(sizeof(hash_state));
       ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/libtomcrypt/src/hashes/hash_memory.c:62:5: error: implicit declaration of function 'LibTomFree' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
    XFREE(md);
    ^
src/libtomcrypt/src/hashes/../headers/tomcrypt_custom.h:45:18: note: expanded from macro 'XFREE'
#define XFREE    LibTomFree
                 ^
1 warning and 2 errors generated.
make: *** [src/libtomcrypt/src/hashes/hash_memory.o] Error 1
-- StormLib NOT built successfully
-- Configuring done
-- Generating done
-- Build files have been written to: /Users/alexbenedict/repos/wargus/build

To Reproduce Steps to reproduce the behavior:

  1. Install macports for dependencies.
  2. Install dependencies from macports:
    sudo port install toluapp
    sudo port install lua51
    sudo port install libsdl2 (and similar for other sdl libs)
    sudo port install clang-13 (Needed this for Stratagus to build and not have linker errors)
  3. Build Stratagus (had to minorly modify the cmake file for libSDL2main.a to be able to link as well).
  4. Run cmake .. -DSTRATAGUS_INCLUDE_DIR=../../stratagus/gameheaders -DSTRATAGUS=/usr/local/games/stratagus
  5. Get error message.

Expected behavior cmake to run and then I would be able to run make.

Screenshots and Logs See above.

Desktop (please complete the following information):

ghost commented 2 years ago

I have found a very similar issue here: https://githubmemory.com/repo/ladislav-zezula/StormLib/issues/222, which looks like it was resolved about 6 months ago in StormLib. Maybe updating the version of StormLib used, would be good as well. I can try testing that later in the week and submit a pull request if things build and seem reasonable.

timfel commented 2 years ago

Sounds good, we only update the StormLib version when we find issues, so if you can confirm a new version will fix yours, then I'm happy to merge a PR :)

ghost commented 2 years ago

Moving to a new Stormlib seems to bring other issues due to the Makefile.mac being used to build it. I can build StormLib without issue from a fresh clone of StormLib using cmake, but when it is built as part of building Wargus, it has issues.

I am not very familiar with cmake, but I expect I will need to change this part of the CMakeLists.txt:

elseif (${CMAKE_SYSTEM_NAME} STREQUAL Darwin)
          message(STATUS "Attempting to build StormLib submodule")
          execute_process(COMMAND ${MAKE} -f Makefile.mac WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/StormLib TIMEOUT 240 RESULT_VARIABLE STORM_LIB_MAKE_RESULT OUTPUT_QUIET)
        endif()

to just try to run cmake and then make. I should have some time later when I can look up how to do that.

ghost commented 2 years ago

It might make sense to build StormLib as an ExternalProject, I am basing this off of (https://stackoverflow.com/questions/56528011/cmake-force-install-after-add-subdirectory) and (https://cmake.org/cmake/help/latest/module/ExternalProject.html), since that should allow StormLib to be built as part of the build process for Wargus in a way that only requires cmake and not externally generated make files, which ought to make it easier for porting to other systems.

Since cmake is needed to build Wargus anyway, it might make sense to make this change more broadly(for non-macOS platforms like linux) since StormLib officially supports building using cmake.