Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update worker dependencies #1201

Merged
merged 18 commits into from
Dec 4, 2023
Merged

Update worker dependencies #1201

merged 18 commits into from
Dec 4, 2023

Conversation

jmillan
Copy link
Member

@jmillan jmillan commented Oct 27, 2023

Not only in order to use a newer version (we can use a newer abseil version now that we use a newer C++ version), but also because previous abseil would emit a lot of warnings with clang 15.

@jmillan jmillan changed the title Update abseil to 20230125.1-5 Update worker dependencies Oct 27, 2023
@ibc
Copy link
Member

ibc commented Oct 27, 2023

CI fails 😖

@jmillan
Copy link
Member Author

jmillan commented Oct 27, 2023

I'll check after the weekend

@ibc
Copy link
Member

ibc commented Oct 27, 2023

Compiling this branch in my macbook Intel 2019 I get some warnings:

[160/311] Linking static target subprojects/abseil-cpp-20230125.1/libabsl_debugging.a
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: subprojects/abseil-cpp-20230125.1/libabsl_debugging.a(absl_debugging_internal_elf_mem_image.cc.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: subprojects/abseil-cpp-20230125.1/libabsl_debugging.a(absl_debugging_internal_stack_consumption.cc.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: subprojects/abseil-cpp-20230125.1/libabsl_debugging.a(absl_debugging_internal_vdso_support.cc.o) has no symbols

but it does compile.

@jmillan
Copy link
Member Author

jmillan commented Oct 30, 2023

The interesting part here is that the error happens only building mediasoup-sys for Rust. Mediasoup worker is compiling and linking properly.

@nazar-pc
Copy link
Collaborator

Rust also links standard C++ library, it does so differently on different platforms, something might be off with that.

@jmillan
Copy link
Member Author

jmillan commented Nov 22, 2023

Rust also links standard C++ library, it does so differently on different platforms, something might be off with that.

@nazar-pc do you happen to have any idea on how to overcome this problem?

Using the abseil version in v3 with clang-15 is so noisy (many deprecation messages) that we are using clang-14 back, rather than the newer clang-15 in local dev just to avoid all the noise. Using the newer abseil version does fix those warnings but comes with the issues shown here.

@jmillan
Copy link
Member Author

jmillan commented Nov 22, 2023

I've just updated to a newer abseil version that is available in wrapdb.

@nazar-pc
Copy link
Collaborator

nazar-pc commented Nov 22, 2023

Not sure off top of my head, will have to tinker with it. All we do is simply linking it.

multiple definition of main

is because because someone updated abseil in Meson incorrectly AGAIN by including test files with main functions in library code 😞

@eli-schwartz I fixed this for abseil several times in wraps already, maybe there is some sort of automation that can be done to prevent this in the future? I guess writing a test case.

@eli-schwartz
Copy link

Yes, if the wrap could include an example or test that gets built by the wrapdb and fails to link when this porting mistake is made, that would be fantastic.

@ibc
Copy link
Member

ibc commented Nov 22, 2023

is because because someone updated abseil in Meson incorrectly AGAIN by including test files with main functions in library code

So this can be fixed by publishing a new path version in Meson with those tests removed, right?

@nazar-pc
Copy link
Collaborator

Yes, and ideally we need to write a test that will run in Meson's CI that will ensure extra main functions will not appear in libraries anymore.

@ibc
Copy link
Member

ibc commented Nov 22, 2023

Yes, and ideally we need to write a test that will run in Meson's CI that will ensure extra main functions will not appear in libraries anymore.

I see. And when you say "we", do you mean you? or We The Team©?

@nazar-pc
Copy link
Collaborator

I mean you're welcome to do that for sure, otherwise I'll have to 😂

@jmillan
Copy link
Member Author

jmillan commented Nov 22, 2023

It seems that the culprit is this file here:

https://github.com/mesonbuild/wrapdb/blob/master/subprojects/packagefiles/abseil-cpp/meson.build#L258
https://github.com/abseil/abseil-cpp/blob/master/absl/hash/internal/print_hash_of.cc#L4

I can check it and create a new wrapdb version tomorrow if it's not done before by you.

@ibc
Copy link
Member

ibc commented Nov 22, 2023

This is the patch file of the latest flatbuffers-23.3.3 wrap file in Meson DB:

project('flatbuffers', 'cpp', version: '23.3.3', license: 'Apache-2.0', meson_version: '>=0.49.0')

cpp = meson.get_compiler('cpp')

if host_machine.system() == 'windows'
  add_project_arguments('-D_CRT_SECURE_NO_WARNINGS', language: 'cpp')
endif

add_project_arguments('-DFLATBUFFERS_LOCALE_INDEPENDENT=@0@'.format(cpp.has_function('strtoull_l')), language: 'cpp')

# Certain platforms such as ARM do not use signed chars by default
# which causes issues with certain bounds checks.
add_project_arguments(cpp.get_supported_arguments('-fsigned-char'), language: 'cpp')

includes = include_directories('include', 'grpc')

headers = files(
  'include/flatbuffers/allocator.h',
  'include/flatbuffers/array.h',
  'include/flatbuffers/base.h',
  'include/flatbuffers/buffer.h',
  'include/flatbuffers/buffer_ref.h',
  'include/flatbuffers/code_generators.h',
  'include/flatbuffers/default_allocator.h',
  'include/flatbuffers/detached_buffer.h',
  'include/flatbuffers/flatbuffers.h',
  'include/flatbuffers/flatc.h',
  'include/flatbuffers/flex_flat_util.h',
  'include/flatbuffers/flexbuffers.h',
  'include/flatbuffers/grpc.h',
  'include/flatbuffers/hash.h',
  'include/flatbuffers/idl.h',
  'include/flatbuffers/minireflect.h',
  'include/flatbuffers/reflection.h',
  'include/flatbuffers/reflection_generated.h',
  'include/flatbuffers/registry.h',
  'include/flatbuffers/stl_emulation.h',
  'include/flatbuffers/string.h',
  'include/flatbuffers/struct.h',
  'include/flatbuffers/table.h',
  'include/flatbuffers/util.h',
  'include/flatbuffers/vector.h',
  'include/flatbuffers/vector_downward.h',
  'include/flatbuffers/verifier.h',
)

# flatbuffers
install_headers(
  headers,
  subdir: 'flatbuffers',
)

flatbuffers_lib_sources = files(
  'src/code_generators.cpp',
  'src/idl_gen_binary.cpp',
  'src/idl_gen_text.cpp',
  'src/idl_parser.cpp',
  'src/reflection.cpp',
  'src/util.cpp',
)

#shared libraries not supported with msvc
flatbuffers_lib = build_target(
  'flatbuffers',
  flatbuffers_lib_sources,
  target_type: cpp.get_argument_syntax() == 'msvc' ? 'static_library' : 'library',
  include_directories: includes,
  install: true,
)

flatbuffers_dep = declare_dependency(
  include_directories: includes,
  link_with: flatbuffers_lib,
)

# grpc libraries
cpp_gen_lib = static_library(
  'cpp_generator',
  'grpc/src/compiler/cpp_generator.cc',
  include_directories: includes,
)

# grpc libraries
go_gen_lib = static_library(
  'go_generator',
  'grpc/src/compiler/go_generator.cc',
  include_directories: includes,
)

java_gen_lib = static_library(
  'java_generator',
  'grpc/src/compiler/java_generator.cc',
  include_directories: includes,
)

python_gen_lib = static_library(
  'python_generator',
  'grpc/src/compiler/python_generator.cc',
  include_directories: includes,
)

swift_gen_lib = static_library(
  'swift_generator',
  'grpc/src/compiler/swift_generator.cc',
  include_directories: includes,
)

ts_gen_lib = static_library(
  'ts_generator',
  'grpc/src/compiler/ts_generator.cc',
  include_directories: includes,
)

# flatc_library
flatc_lib = static_library(
  'flatc_library',
  'src/flatc.cpp',
  include_directories: includes,
  install: true,
  link_with: flatbuffers_lib,
)

flatc_lib_dep = declare_dependency(
  include_directories: includes,
  link_with: flatc_lib,
)

flatc_sources = files(
  'src/annotated_binary_text_gen.cpp',
  'src/bfbs_gen_lua.cpp',
  'src/bfbs_gen_nim.cpp',
  'src/binary_annotator.cpp',
  'src/flatc_main.cpp',
  'src/idl_gen_cpp.cpp',
  'src/idl_gen_csharp.cpp',
  'src/idl_gen_dart.cpp',
  'src/idl_gen_fbs.cpp',
  'src/idl_gen_go.cpp',
  'src/idl_gen_grpc.cpp',
  'src/idl_gen_java.cpp',
  'src/idl_gen_json_schema.cpp',
  'src/idl_gen_kotlin.cpp',
  'src/idl_gen_lobster.cpp',
  'src/idl_gen_lua.cpp',
  'src/idl_gen_php.cpp',
  'src/idl_gen_python.cpp',
  'src/idl_gen_rust.cpp',
  'src/idl_gen_swift.cpp',
  'src/idl_gen_text.cpp',
  'src/idl_gen_ts.cpp',
  'src/util.cpp',
)

flatc = executable(
  'flatc',
  flatc_sources,
  include_directories: includes,
  install: true,
  link_with: [flatc_lib, cpp_gen_lib, go_gen_lib, java_gen_lib, python_gen_lib, swift_gen_lib, ts_gen_lib],
)

meson.override_find_program('flatc', flatc)

flathash = executable(
  'flathash',
  'src/flathash.cpp',
  include_directories: includes,
)

meson.override_find_program('flathash', flathash)

How to know which of those files are test files that contain main? I don't think there are any test files in there.

@ibc
Copy link
Member

ibc commented Nov 22, 2023

It seems that the culprit is this file here:

https://github.com/mesonbuild/wrapdb/blob/master/subprojects/packagefiles/abseil-cpp/meson.build#L258 https://github.com/abseil/abseil-cpp/blob/master/absl/hash/internal/print_hash_of.cc#L4

I can check it and create a new wrapdb version tomorrow if it's not done before by you.

Do please and I promise I'll learn for next time.

BTW I love how you highlighted line 4 of https://github.com/abseil/abseil-cpp/blob/master/absl/hash/internal/print_hash_of.cc#L4 which literally is a comment saying:

you may not use this file except in compliance with the License.

@jmillan
Copy link
Member Author

jmillan commented Nov 23, 2023

This is the patch file of the latest flatbuffers-23.3.3 wrap file in Meson DB:

This is not about flatbuffers but about abseil 😄

@ibc
Copy link
Member

ibc commented Nov 23, 2023

This is the patch file of the latest flatbuffers-23.3.3 wrap file in Meson DB:

This is not about flatbuffers but about abseil 😄

Mmmm, that could be the reason

@jmillan
Copy link
Member Author

jmillan commented Nov 23, 2023

PR created mesonbuild/wrapdb#1309.

@jmillan jmillan closed this Nov 23, 2023
@jmillan jmillan reopened this Nov 23, 2023
@jmillan
Copy link
Member Author

jmillan commented Nov 28, 2023

So probably upgrading to libuv 1.47.0 would fix the issue, but it needs PR in Meson Wrap DB.

Definitety we should use a newer version, it may help here. Just needs to done and publish to wrapDB.

@jmillan
Copy link
Member Author

jmillan commented Nov 28, 2023

libuv: update to v1.47.0

At least one of the missing symbols error must be fixed with this.

@jmillan
Copy link
Member Author

jmillan commented Dec 4, 2023

OMFG

@jmillan
Copy link
Member Author

jmillan commented Dec 4, 2023

I've got it. On it..

@jmillan
Copy link
Member Author

jmillan commented Dec 4, 2023

Same as before:

 = note: mediasoup-worker.lib(src_win_process.c.obj) : error LNK2019: unresolved external symbol __imp_CoTaskMemFree referenced in function uv__kill

          mediasoup-worker.lib(src_win_process.c.obj) : error LNK2019: unresolved external symbol SHGetKnownFolderPath referenced in function uv__kill

          D:\a\mediasoup\mediasoup\target\debug\deps\mediasoup_sys-5d8110c20034dd87.exe : fatal error LNK1120: 2 unresolved externals

Even though ole32 and shell32 libraries are included here, the corresponding -l linking directives are not present:

-l psapi -l user32 -l advapi32 -l iphlpapi -l userenv -l ws2_32 -l ws2_32 -l gdi32 -l advapi32 -l crypt32 -l user32 -l static=mediasoup-worker`

Only failing for Rust.

@ibc
Copy link
Member

ibc commented Dec 4, 2023

So CoTaskMemFree is only used here:

$ ack CoTaskMemFree
src/win/process.c
1221:        CoTaskMemFree(localappdata);

As per Win docs, CoTaskMemFree is included by combaseapi.h: https://learn.microsoft.com/es-es/windows/win32/api/combaseapi/nf-combaseapi-cotaskmemfree

But there is no combaseapi include at all in libuv.

And similar for SHGetKnownFolderPathwhich is used here:

$ ack GetKnownFolderPath
src/win/process.c
1215:        SHGetKnownFolderPath(&FOLDERID_LocalAppData, 0, NULL, &localappdata);

and it is supposed to be defined in shlobj_core.h, but this lib is not included in libuv: https://learn.microsoft.com/en-us/windows/win32/api/shlobj_core/nf-shlobj_core-shgetknownfolderpath

@jmillan
Copy link
Member Author

jmillan commented Dec 4, 2023

So CoTaskMemFree is only used here:

ibc  in /tmp/kk/libuv
$ ack CoTaskMemFree
src/win/process.c
1221:        CoTaskMemFree(localappdata);

As per Win docs, CoTaskMemFree is included by combaseapi.h: https://learn.microsoft.com/es-es/windows/win32/api/combaseapi/nf-combaseapi-cotaskmemfree

But there is no combaseapi include at all in libuv.

It's ole32 the library where it's defined. And such dependency is present in meson. The corresponding linking -l ole32 directive (same as with shell32) should be there, but it's not. Definitely I have no idea of what the problem is..,

@ibc
Copy link
Member

ibc commented Dec 4, 2023

It's ole32 the library where it's defined. And such dependency is present in meson. The corresponding linking -l ole32 directive (same as with shell32) should be there, but it's not. Definitely I have no idea of what the problem is..,

The original CMakeLists.txt conf looks very similar to what the libuv wrap patch does:

CleanShot-2023-12-04-at-11 49 33@2x

@ibc
Copy link
Member

ibc commented Dec 4, 2023

So these are the libs that mediasoup-worker should be linked with according to meson.build in libuv 1.47.0 wrapdb file:

# system-depending config
if win32
  libuv_cargs += [
    '-DWIN32_LEAN_AND_MEAN',
    '-D_WIN32_WINNT=0x0602',
  ]
  libuv_libs += [
    'psapi',
    'user32',
    'advapi32',
    'iphlpapi',
    'userenv',
    'ws2_32',
    'dbghelp',
    'ole32',
    'uuid',
    'shell32',
  ]

And this is the Rust build command:

     Running `rustc --crate-name mediasoup_sys --edition=2021 worker\src\lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 -C metadata=f9d6d2d81d2268d8 -C extra-filename=-f9d6d2d81d2268d8 --out-dir D:\a\mediasoup\mediasoup\target\debug\deps -C incremental=D:\a\mediasoup\mediasoup\target\debug\incremental -L dependency=D:\a\mediasoup\mediasoup\target\debug\deps --extern planus=D:\a\mediasoup\mediasoup\target\debug\deps\libplanus-568447ac982c3f79.rmeta --extern serde=D:\a\mediasoup\mediasoup\target\debug\deps\libserde-67dc188038902225.rmeta -L native=D:\a\mediasoup\mediasoup\target\debug\build\mediasoup-sys-a1c2b48b540f9bcc\out -l psapi -l user32 -l advapi32 -l iphlpapi -l userenv -l ws2_32 -l ws2_32 -l gdi32 -l advapi32 -l crypt32 -l user32 -l static=mediasoup-worker`

To be perfectly clear, there are various libs that are missing:

-l psapi 
-l user32 
-l advapi32 
-l iphlpapi 
-l userenv 
-l ws2_32 
-l ws2_32 
-l gdi32 
-l advapi32 
-l crypt32 
-l user32 
-l static=mediasoup-worker
  • ws2_32 is included twice in the Rust build command (probably because it's included by another meson subproject).
  • dbghelp, ole32, uuid and shell32 are not present in the Rust build command.

The most interesting thing here is that those libs in the libuv meson patch file are NOT included in the Rust build command. Those are included explicitly by our mediasoup/worker/build.rs file:

CleanShot-2023-12-04-at-12 37 32@2x

@jmillan
Copy link
Member Author

jmillan commented Dec 4, 2023

@nazar-pc,

Why do those libraries which are dependencies of our deps need to be included in our build.rs file?

#1201 (comment)

@ibc
Copy link
Member

ibc commented Dec 4, 2023

CI is now passing. However this branch is out-of-date with v3. I'll resolve conflicts and push.

@ibc ibc marked this pull request as ready for review December 4, 2023 12:11
@ibc ibc merged commit 2e6913b into v3 Dec 4, 2023
36 checks passed
@ibc ibc deleted the update-abseil-dep branch December 4, 2023 12:14
@nazar-pc
Copy link
Collaborator

nazar-pc commented Dec 4, 2023

Why do those libraries which are dependencies of our deps need to be included in our build.rs file?

Because they are not static libraries, so linking just libmediasoup-worker is not sufficient.

@ibc
Copy link
Member

ibc commented Dec 4, 2023

Why do those libraries which are dependencies of our deps need to be included in our build.rs file?

Because they are not static libraries, so linking just libmediasoup-worker is not sufficient.

We could tell libuv wrap patch to link those libraries statically but that would only make sense for us (mediasoup), right?

@nazar-pc
Copy link
Collaborator

nazar-pc commented Dec 4, 2023

We could tell libuv wrap patch to link those libraries statically but that would only make sense for us (mediasoup), right?

My understanding is that those are system libraries on Windows and are not possible to link statically at all.

@lelikg
Copy link

lelikg commented Dec 4, 2023

Hello I believe this broke our npm install

#12 177.3 npm ERR! [778/1349] Compiling C++ object mediasoup-worker.p/src_RTC_Transport.cpp.o
#12 177.3 npm ERR! FAILED: mediasoup-worker.p/src_RTC_Transport.cpp.o 
#12 177.3 npm ERR! c++ -Imediasoup-worker.p -I. -I../../.. -I../../../include -Isubprojects/abseil-cpp-20230802.0 -I../../../subprojects/abseil-cpp-20230802.0 -I../../../subprojects/openssl-3.0.8/include -I../../../subprojects/openssl-3.0.8/crypto -I../../../subprojects/openssl-3.0.8/crypto/modes -I../../../subprojects/openssl-3.0.8/crypto/ec/curve448 -I../../../subprojects/openssl-3.0.8/crypto/ec/curve448/arch_32 -I../../../subprojects/openssl-3.0.8/providers/common/include -I../../../subprojects/openssl-3.0.8/providers/implementations/include -Isubprojects/openssl-3.0.8/generated-config/archs/linux-x86_64/asm -I../../../subprojects/openssl-3.0.8/generated-config/archs/linux-x86_64/asm -I../../../subprojects/openssl-3.0.8/generated-config/archs/linux-x86_64/asm/include -I../../../subprojects/openssl-3.0.8/generated-config/archs/linux-x86_64/asm/crypto -I../../../subprojects/openssl-3.0.8/generated-config/archs/linux-x86_64/asm/providers/common/include -I../../../subprojects/libuv-v1.47.0/include -Isubprojects/libsrtp-2.5.0/include -I../../../subprojects/libsrtp-2.5.0/include -Isubprojects/usrsctp-ebb18adac6501bad4501b1f6dccb67a1c85cc299/usrsctplib -I../../../subprojects/usrsctp-ebb18adac6501bad4501b1f6dccb67a1c85cc299/usrsctplib -Isubprojects/usrsctp-ebb18adac6501bad4501b1f6dccb67a1c85cc299/usrsctplib/netinet -I../../../subprojects/usrsctp-ebb18adac6501bad4501b1f6dccb67a1c85cc299/usrsctplib/netinet -Isubprojects/usrsctp-ebb18adac6501bad4501b1f6dccb67a1c85cc299/usrsctplib/netinet6 -I../../../subprojects/usrsctp-ebb18adac6501bad4501b1f6dccb67a1c85cc299/usrsctplib/netinet6 -I../../../subprojects/flatbuffers-23.3.3/include -I../../../subprojects/flatbuffers-23.3.3/grpc -Ifbs -I../../../fbs -Ideps/libwebrtc -I../../../deps/libwebrtc -I../../../deps/libwebrtc/libwebrtc -Isubprojects/liburing-liburing-2.4/src/include -I../../../subprojects/liburing-liburing-2.4/src/include -fdiagnostics-color=always -DNDEBUG -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c++17 -O3 -fPIE -pthread -DMS_LITTLE_ENDIAN -DMS_LIBURING_SUPPORTED -DMS_EXECUTABLE -MD -MQ mediasoup-worker.p/src_RTC_Transport.cpp.o -MF mediasoup-worker.p/src_RTC_Transport.cpp.o.d -o mediasoup-worker.p/src_RTC_Transport.cpp.o -c ../../../src/RTC/Transport.cpp
#12 177.3 npm ERR! In file included from ../../../subprojects/liburing-liburing-2.4/src/include/liburing.h:24,
#12 177.3 npm ERR!                  from ../../../include/DepLibUring.hpp:7,
#12 177.3 npm ERR!                  from ../../../src/RTC/Transport.cpp:7:
#12 177.3 npm ERR! subprojects/liburing-liburing-2.4/src/include/liburing/compat.h:5:10: fatal error: linux/time_types.h: No such file or directory
#12 177.3 npm ERR!  #include <linux/time_types.h>
#12 177.3 npm ERR!           ^~~~~~~~~~~~~~~~~~~~
#12 177.3 npm ERR! compilation terminated.
#12 177.3 npm ERR! [779/1349] Compiling C++ object mediasoup-worker.p/src_RTC_TcpServer.cpp.o

@jmillan
Copy link
Member Author

jmillan commented Dec 4, 2023

@lelikg, this is not the PR that originated this problem. Can you please open a GH issue with this log extract?

I'll look into this tomorrow.

@jmillan
Copy link
Member Author

jmillan commented Dec 4, 2023

@lelikg , specify in the issue the Linux flavor and version alomng with any relevant environment details

@lelikg
Copy link

lelikg commented Dec 4, 2023

@lelikg , specify in the issue the Linux flavor and version alomng with any relevant environment details

done, thank you

jmillan added a commit that referenced this pull request Dec 5, 2023
While investigating #1201,
disable liburing by default.

In order to enable it, use MESON_ARGS as usual. ie:

MESON_ARGS=-Dms_enable_liburing=true npm install
jmillan added a commit that referenced this pull request Dec 5, 2023
Disable liburing by default

While investigating #1201,
disable liburing by default.

In order to enable it, use MESON_ARGS as usual. ie:

MESON_ARGS=-Dms_enable_liburing=true npm install
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants