@microsoft-github-policy-service agree

Hi @constcuriosity, thank you for the contribution. Unfortunately, all feature sets in ports should be side by side installable. This is implementing alternatives as features. See -> https://learn.microsoft.com/en-us/vcpkg/contributing/maintainer-guide#do-not-use-features-to-implement-alternatives

Currently, vcpkg install imgui[sdl2-binding,sdl3-binding] will not build.

Marking this vcpkg-team-review for a path forward.

Hi @JavierMatosD, thanks for taking a look at this pull request. I'll take a look at seeing if there's a way forward for these two features to compile together.

Do you know offhand if all of the directx version features are able to compile together? I'm just looking to extend the pattern that's already been established by the port.

Hi @JavierMatosD, thanks for taking a look at this pull request. I'll take a look at seeing if there's a way forward for these two features to compile together.

I spoke with my coworkers and it seems that the SDL project currently calls 2.x "current". See -> https://www.libsdl.org/. We think SDL2 should remain the canonical one for now. We encourage you to create an overlay port with this added functionality for your own purposes. See -> https://learn.microsoft.com/vcpkg/concepts/overlay-ports

Do you know offhand if all of the directx version features are able to compile together? I'm just looking to extend the pattern that's already been established by the port.

These features were probably added erroneously.

I'm closing this PR for now. Please feel free to reopen the PR if you feel we are mistaken.

I spoke with my coworkers and it seems that the SDL project currently calls 2.x "current". See -> https://www.libsdl.org/. We think SDL2 should remain the canonical one for now. We encourage you to create an overlay port with this added functionality for your own purposes. See -> https://learn.microsoft.com/vcpkg/concepts/overlay-ports

SDL3 has hit API and ABI lock as listed at https://wiki.libsdl.org/SDL3/FrontPage as well as in the formation of the sdl3 port a bit ago #40867. While not official yet, projects are now encouraged to move towards adoption of SDL3.

While you can have a vcpkg.json file that includes both sdl2 and sdl3, you can't follow the usage lines for both of them in your own cmake files. Doing so will have the 2nd library inclusion complain about a difference between the INTERFACE_SDL_VERSION and the SDL_VERSION of the next to-be-included library. And this is what causes the sdl2-binding and sdl3-binding incompatibility. Those features request either sdl2 or sdl3 as dependencies, and turning on both features will attempt to enforce that both are included, which triggers the above error.

This feels like it breaks the spirit of Ports in the current baseline must be installable simultaneously, since while you can technically install both SDL2 and SDL3 simultaneously, you can't have both of them as a dependency. If that was possible, then these features would work fine together.

Practically though... I don't know when that'd really happen. I think the vast majority of projects would only ever use one or the other. And same would go for these features on the imgui port. I guess the workaround would be to remove the dependency on each of the features, and rely on the project author add the dependencies themselves, but that's counter to the goal of vcpkg.

So with the TL;DR "It's not the features' fault, it's SDL2 and SDL3 incompatibility", I'd like to request an exception to the "no mutually exclusive features" policy. Making an overlay port would certainly work but it introduces a continuous maintenance burden by forking a frequently updated port (the version was even bumped within the short lifetime of this PR), and I believe the likelihood of a project purposefully including both SDL versions and both features to be very small.

Thanks for your time.

@JavierMatosD Sorry to ping you directly but it doesn't look like I have permission to re-open the PR myself.

This feels like it breaks the spirit of Ports in the current baseline must be installable simultaneously, since while you can technically install both SDL2 and SDL3 simultaneously, you can't have both of them as a dependency. If that was possible, then these features would work fine together.

Practically though... I don't know when that'd really happen. I think the vast majority of projects would only ever use one or the other. And same would go for these features on the imgui port. I guess the workaround would be to remove the dependency on each of the features, and rely on the project author add the dependencies themselves, but that's counter to the goal of vcpkg.

So with the TL;DR "It's not the features' fault, it's SDL2 and SDL3 incompatibility", I'd like to request an exception to the "no mutually exclusive features" policy. Making an overlay port would certainly work but it introduces a continuous maintenance burden by forking a frequently updated port (the version was even bumped within the short lifetime of this PR), and I believe the likelihood of a project purposefully including both SDL versions and both features to be very small.

Thanks for your time.

I want to voice my support for @constcuriosity's request for an exception to the abovementioned rule.
SDL3 is just about to become the stable version. If you look at SDL's GitHub repository, you will see that they already switched the main branch to SDL3 and moved SDL2 into a separate branch.
As SDL2 will undoubtedly continue to be used by many existing applications due to the complexity of completely switching from SDL2 to SDL3, not exposing both imgui backend APIs for SDL2/3 as features is, IMHO, not an option for the foreseeable future.
I'd also like to request an exception to the mentioned rule since SDL2 and SDL3 are not compatible with each other anyway, and it would make no sense to install both features simultaneously.

I've authored a compromise where the sdl2-binding, sdl3-binding, sdl2-renderer-binding, and sdl3-renderer-binding features can all be included as enabled features. When both SDL2 and SDL3 features are enabled, the SDL2 features are basically ignored and a warning is logged that says that the SDL2 dependency was not enforced.

So the ports and features now technically satisfy the maintainer guidelines, but I wouldn't say it adheres to the spirit of them. Nevertheless it continues with my statement about that including both SDL2 and SDL3 features is not expected and very much not desired behavior.

So, what's the next move or general consensus of what to do?

@constcuriosity can you resolve the file conflicts and change the PR as ready for review.

@Cheney-W please test features and mark as reviewed when ready.

Hey, sorry I've been radio silent. My main work caught up with me post my holiday break.

I've gotten latest on master and updated the versions. Otherwise the functionality of the CMAKE files has stayed the same.

We wait until SDL3 is officially released and then drop SDL2 for SDL3 in the imgui port as proposed above.
=> This would, from that point onwards, prevent users with legacy SDL2 applications from upgrading to newer imgui versions via vcpkg.

After discussing this with @vicroms, @ras0219-msft, @AugP, @BillyONeal we believe that this is the right approach. Please more this out of draft when SDL3 is officially released.

Reminder, sdl2 features are expected to be removed at this point.

Hi, sorry as well for my radio silence the last few weeks... Work work...
I was about to say that the changes of commit ee88b23 would have to be reversed and all references to SDL2 would have to be removed before continuing. 🤔

We wait until SDL3 is officially released and then drop SDL2 for SDL3 in the imgui port as proposed above.
=> This would, from that point onwards, prevent users with legacy SDL2 applications from upgrading to newer imgui versions via vcpkg.

After discussing this with @vicroms, @ras0219-msft, @AugP, @BillyONeal we believe that this is the right approach. Please more this out of draft when SDL3 is officially released.

Alright... That doesn't bother my side project because I'm starting fresh with sdl3 and haven't used sdl2 before, but what's the recourse for anyone that was using sdl2 before? Are they expected to maintain their own overlay port at that point? Just want it to be very explicit for anyone that needs to deal with the downstream wake.

Also, it looks like because I did a merge on some of the files instead of a complete stomp and edit, it's blocking the pull request completely. Is there a way to salvage the pull, steam roll everything, or is it just cleaner to make a fresh pull request? Not really a fan of losing the context of this thread though.

Also, it looks like because I did a merge on some of the files instead of a complete stomp and edit, it's blocking the pull request completely. Is there a way to salvage the pull, steam roll everything, or is it just cleaner to make a fresh pull request? Not really a fan of losing the context of this thread though.

The "Merging is blocked ... This branch must not contain merge commits" also appears on clean PRs ATM. I suggest to ignore it.

Alright... That doesn't bother my side project because I'm starting fresh with sdl3 and haven't used sdl2 before, but what's the recourse for anyone that was using sdl2 before? Are they expected to maintain their own overlay port at that point? Just want it to be very explicit for anyone that needs to deal with the downstream wake.

I'll work on a new vcpkg port for the new "sdl2-compat"-layer I mentioned above. I plan to create a pull request after the sdl2-compat port is approved and merged, which adds the sdl2 feature back to the imgui port via the sdl2-compat layer. This way, users with existing sdl2 projects should still be able to continue using recent versions of imgui.

Ok, latest changes remove the SDL2 feature bindings and replaces them with the SDL3 ones.

New feature test passed with below triplets:

x86-windows
x64-windows
x64-windows-static

Finally, big thanks to everyone!
can't wait to actually use sdl3 :D