Skip to content

quakespasm: fix build on Darwin#346972

Merged
emilazy merged 1 commit intoNixOS:masterfrom
reckenrode:push-rmkwsrqpxnwm
Oct 7, 2024
Merged

quakespasm: fix build on Darwin#346972
emilazy merged 1 commit intoNixOS:masterfrom
reckenrode:push-rmkwsrqpxnwm

Conversation

@reckenrode
Copy link
Contributor

QuakeSpasm tries to use a 10.6 deployment target on Darwin, which is too old for SDL2. Use the default to fix the build.

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.

Copy link
Member

Choose a reason for hiding this comment

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

We could simply remove this to avoid the dependency on the deployment target variable, right? And in that case it could be unconditional, as it only affects Darwin compilation anyway and keeping patches unconditional is good hygiene.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably the way to go is to use sed to delete the lines that try to set the version. It uses different ones depending on the platform.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds reasonable. There’s already a patch to the Darwin Makefile so it could just be rolled into that.

@ofborg ofborg bot added the 6.topic: darwin Running or building packages on Darwin label Oct 7, 2024
@ofborg ofborg bot requested a review from mikroskeem October 7, 2024 01:57
@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 Oct 7, 2024
QuakeSpasm tries to use a 10.6 deployment target on Darwin, which is too
old for SDL2. Use the default to fix the build.
Copy link
Member

@emilazy emilazy left a comment

Choose a reason for hiding this comment

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

Confirmed build on x86_64-darwin and x86_64-linux.

@emilazy emilazy merged commit 10b89ee into NixOS:master Oct 7, 2024
@reckenrode reckenrode deleted the push-rmkwsrqpxnwm branch October 7, 2024 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: darwin Running or building packages on Darwin 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

Development

Successfully merging this pull request may close these issues.

2 participants