haasn / libplacebo

Official mirror of libplacebo
http://libplacebo.org/
GNU Lesser General Public License v2.1
524 stars 63 forks source link

build failed with meson 1.2.1 #191

Closed nanake closed 10 months ago

nanake commented 10 months ago

F39 recently updated the meson to 1.2.1 and libplacebo build has failed with this update.

meson setup --prefix=/opt/ffbuild --buildtype=release -Ddefault_library=static -Dd3d11=enabled -Dvulkan=enabled -Dshaderc=enabled -Dbench=false -Ddemos=false -Dfuzz=false -Dtests=false -Dglslang=disabled -Dvk-proc-addr=disabled -Dvulkan-registry=/opt/ffbuild/share/vulkan/registry/vk.xml --cross-file=/cross.meson ..
The Meson build system
Version: 1.2.1
Source dir: /stage/pl
Build dir: /stage/pl/build
Build type: cross build
Project name: libplacebo
Project version: 6.309.0
C compiler for the host machine: x86_64-w64-mingw32ucrt-gcc (gcc 13.2.1 "x86_64-w64-mingw32ucrt-gcc (GCC) 13.2.1 20230728 (Fedora MinGW 13.2.1-5.fc39)")
C linker for the host machine: x86_64-w64-mingw32ucrt-gcc ld.bfd 2.40-4
C++ compiler for the host machine: x86_64-w64-mingw32ucrt-g++ (gcc 13.2.1 "x86_64-w64-mingw32ucrt-g++ (GCC) 13.2.1 20230728 (Fedora MinGW 13.2.1-5.fc39)")
C++ linker for the host machine: x86_64-w64-mingw32ucrt-g++ ld.bfd 2.40-4
C compiler for the build machine: cc (gcc 13.2.1 "cc (GCC) 13.2.1 20230728 (Red Hat 13.2.1-1)")
C linker for the build machine: cc ld.bfd 2.40-13
C++ compiler for the build machine: c++ (gcc 13.2.1 "c++ (GCC) 13.2.1 20230728 (Red Hat 13.2.1-1)")
C++ linker for the build machine: c++ ld.bfd 2.40-13
Build machine cpu family: x86_64
Build machine cpu: x86_64
Host machine cpu family: x86_64
Host machine cpu: x86_64
Target machine cpu family: x86_64
Target machine cpu: x86_64
Compiler for C supports arguments -Wincompatible-pointer-types: YES
Compiler for C supports link arguments -Wl,--exclude-libs=ALL: YES
Library m found: YES
Program python3 found: YES (/usr/bin/python3)
Found pkg-config: /usr/bin/pkg-config (1.9.5)
Found CMake: NO
Run-time dependency libunwind found: NO (tried pkgconfig and cmake)
Library execinfo found: NO
Checking for function "backtrace_symbols" with dependency -lexecinfo: NO
Check usable header "dbghelp.h" : YES
Library shlwapi found: YES
Fetching value of define "__MINGW32__" : 1
Program x86_64-w64-mingw32ucrt-dlltool found: YES
Run-time dependency shaderc found: YES 2023.6.0
Check usable header "d3d11.h" : YES
Check usable header "d3d11_4.h" : YES
Check usable header "dxgi1_6.h" : YES
Run-time dependency spirv-cross-c-shared found: YES 0.57.0
Library version found: YES
Header "dxgidebug.h" has symbol "IID_IDXGIInfoQueue" : YES
Header "d3d11sdklayers.h" has symbol "DXGI_DEBUG_D3D11" : YES
Run-time dependency vulkan found: YES 1.3.261
Header "vulkan/vulkan_core.h" has symbol "VK_VERSION_1_3" : YES
Run-time dependency lcms2 found: NO (tried pkgconfig and cmake)
Run-time dependency dovi found: NO (tried pkgconfig and cmake)
Configuring config.h using configuration
Configuring config_internal.h using configuration

../src/meson.build:217:6: ERROR: Cannot link_whole a custom or Rust target 'ucrt_math.lib' into a static library 'placebo'. Instead, pass individual object files with the "objects:" keyword argument if possible. Meson had to promote link to link_whole because 'placebo' is installed but not 'ucrt_math.lib', and thus has to include objects from 'ucrt_math.lib' to be usable.

maybe triggered by mesonbuild/meson@8f6fdb72fd099af1290953ccc272d80794d37bc3

DeadSix27 commented 10 months ago

Same issue here, I think you'd need to use objects in library(), which you'd get via ucrt_math.get_outputs()? I don't know enough about meson though.

haasn commented 10 months ago

@kasper93

kasper93 commented 10 months ago

https://code.videolan.org/videolan/libplacebo/-/merge_requests/532

nanake commented 10 months ago

https://code.videolan.org/videolan/libplacebo/-/merge_requests/532

libplacebo build fine with this PR but it can result in other problem with @BtbN BtbN/FFmpeg-Builds when build FFmpeg because a file libplacebo_umath.lib will be washed out after build (https://github.com/BtbN/FFmpeg-Builds/blob/0a5e19d524d224bfc3ee1d9678ae562bae4265b7/images/base/run_stage.sh#L22)

config.log

require_pkg_config libplacebo libplacebo >= 4.192.0 libplacebo/vulkan.h pl_vulkan_create
check_pkg_config libplacebo libplacebo >= 4.192.0 libplacebo/vulkan.h pl_vulkan_create
test_pkg_config libplacebo libplacebo >= 4.192.0 libplacebo/vulkan.h pl_vulkan_create
pkg-config --exists --print-errors libplacebo >= 4.192.0
check_func_headers libplacebo/vulkan.h pl_vulkan_create -I/opt/ffbuild/include -isystem/stage/placebo/3rdparty/Vulkan-Headers/include -DPL_STATIC -I/opt/ffbuild/include/spirv_cross -L/opt/ffbuild/lib -lplacebo -lm -lshlwapi -lversion -L/opt/ffbuild/bin -lplacebo_umath -lshaderc_combined -lspirv-cross-c -lspirv-cross-glsl -lspirv-cross-hlsl -lspirv-cross-reflect -lspirv-cross-msl -lspirv-cross-util -lspirv-cross-core -lstdc++
test_ld cc -I/opt/ffbuild/include -isystem/stage/placebo/3rdparty/Vulkan-Headers/include -DPL_STATIC -I/opt/ffbuild/include/spirv_cross -L/opt/ffbuild/lib -lplacebo -lm -lshlwapi -lversion -L/opt/ffbuild/bin -lplacebo_umath -lshaderc_combined -lspirv-cross-c -lspirv-cross-glsl -lspirv-cross-hlsl -lspirv-cross-reflect -lspirv-cross-msl -lspirv-cross-util -lspirv-cross-core -lstdc++
test_cc -I/opt/ffbuild/include -isystem/stage/placebo/3rdparty/Vulkan-Headers/include -DPL_STATIC -I/opt/ffbuild/include/spirv_cross -L/opt/ffbuild/lib -L/opt/ffbuild/bin
BEGIN /tmp/ffconf.GZHyaxKC/test.c
    1   #include <libplacebo/vulkan.h>
    2   #include <stdint.h>
    3   long check_pl_vulkan_create(void) { return (long) pl_vulkan_create; }
    4   int main(void) { int ret = 0;
    5    ret |= ((intptr_t)check_pl_vulkan_create) & 0xFFFF;
    6   return ret; }
END /tmp/ffconf.GZHyaxKC/test.c
x86_64-w64-mingw32ucrt-gcc -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -U__STRICT_ANSI__ -D__USE_MINGW_ANSI_STDIO=1 -D__printf__=__gnu_printf__ -D_POSIX_C_SOURCE=200112 -D_XOPEN_SOURCE=600 -DPIC -static-libgcc -static-libstdc++ -I/opt/ffbuild/include -O2 -pipe -D_FORTIFY_SOURCE=2 -fstack-protector-strong -std=c11 -fomit-frame-pointer -I/opt/ffbuild/include -pthread -I/opt/ffbuild/include -I/opt/ffbuild/include -I/opt/ffbuild/include/freetype2 -I/opt/ffbuild/include/libxml2 -DLIBXML_STATIC -I/opt/ffbuild/include/freetype2 -I/opt/ffbuild/include -I/opt/ffbuild/include/fribidi -DFRIBIDI_LIB_STATIC -I/opt/ffbuild/include/harfbuzz -I/opt/ffbuild/include/freetype2 -I/opt/ffbuild/include -I/opt/ffbuild/include -isystem/stage/placebo/3rdparty/Vulkan-Headers/include -DPL_STATIC -I/opt/ffbuild/include/spirv_cross -L/opt/ffbuild/lib -L/opt/ffbuild/bin -c -o /tmp/ffconf.GZHyaxKC/test.o /tmp/ffconf.GZHyaxKC/test.c
/tmp/ffconf.GZHyaxKC/test.c: In function 'check_pl_vulkan_create':
/tmp/ffconf.GZHyaxKC/test.c:3:44: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
    3 | long check_pl_vulkan_create(void) { return (long) pl_vulkan_create; }
      |                                            ^
x86_64-w64-mingw32ucrt-gcc -static-libgcc -static-libstdc++ -L/opt/ffbuild/lib -O2 -pipe -fstack-protector-strong -Wl,--nxcompat,--dynamicbase -Wl,--high-entropy-va -Wl,--as-needed -Wl,--pic-executable,-e,mainCRTStartup -Wl,--image-base,0x140000000 -I/opt/ffbuild/include -isystem/stage/placebo/3rdparty/Vulkan-Headers/include -DPL_STATIC -I/opt/ffbuild/include/spirv_cross -L/opt/ffbuild/lib -L/opt/ffbuild/bin -o /tmp/ffconf.GZHyaxKC/test.exe /tmp/ffconf.GZHyaxKC/test.o -lplacebo -lm -lshlwapi -lversion -lplacebo_umath -lshaderc_combined -lspirv-cross-c -lspirv-cross-glsl -lspirv-cross-hlsl -lspirv-cross-reflect -lspirv-cross-msl -lspirv-cross-util -lspirv-cross-core -lstdc++
/usr/lib/gcc/x86_64-w64-mingw32ucrt/13.2.1/../../../../x86_64-w64-mingw32ucrt/bin/ld: cannot find -lplacebo_umath: No such file or directory
collect2: error: ld returned 1 exit status
ERROR: libplacebo >= 4.192.0 not found using pkg-config

and yes, that's not your fault.

My question is, as states in commit, if it was never intended to be used when statically linking libplacebo, is it SAFE for me to just strip out -L${prefix}/bin -lplacebo_umath from a pc file so it can build into FFmpeg without modify BtbN build script?

kasper93 commented 10 months ago

libplacebo build fine with this PR but it can result in other problem with @BtbN BtbN/FFmpeg-Builds when build FFmpeg because a file libplacebo_umath.lib will be washed out after build

The problem was that meson strips lib prefix from import libs, which doesn't really work on Windows like that... I renamed the lib to not have prefix. ¯¯\(ツ)/¯ Works ok for me now, but I don't use BtbN scripts.

If BtbN scripts nuke bin dir, it will not work. But please give it a try with new version of patch, same MR. To make sure.

And if it fails, try with, installing it to libdir, while import libs are suppose to go into bin dir, we don't install .dll anyway, so it is fine.

diff --git a/src/meson.build b/src/meson.build
index 946f2f0b..29f692ff 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -29,7 +29,7 @@ if host_machine.system() == 'windows' and mingw32 != '' and host_machine.cpu() i
                             input : 'libplacebo_umath.def',
                             command : [dlltool, '-d', '@INPUT@', '-l', '@OUTPUT@'],
                             install: true,
-                            install_dir: get_option('bindir'))
+                            install_dir: get_option('libdir'))
   build_deps += declare_dependency(link_with: placebo_umath)
   # MinGW-w64 inlines functions like powf, rewriting them to pow. We want to use
   # the powf specialization from UCRT, so disable inlining.

My question is, as states in commit, if it was never intended to be used when statically linking libplacebo, is it SAFE for me to just strip out -L${prefix}/bin -lplacebo_umath from a pc file so it can build into FFmpeg without modify BtbN build script?

Yes, you can remove this https://code.videolan.org/videolan/libplacebo/-/blob/e9043c5d5089fda54814bc4749bb5625e1ab4e27/src/meson.build#L19-35 block of code from meson and it will work fine.

But I would like either fix it so it works for everyone, or remove it completely. So please help me testing ;)

nanake commented 10 months ago

But I would like either fix it so it works for everyone, or remove it completely. So please help me testing ;)

I'm happy to help :)

First, if I wash off -L${prefix}/bin -lplacebo_umat from pc file. It can build into ffmpeg successfully.

But with recent version merge request plus a patch from prior comment, there're symbol collisions with other package.

LD  ffmpeg_g.exe
LD  ffplay_g.exe
/usr/lib/gcc/x86_64-w64-mingw32ucrt/13.2.1/../../../../x86_64-w64-mingw32ucrt/bin/ld: /usr/lib/gcc/x86_64-w64-mingw32ucrt/13.2.1/../../../../x86_64-w64-mingw32ucrt/bin/ld/opt/ffbuild/lib/libzimg.a(libzimg_internal_la-libm_wrapper.o):libm_wrapper.c:(.text$expf[expf]+0x0): multiple definition of `expf'; /opt/ffbuild/lib/placebo_umath.lib(src_placebo_umath_lib_s00182.o):(.text+0x0): first defined here
/usr/lib/gcc/x86_64-w64-mingw32ucrt/13.2.1/../../../../x86_64-w64-mingw32ucrt/bin/ld: /opt/ffbuild/lib/libzimg.a(libzimg_internal_la-libm_wrapper.o):libm_wrapper.c:(.text$powf[powf]+0x0): multiple definition of `powf'; /opt/ffbuild/lib/placebo_umath.lib(src_placebo_umath_lib_s00255.o):(.text+0x0): first defined here
: /opt/ffbuild/lib/libzimg.a(libzimg_internal_la-libm_wrapper.o):libm_wrapper.c:(.text$expf[expf]+0x0): multiple definition of `expf'; /opt/ffbuild/lib/placebo_umath.lib(src_placebo_umath_lib_s00182.o):(.text+0x0): first defined here
/usr/lib/gcc/x86_64-w64-mingw32ucrt/13.2.1/../../../../x86_64-w64-mingw32ucrt/bin/ld: /opt/ffbuild/lib/libzimg.a(libzimg_internal_la-libm_wrapper.o):libm_wrapper.c:(.text$powf[powf]+0x0): multiple definition of `powf'; /opt/ffbuild/lib/placebo_umath.lib(src_placebo_umath_lib_s00255.o):(.text+0x0): first defined here
collect2: error: ld returned 1 exit status
make: *** [Makefile:133: ffmpeg_g.exe] Error 1
make: *** Waiting for unfinished jobs....
collect2: error: ld returned 1 exit status
make: *** [Makefile:133: ffplay_g.exe] Error 1
Error: Process completed with exit code 2.
kasper93 commented 10 months ago

Ah, zimg. It is funny, because they export math functions (too). And if fact all symbols. It was fixed in MSYS2 https://github.com/msys2/MINGW-packages/commit/92d0f0f63f0f42760eae61a71be04d23ab6eb9e6, but rejected upstream https://github.com/sekrit-twc/zimg/pull/200.

Anyway, this is what I expected, we cannot really as libplacebo force imports of math functions when statically linking. It is just not nice.

BtbN commented 10 months ago

Import-Libs very much do get installed into .../lib. The corresponding .dll ends up in .../bin, but its .dll.a or .lib import-lib goes into the lib dir.

Also, is this issue with the math functions even still a thing? Looking at the mingw ucrt implibs, it seems to very much export them all?

kasper93 commented 10 months ago

Import-Libs very much do get installed into .../lib. The corresponding .dll ends up in .../bin, but its .dll.a or .lib import-lib goes into the lib dir.

At least meson by default install .lib alongside .dll in /bin dir. So I tried to mimic that. But installing in /lib is ok too.

Also, is this issue with the math functions even still a thing? Looking at the mingw ucrt implibs, it seems to very much export them all?

Yes. Take a look at https://github.com/mingw-w64/mingw-w64/blob/2a7e9255dfe46be06cd45bce91c5aa2e8bc1be25/mingw-w64-crt/lib-common/ucrtbase.def.in Those functions marked with DATA will use statically linked versions form libmingwex.a. You search for sin, cos, pow... they are all not imported from UCRT.

BtbN commented 10 months ago

That's strange. I wonder why they opted to do that. Are there some flavours of Windows that are lacking those functions? In any case, it seems to mainly affect 32bit, and I'm not too hard-pressed to just accept that 32bit is slow. That's kinda what you sign up for when not using 64bit.

kasper93 commented 10 months ago

That's strange. I wonder why they opted to do that. Are there some flavours of Windows that are lacking those functions?

Probably historical reasons and things like rounding mode. UCRT support evolved a lot over recent years, maybe this could be improved too.

In any case, it seems to mainly affect 32bit, and I'm not too hard-pressed to just accept that 32bit is slow. That's kinda what you sign up for when not using 64bit.

It affects x86 in generaly. F_X86_ANY(DATA) means x86, both 32-bit and 64-bit. F_X64 is for 64-bit and F_I386 is for 32-bit.

I wouldn't care about 32-bit build at all.

xclaesse commented 10 months ago

I think the root cause here is Meson thinks that ucrt_math.lib is a static library, because there is no way to distinguish an import library and static library with that name. I would suggest to rename it to ucrt_math.dll.lib or ucrt_math.dll.a and then Meson could know it's not a static library. We could then get it fixed in Meson 1.2.2 with https://github.com/mesonbuild/meson/pull/12121.

Note that you always had a warning about this, and Meson 1.2.1 turned that to an hard error, because uninstalled static libraries caused other issues.

BtbN commented 10 months ago

An import library for all intents and purposes is a static library. It's a static library with a shim for every function, which then under the hood loads those functions from the DLL on load/call. So there's no difference in regards to how it functions.

xclaesse commented 10 months ago

While on paper that's true, in practice there is a big difference, otherwise libplacebo.a would not be usable at all without installing ucrt_math.lib.

BtbN commented 10 months ago

It's usable because the linker implicitly has another import lib implicitly linked in, the mingw/toolchain one. The one supplied by libplacebo just takes precedence over it.

xclaesse commented 10 months ago

Note that it's not only an issue with Meson, it also fails with at least mingw from Ubuntu 23.04: https://github.com/mesonbuild/meson/pull/12011#issuecomment-1677301571

BtbN commented 10 months ago

I think the only correct solution is to just not use that ucrt math shim when building a static library. Since it will cause other things linked with it to then also use those separate math functions, which they might not be fine with.

The workaround to all these issues would be to manually write an import-lib which dlopens the repective library and then loads the symbols via dlsym, and then use those manually loaded functions directly.

kasper93 commented 10 months ago

I think the only correct solution is to just not use that ucrt math shim when building a static library.

Correct, but as far as I can tell when building -Ddefault_library=both there is no control over parameters for each version. For other cases we could do get_option('default_library') and just exude offending part.

The workaround to all these issues would be to manually write an import-lib which dlopens the repective library and then loads the symbols via dlsym, and then use those manually loaded functions directly.

I know, but the point for the beginning was to have pretty minimal (little bit hacky) way to override math functions. It is not worth extending this to something bigger than few lines on code in meson. And we spent today too much time on this already, so I just removed it. And all our issue will be gone.

https://code.videolan.org/videolan/libplacebo/-/merge_requests/532

xclaesse commented 10 months ago

I think this works?

diff --git a/src/meson.build b/src/meson.build
index 4a7dd6bc..93d6d8d0 100644
--- a/src/meson.build
+++ b/src/meson.build
@@ -15,6 +15,9 @@ elif has_execinfo
   build_deps += libexecinfo
 endif

+link_args = []
+link_depends = []
+
 # Looks like meson in certain configuration returns ' ' instead of empty string
 mingw32 = cc.get_define('__MINGW32__').strip()
 if host_machine.system() == 'windows' and mingw32 != '' and host_machine.cpu() in ['aarch64', 'arm', 'x86_64']
@@ -28,7 +31,8 @@ if host_machine.system() == 'windows' and mingw32 != '' and host_machine.cpu() i
                             output : ['ucrt_math.lib'],
                             input : 'ucrt_math.def',
                             command : [dlltool, '-d', '@INPUT@', '-l', '@OUTPUT@'])
-  build_deps += declare_dependency(link_with: ucrt_math)
+  link_args += ucrt_math.full_path()
+  link_depends = ucrt_math
   # MinGW-w64 inlines functions like powf, rewriting them to pow. We want to use
   # the powf specialization from UCRT, so disable inlining.
   add_project_arguments(['-D__CRT__NO_INLINE'], language: ['c', 'cpp'])
@@ -221,6 +225,7 @@ lib = library('placebo', sources,
   soversion: apiver,
   include_directories: inc,
   link_args: link_args,
+  link_depends: link_depends,
   gnu_symbol_visibility: 'hidden',
   name_prefix: 'lib'
 )

Note that link_args was already passed to library() but was not needed because it already does add_project_link_arguments(link_args, language: 'c'). So it's fine to override it here.

kasper93 commented 10 months ago

My brain is split in 3 threads about the same.

Note that it's not only an issue with Meson, it also fails with at least mingw from Ubuntu 23.04: mesonbuild/meson#12011 (comment)

This is already fixed and not related to specifics of libplacebo. https://sourceware.org/bugzilla/show_bug.cgi?id=30079 https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=b7eab2a9d4f4e92692daf14b09fc95ca11b72e30

I think this works?

Like mentioned in other thread, yes this works. Thanks for the idea about link_depends this is what was missing. Now we can directly inject link_args and trigger custom_target build.

nanake commented 10 months ago

I can confirm this is fixed for me in latest version MR532. 🎉