Skip to content

SDL_compat: patch sdl-config to use setup-hook#387357

Merged
pbsds merged 2 commits intoNixOS:masterfrom
marcin-serwin:push-qynqknmwzyux
Mar 10, 2025
Merged

SDL_compat: patch sdl-config to use setup-hook#387357
pbsds merged 2 commits intoNixOS:masterfrom
marcin-serwin:push-qynqknmwzyux

Conversation

@marcin-serwin
Copy link
Contributor

@marcin-serwin marcin-serwin commented Mar 5, 2025

The setup hook creating SDL_PATH variable was copied from the SDL1, however, the necessary patch to sdl-config.in was not adapted making the hook useless.

This will allow to use the SDL_compat as SDL replacement without manual hacks like this:

# when using SDL_compat instead of SDL1, SDL_mixer isn't correctly detected,
# but there is no harm just specifying it
env.NIX_CFLAGS_COMPILE = toString [
"-I${lib.getDev SDL_mixer}/include/SDL"
];

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Mar 5, 2025
@nix-owners nix-owners bot requested a review from peterhoeg March 5, 2025 18:50
@pbsds
Copy link
Member

pbsds commented Mar 9, 2025

Diff LGTM, running a nixpkgs-review...

@pbsbot
Copy link

pbsbot commented Mar 9, 2025

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 387357 --checkout commit


x86_64-linux

✅ 15 packages built:
  • SDL_compat
  • bottles
  • dosbox
  • katawa-shoujo
  • keen4
  • open-watcom-v2
  • open-watcom-v2-unwrapped
  • openxcom
  • rott
  • rott-shareware
  • stardust
  • zandronum
  • zandronum-alpha
  • zandronum-alpha-server
  • zandronum-server

@pbsds
Copy link
Member

pbsds commented Mar 9, 2025

This will allow to use the SDL_compat as SDL replacement without manual hacks like this:

Could you remove one such patch in this PR to show it works?

@github-actions github-actions bot added the 6.topic: games Gaming on NixOS label Mar 9, 2025
@marcin-serwin
Copy link
Contributor Author

Could you remove one such patch in this PR to show it works?

I've removed it from the rott package.

@nix-owners nix-owners bot requested a review from svanderburg March 9, 2025 20:40
Copy link
Contributor

@LordGrimmauld LordGrimmauld left a comment

Choose a reason for hiding this comment

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

Nice work! The CI seems successful (tho i am running nixpkgs-review again just to be sure.

@marcin-serwin marcin-serwin force-pushed the push-qynqknmwzyux branch 2 times, most recently from d676afb to 69924c6 Compare March 10, 2025 16:05
The setup hook creating SDL_PATH variable was copied from the SDL1,
however, the necessary patch to `sdl-config.in` was not adapted making
the hook useless.
@LordGrimmauld
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 387357 --skip-package open-watcom-v2-unwrapped


x86_64-linux

✅ 14 packages built:
  • SDL_compat
  • bottles
  • dosbox
  • katawa-shoujo
  • keen4
  • open-watcom-v2
  • openxcom
  • rott
  • rott-shareware
  • stardust
  • zandronum
  • zandronum-alpha
  • zandronum-alpha-server
  • zandronum-server

@pbsds pbsds merged commit 109737e into NixOS:master Mar 10, 2025
27 checks passed
@pbsds
Copy link
Member

pbsds commented Mar 10, 2025

Thanks!

@marcin-serwin marcin-serwin deleted the push-qynqknmwzyux branch March 10, 2025 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: games Gaming on NixOS 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants