pingus: unstable-0.7.6.0.20191104 -> 0.7.6-unstable-2025-07-21 by SchweGELBin · Pull Request #451098 · NixOS/nixpkgs

SchweGELBin · 2025-10-11T19:31:11Z

I thought it would be a simple package update. I was wrong...

The package builds but doesn't seem to act right.
The upstream repo moved and doesn't use submodules anymore.
I ported all the dependencies. I might have made some mistakes, but checked everything twice.

Things done

Add a 👍 reaction to pull requests you find important.

Yarny0

Review per https://github.com/NixOS/nixpkgs/tree/0b4defa2584313f3b781240b29d61f6f9f7e0df3/pkgs#package-updates

Reviewed points

package name fits guidelines
package version fits guidelines (see below)
package builds on x86_64-linux (see below)
executables tested on x86_64-linux (see below)
any change of upstream are verified (see below)
the motives for any special packaging choices are documented
all depending packages build
patches have a comment describing either the upstream URL or a reason why the patch wasn't upstreamed (none)
patches that are remotely available are fetched rather than vendored (none)

Possible improvements

According to https://github.com/NixOS/nixpkgs/tree/0b4defa2584313f3b781240b29d61f6f9f7e0df3/pkgs#versioning , versions must start with a digit, so those need to be fixed (see also my inline comment at pingus package).
Non-blocking suggestion: Please move the commit "pingus: unstable-0.7.6.0.20191104 -> unstable-0.8.0-20250721" to the tip of the branch. That commit doesn't make sense without the other packages. nixpkgs probably even has problems evaluating with that commit alone, as it references missing packages. Reordering of commits would make it easier to do bisections in the repo later (even bisections that don't concern pingus or one of the new libraries would profit).

With hash = "sha256-jQYZM7VLqbl9/+QXyswEXdGmwOq/nxRzWARvcDqNM9M="; in pingus.src, I am able to build everything on my machine. However, starting $out/bin/pingus immediatelly fails with an exception that is not further explained:

$ rm -rf ~/.config/pingus*
$ LANG=en_US.UTF-8 result/bin/pingus -D
[INFO ] /build/source/src/util/system.cpp:257: System::create_dir: %1
[INFO ] /build/source/src/util/system.cpp:264: Successfully created: /home/pingu/.config/pingus-0.8
[INFO ] /build/source/src/util/system.cpp:257: System::create_dir: %1
[INFO ] /build/source/src/util/system.cpp:264: Successfully created: /home/pingu/.config/pingus-0.8/levels
[INFO ] /build/source/src/util/system.cpp:257: System::create_dir: %1
[INFO ] /build/source/src/util/system.cpp:264: Successfully created: /home/pingu/.config/pingus-0.8/levels/dist
[INFO ] /build/source/src/util/system.cpp:257: System::create_dir: %1
[INFO ] /build/source/src/util/system.cpp:264: Successfully created: /home/pingu/.config/pingus-0.8/themes
[INFO ] /build/source/src/util/system.cpp:257: System::create_dir: %1
[INFO ] /build/source/src/util/system.cpp:264: Successfully created: /home/pingu/.config/pingus-0.8/savegames
[INFO ] /build/source/src/util/system.cpp:257: System::create_dir: %1
[INFO ] /build/source/src/util/system.cpp:264: Successfully created: /home/pingu/.config/pingus-0.8/images
[INFO ] /build/source/src/util/system.cpp:257: System::create_dir: %1
[INFO ] /build/source/src/util/system.cpp:264: Successfully created: /home/pingu/.config/pingus-0.8/cache
[INFO ] /build/source/src/util/system.cpp:257: System::create_dir: %1
[INFO ] /build/source/src/util/system.cpp:264: Successfully created: /home/pingu/.config/pingus-0.8/demos
[INFO ] /build/source/src/util/system.cpp:257: System::create_dir: %1
[INFO ] /build/source/src/util/system.cpp:264: Successfully created: /home/pingu/.config/pingus-0.8/backup
[INFO ] /build/source/src/util/system.cpp:257: System::create_dir: %1
[INFO ] /build/source/src/util/system.cpp:264: Successfully created: /home/pingu/.config/pingus-0.8/screenshots
[INFO ] /build/source/src/pingus/pingus_main.cpp:103: /home/pingu/.config/pingus-0.8/config: config file not found
Welcome to Pingus 0.7.6-631-gb0ceeeeb9!
=======================================
userdir:                 /home/pingu/.config/pingus-0.8/
datadir:                 /nix/store/6f61bagndzg5jnvdzahsyjr0d2ypqyra-pingus-unstable-0.8.0-20250721/share/pingus
language:                English (en)
sound support:           enabled
music support:           enabled
fullscreen:              disabled

[INFO ] /build/source/src/engine/display/display.cpp:95: sdl isize(1024, 768) window
[INFO ] /build/source/src/pingus/savegame_manager.cpp:48: /home/pingu/.config/pingus-0.8/savegames/savegames.scm: savegame file not found
[DEBUG] /build/source/src/util/system.cpp:719: writing /home/pingu/.config/pingus-0.8/savegames/variables.scm
[DEBUG] /build/source/src/util/system.cpp:719: writing /home/pingu/.config/pingus-0.8/savegames/variables.scm
Pingus: Standard exception caught!:
unknown format

I tried this on a NixOS 25.05 machine and again inside a VirtualBox with NixOS unstable running, each time within plasma6.

I verified all licenses, mostly by looking at what Github project page says. See my inline comments on where changes are or might be indicated.

I suggest we add some notes in commit messages about why the license is chosen as it is, so we can reconstruct that if questions arise later.

Should we open some issues upstream about missing/unclear licenses?

Dear @SchweGELBin , thank you so much for you efforts in rescuing pingus! I really and wholeheartedly appreciate that!

Edit: For the record: I performed these tests after rebasing this PR's branch onto current nixos-unstable.

pkgs/by-name/ar/argpp/package.nix

pkgs/by-name/ge/geomcpp/package.nix

pkgs/by-name/pi/pingus/package.nix

pkgs/by-name/pr/priocpp/package.nix

pkgs/by-name/st/strutcpp/package.nix

pkgs/by-name/ti/tinygettext/package.nix

pkgs/by-name/ws/wstsound/package.nix

pkgs/by-name/pi/pingus/package.nix

SchweGELBin · 2025-10-12T10:54:13Z

@Yarny0, thank you very much for your detailed and helpful review!
I really appreciate this.

I answered most of your inline comments and want to add the following:

Regarding the licenses:
I've used free for those that don't have a explicitely named license in the repo.
I would like to keep that to avoid confusion.
But if it should be changed, I can definitely do that.

I will put the pingus changes at the end of the commits.
I just didn't change the order while working on this pr.

Oh, I forgot to add:
$ ./result/bin/pingus --version outputs "Pingus unknown-version"

SchweGELBin · 2025-10-12T13:38:01Z

Hey @Yarny0,

I've implemented all your suggestions.
Can you take another look?

I've also implemented checks/tests, added sexp-cpp as a dependency to priocpp and pingus works now:
No more "unknown format" or "unknown-version" anymore and the program can be launched normally.

There was already sexpp in nixpkgs, but this version did not work.

1 check in wstsound fails, so I commented it and disabled this check.
Everything else works perfectly.

Yarny0

Besides the homepage, this looks alright.

I built everything on x86_64-linux (again after rebasing onto current nixos-unstable). Now pingus starts and I was able to play some levels without any issues. Sound effects and background music sound lovely. This is great!

`nixpkgs-review` result

Generated using nixpkgs-review.

Command: nixpkgs-review
Commit: c3723d6b93967519dcd42c7fbeb4789ba5a2cd09

`x86_64-linux`

✅ 12 packages built:

argpp
geomcpp
logmich
pingus
priocpp
sexp-cpp
strutcpp
tinycmmc
tinygettext
uitest
wstsound
xdgcpp

pkgs/by-name/pi/pingus/package.nix

Yarny0

I have no further objections or remarks. This is great work, thanks!

nixos-discourse · 2025-10-12T17:07:33Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/2588

iljod · 2025-10-13T07:40:51Z

one thing, we really should keep it 1 commit that should get merged, not 13
So could you squash?

SchweGELBin · 2025-10-13T09:31:09Z

I can, but I thought that splitting them would make more sense.

Yarny0 · 2025-10-13T10:03:21Z

https://github.com/NixOS/nixpkgs/tree/0b4defa2584313f3b781240b29d61f6f9f7e0df3/pkgs#commit-conventions encourages one commit per new package and explains that proper commit titles even help automation. I don't really know anything about what's going on behind the scenes with CI, but I also don't see any problem with multiple commits. In light of that, I suggest to keep one commit per package. (However, I'm not a committer, so in the end, it's not my call.)

phanirithvij · 2025-10-14T17:34:03Z

`nixpkgs-review` result

Generated using nixpkgs-review-gha

Command: nixpkgs-review pr 451098

Logs: https://github.com/phanirithvij/nixpkgs-review-gha/actions/runs/18504770844

`x86_64-linux`

✅ 12 packages built:

argpp
geomcpp
logmich
pingus
priocpp
sexp-cpp
strutcpp
tinycmmc
tinygettext
uitest
wstsound
xdgcpp

`aarch64-linux`

✅ 12 packages built:

argpp
geomcpp
logmich
pingus
priocpp
sexp-cpp
strutcpp
tinycmmc
tinygettext
uitest
wstsound
xdgcpp

pkgs/by-name/xd/xdgcpp/package.nix

7c6f434c · 2025-10-15T01:37:58Z

I am not sure why CI does not build the added packages based on commit titles here.

One commit per package looks fine though.

Looks fine to me except for apparent duplication of one description.

7c6f434c · 2025-10-18T06:24:25Z

@ofborg build pingus

nixpkgs-ci bot added 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. labels Oct 11, 2025

nix-owners bot requested a review from 7c6f434c October 11, 2025 19:36

Yarny0 suggested changes Oct 12, 2025

View reviewed changes

SchweGELBin force-pushed the pingus-update branch 2 times, most recently from c7e66ef to 701e18f Compare October 12, 2025 12:51

SchweGELBin added 10 commits October 12, 2025 15:26

tinycmmc: init at 0.1.0-unstable-2025-07-21

6f4198f

logmich: init at 0.1.0-unstable-2025-07-21

0303b0a

sexp-cpp: init at 0.1.0-unstable-2025-07-21

cc5f51f

argpp: init at 0-unstable-2022-08-14

acba1fa

geomcpp: init at 0.0.0-unstable-2025-06-11

2557cfe

priocpp: init at 0.0.0-unstable-2025-07-21

c9e5439

strutcpp: init at 0-unstable-2025-07-21

28de29e

tinygettext: init at 0.1.0-unstable-2025-05-07

98f024e

uitest: init at 0-unstable-2024-05-26

ba8b10f

wstsound: init at 0.3.0-unstable-2025-07-21

b87fcf0

SchweGELBin force-pushed the pingus-update branch from 701e18f to c3723d6 Compare October 12, 2025 13:31

SchweGELBin changed the title ~~pingus: unstable-0.7.6.0.20191104 -> unstable-0.8.0-20250721~~ pingus: unstable-0.7.6.0.20191104 -> 0.7.6-unstable-2025-07-21 Oct 12, 2025

SchweGELBin requested a review from Yarny0 October 12, 2025 13:38

Yarny0 suggested changes Oct 12, 2025

View reviewed changes

pkgs/by-name/pi/pingus/package.nix Show resolved Hide resolved

SchweGELBin force-pushed the pingus-update branch from c3723d6 to e04d766 Compare October 12, 2025 16:22

Sigmanificient mentioned this pull request Oct 12, 2025

Tracking: Build Failures with CMake 4 #445447

Open

Yarny0 approved these changes Oct 12, 2025

View reviewed changes

nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Oct 12, 2025

7c6f434c reviewed Oct 15, 2025

View reviewed changes

pkgs/by-name/xd/xdgcpp/package.nix Outdated Show resolved Hide resolved

SchweGELBin added 3 commits October 15, 2025 12:00

xdgcpp: init at 0-unstable-2024-05-26

999175c

pingus: unstable-0.7.6.0.20191104 -> 0.7.6-unstable-2025-07-21

253987b

pingus: add SchweGELBin as maintainer

392900b

SchweGELBin force-pushed the pingus-update branch from e04d766 to 392900b Compare October 15, 2025 10:00

SchweGELBin requested a review from 7c6f434c October 15, 2025 10:09

7c6f434c added this pull request to the merge queue Oct 18, 2025

Merged via the queue into NixOS:master with commit 1a8abfd Oct 18, 2025
27 of 30 checks passed

SchweGELBin deleted the pingus-update branch October 18, 2025 10:11

Uh oh!

Conversation

SchweGELBin commented Oct 11, 2025

Things done

Uh oh!

Yarny0 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Review per https://github.com/NixOS/nixpkgs/tree/0b4defa2584313f3b781240b29d61f6f9f7e0df3/pkgs#package-updates

Reviewed points

Possible improvements

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SchweGELBin commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SchweGELBin commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Yarny0 left a comment

Choose a reason for hiding this comment

nixpkgs-review result

x86_64-linux

Uh oh!

Uh oh!

Yarny0 left a comment

Choose a reason for hiding this comment

Uh oh!

nixos-discourse commented Oct 12, 2025

Uh oh!

iljod commented Oct 13, 2025

Uh oh!

SchweGELBin commented Oct 13, 2025

Uh oh!

Yarny0 commented Oct 13, 2025

Uh oh!

phanirithvij commented Oct 14, 2025

nixpkgs-review result

x86_64-linux

aarch64-linux

Uh oh!

Uh oh!

7c6f434c commented Oct 15, 2025

Uh oh!

7c6f434c commented Oct 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Yarny0 left a comment •

edited

Loading

SchweGELBin commented Oct 12, 2025 •

edited

Loading

SchweGELBin commented Oct 12, 2025 •

edited

Loading

`nixpkgs-review` result

`x86_64-linux`

`nixpkgs-review` result

`x86_64-linux`

`aarch64-linux`