Skip to content

gdlauncher-carbon: init at 2.0.20#297096

Merged
FliegendeWurst merged 2 commits intoNixOS:masterfrom
huantianad:gdlauncher-carbon
Dec 14, 2024
Merged

gdlauncher-carbon: init at 2.0.20#297096
FliegendeWurst merged 2 commits intoNixOS:masterfrom
huantianad:gdlauncher-carbon

Conversation

@huantianad
Copy link
Contributor

@huantianad huantianad commented Mar 19, 2024

Description of changes

Package request issue: #269067

Packaged by extracting app.asar as well as binary blob from provided AppImage, and wrapping with our own electron.

Haven't done much testing, so putting this as a draft for now, just to get some eyes on this. I also haven't added any of the libraries that MC itself needs. I wonder if we could borrow some code from prismlauncher.

If wanted, we could also provide a derivation built from source; however as mentioned in the request issue thread, it won't have all the features as the pre-built version comes with an API key for their own servers.

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.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 0 This PR does not cause any packages 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 Mar 19, 2024
@huantianad
Copy link
Contributor Author

@PassiveLemon You might be interested!

Comment on lines 52 to 93
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'm not 100% sure this is needed, but I'm assuming that app does depend on xdg-utils to open up browser pages, including for signin. Would have to test on a system without it though.

@PassiveLemon
Copy link
Contributor

I'll check it out later, looks promising though

@PassiveLemon
Copy link
Contributor

Upon running GDL, it returns: cannot execute /nix/store/lcvhr3s23c2i53ackxh8c39lggcfs0mq-gdlauncher-carbon-2.0.1/share/gdlauncher-carbon/resources/binaries/core_module: You are trying to run an unpatched binary on nixos, but you have not configured NIX_LD or NIX_LD_x86_64-linux. See https://github.com/Mic92/nix-ld for more details
The icon is also not used.
Seems like something is up. My personal package works fine however.

@huantianad
Copy link
Contributor Author

huantianad commented Mar 19, 2024

Wups I've been testing this with mix-ld 🤦

Also wdym about the icon?

@PassiveLemon
Copy link
Contributor

Also wdym about the icon?

The icon is simply not in my app launcher.
image

@huantianad
Copy link
Contributor Author

Man I thought I fixed that with my recent force pushes, can you check to see if the icons were installed in the right folders?

@PassiveLemon
Copy link
Contributor

I see icons in both $out/share/icons/hicolor/128x128/apps/gdlauncher-carbon.png and 512x512 and I can open both in feh so I'm not really sure what the issue is there.

In my package, the icon was installed in $out/share/pixmaps/gdlauncher-carbon.png so I added a line to install in there in the derivation and it does find the icon after that.

@huantianad
Copy link
Contributor Author

That's strange, pixmap shouldn't be the place to put these icons tho. Maybe I should also add a 48x48 and 96x96 size?

@PassiveLemon
Copy link
Contributor

PassiveLemon commented Mar 19, 2024

Nvm, it actually does work at those sizes. Didn't realize I had to restart my WM for the icon to update. Guess there was some caching, sorry for the concern lol

@huantianad huantianad changed the title gdlauncher-carbon: init at 2.0.1 gdlauncher-carbon: init at 2.0.2 Mar 26, 2024
@huantianad
Copy link
Contributor Author

I've fixed the core binary issue as well as added some of the libraries from prismlauncher. Not sure if all of them are necessary, or if the ones I skipped are also needed. Perhaps @Scrumplex can give some insight here.

@Scrumplex
Copy link
Member

glfw and openal might not be needed here, as Prism Launcher has a feature to override the versions shipped by LWJGL, while I assume GDLauncher does not

@PassiveLemon
Copy link
Contributor

Tested the package, it works

@huantianad
Copy link
Contributor Author

glfw and openal might not be needed here, as Prism Launcher has a feature to override the versions shipped by LWJGL, while I assume GDLauncher does not

Makes sense, I've mentioned the feature to the dev and they seem interested in possibly adding in the future. For now, removing.

@huantianad huantianad marked this pull request as ready for review March 30, 2024 00:37
@FliegendeWurst FliegendeWurst added the 6.topic: games Gaming on NixOS label Nov 17, 2024
Copy link
Member

Choose a reason for hiding this comment

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

I think this is done automatically by all relevant icon users?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps I can try again; last time I checked, it didn't seem to work on KDE Plasma.

@github-actions github-actions bot removed the 6.topic: games Gaming on NixOS label Nov 20, 2024
@huantianad huantianad changed the title gdlauncher-carbon: init at 2.0.2 gdlauncher-carbon: init at 2.0.20 Nov 20, 2024
Copy link
Member

Choose a reason for hiding this comment

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

Use addDriverRunPath instead, see #275241

@Pandapip1
Copy link
Member

Currently failing with:

       error: evaluation aborted with the following error message: 'lib.customisation.callPackageWith: Function called without required argument "addOpenGLRunpath" at /ofborg/checkout/1/repo/38dca4e3aa6bca43ea96d2fcc04e8229/mr-est/ofborg-evaluator-3/pkgs/by-name/gd/gdlauncher-carbon/package.nix:16'

@huantianad
Copy link
Contributor Author

By the way, if someone could help co-maintain this package, would be appreciated, as I don't use it myself at all.

@FliegendeWurst FliegendeWurst self-requested a review December 6, 2024 14:15
@FliegendeWurst FliegendeWurst added the 9.needs: maintainer Package or module needs active maintainers label Dec 6, 2024
@TsubakiDev
Copy link
Member

I have a lot of free time, can I apply to be a co-maintainer of this package?

@FliegendeWurst
Copy link
Member

I have a lot of free time, can I apply to be a co-maintainer of this package?

Yes, certainly :)

Since you are not listed in maintainer-list.nix yet, huantianad can add your details there and then add you to meta.maintainers of this package.

@TsubakiDev
Copy link
Member

TsubakiDev commented Dec 8, 2024 via email

@huantianad
Copy link
Contributor Author

Not sure what the procedure for this is, @TsubakiDev can you let me know what you want your entry in https://github.com/NixOS/nixpkgs/blob/master/maintainers/maintainer-list.nix to look like? Comment at the start has instructions.

I'll make a commit with that myself, or I suppose if you'd like you could make the commit in your own fork and I'll cherry-pick it into this PR branch (though that's probably unnecessarily complex).

@TsubakiDev
Copy link
Member

Not sure what the procedure for this is, @TsubakiDev can you let me know what you want your entry in https://github.com/NixOS/nixpkgs/blob/master/maintainers/maintainer-list.nix to look like? Comment at the start has instructions.

I'll make a commit with that myself, or I suppose if you'd like you could make the commit in your own fork and I'll cherry-pick it into this PR branch (though that's probably unnecessarily complex).

TsubakiDev = {
    email = i@tsubakidev.cc";
    github = "TsubakiDev";
    githubId = 132794625;
    name = "Daniel Wang";
  };

This is my entry at maintainer-list.nix.

@github-actions github-actions bot added the 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` label Dec 9, 2024
@github-actions github-actions bot removed the 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. label Dec 9, 2024
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Dec 9, 2024
@ofborg ofborg bot requested a review from TsubakiDev December 10, 2024 04:01
@ofborg ofborg bot added the 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. label Dec 10, 2024
@wegank wegank added the 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. label Dec 10, 2024
@FliegendeWurst FliegendeWurst removed the 9.needs: maintainer Package or module needs active maintainers label Dec 10, 2024
@github-actions github-actions bot removed the 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. label Dec 11, 2024
@ofborg ofborg bot requested a review from TsubakiDev December 12, 2024 00:04
@ofborg ofborg bot added the 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. label Dec 12, 2024
@FliegendeWurst FliegendeWurst merged commit 7a5df46 into NixOS:master Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages 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. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants