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

Compiling with CMake for Windows/MSVC only supports DLL runtime libraries #333

Open
Malcolmnixon opened this issue Apr 2, 2024 · 9 comments

Comments

@Malcolmnixon
Copy link
Contributor

I'm attempting to consume opus as a git submodule from an official tag, and build a Godot extension cross-compiling for Windows, Linux, and Android.

The current CMakeLists.txt targets cmake 3.1 which hard-codes the C runtime library to either MultiThreadedDebugDLL or MultiThreadedDLL. The resulting code therefore has to ship the Microsoft runtime library DLL.

Upgrading CMakeLists.txt to cmake_minimum_required(VERSION 3.15) unlocks the options for specifying the runtime library options on the command-line such as cmake -DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreaded .

It would be preferable to support an OPUS_STATIC_RUNTIME option, which for MSVC would use CMAKE_MSVC_RUNTIME_LIBRARY Generator Expressions to correctly specify the Debug or Release Static or DLL runtimes.

Possibly something like:

if(MSVC)
  if(OPUS_STATIC_RUNTIME)
    set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
  else()
    set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>DLL")
  endif()
endif()
@jmvalin
Copy link
Member

jmvalin commented Apr 2, 2024

@xnorpx Any opinion here? unfortunately I don't know anything on the topic.

@xnorpx
Copy link
Contributor

xnorpx commented Apr 2, 2024

Last time we tried to upgrade min version people on ancient Linux distros got "upset" and we reverted back.

I don't know on top of my head if it's possible to use different min version per platform? But if possible then we could apply a patch.

@jmvalin
Copy link
Member

jmvalin commented Apr 2, 2024

Do you remember when you had to revert and what ancient distro had the problem?

@xnorpx
Copy link
Contributor

xnorpx commented Apr 2, 2024

Found it in an old pr. Ancient might been exaggeration.

Requirement was set to 3.12 and the user wanted 3.5 or less as that was the default in Ubuntu 16.04 (which was at that time only 2 years old)

3.15 is ~5 years old: https://github.com/Kitware/CMake/releases/tag/v3.15.0

Is there is an easy way to find what versions the common distros is on?

@xnorpx
Copy link
Contributor

xnorpx commented Apr 2, 2024

https://launchpad.net/ubuntu/+source/cmake/

Looks like 3.16 is fine for 20.04 LTS so seems resonable for Ubuntu.

@xnorpx
Copy link
Contributor

xnorpx commented Apr 2, 2024

For debian:

https://tracker.debian.org/pkg/cmake

Not sure how "old" old old stable is and how many users.

@Malcolmnixon
Copy link
Contributor Author

Assuming 3.15 (or 3.16) is considered acceptable I could construct a PR for a new OPUS_STATIC_RUNTIME option, or is this something preferably done by the build team to keep consistency across cmake/meson/autoconf/etc?

@xnorpx
Copy link
Contributor

xnorpx commented Apr 3, 2024

Consistency is best effort :) not sure if Meson have that Opion.

Given it's 5 years i think it's fine. So pr would be good.

Add it's so the option is visible only on Windows also please test the different combinations.

@xnorpx
Copy link
Contributor

xnorpx commented Apr 3, 2024

I would say update to 3.16 since that is 20.04 LTS Ubuntu and Debian seems to support it (except the oldest)

Also update this test to 3.16: https://github.com/xiph/opus/blob/0e30966b198ad28943799eaf5b3b08100b6f70c3/.github/workflows/cmake.yml#L6C3-L9C11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants