jtothebell / fake-08

A Pico-8 player/emulator for console homebrew
Other
583 stars 50 forks source link

Build errors with MinGW, easy to fix but I'm not sure how to handle the #ifdef properly :/ #209

Closed bslenul closed 1 year ago

bslenul commented 1 year ago

Hello!

So yesterday I wanted to mess a bit with the Libretro core but couldn't build it (for Windows, using MinGW), first there was some includes missing due to GCC13 (now partially fixed thanks to #208, I still had to add one more in libretrohosthelpers.h) but then I'm getting this:

../../platform/libretro/libretrohost.cpp: In member function 'void Host::overrideLogFilePrefix(const
 char*)':
../../platform/libretro/libretrohost.cpp:130:14: error: too many arguments to function 'int mkdir(co
nst char*)'
  130 |         mkdir(cartdatadir.c_str(), 0777);
      |         ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from G:/msys64/mingw64/include/sys/stat.h:14,
                 from ../../platform/libretro/libretrohost.cpp:6:
G:/msys64/mingw64/include/io.h:282:15: note: declared here
  282 |   int __cdecl mkdir (const char *) __MINGW_ATTRIB_DEPRECATED_MSVC2005;
      |               ^~~~~

I'm building on Windows with MinGW, so I simply changed:

    if (stat(cartdatadir.c_str(), &st) == -1) {
        mkdir(cartdatadir.c_str(), 0777);
    }

to:

    if (stat(cartdatadir.c_str(), &st) == -1) {
#ifdef _WIN32
        mkdir(cartdatadir.c_str());
#else
        mkdir(cartdatadir.c_str(), 0777);
#endif
    }

and it built just fine, I wanted to submit a PR but I'm not sure about the define, is this correct? Should it be __MINGW32__ instead? Idk, I'm way too noob at this :/

And same thing happens when I try to build standalone:

G:/msys64/home/B-S/fake-08/platform/windows/source/WindowsHost.cpp: In constructor 'Host::Host()':
G:/msys64/home/B-S/fake-08/platform/windows/source/WindowsHost.cpp:60:20: error: too many arguments
to function 'int mkdir(const char*)'
   60 |         res = mkdir(_desktopSdl2SettingsDir.c_str(), 0777);
      |               ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from G:/msys64/mingw64/include/dirent.h:15,
                 from G:/msys64/home/B-S/fake-08/platform/windows/source/WindowsHost.cpp:4:
G:/msys64/mingw64/include/io.h:282:15: note: declared here
  282 |   int __cdecl mkdir (const char *) __MINGW_ATTRIB_DEPRECATED_MSVC2005;
      |               ^~~~~
G:/msys64/home/B-S/fake-08/platform/windows/source/WindowsHost.cpp:67:20: error: too many arguments
to function 'int mkdir(const char*)'
   67 |         res = mkdir(cartdatadir.c_str(), 0777);
      |               ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~
G:/msys64/mingw64/include/io.h:282:15: note: declared here
  282 |   int __cdecl mkdir (const char *) __MINGW_ATTRIB_DEPRECATED_MSVC2005;
      |               ^~~~~
G:/msys64/home/B-S/fake-08/platform/windows/source/WindowsHost.cpp:72:20: error: too many arguments
to function 'int mkdir(const char*)'
   72 |         res = mkdir(cartdir.c_str(), 0777);
      |               ~~~~~^~~~~~~~~~~~~~~~~~~~~~~
G:/msys64/mingw64/include/io.h:282:15: note: declared here
  282 |   int __cdecl mkdir (const char *) __MINGW_ATTRIB_DEPRECATED_MSVC2005;
      |               ^~~~~

And again, it builds fine if I use the #fidef _WIN32.

jtothebell commented 1 year ago

I think#ifdef _WIN32 works, but I'm not familiar with building C/C++ on windows either. But looking at libretro code and a couple other projects, that looks like the right thing to do. At present I don't have a windows machine to check, but I did get the libretro core building on windows at one point and I think what you have described sounds like what I did.

bslenul commented 1 year ago

Should I send a PR or you want to test for yourself first and/or find a better alternative maybe?

jtothebell commented 1 year ago

Found my old branch and pushed it. Does https://github.com/jtothebell/fake-08/tree/libretro-windows work for you? I got it building for someone else and they said it was working, but I don't know how thoroughly it was tested.

bslenul commented 1 year ago

#include <cstdint> (or , idk the difference) required in this file: https://github.com/jtothebell/fake-08/blob/libretro-windows/platform/libretro/libretrohosthelpers.h but otherwise it builds and works fine for me now, yes 👍

edit: and putting that same #ifdef block inside https://github.com/jtothebell/fake-08/blob/libretro-windows/source/filehelpers.h let me build standalone as well, might want to add that too ;)

jtothebell commented 1 year ago

Alright, I moved that #ifdef block to host.h since mkdir should only be getting called from host implementations, and that should work for both libretro builds and standalone builds. If that is working, I'll merge that change in.

bslenul commented 1 year ago

Yep all good now, both Libretro and standalone!

Thank you! ❤️

jtothebell commented 1 year ago

@bslenul Can you give me any more details about how you're building with MinGW for windows? I tried today to build the libretro core, and it builds fine, but crashes when I try to load a game (I don't see a smoking gun in the logs, but if I pepper some extra logging in it looks like its having trouble with the call to mkdir). I installed MSYS2 as recommended by the "Windows 7 and later compilation and development guide" in the libretro docs, then tried using MinGW-W64 and MinGW-W32 shells but the builds from both crash RetroArch.

The odd thing is that I have an old build from the branch I mentioned above on this same machine, and that one works still, but I can't remember what I might have done differently back then, so I was hoping you might be able to help. Thanks!

bslenul commented 1 year ago

I installed MSYS2 as recommended by the "Windows 7 and later compilation and development guide" in the libretro docs

Yeah I followed the same guide, then idk I didn't do anything special, just cd to platform/libretro/ then make and that's it. Then in RetroArch: Main Menu > Load Content, select a .p8 file and it works just fine for me.

It seems to crash when loading the core without content however (so Main Menu > Load Core > FAKE-08 > Start Core), looks like it's because of info being NULL in retro_load_game(), it crashes here: https://github.com/jtothebell/fake-08/blob/46cdbcf44582ebebdfe6be417cabd5041e9bfc34/platform/libretro/libretro.cpp#L621

But in any case I don't seem to have any issue with mkdir anymore 🤔

jtothebell commented 1 year ago

@bslenul Well, I'm still not sure what is wrong with that windows machine, but I tried it on someone else's and it worked fine, so I set up a CI build in github actions and that output also seems to be working. I also threw in a quick fix to that crash. It just loads the cart list cart, which isn't really useful as configured, but at least it doesn't crash.