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

Drop gyp, move to CMake #359

Closed
ibc opened this issue Feb 13, 2020 · 34 comments · Fixed by #622
Closed

Drop gyp, move to CMake #359

ibc opened this issue Feb 13, 2020 · 34 comments · Fixed by #622
Labels
feature meson Meson build system related
Milestone

Comments

@ibc
Copy link
Member

ibc commented Feb 13, 2020

Rationale:

So let's move to CMake:

  • Should choose a CMake version available in most common platforms (Debian, Ubuntu, OSX, etc).
  • Avoid the very latest and coolest CMake version since some of its features are not implemented by most common versions such as 3.7.

Ping @saghul because I can.

@saghul
Copy link
Contributor

saghul commented Feb 13, 2020

Current worker deps:

Name CMake support upstream?
catch
getopt
json
lcov 🚫
libsrtp
libuv
libwebrtc 🚫
netstring 🚫
openssl 🚫
usrsctp

Out of these, I'd say OpenSSL is the trickiest one. An option would be to move to BoringSSL, which does support CMake out of the box. The rest are pretty small and writing CMakefiles for them should be trivial.

@ibc
Copy link
Member Author

ibc commented Feb 13, 2020

Wow, thanks! BTW:

  • lcov is not really used in mediasoup (I mean, it's not built).
  • Regarding libwebrtc we only use a subset of files so it shouldn't be hard to make a CMakelists.txt for it (wait, it depends on abseil-cpp lib but it does provide CMakelists.txt).
  • netstring is two files code :)
  • openssl... wow, didn't know that... Moving to BoringSSL is an option, of course, but somehow I don't want to rely on so many Google-managed projects... sad.

@saghul
Copy link
Contributor

saghul commented Feb 13, 2020

* `openssl`... wow, didn't know that... Moving to BoringSSL is an option, of course, but somehow I don't want to rely on so many Google-managed projects... sad.

It already has the necessary APIs for QUIC unlike OpenSSL (still in PR) so maybe it's a sign from, the future! 😎

@torlarse
Copy link

torlarse commented Apr 5, 2020

Please correct me if I am interrupting or misunderstanding, this is just a friendly attempt to contribute. I copied this from the BoringSSL Git 1:

Although BoringSSL is an open source project, it is not intended for general use, as OpenSSL is. We don't recommend that third parties depend upon it. Doing so is likely to be frustrating because there are no guarantees of API or ABI stability.

@barry-ran
Copy link

barry-ran commented Apr 10, 2020

openssl-cmake

@versatica versatica deleted a comment from barry-ran Apr 10, 2020
@barry-ran
Copy link

barry-ran commented May 5, 2020

I am trying to move mediasoup to cmake. I have made some attempts and it should be successful (mediasoup-cmake)

@ibc
Copy link
Member Author

ibc commented May 5, 2020

Interesting, This will be very useful for us.

Just wondering: Did you use openssl-cmake project into it? And what else did you have to modify (other than creating the CMakeLists.txt file)?

BTW I tried it in OSX and it seems to work, but I get these warnings:

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../../libcrypto.a(ebcdic.c.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../../libcrypto.a(async_null.c.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../../libcrypto.a(async_win.c.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../../libcrypto.a(rsaz_exp.c.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../../libcrypto.a(cms_cd.c.o) has no symbols

[...]

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../../libssl.a(ssl_utst.c.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../../libssl.a(t1_trce.c.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../../libssl.a(ssl_utst.c.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../../libssl.a(t1_trce.c.o) has no symbols

@barry-ran
Copy link

barry-ran commented May 5, 2020

libssl.a(ssl_utst.c.o) has no symbols

Yes, I used openssl-cmake. Now the mediasoup-cpp worker can only be compiled and run. It has not been rigorously tested. I will continue to improve it later (for example, there are some compilation options not added, I am not sure What question), now I just hope it can help you.

@ibc
Copy link
Member Author

ibc commented May 5, 2020

Thanks, don't hesitase in updating this issue with your progress, please.

@jmillan
Copy link
Member

jmillan commented Jun 1, 2020

Hi @barry-ran,

I see that mediasoup-cpp is a separate project, not a mediasoup branch.

Could you please make it a mediasoup branch (out of current mediasoup v3 version) so it's easier to compare, collaborate and actually merge? We would like to put some efforts on this.

Thanks

@barry-ran
Copy link

Hi @jmillan ,
Ok i will do it, give me some time

@jmillan
Copy link
Member

jmillan commented Jun 2, 2020

Ok i will do it, give me some time

👍

@barry-ran
Copy link

barry-ran commented Jun 14, 2020

Hi @jmillan ,
Ok i will do it, give me some time

Hi @jmillan ,
Sorry for not finishing till now,I cut off the feat/gyp-move-to-camke branch based on 3.6.7, I try to small changes:

  1. replace openssl with openssl-cmake
  2. add CMakeLists.txt
  3. build on github action

It has not been rigorously tested. There are some compilation options not added, I am not sure What question (Because I don’t know much about these options).

@jmillan
Copy link
Member

jmillan commented Jun 15, 2020

Thanks @barry-ran

@jmillan
Copy link
Member

jmillan commented Jun 17, 2020

Hi @barry-ran,

I'm getting the following error when trying to compile your branch:

Screenshot 2020-06-17 at 17 33 58

Such file is not present in deps/openssl/openssl/crytpo

@barry-ran
Copy link

Hi @barry-ran,

I'm getting the following error when trying to compile your branch:

Screenshot 2020-06-17 at 17 33 58

Such file is not present in deps/openssl/openssl/crytpo

Hi @jmillan ,
You should build it with cmake:

mkdir build
cd build
cmake ..
cmake --build .

@mabuchner
Copy link

mabuchner commented Mar 5, 2021

@barry-ran I tried to compile your branch on Windows with Visual Studio 2017, but CMake gave me the following error during configuration

-- Building for: Visual Studio 15 2017
-- Check if the system is big endian
-- Searching 16 bit integer
CMake Error at C:/Program Files/CMake/share/cmake-3.19/Modules/TestBigEndian.cmake:30 (message):
  TEST_BIG_ENDIAN needs either C or CXX language enabled
Call Stack (most recent call first):
  CMakeLists.txt:4 (TEST_BIG_ENDIAN)

To fix this, I had to move the project definition to the top of the CMakeLists.txt file

cmake_minimum_required(VERSION 3.16)

set(PROJECT_NAME "mediasoup-worker")
project(${PROJECT_NAME})

...

Afterwards I was able to configure and build with CMake

cmake -S . -B build -G "Visual Studio 15 2017" -A x64
cmake --build build --config Release

Finally, I had to copy mediasoup-worker.exe, usrsctp.dll and uv.dll to mediasoup/worker/out/Release. I also tried to specify the CMAKE_INSTALL_PREFIX and build the INSTALL target, but nothing was installed.

@jmillan
Copy link
Member

jmillan commented Mar 11, 2021

Hi @barry-ran,

Sorry for the long time without feedback.

Would you be willing to make a PR agains current v3?

@jmillan
Copy link
Member

jmillan commented Mar 11, 2021

I've just tried and had the same issue @mabuchner had.

@barry-ran
Copy link

@barry-ran I tried to compile your branch on Windows with Visual Studio 2017, but CMake gave me the following error during configuration

-- Building for: Visual Studio 15 2017
-- Check if the system is big endian
-- Searching 16 bit integer
CMake Error at C:/Program Files/CMake/share/cmake-3.19/Modules/TestBigEndian.cmake:30 (message):
  TEST_BIG_ENDIAN needs either C or CXX language enabled
Call Stack (most recent call first):
  CMakeLists.txt:4 (TEST_BIG_ENDIAN)

To fix this, I had to move the project definition to the top of the CMakeLists.txt file

cmake_minimum_required(VERSION 3.16)

set(PROJECT_NAME "mediasoup-worker")
project(${PROJECT_NAME})

...

Afterwards I was able to configure and build with CMake

cmake -S . -B build -G "Visual Studio 15 2017" -A x64
cmake --build build --config Release

Finally, I had to copy mediasoup-worker.exe, usrsctp.dll and uv.dll to mediasoup/worker/out/Release. I also tried to specify the CMAKE_INSTALL_PREFIX and build the INSTALL target, but nothing was installed.

try this?

cmake --build . --config Release --target install

@barry-ran
Copy link

Hi @barry-ran,

Sorry for the long time without feedback.

Would you be willing to make a PR agains current v3?

Ok i will do it

@jmillan
Copy link
Member

jmillan commented Apr 7, 2021

Thank you!

@mabuchner
Copy link

try this?

cmake --build . --config Release --target install

I tried

cmake -S . -B build -G "Visual Studio 15 2017" -A x64 -D CMAKE_INSTALL_PREFIX="build/install"
cmake --build build --config Release --target install

The CMake summary says

-- Selecting Windows SDK version 10.0.17763.0 to target Windows 10.0.18363.
-- summary of build options:
    Install prefix:  C:/Users/user.name/project-name/node_modules/mediasoup/worker/build/install
    Target system:   Windows
    Compiler:
      C compiler:    C:/Program Files (x86)/Microsoft Visual Studio/2017/Professional/VC/Tools/MSVC/14.16.27023/bin/Hostx86/x64/cl.exe
      CFLAGS:         /DWIN32 /D_WINDOWS

But as before nothing was installed.

@barry-ran
Copy link

Hi @barry-ran,
Sorry for the long time without feedback.
Would you be willing to make a PR agains current v3?

Ok i will do it

Hi @jmillan @mabuchner

I rebased current v3 and fixed some error, this pr
I've tested the following command:

cmake .. -D CMAKE_INSTALL_PREFIX=./dist
cmake --build . --target install

mediasoup-worker.exe and uv.dll is installed in the ./dist/lib/Debug directory (usrsctp is static now)

@nazar-pc
Copy link
Collaborator

nazar-pc commented May 2, 2021

BTW, was Meson under consideration?
I see many projects transition to it eventually.

@ibc
Copy link
Member Author

ibc commented May 2, 2021

BTW, was Meson under consideration?

I see many projects transition to it eventually.

We want something that it's already integrated into most of the C/C++ libraries used by mediasoup. Meson is not.

@nazar-pc
Copy link
Collaborator

nazar-pc commented May 2, 2021

I'm far from either of those, but it seems like using CMake projects in Meson is relatively straightforward: https://mesonbuild.com/CMake-module.html

So you'd use dependencies that already have CMake support as is.

@ibc
Copy link
Member Author

ibc commented May 2, 2021

I don't even know CMake so I'm not gonna learn Meson to integrate CMake into it XD

@nazar-pc
Copy link
Collaborator

nazar-pc commented Jun 6, 2021

I found this option for OpenSSL: https://github.com/viaduck/openssl-cmake
It will download desired OpenSSL version during build step, thus should be easier to maintain and keep up to date with OpenSSL updates. Will also reduce size of NPM package and Rust crate (I had to request to bump the limit for crate size to even publish mediasoup-sys).

@ibc
Copy link
Member Author

ibc commented Jul 8, 2021

Late thanks, @nazar-pc. Unclear whether such a OpenSSL will come with the optimized build options that have been merged into mediasoup today. Will see when we come back to this topic.

@nazar-pc
Copy link
Collaborator

nazar-pc commented Jul 8, 2021

I'm learning CMake to make this happen, but approaching from a different angle.

GYP can actually produce CMakeLists.txt files, with current GYP config it actually produces two (Release and Debug).

After minor tweaks it seems to build most of the stuff just fine as is. I'll try to combine both files into one and then swapping explicit instructions for building dependencies with includes/imports/whatever it is called in CMake. That will make ensure there is a working build all along and wouldn't necessarily block CMake introduction on upstream CMake support on every dependency too much.

I'll see where it goes, but no promises, no ETA.

@ibc
Copy link
Member Author

ibc commented Jul 8, 2021

Are you learning CMake? 😀😀😀😀

@ibc
Copy link
Member Author

ibc commented Jul 9, 2021

Just in case, here another approach: #602

@nazar-pc
Copy link
Collaborator

If some of subscribed to this issue have time to look at and maybe even test #622, it would be very helpful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature meson Meson build system related
Development

Successfully merging a pull request may close this issue.

7 participants