luvit / luv

Bare libuv bindings for lua
Apache License 2.0
823 stars 185 forks source link

MINGW64 stat defines interfere with FS_CALL macro expansion #624

Closed rdw-software closed 1 year ago

rdw-software commented 1 year ago

When trying to build luv with gcc on MSYS2/MINGW64 (Windows), the uv_fs_stat and uv_fs_fstat APIs aren't exported.

This is likely due to some MINGW defines for stat and fstatmessing up the FS_CALL (and FS_CALL_NORETURN) macros in src/fs.c: Instead of uv_fs_stat the output becomes uv_fs__stat64 and uv_fs_fstat becomes uv_fs__fstat64.

More context: https://github.com/msys2/MINGW-packages/issues/10591


Steps to reproduce:

  1. Start MSYS2 bash with MINGW64 system (toolchain was set up via pacman -S git make mingw-w64-x86_64-gcc ninja mingw-w64-x86_64-cmake --noconfirm)
  2. git clone https://github.com/luvit/luv.git --recursive
  3. cd luv
  4. cmake -S . -B cmakebuild-msys2 -G Ninja -DBUILD_MODULE=OFF -DBUILD_SHARED_LIBS=OFF -DBUILD_STATIC_LIBS=ON
  5. cmake --build cmakebuild-msys2 --clean-first

Expected result: All compilation steps succeed without issues

Actual result: The following warning appears:

[55/59] Building C object CMakeFiles/libluv_a.dir/src/luv.c.obj
In file included from E:/GitHub/Forks/luv/src/luv.c:28:
E:/GitHub/Forks/luv/src/fs.c: In function 'luv_fs_stat':
E:/GitHub/Forks/luv/src/fs.c:433:9: warning: implicit declaration of function 'uv_fs__stat64'; did you mean 'uv_fs_statfs'? [-Wimplicit-function-declaration]
  433 |   ret = uv_fs_##func(data->ctx->loop, req, __VA_ARGS__,   \
      |         ^~~~~~
E:/GitHub/Forks/luv/src/fs.c:433:9: note: in definition of macro 'FS_CALL_NORETURN'
  433 |   ret = uv_fs_##func(data->ctx->loop, req, __VA_ARGS__,   \
      |         ^~~~~~
E:/GitHub/Forks/luv/src/fs.c:629:3: note: in expansion of macro 'FS_CALL'
  629 |   FS_CALL(stat, req, path);
      |   ^~~~~~~
E:/GitHub/Forks/luv/src/fs.c: In function 'luv_fs_fstat':
E:/GitHub/Forks/luv/src/fs.c:433:9: warning: implicit declaration of function 'uv_fs__fstat64'; did you mean 'uv_fs_fstat'? [-Wimplicit-function-declaration]
  433 |   ret = uv_fs_##func(data->ctx->loop, req, __VA_ARGS__,   \
      |         ^~~~~~
E:/GitHub/Forks/luv/src/fs.c:433:9: note: in definition of macro 'FS_CALL_NORETURN'
  433 |   ret = uv_fs_##func(data->ctx->loop, req, __VA_ARGS__,   \
      |         ^~~~~~
E:/GitHub/Forks/luv/src/fs.c:638:3: note: in expansion of macro 'FS_CALL'
  638 |   FS_CALL(fstat, req, file);
      |   ^~~~~~~

Needless to say, the resulting library is broken (undefined reference to `uv_fs__stat64' error when trying to link with it).


To confirm the problem is as I suspected, I applied a naive "hackfix" to fs.c.

Insert before definitions of luv_fs_stat and luv_fs_fstat:

#ifdef stat
#define MINGW_STAT_BACKUP stat
#undef stat
#endif

#ifdef fstat
#define MINGW_FSTAT_BACKUP fstat
#undef fstat
#endif

Then I reset the defines afterwards:

#ifdef MINGW_STAT_BACKUP
#define stat MINGW_STAT_BACKUP
#undef MINGW_STAT_BACKUP
#endif

#ifdef MINGW_FSTAT_BACKUP
#define fstat MINGW_FSTAT_BACKUP
#undef MINGW_FSTAT_BACKUP
#endif

Not exactly a great solution, but it should illustrate the nature of the problem.

Happy to submit a PR if you can suggest a better approach - or maybe this is fine?

squeek502 commented 1 year ago

An alternative that avoids any hacks would be to ditch the concatenation in the macro and have it take the full function name, i.e.

- ret = uv_fs_##func(data->ctx->loop, req, __VA_ARGS__,
+ ret = func(data->ctx->loop, req, __VA_ARGS__,

and then at each FS_CALL usage:

- FS_CALL(open, req, path, flags, mode);
+ FS_CALL(uv_fs_open, req, path, flags, mode);

func is only used at that one spot in the FS_CALL macro so there's no real benefit to having it only take the part after the uv_fs_ prefix.