Skip to content

Improve SDL setup hook#296015

Merged
Artturin merged 2 commits intoNixOS:stagingfrom
sergv:sdl-setup-hook-improvements
Mar 18, 2024
Merged

Improve SDL setup hook#296015
Artturin merged 2 commits intoNixOS:stagingfrom
sergv:sdl-setup-hook-improvements

Conversation

@sergv
Copy link
Contributor

@sergv sergv commented Mar 14, 2024

This is a resurrection of #254465 which got accidentally closed without merging relevant changes.

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/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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.

sergv added 2 commits March 14, 2024 23:13
Putting everything that has lib/ directory is redundant and bloats
environment for projects with many dependencies.
@sergv sergv mentioned this pull request Mar 14, 2024
12 tasks
@ofborg ofborg bot added 10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches. labels Mar 14, 2024
@Artturin Artturin merged commit 3e00d41 into NixOS:staging Mar 18, 2024
# will contain "include/SDL/" and "lib/" directories.
#
# However the SDL_LIB_PATH is consumed by SDL itself and serves to locate
# libraries like SDL2_mixer, SDL2_image, etc which are not split-package
Copy link
Member

Choose a reason for hiding this comment

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

sdl2-mixer has out and dev however sdl-mixer doesn't

and there's no sdl-* which have split outputs fd --type d 'sdl_' pkgs/development/libraries | xargs -I {} cat "{}/default.nix" | grep outputs

#296919

Copy link
Member

@Artturin Artturin Mar 18, 2024

Choose a reason for hiding this comment

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

Maybe the lib file names could be matched for sdl? that could work with split outputs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we're accidentally fine - SDL_PATH is only used by the SDL package. For SDL2 there's SDL2_PATH and different setup hook, still employing similar structure and probably susceptible to sdl2-mixer having split outputs: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/libraries/SDL2/setup-hook.sh.

Maybe the lib file names could be matched for sdl? that could work with split outputs

Do you mean that shell in setup-hook.sh would somehow analyse its argument to determine whether we're looking at an sdl components package?

@sergv sergv deleted the sdl-setup-hook-improvements branch March 25, 2024 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 101-500 This PR causes between 101 and 500 packages to rebuild on Darwin. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 2501-5000 This PR causes many rebuilds on Linux and should target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants