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
signal-desktop: use autoPatchelfHook, wrap properly #77850
Conversation
Most prominently we don't use `phases` because of NixOS#28910. This is also problematic when using wrapGAppsHook. In order to use wrapGAppsHook's automatic wrapping (this was done manually before because there was no fixupPhase) we need to install signal at lib/Signal instead of just into libexec. That's because it would try to wrap .so files.
1246481
to
3a94b5d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rebased on master to resolve a new conflict, since I had to merge #77924 first.
The change itself LGTM and should make the code more robust and readable as well, thanks :)
However, unfortunately this seems to break Signal-Desktop on my setup:
$ signal-desktop
Set Windows Application User Model ID (AUMID) { appUserModelId: 'org.whispersystems.signal-desktop' }
NODE_ENV production
NODE_CONFIG_DIR /nix/store/59r8ddzw0nc1fh6yvyd04rmhi2gnd74j-signal-desktop-1.29.6/lib/Signal/resources/app.asar/config
NODE_CONFIG {}
ALLOW_CONFIG_MUTATIONS undefined
HOSTNAME undefined
NODE_APP_INSTANCE undefined
SUPPRESS_NO_CONFIG_WARNING undefined
userData: /home/michael/.config/Signal
config/get: Successfully read user config file
config/get: Successfully read ephemeral config file
Unhandled Error: Error: libstdc++.so.6: cannot open shared object file: No such file or directory
at process.func (electron/js2c/asar.js:138:31)
at process.func [as dlopen] (electron/js2c/asar.js:138:31)
at Object.Module._extensions..node (internal/modules/cjs/loader.js:828:18)
at Object.func (electron/js2c/asar.js:138:31)
at Object.func [as .node] (electron/js2c/asar.js:147:18)
at Module.load (internal/modules/cjs/loader.js:645:32)
at Function.Module._load (internal/modules/cjs/loader.js:560:12)
at Module.require (internal/modules/cjs/loader.js:685:19)
at require (internal/modules/cjs/helpers.js:16:16)
at Object.<anonymous> (/nix/store/59r8ddzw0nc1fh6yvyd04rmhi2gnd74j-signal-desktop-1.29.6/lib/Signal/resources/app.asar/node_modules/@journeyapps/sqlcipher/lib/sqlite3.js:4:15)
Can you reproduce this error or does it work fine on your system?
Edit: I guess --prefix LD_LIBRARY_PATH : "${stdenv.cc.cc.lib}/lib"
was there to prevent this.
pkgs/applications/networking/instant-messengers/signal-desktop/default.nix
Show resolved
Hide resolved
pkgs/applications/networking/instant-messengers/signal-desktop/default.nix
Outdated
Show resolved
Hide resolved
@primeos I believe |
@GrahamcOfBorg test signal-desktop (To double check, while I test it locally) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works flawlessly now, I didn't notice anything strange while testing it :)
Thanks for this awesome refactoring and improvement of the wrapper + ELF patching :)
Most prominently we don't use `phases` because of NixOS#28910. This is also problematic when using wrapGAppsHook. In order to use wrapGAppsHook's automatic wrapping (this was done manually before because there was no fixupPhase) we need to install signal at lib/Signal instead of just into libexec. That's because it would try to wrap .so files. (cherry picked from commit 15e0b23)
Most prominently we don't use
phases
because of #28910.This is also problematic when using wrapGAppsHook.
In order to use wrapGAppsHook's automatic wrapping
(this was done manually before because there was no fixupPhase)
we need to install signal at lib/Signal instead of just into libexec.
That's because it would try to wrap .so files.
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)