This was previously blocked because it depended on #386759, but that has been merged four days ago. What is the status on this? Any blockers?

I am also a little concerned about the seemingly flaky tests reported in #386759 (comment). If this becomes a channel blocker over at hydra, things will be ugly.

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 386495

x86_64-linux

I now set my main system to use SDL_compat through sdl12-compat, and everything still builds and works perfectly. Including bottles, which sometimes can be fragile to dosbox changes. From a functionality perspective, i call this ready.

I am also a little concerned about the seemingly flaky tests reported in #386759 (comment). If this becomes a channel blocker over at hydra, things will be ugly.

Good point. There is an environment variable that skips timer tests. Perhaps we should set it preemptively.

I am also a little concerned about the seemingly flaky tests reported in #386759 (comment). If this becomes a channel blocker over at hydra, things will be ugly.

Good point. There is an environment variable that skips timer tests. Perhaps we should set it preemptively.

if this is as easy as an export in preCheck then i am in favor of that change. And that can be a separate commit with its own explanation too, will make the git blame more helpful.

if this is as easy as an export in preCheck then i am in favor of that change. And that can be a separate commit with its own explanation too, will make the git blame more helpful.

Done

Oh uh... Crap, you are setting yourself up for merge conflicts. This commit should probably go into #388079 now that i think about it? Or you rebase one of the two PRs when the other gets the merge. but currently this will conflict. I am an idiot for telling you to do it in here, sorry!

Eh, you are smart, you'll figure it out.

#388079 addresses the libx11 thing, correct?

#388079 addresses the libx11 thing, correct?

i just tried, and no; commenting out libX11 here, SDL_compat already fails to build.

After some experimenting and reading about dependency propagation my current
understanding is that the proper fix is to remove propagatedBuildInputs from
SDL{1,2,3} and SDL_compat.

My reasoning: Package B should add package A to propagatedBuildInputs if any
package C depending on package B also depends (directly) on package A, for
example, because compiling package C requires passing -lB -lA to the compiler.
This is not the case for any of the SDL libs; it is possible to compile a
package that only depends on the SDL itself (excluding standard dependencies
like libc). If you look at the outputs of *-config tools you can see that on
NixOS they only link to the SDL library.

$ sdl-config --libs
-L/nix/store/ryxfmihhya90il7fij24g5l5l23vql9s-SDL-1.2.15/lib -Wl,-rpath,/nix/store/ryxfmihhya90il7fij24g5l5l23vql9s-SDL-1.2.15/lib -lSDL -lpthread -L/nix/store/nhrql1l31nyz7hg8n16mw3g4vza0r8zy-SDL-1.2.15-dev/lib -L/nix/store/nhrql1l31nyz7hg8n16mw3g4vza0r8zy-SDL-1.2.15-dev/lib -L/nix/store/nhrql1l31nyz7hg8n16mw3g4vza0r8zy-SDL-1.2.15-dev/lib

$ sdl2-config --libs
-L/nix/store/0k3g1sgznf7mpz1ycg0sdnlfb3r5405f-SDL2-2.32.0/lib -Wl,-rpath,/nix/store/0k3g1sgznf7mpz1ycg0sdnlfb3r5405f-SDL2-2.32.0/lib -Wl,--enable-new-dtags -lSDL2

$ pkg-config --libs sdl3
-L/nix/store/1lc2wbfgf3ll0qa5capjkp1b0dhi81m3-sdl3-3.2.6-lib/lib -Wl,-rpath,/nix/store/1lc2wbfgf3ll0qa5capjkp1b0dhi81m3-sdl3-3.2.6-lib/lib -Wl,--enable-new-dtags -lSDL3

It makes sense that this is the case: If an SDL app wants to create a window
then it calls SDL_CreateWindow which transfers the control to the shared
library libSDL.so which then either dlopens or links directly to the
appropriate backend, like X11, and calls the appropriate functions there. Notice
that the app is oblivious to whether X11 is present in the system, it may as
well be talking to Wayland. The actual backend is a direct dependency of the
libSDL.so so it's present on the system due to being in buildInputs of SDL
package.

Of course, some packages may break the abstraction and choose to talk to
X11/Pipewire/Vulkan/whatever directly, bypassing the SDL, but then they should
declare it via their buildInputs and not rely on SDL to propagate it. This is
the case here, with SDL_compat depending on X11.

I have managed to build some simple applications after removing propagated build
inputs from the SDL packages and they are running fine so experimentally it
seems to be working too.

Is this correct or is there some other reason requiring the SDL packages to
propagate deps? @getchoo, perhaps you know since you've recently packaged SDL3.

Some context about these: https://github.com/libsdl-org/sdl12-compat/blob/35ef746cf1a4fde8c442dc92fe9b18c3f90d7855/include/SDL/SDL_opengl.h#L51
Turns out SDL_compat does explicitly use GL headers. Surprisingly many packages use that include and just expect GL to be there. I am happy to fix these, but i am no longer certain killing the propagatedBuildInputs was the correct decision. At least this is out of the mass-rebuild path, so these breaks all only have minor impact.

Some context about these: https://github.com/libsdl-org/sdl12-compat/blob/35ef746cf1a4fde8c442dc92fe9b18c3f90d7855/include/SDL/SDL_opengl.h#L51 Turns out SDL_compat does explicitly use GL headers.

This is an optional include file and if a package uses it then it should declare OpenGL in its dependencies. This is also the position of the SDL contributors: libsdl-org/sdl2-compat#405 (comment).

Surprisingly many packages use that include and just expect GL to be there.

If a package uses it and does not depend on libGL then it's a problem with the derivation. Failures like these were expected.

Yeah, I don't think it makes sense to propagate optional dependencies that are not used by a lot of things.