nuclear: 0.6.6 -> unstable-2022-04-20

[WolfangAukang]

Description of changes

Latest stable version (0.6.17) does not work, but one of the latest pre-releases works fine. Also, as this app is still an alpha, using an unstable version shouldn't be a problem.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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/)
• 22.05 Release Notes (or backporting 21.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
  • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
• Fits CONTRIBUTING.md.

[IvarWithoutBones]

I'm getting the following error when running this from nixpkgs-review:

[nix-shell:~/.cache/nixpkgs-review/pr-169347-1]$ ./results/nuclear/bin/nuclear
      main › (node:2) [DEP0005] DeprecationWarning: Buffer() is deprecated due to security and usability issues. Please use the Buffer.alloc(), Buffer.allocUnsafe(), or Buffer.from() methods instead.
(Use `nuclear --trace-deprecation ...` to show where the warning was created)
[2:0421/193633.580170:FATAL:gpu_data_manager_impl_private.cc(415)] GPU process isn't usable. Goodbye.

Adding the --no-sandbox flags fixes it, although that's a bit annoying. According to LedgerHQ/ledger-live-desktop#4253 (comment) this should be fixed with an upstream electron update, we might want to wrap the binary in the meantime.

Other than that, this looks good to me 👍

[WolfangAukang]

we might want to wrap the binary in the meantime.

Sure thing, just wrapped it

[IvarWithoutBones]

I have created an issue upstream so that we can hopefully get this fixed in a future release. See nukeop/nuclear#1268.

[IvarWithoutBones]

This way the fix only applies to the desktop file, so launching the app from the terminal would still cause issues. You should use wrapProgram instead:

Suggested change

	--replace 'Exec=AppRun' 'Exec=${pname} --no-sandbox'
	--replace 'Exec=AppRun' 'Exec=${pname}'
	# Temporary fix for https://github.com/nukeop/nuclear/issues/1268
	wrapProgram $out/bin/nuclear --add-flags "--no-sandbox"

[WolfangAukang]

Here is the thing: To do this, you need to run wrapProgram into a nativeBuildInputs list so it can be recognized for its usage on the phase. As this is an AppImage, I'm not sure of the correct approach, as nativeBuildInputs is an anonymous function at this scope. That's why I only wrapped the desktop file.

One plan is to try using stdenv.mkDerivation, but I haven't found an example of this with AppImages. and I've found an example with blockbench-electron, going to try it

[WolfangAukang]

Ok, I was able to move it into an mkDerivation, but some considerable observations:

• As we are just wrapping to wait for a fix, it would be preferred to revert it into the old structure (the AppImage wrapType2) after the fix is finally applied.
• Nuclear maximum supported Electron version is 13, which is currently marked as insecure.

[IvarWithoutBones]

Mhmm, that does seem a bit annoying. I saw the other PR that updates this used extraPkgs = pkgs: with pkgs; [ wrapGAppsHook ];, which would only do anything if it's an nativeBuildInput right? Wouldn't that work the same with makeWrapper?

[WolfangAukang]

Unfortunately I still get a wrapProgram: command not found. I'm using makeWrapper as an input.

[WolfangAukang]

Moving to NUR