Skip to content

Conversation

withSang
Copy link
Contributor

Description

Replaces the hardcoded path traversal for finding the SDL3 framework root with a flexible upward search loop.
The hardcoded path traversal of 4 levels is not suitable for iOS frameworks(SDL3.framework/CMake/SDL3Config.cmake).
The new method is more robust and less likely to break.

Existing Issue(s)

Hopefully #14121

Copy link
Contributor

@madebr madebr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Xcode/SDL/pkg-support/resources/CMake/SDL3Config.cmake is meant to be put in a very specific location during package creation.

I don't know what this fixes. Can you please provide a reproducer for the error?

@withSang
Copy link
Contributor Author

Xcode/SDL/pkg-support/resources/CMake/SDL3Config.cmake is meant to be put in a very specific location during package creation.

I don't know what this fixes. Can you please provide a reproducer for the error?

Of course! I confirm the official release (https://github.com/libsdl-org/SDL/releases/download/release-3.2.24/SDL3-3.2.24.dmg) has the same issue.

In the SDL3.xcframework of the SDL3-3.2.24.dmg, the SDL3Config.cmake file location differs across the platforms.

  • macOS: SDL3.xcframework/macos-arm64_x86_64/SDL3.framework/Versions/A/Resources/CMake/SDL3Config.cmake
  • iOS: SDL3.xcframework/ios-arm64/SDL3.framework/CMake/SDL3Config.cmake
  • tvOS: SDL3.xcframework/tvos-arm64/SDL3.framework/CMake/SDL3Config.cmake, which makes sense since tvOS started as a fork of iOS.

I'm getting the same issue when I build the XCFramework directly from the main branch.
I've used this command at the project root.

xcodebuild -project Xcode/SDL/SDL.xcodeproj -target "SDL3.xcframework" -configuration Release SYMROOT="build/"

@withSang
Copy link
Contributor Author

withSang commented Oct 16, 2025

https://github.com/mpaghq/fluidsynth/actions/runs/18559446061/job/52918890912

This is a project I'm using SDL.
I'm using iOS toolchain from https://github.com/leetal/ios-cmake to link SDL to the FluidSynth library.

As seen in the 'build' step logs, finding the header fails due to CMake configuration.
If I change the configuration as the PR and try to build again, there's no problem!

@withSang withSang requested a review from madebr October 16, 2025 14:07
@madebr
Copy link
Contributor

madebr commented Oct 16, 2025

Of course! I confirm the official release (https://github.com/libsdl-org/SDL/releases/download/release-3.2.24/SDL3-3.2.24.dmg) has the same issue.

Thanks! I can reproduce your error (on ci, I don't own Apple hw).
I just pushed 57ac8fc which verifies all frameworks are behaving correctly after creation.
For iOS this indeed failed:

CMake Error (dev) at /Volumes/SDL3/SDL3.xcframework/ios-arm64/SDL3.framework/CMake/SDL3ConfigVersion.cmake:9 > (message):
Could not find SDL_version.h. This script is meant to be placed in the
Resources/CMake directory of SDL2.framework

So the first error actually happens in SDL3ConfigVersion.cmake, which is not modified here.

(While you're at it, can you also fix the error message? SDL2 -> SDL3)

@withSang
Copy link
Contributor Author

@madebr Thanks for the reply!
I've changed SDL3ConfigVersion.cmake to look for header paths for both macOS and iOS (and the error message as well).
Maybe the tests should pass this time :)

@madebr
Copy link
Contributor

madebr commented Oct 17, 2025

Maybe the tests should pass this time :)

Thanks to the power of GitHub, you can run these on your fork!

Assuming you have gh installed, run the following command.
A release job should start on the GitHub Actions servers (like here, but then on your account)
(This is what I did to trigger the error above)

python3 build-scripts/create-release.py -R mpaghq/SDL --ref main

@withSang
Copy link
Contributor Author

Maybe the tests should pass this time :)

Thanks to the power of GitHub, you can run these on your fork!

Assuming you have gh installed, run the following command. A release job should start on the GitHub Actions servers (like here, but then on your account) (This is what I did to trigger the error above)

python3 build-scripts/create-release.py -R mpaghq/SDL --ref main

Oh cool thanks! It runs fine :)

https://github.com/mpaghq/SDL/actions/runs/18582543141/job/52980355332

@madebr
Copy link
Contributor

madebr commented Oct 17, 2025

Looks good!
I suggest the following small change(s) which I think makes it more robust:

diff --git a/Xcode/SDL/pkg-support/resources/CMake/SDL3Config.cmake b/Xcode/SDL/pkg-support/resources/CMake/SDL3Config.cmake
index d63c926ae..87df5a2bb 100644
--- a/Xcode/SDL/pkg-support/resources/CMake/SDL3Config.cmake
+++ b/Xcode/SDL/pkg-support/resources/CMake/SDL3Config.cmake
@@ -3,7 +3,7 @@
 # or in the CMake directory of a SDL3 framework for iOS / tvOS / visionOS.
 
 # INTERFACE_LINK_OPTIONS needs CMake 3.12
-cmake_minimum_required(VERSION 3.12)
+cmake_minimum_required(VERSION 3.12...4.0)
 
 include(FeatureSummary)
 set_package_properties(SDL3 PROPERTIES
@@ -32,26 +32,28 @@ endmacro()
 
 set(SDL3_FOUND TRUE)
 
-# Compute the installation prefix relative to this file.
-set(_sdl3_framework_path "${CMAKE_CURRENT_LIST_DIR}")
-get_filename_component(_sdl3_framework_path "${_sdl3_framework_path}" REALPATH)
+# Compute the installation prefix relative to this file:
+# search upwards for the .framework directory
+set(_current_path "${CMAKE_CURRENT_LIST_DIR}")
+get_filename_component(_current_path "${_current_path}" REALPATH)
+set(_sdl3_framework_path "")
 
-# Search upwards for the .framework directory
 set(_current_path "${_sdl3_framework_path}")
-set(_found_framework FALSE)
-foreach(i RANGE 10) # max 10 levels up
-    if (IS_DIRECTORY "${_current_path}" AND "${_current_path}" MATCHES "\\.framework$")
+while(NOT _sdl3_framework_path)
+    if (IS_DIRECTORY "${_current_path}" AND "${_current_path}" MATCHES "/SDL3\\.framework$")
         set(_sdl3_framework_path "${_current_path}")
-        set(_found_framework TRUE)
         break()
     endif()
-    if ("${_current_path}" STREQUAL "")
+    get_filename_component(_next_current_path "${_current_path}" DIRECTORY)
+    if ("${_current_path}" STREQUAL "${_next_current_path}")
         break()
     endif()
-    get_filename_component(_current_path "${_current_path}" DIRECTORY)
+    set(_next_current_path "${_current_path}")
 endforeach()
+set(_next_current_path)
+set(_next_current_path)
 
-if(NOT _found_framework)
+if(NOT _sdl3_framework_path)
     message(FATAL_ERROR "Could not find SDL3.framework root from ${CMAKE_CURRENT_LIST_DIR}")
 endif()
 
diff --git a/Xcode/SDL/pkg-support/resources/CMake/SDL3ConfigVersion.cmake b/Xcode/SDL/pkg-support/resources/CMake/SDL3ConfigVersion.cmake
index ad331216f..80a127872 100644
--- a/Xcode/SDL/pkg-support/resources/CMake/SDL3ConfigVersion.cmake
+++ b/Xcode/SDL/pkg-support/resources/CMake/SDL3ConfigVersion.cmake
@@ -4,7 +4,7 @@
 # This file is meant to be placed in Resources/CMake of a SDL3 framework for macOS,
 # or in the CMake directory of a SDL3 framework for iOS / tvOS / visionOS.
 
-cmake_minimum_required(VERSION 3.12)
+cmake_minimum_required(VERSION 3.12...4.0)
 
 # Find SDL_version.h
 set(_sdl_version_h_path "")
@@ -16,6 +16,7 @@ endif()
 
 if(NOT _sdl_version_h_path)
     message(AUTHOR_WARNING "Could not find SDL_version.h. This script is meant to be placed in the Resources/CMake directory or the CMake directory of SDL3.framework.")
+    set(PACKAGE_VERSION_UNSUITABLE TRUE)
     return()
 endif()
 

@withSang
Copy link
Contributor Author

@madebr Thanks a lot! I've made the changes as yours :-)
The tests pass as well

https://github.com/mpaghq/SDL/actions/runs/18627125570

@slouken slouken added this to the 3.4.0 milestone Oct 19, 2025
Copy link
Contributor

@madebr madebr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Some styling nits, but ok to merge!
(I cannot commit on your branch, otherwise I would've applied these myself)

@withSang
Copy link
Contributor Author

withSang commented Oct 20, 2025

Thanks!! I've applied the patch :)

@slouken slouken merged commit 792bde9 into libsdl-org:main Oct 20, 2025
43 checks passed
@slouken
Copy link
Collaborator

slouken commented Oct 20, 2025

Merged, thanks!

@withSang
Copy link
Contributor Author

Merged, thanks!

Thanks a lot!! Have a good day :-)

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.

3 participants