sdl2-compat: add setup hook and minor fixes#388079
Conversation
marcin-serwin
force-pushed
the
push-yqknyylorstk
branch
from
89dd642 to
e3db818
Compare
github-actions
bot
added
10.rebuild-darwin: 1
marcin-serwin
force-pushed
the
push-yqknyylorstk
branch
from
e3db818 to
734cbb2
Compare
|
Seems good to me. I wasn't exactly sure ehat the purpose of the setup hook for SDL2 was, and it seemed to work fine without, but we definitely need this if it helps with using sdl2-compat as a replacement for SDL2. |
|
I have a wip branch with update to ltris that relies on this if you want to
test. You can check that this updated derivation doesn't build on master.
marcin-serwin@ba65f6a
|
There was a problem hiding this comment.
cmake+ninja is usually way faster than just cmake+make.
There was a problem hiding this comment.
I'm a bit surprised, I always thought that ninja is only faster for incremental builds as claimed, e.g., here. (The article is a bit old though.)
I've benchmarked it for this specific package and ninja is faster by about 2 seconds on my machine:
$ time (runPhase configurePhase && runPhase buildPhase) # cmake+ninja
real 0m7,871s
user 0m39,559s
sys 0m7,954s$ time (runPhase configurePhase && runPhase buildPhase) # cmake+make
real 0m9,968s
user 0m37,053s
sys 0m10,434sI'm not sure if this speedup is worth the extra dependency though, especially since CPU time is about the same. (I'm guessing that CPU time is more important for Hydra, is that true?)
What do you think?
There was a problem hiding this comment.
For some big applications it really makes a difference but regardless I would just leave it here as we plan to use ninja for everything cmake related anyway.
There was a problem hiding this comment.
did you test this, especially on darwin?
There was a problem hiding this comment.
I don't use nix on darwin so I don't have any way to test this currently. I've tested the change on linux.
Note that this line doesn't add darwin to platforms - darwin is unix so it was already there - effectively it only adds windows which I wanted to test but I was blocked by #388297, I will try again shortly now that it's fixed.
There was a problem hiding this comment.
ah, good point. If it was previously unix it should be fine.
There was a problem hiding this comment.
I managed to cross compile the package itself for windows after fixing the rpath setting, but cross compiling anything dependent on it is problematic, since I realized that the setup hook assumes that host == target. I've tried building a simple game by supplying necessary flags manually but it failed to run in wine with Failed to initialize registry display settings for L"\\\\.\\DISPLAY1". I'm still leaving the platforms set to all since there is nothing unixy required by the library, and the issues are likely caused by cross compiling.
marcin-serwin
force-pushed
the
push-yqknyylorstk
branch
from
734cbb2 to
ccf25dc
Compare
github-actions
bot
removed
the
10.rebuild-linux: 1
wegank
added
12.approvals: 1
This adds a setup hook analogous to the one used by SDL2 package that picks up satelitte libraries during build time. This makes it possible to replace SDL2 with sdl2-compat in packages using satelitte libraries without further changes to build flags.
The SDL library has backends for many platforms, not just unix ones, however, it's currently restricted by its dependencies.
The slow tests rely on timers executing within a specified time which is not necessarily true if system is under a heavy load [1]. These tests are also disabled in the official CI pipeline [2]. [1]: NixOS#386759 (comment) [2]: https://github.com/libsdl-org/sdl2-compat/blob/9544651fb39deefb2dd1aed9113af3b60898a1bb/.github/workflows/main.yml#L116
marcin-serwin
force-pushed
the
push-yqknyylorstk
branch
from
ccf25dc to
c6e964d
Compare
6 participants
This adds a setup hook analogous to the one used by SDL2 package that
picks up satelitte libraries during build time. This makes it possible
to replace SDL2 with sdl2-compat in packages using satelitte libraries
without further changes to build flags.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.