-
Notifications
You must be signed in to change notification settings - Fork 906
Add initial support for build system generation with CMake #4494
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
Conversation
The use of `ifdef` makes the first branch guaranteed to be taken
This is still missing: * Dependency resolving at import time (should be in PjConfig.cmake.in) * Some `pkg-config` variables
It's not the library itself that decides it. The option needs to be explicitly set during project generation to have a different behaviour from the default one. |
Setting Leaving |
Thanks, the configure step completed successfully now. Just FYI, there was a warning regarding the SDL2 probe (due to missing explicit settings and the absence of a CMake detection config?): Then the generate step failed, looks like the Perhaps it could check whether one or more header files exist. If not, it could copy all the G7221 header files. However, I'm unsure whether this approach (especially the copying part) would behave correctly if the symbolic link is created successfully. |
That option is also for the user to set. It is simpler to set an ON/OFF option than to pass a generator expression.
This gives errors during linking. |
MSVC project files and Makefile that exist in the repo add third_party folder to include folders. |
This is equivalent to adding a switch to select which compiler to use. Sure, it may work sometimes (since C has a stable ABI), but not every time. Giving users this option is like intentionally giving them a gun to shoot their feet.
No, it wont. Using shared libraries does not involve compile-time linkage, but rather the libraries are dynamically-linked (hence the name) at load-time by the loader. Feel free to correct me if I am wrong. |
The existing build system also (mostly) includes everything to everything. Dependency isolation is one of the biggest selling points of CMake, why give it up? |
E.g., linking a log4cplus library built with static runtime in a project built with dynamic runtime gives errors like the following in Visual Studio: |
Current solution looks a bit too much for this. I think changing #include directives would be better. It looks like only one file uses such includes, that start with g7221, third_party/g7221/common/defs.h, and it includes files in the same folder. |
|
Update on MSVC, it is now built successfully for the libs & and the pjsua app, however it is still failed on test apps ( The Btw, |
Yes, its use requires WIN32 option for add_executable as it's not a console app but rather a window app.
Agree. |
It seems that MSVC's linker does check for conflicting symbols (in this case, |
I had to use that method to make sure this PR is add-only, without changing the current codebase. The code inside |
Windows builds now should use
You're right. |
|
@nanangizz While you're at it, can you try building with other toolchains like LLVM/Clang, MinGW, and/or CYGWIN? |
Tried Visual Studio with clangcl toolset for x64. webrtc_aec3 project succeeds after AVX2 architecture is specified, Other binaries are built without problems. |
|
Yes, Windows Phone may also use FYI, I experienced issues configuring for MinGW (I am a newbie, remember? :), when I use cmake-gui (the same app I used for configuring for MSVC), with generator set to "MinGW Makefiles" & I also tried to use cmake-3.30.5, installed using Any idea? |
What installation package have you used? Does it correspond to your platform? I have installed msys2-x86_64-20250830.exe, installed packages mingw-w64-x86_64-gcc git cmake ninja openssl-devel. After adding WEBRTC_WIN compiler definition for MINGW compilation failed with __try/__except are Microsoft-specific. |
I already have MinGW/msys2 installed for development, with compiler package.
Haven't tried to generate project files for ninja though. |
|
Tried with host OS cmake and got similar errors. |
|
I can generate the project files on MinGW successfully now. However, in |
|
Just realized that using the existing So I guess we can just disabled the WebRTC AEC3 for MinGW for now? |
Why CMake?
compile_commands.jsonfiles used by stand-alone tooling.What does this PR add?
RPATHvalues.What is still needed?
find_packagein config mode) requires more work.Suggestions for improvements