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
kitty: fix loading of library #74874
Conversation
When trying to play a sound, kitty prints an error message because it cannot find `libcanberra.so`: ``` Failed to load libcanberra.so, cannot play beep sound, with error: libcanberra.so.0.2.5: cannot open shared object file: No such file or directory ``` This is fixed by patching the path to the library.
BTW, is there a good way to do this upstream in kitty so that we don't need this patch? |
Testing build on darwin |
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 on NixOS unstable (tested with nix-review
).
printf \\a
in kitty
makes a sound, no warning about libcanberra
.
Still, Darwin build should be checked too.
Could |
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.
One of the reasons we prefer patches to environment variables is that env vars get inherited by child programs. This is especially bad with terminal emulator, which will be running other programs. LD_LIBRARY_PATH
will take precedence over their DT_RUNPATH
entries, which Nix heavily relies on.
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.
One of the reasons we prefer patches to environment variables is that env vars get inherited by child programs. This is especially bad with terminal emulator, which will be running other programs. LD_LIBRARY_PATH
will take precedence over their DT_RUNPATH
entries, which Nix heavily relies on.
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.
One of the reasons we prefer patches to environment variables is that env vars get inherited by child programs. This is especially bad with terminal emulator, which will be running other programs. LD_LIBRARY_PATH
will take precedence over their DT_RUNPATH
entries, which Nix heavily relies on.
++ to @jtojnar requested changes here. I actually ran into issues because my preferred terminal program had a wrapper with Though this isn't meant to trivialize @Ma27 generosity for helping here ❤️ Also about #74874 (comment), you can split the patch into a separate file and apply it conditionally. |
I see, thanks for elaborating! I can confirm that the author's change fixes the issue, so I'd suggest I alter the patch to apply the librarie's changes on Linux only, then this should be good to go. As the issue exists on release-19.09 as well (the libcanberra dependency was introduced somewhere in 0.14), I'd also do the backport :) |
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.
The original change fixes the issue, I pushed a fix to ensure that stuff like libcanberra is only patched on linux. @jtojnar anything to add? :)
Not from my side, I actually wanted to wait until @jtojnar approves, but as it seems fine from my side, I'd merge tomorrow :) |
Btw I was wrong about the backport, it seems as the lazy loading of libcanberra has been introduced in 0.15.0 (previously it was a build dependency): kovidgoyal/kitty#2089 |
When trying to play a sound, kitty prints an error message because it cannot find
libcanberra.so
:This is fixed by patching the path to the library.
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nix-review --run "nix-review wip"
./result/bin/
)nix path-info -S
before and after)Notify maintainers
cc @tex @rvolosatovs @Ma27