Skip to content

Commit e7ead74

Browse files
authored
fix: stop compat.freetype and compat.zlib leaking GCC-only cflags to MSVC (#208)
Both declared compiler flags that only GCC and Clang understand in their COMMON cflags, so MSVC received them verbatim. Found while building XRGUI (Sunrisepeak/xrgui#3) with mcpp on windows-latest with MSVC 14.5x: cl : Command line error D8021 : invalid numeric argument '/Wno-implicit-function-declaration' That is compat.freetype, and it is fatal -- the package cannot build on Windows at all. Every consumer goes down with it: harfbuzz's FreeType bridge, msdfgen's ext/import-font, and anything drawing text. compat.zlib's is the quieter and worse-behaved half: cl : Command line warning D9002 : ignoring unknown option '-include' cl : Command line warning D9024 : unrecognized source file type 'mcpp_zlib_config.h', object file assumed cl does not reject `-include`; it warns, drops the flag, and builds. So mcpp_zlib_config.h was never included and the package compiled with a silently different configuration than the recipe describes. On Windows that header is empty by construction (everything in it is behind `#if !defined(_WIN32)`), so nothing was actually miscompiled this time -- but the mechanism would not have told us if it had been. THE FIX Both descriptors already had per-OS sections; the platform-specific flags now live in them. compat.freetype common cflags keep only the FT_* defines. cl accepts -D, so those reach MSVC unchanged. -Wno-implicit-function-declaration -> linux + macosx -D_DARWIN_C_SOURCE -> macosx. It was on every platform, which was wrong on Linux too, just harmlessly so. compat.zlib -D_GNU_SOURCE -> linux; -include mcpp_zlib_config.h -> linux + macosx. Windows needs neither. Also switched to the two-element {"-include", "file"} form the rest of the index uses, rather than one string with an embedded space. VERIFIED Linux: tests/examples/freetype, msdfgen and harfbuzz all still pass (1 passed, 0 failed each) -- msdfgen and harfbuzz because they are freetype's consumers and a fix that only satisfies Windows would be no fix at all. Windows: the point of the change; this repo's windows workspace leg builds tests/examples/freetype, and it only rebuilds members a PR touches -- which is why the defect survived until a project outside this repo pulled freetype on MSVC. THE SAME DEFECT, NOT TOUCHED HERE Sweeping every descriptor for platform-specific flags in common cflags turns up four more, all with a windows xpm section, so all reachable on MSVC today: compat.lua -include mcpp_lua_platform_config.h compat.godot-cpp -include cstdlib compat.redis-plus-plus -include cstdint compat.eui-neo -include mcpp_eui_backends.h, -fno-char8_t Left alone deliberately: each needs its own judgement about what the MSVC equivalent should be (/FI, /Zc:char8_t-) or whether the flag is needed there at all, and I have no evidence about those packages on Windows the way I do for these two. Flagging rather than blind-editing. (The X11 packages also carry -D_GNU_SOURCE in common cflags. cl accepts -D and those packages are Linux-only, so it is untidy rather than broken.)
1 parent 78bcbd6 commit e7ead74

2 files changed

Lines changed: 22 additions & 4 deletions

File tree

pkgs/c/compat.freetype.lua

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,24 @@ package = {
7676
},
7777
targets = { ["freetype"] = { kind = "lib" } },
7878
deps = { ["compat.libpng"] = "1.6.43" },
79-
cflags = { "-DFT2_BUILD_LIBRARY", "-DFT_DISABLE_ZLIB", "-DFT_DISABLE_BZIP2", "-DFT_DISABLE_HARFBUZZ", "-DFT_DISABLE_BROTLI", "-Wno-implicit-function-declaration", "-D_DARWIN_C_SOURCE" },
79+
-- Only the FT_* configuration defines are portable. `cl` accepts -D, so
80+
-- these reach MSVC unchanged; a -W switch does not -- see below.
81+
cflags = { "-DFT2_BUILD_LIBRARY", "-DFT_DISABLE_ZLIB", "-DFT_DISABLE_BZIP2", "-DFT_DISABLE_HARFBUZZ", "-DFT_DISABLE_BROTLI" },
8082
linux = {
8183
ldflags = { "-lm" },
8284
sources = { "*/builds/unix/ftsystem.c", "*/src/base/ftdebug.c" },
83-
cflags = { "-include", "fcntl.h" },
85+
-- -Wno-implicit-function-declaration is a GCC/Clang switch. It used
86+
-- to sit in the common cflags, where MSVC rejected it outright with
87+
-- `D8021: invalid numeric argument`, so the package could not build
88+
-- on Windows at all.
89+
cflags = { "-include", "fcntl.h", "-Wno-implicit-function-declaration" },
8490
},
8591
macosx = {
8692
ldflags = { "-lm" },
8793
sources = { "*/builds/unix/ftsystem.c", "*/src/base/ftdebug.c" },
88-
cflags = { "-include", "fcntl.h" },
94+
-- _DARWIN_C_SOURCE belongs here and nowhere else; it was in the
95+
-- common cflags, which put an Apple feature macro on every platform.
96+
cflags = { "-include", "fcntl.h", "-Wno-implicit-function-declaration", "-D_DARWIN_C_SOURCE" },
8997
},
9098
windows = {
9199
sources = {

pkgs/c/compat.zlib.lua

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,17 @@ package = {
4242
import_std = false,
4343
c_standard = "c11",
4444
include_dirs = {"*", "mcpp_generated/include"},
45-
cflags = { "-D_GNU_SOURCE", "-include mcpp_zlib_config.h" },
45+
-- Both of these are GCC/Clang-only and used to be unconditional.
46+
-- `-include` is the worse of the two on MSVC: cl does not reject it, it
47+
-- warns (`D9002 ignoring unknown option`) and carries on, so the config
48+
-- header was silently never included. The header only defines anything
49+
-- when _WIN32 is absent, so Windows needs neither.
50+
linux = {
51+
cflags = { "-D_GNU_SOURCE", "-include", "mcpp_zlib_config.h" },
52+
},
53+
macosx = {
54+
cflags = { "-include", "mcpp_zlib_config.h" },
55+
},
4656
generated_files = {
4757
["mcpp_generated/include/mcpp_zlib_config.h"] = "#ifndef MCPP_ZLIB_CONFIG_H\n#define MCPP_ZLIB_CONFIG_H\n#if !defined(_WIN32)\n#define Z_HAVE_UNISTD_H 1\n#endif\n#endif\n",
4858
},

0 commit comments

Comments
 (0)