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

package(sdl): Add version 3.2.0 #26464

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Sil3ntStorm
Copy link
Contributor

@Sil3ntStorm Sil3ntStorm commented Jan 24, 2025

Summary

Changes to recipe: sdl/3.2.0

Motivation

Add latest versions of SDL3 (3.2.0 and 3.2.2) as I need it. Also requested in #25605
Fixes #25605

Details

Add a new recipe folder due to changes required to CMakeLists.txt in test_package.
Pretty much a copy of the existing one minus a few conditions.

Only tested on Windows.


@Sil3ntStorm
Copy link
Contributor Author

Whoops I guess there is one already after all, which didn't show up in search... Feel free to close if desired.

I don't know what CI is complaining about, builds locally.

@Sil3ntStorm Sil3ntStorm marked this pull request as ready for review January 25, 2025 12:09
@elvisdukaj
Copy link
Contributor

elvisdukaj commented Feb 3, 2025

I've tested this MR on

🟢 windows with msvc
🔴 windows with mingw (gcc-14)
🟢 windows with android-ndk/27c
🟢 Linux (Ubuntu 24) with GCC
🟢 Macos (apple-clang)

I would be in favour of this MR since #25873 seems behind

@elvisdukaj
Copy link
Contributor

This is the issue on windows with mingw build:

======== Testing the package: Building ========
sdl/3.2.0 (test package): Calling build()
sdl/3.2.0 (test package): Running CMake.configure()
sdl/3.2.0 (test package): RUN: cmake -G ""Ninja"" -DCMAKE_TOOLCHAIN_FILE="generators/conan_toolchain.cmake" -DCMAKE_INSTALL_PREFIX="C:/Users/EDUKAJ/projects/dev/conan-center-index/recipes/sdl/3.x/test_package" -DCMAKE_POLICY_DEFAULT_CMP0091="NEW" -DCMAKE_BUILD_TYPE="Release" "C:/Users/EDUKAJ/projects/dev/conan-center-index/recipes/sdl/3.x/test_package"
-- Using Conan toolchain: C:/Users/EDUKAJ/projects/dev/conan-center-index/recipes/sdl/3.x/test_package/build/windows-x86_64-gcc-14.2/Release/generators/conan_toolchain.cmake
-- Conan toolchain: Defining architecture flag: -m64
-- Conan toolchain: C++ Standard 23 with extensions OFF
-- The C compiler identification is GNU 14.2.0
-- The CXX compiler identification is GNU 14.2.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: C:/Users/EDUKAJ/.conan2/p/mingwe8c4b6043d588/p/bin/gcc.exe - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: C:/Users/EDUKAJ/.conan2/p/mingwe8c4b6043d588/p/bin/g++.exe - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Conan: Component target declared 'SDL3::SDL3'
-- Conan: Target declared 'Iconv::Iconv'
CMake Error at build/windows-x86_64-gcc-14.2/Release/generators/cmakedeps_macros.cmake:66 (message):
  Library 'iconv' not found in package.  If 'iconv' is a system library,
  declare it with 'cpp_info.system_libs' property
Call Stack (most recent call first):
  build/windows-x86_64-gcc-14.2/Release/generators/Iconv-Target-release.cmake:23 (conan_package_library_targets)
  build/windows-x86_64-gcc-14.2/Release/generators/IconvTargets.cmake:24 (include)
  build/windows-x86_64-gcc-14.2/Release/generators/IconvConfig.cmake:16 (include)
  C:/Users/EDUKAJ/.conan2/p/cmake7f2257f292deb/p/share/cmake-3.31/Modules/CMakeFindDependencyMacro.cmake:76 (find_package)
  build/windows-x86_64-gcc-14.2/Release/generators/SDL3Config.cmake:24 (find_dependency)
  CMakeLists.txt:4 (find_package)


-- Configuring incomplete, errors occurred!

Tomorrow I will try to fix this issue :)

Well done! 🥳

@Sil3ntStorm
Copy link
Contributor Author

Sil3ntStorm commented Feb 4, 2025

The only mention of iconv in SDL seems to be within hidapi (specifically libusb).
I am happy with adding an override in the conanfile or applying some small patch to the sources. Other than that my knee jerk reaction would be to just raise ConanInvalidConfiguration.

However I do not have a MinGW setup to test it.

@elvisdukaj
Copy link
Contributor

I did test it but I don't understand the issue; the test package is failing because iconv is not there, but it is there: sdl is having it as requirement in the package_info method.

Testing is pretty straightforward since mingw-builds/14.2.0 is a Conan package and can be used as tool requirement.

In the end I think it's even not that important since SDL on Windows with MinGW it's not very common.

@elvisdukaj
Copy link
Contributor

I can try to have the same scenario with sdl2. I will let you know tomorrow.

@elvisdukaj
Copy link
Contributor

I tried to build SDL 2.30.9 with the same profile and still get the same error on the test package. So this PR didn't add any new odd behavior.

For clarity I am posting the used profile:

======== Input profiles ========
Profile host:
[settings]
arch=x86_64
build_type=Release
compiler=gcc
compiler.cppstd=23
compiler.exception=seh
compiler.libcxx=libstdc++11
compiler.threads=posix
compiler.version=14.2
os=Windows
[tool_requires]
*: ninja/1.12.1, cmake/3.31.3, mingw-builds/14.2.0
[conf]
tools.cmake.cmake_layout:build_folder_vars=['settings.os', 'settings.arch', 'settings.compiler', 'settings.compiler.version']
tools.cmake.cmaketoolchain:extra_variables={'CMAKE_EXPORT_COMPILE_COMMANDS': 'TRUE'}
tools.cmake.cmaketoolchain:generator="Ninja"
tools.env.virtualenv:powershell=!

Profile build:
[settings]
arch=x86_64
build_type=Release
compiler=msvc
compiler.cppstd=14
compiler.runtime=dynamic
compiler.runtime_type=Release
compiler.version=194
os=Windows
[conf]
tools.cmake.cmaketoolchain:generator=Ninja
tools.env.virtualenv:powershell=True

@Sil3ntStorm
Copy link
Contributor Author

Sil3ntStorm commented Feb 5, 2025

So you convinced me to run the mingw, and it seems to complete without any issues. The only difference is I don't use an ancient c++ standard.

======== Input profiles ========
Profile host:
[settings]
arch=x86_64
build_type=Release
compiler=gcc
compiler.cppstd=23
compiler.exception=seh
compiler.libcxx=libstdc++11
compiler.threads=posix
compiler.version=14.2
os=Windows
[tool_requires]
*: ninja/1.12.1, cmake/3.31.3, mingw-builds/14.2.0
[conf]
tools.cmake.cmake_layout:build_folder_vars=['settings.os', 'settings.arch', 'settings.compiler', 'settings.compiler.version']
tools.cmake.cmaketoolchain:extra_variables={'CMAKE_EXPORT_COMPILE_COMMANDS': 'TRUE'}
tools.cmake.cmaketoolchain:generator="Ninja"

Profile build:
[settings]
arch=x86_64
build_type=Release
compiler=msvc
compiler.cppstd=23
compiler.runtime=dynamic
compiler.runtime_type=Release
compiler.version=194
os=Windows
[conf]
tools.cmake.cmaketoolchain:generator=Ninja

since it was released during the lifetime of the PR
@Sil3ntStorm
Copy link
Contributor Author

Interestingly I seem to have crashed the Linter 🗡️

@elvisdukaj
Copy link
Contributor

Oh that's interesting, but we're both using the same version of C++ the build profile is not used for SDL.

@perseoGI
Copy link
Contributor

perseoGI commented Feb 6, 2025

Interestingly I seem to have crashed the Linter 🗡️

Please don't worry about linter crashing. The issue has just been resolved. It does not have anything to do with your PR 🐸

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

Successfully merging this pull request may close these issues.

[request] SDL3
4 participants