Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hyperrogue: 13.0v -> 13.0w #367624

Merged
merged 1 commit into from
Dec 30, 2024
Merged

hyperrogue: 13.0v -> 13.0w #367624

merged 1 commit into from
Dec 30, 2024

Conversation

linsui
Copy link
Contributor

@linsui linsui commented Dec 23, 2024

Closes #367519

Fix font not found error.

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.

@DimitarNestorov
Copy link
Member

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 367624


aarch64-darwin

✅ 1 package built:
  • hyperrogue

@DimitarNestorov DimitarNestorov added 8.has: package (update) This PR updates a package to a newer version 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Dec 28, 2024
Copy link
Contributor

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

Can you please:

  • Remove the with lib; in the meta attrs
  • Add
nativeInstallCheckInputs = [
  versionCheckHook
];
versionCheckProgramArg = [ "--version" ];
doInstallCheck = true;

after the installPhase.

  • Add
passthru = {
  updateScript = nix-update-script { };
};

just above meta.

  • Add
changelog = "https://github.com/zenorogue/hyperrogue/releases/tag/v${version}";

in meta

}:

stdenv.mkDerivation rec {
pname = "hyperrogue";
version = "13.0v";
version = "13.0w";

src = fetchFromGitHub {
owner = "zenorogue";
repo = "hyperrogue";
rev = "v${version}";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
rev = "v${version}";
tag = "v${version}";

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 367624


x86_64-linux

✅ 1 package built:
  • hyperrogue

aarch64-linux

✅ 1 package built:
  • hyperrogue

x86_64-darwin

✅ 1 package built:
  • hyperrogue

aarch64-darwin

✅ 1 package built:
  • hyperrogue

@linsui
Copy link
Contributor Author

linsui commented Dec 29, 2024

Suggestions applied. :)

@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 29, 2024
makeDesktopItem,
fontconfig,
versionCheckHook,
nix-update-script,
}:

stdenv.mkDerivation rec {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
stdenv.mkDerivation rec {
stdenv.mkDerivation (finalAttrs: {


src = fetchFromGitHub {
owner = "zenorogue";
repo = "hyperrogue";
rev = "v${version}";
sha256 = "sha256-bH5dnXLWU2D6Y70u7Cy2vqsli1GhID9af4J+21NSPHk=";
tag = "v${version}";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
tag = "v${version}";
tag = "v${finalAttrs.version}";

homepage = "https://www.roguetemple.com/z/hyper/";
description = "A roguelike game set in hyperbolic geometry";
changelog = "https://github.com/zenorogue/hyperrogue/releases/tag/v${version}";
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
changelog = "https://github.com/zenorogue/hyperrogue/releases/tag/v${version}";
changelog = "https://github.com/zenorogue/hyperrogue/releases/tag/v${finalAttrs.version}";

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 367624


x86_64-linux

✅ 1 package built:
  • hyperrogue

aarch64-linux

✅ 1 package built:
  • hyperrogue

x86_64-darwin

❌ 1 package failed to build:
  • hyperrogue

aarch64-darwin

❌ 1 package failed to build:
  • hyperrogue

@GaetanLepage
Copy link
Contributor

Are you sure that this game is compatible with darwin ?
While it builds fine, it crashes at startup during the version check:

Executing versionCheckPhase
Did not find version 13.0w in the output of the command /nix/store/a9pq2lvv0vrhgk9rcvqj7p6kxmh4j2bx-hyperrogue-13.0w/bin/hyperrogue --version
*** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Critical error: required built-in appearance SystemAppearance not found'
*** First throw call stack:
(
        0   CoreFoundation                      0x000000019e7fae80 __exceptionPreprocess + 176
        1   libobjc.A.dylib                     0x000000019e2e2cd8 objc_exception_throw + 88
        2   CoreFoundation                      0x000000019e7fad70 +[NSException exceptionWithName:reason:userInfo:] + 0
        3   AppKit                              0x00000001a22bfa30 __33+[NSAppearance _initializeCoreUI]_block_invoke + 88
        4   libdispatch.dylib                   0x000000019e4ed5b4 _dispatch_client_callout + 20
        5   libdispatch.dylib                   0x000000019e4eee00 _dispatch_once_callout + 32
        6   AppKit                              0x00000001a22e05c0 +[NSAppearance _aquaAppearance] + 60
        7   AppKit                              0x00000001a22bf19c +[NSAppearance appearanceNamed:] + 32
        8   AppKit                              0x00000001a22be814 -[NSSystemAppearanceProxy init] + 124
        9   AppKit                              0x00000001a22be788 __38+[NSSystemAppearanceProxy systemProxy]_block_invoke + 24
        10  libdispatch.dylib                   0x000000019e4ed5b4 _dispatch_client_callout + 20
        11  libdispatch.dylib                   0x000000019e4eee00 _dispatch_once_callout + 32
        12  AppKit                              0x00000001a22be76c +[NSSystemAppearanceProxy systemProxy] + 64
        13  AppKit                              0x00000001a22be6f8 -[NSApplication(NSApplicationAppearance_Internal) _registerForAppearanceNotifications] + 32
        14  AppKit                              0x00000001a22bc314 -[NSApplication init] + 908
        15  AppKit                              0x00000001a22bbdbc +[NSApplication sharedApplication] + 128
        16  hyperrogue                          0x00000001031cbe90 main + 192
        17  dyld                                0x000000019e320274 start + 2840
)
libc++abi: terminating due to uncaught exception of type NSException

@GaetanLepage
Copy link
Contributor

Also, it might just not be possible to start the game in the sandbox on darwin.
@khaneliman could I ask you to try to run this on your HW please ? (you'd have to disable the version check I guess).

@linsui
Copy link
Contributor Author

linsui commented Dec 29, 2024

It does have a binary for macos on https://zenorogue.itch.io/hyperrogue so I thought darwin is supported. But I don't have a mac to fix it. 🤷

@khaneliman
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 367624


x86_64-darwin

✅ 1 package built:
  • hyperrogue

aarch64-darwin

✅ 1 package built:
  • hyperrogue

Copy link
Contributor

@khaneliman khaneliman left a comment

Choose a reason for hiding this comment

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

Builds and plays for me on my mac. Might just be a sandbox issue.

@GaetanLepage
Copy link
Contributor

Builds and plays for me on my mac. Might just be a sandbox issue.

Thanks a lot for testing :)

@GaetanLepage
Copy link
Contributor

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 367624


x86_64-linux

✅ 1 package built:
  • hyperrogue

aarch64-linux

✅ 1 package built:
  • hyperrogue

x86_64-darwin

✅ 1 package built:
  • hyperrogue

aarch64-darwin

✅ 1 package built:
  • hyperrogue

Copy link
Contributor

@GaetanLepage GaetanLepage left a comment

Choose a reason for hiding this comment

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

Thanks all !

@GaetanLepage GaetanLepage merged commit 583d514 into NixOS:master Dec 30, 2024
23 of 24 checks passed
@linsui linsui deleted the hyperrogue branch December 30, 2024 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants