Skip to content

{SDL1, SDL_compat}: migrate to by-name#339311

Merged
philiptaron merged 14 commits intoNixOS:masterfrom
museoa:rework-sdl
Sep 5, 2024
Merged

{SDL1, SDL_compat}: migrate to by-name#339311
philiptaron merged 14 commits intoNixOS:masterfrom
museoa:rework-sdl

Conversation

@AndersonTorres
Copy link
Member

@AndersonTorres AndersonTorres commented Sep 3, 2024

Description of 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.11 Release Notes (or backporting 23.11 and 24.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.

@AndersonTorres AndersonTorres marked this pull request as ready for review September 3, 2024 18:29
@AndersonTorres
Copy link
Member Author

AndersonTorres commented Sep 3, 2024

Related: #274836

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Sep 3, 2024
@ofborg ofborg bot requested a review from lovek323 September 3, 2024 19:11
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Sep 3, 2024
@emilazy
Copy link
Member

emilazy commented Sep 3, 2024

I’d rather drop SDL1 in favour of the compatibility library than refactor it.

@ofborg ofborg bot added the 8.has: clean-up This PR removes packages or removes other cruft label Sep 4, 2024
@peterhoeg
Copy link
Member

peterhoeg commented Sep 4, 2024

I’d rather drop SDL1 in favour of the compatibility library than refactor it.

I don't have any real numbers to back this up and my info is at least a year old, but there were problems in the past with SDL_compat breaking applications when swapping it in for SDL1. I don't know how big a problem it is and I appreciate that that number will continue to go down but for now, I am very much in favour of retaining both. That being said, we should absolutely try migrating as many as possible to SDL_compat.

@ofborg ofborg bot requested review from bjornfor, prusnak and svanderburg September 4, 2024 14:03
@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. and removed 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Sep 4, 2024
@AndersonTorres
Copy link
Member Author

AndersonTorres commented Sep 4, 2024

If you're having trouble, ping @NixOS/nixpkgs-vet

I do not even know what is happening here...

@AndersonTorres
SDL_compat: migrate to by-name
77b1cef
@AndersonTorres
SDL_compat: rewrite
ad2899f 
- finalAttrs design pattern
- get rid of nested with in meta
@AndersonTorres
SDL_classic: hide parameters under package expression
245276a
@AndersonTorres
SDL1: migrate to by-name
7159fac 
And green-alias both SDL1 and SDL_classic to it.
@AndersonTorres
Copy link
Member Author

AndersonTorres commented Sep 4, 2024

By running it locally, SDL_compat zero-rebuilds.
Given that this PR is mass-rebuilding, I will try to split it.

@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. and removed 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 Sep 4, 2024
@AndersonTorres
Copy link
Member Author

AndersonTorres commented Sep 4, 2024 •
edited
Loading

Hey!
Why giving name s for the patches caused mass rebuilds?

@philiptaron
Copy link
Contributor

philiptaron commented Sep 4, 2024 •
edited
Loading

Hey! Why giving name s for the patches caused mass rebuilds?

nix-diff will tell you. That being said, it's not so bad to target staging and call it a day if you like the change you've made in the PR.

@AndersonTorres
Copy link
Member Author

AndersonTorres commented Sep 4, 2024

Since @pbsds is dealing with the SDL1 "upgrade" (basically, packaging HEAD) at #274836, doing this particular cleanup is not worth the hassle.

@AndersonTorres AndersonTorres marked this pull request as ready for review September 4, 2024 22:54
@philiptaron philiptaron merged commit 0223e69 into NixOS:master Sep 5, 2024
@AndersonTorres AndersonTorres deleted the rework-sdl branch September 5, 2024 12:24
@LordGrimmauld LordGrimmauld mentioned this pull request Mar 11, 2025
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@philiptaron philiptaron philiptaron approved these changes

@peterhoeg peterhoeg Awaiting requested review from peterhoeg

@lovek323 lovek323 Awaiting requested review from lovek323

@prusnak prusnak Awaiting requested review from prusnak

@svanderburg svanderburg Awaiting requested review from svanderburg

@bjornfor bjornfor Awaiting requested review from bjornfor

Assignees

No one assigned

Labels

8.has: clean-up This PR removes packages or removes other cruft 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@AndersonTorres @emilazy @peterhoeg @Aleksanaa @philiptaron