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

firefox*: fix nativeMessagingHosts #294972

Closed
wants to merge 1 commit into from

Conversation

zeorin
Copy link
Contributor

@zeorin zeorin commented Mar 11, 2024

wrapFirefox calls lib.getBin on each of the packages in nativeMessagingHosts. I checked plasma-browser-integration and tridactyl-native, they both don't have a bin output defined, nor any other specific output, which IIRC means that everything that is built is put in the default out output that a derivation normally gives you. The native messenger files actually live in the lib directory.

What this means is that the wrapper script cannot find the native messaging files in the packages because it's looking in the wrong output.

By instead looking in the default output of the derivations, the native messaging JSON files are found correctly.

Fixes #281710.

Description of changes

By removing the call to lib.getBin and instead looking in the default output of the derivations, the native messaging JSON files are found correctly.

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.

`wrapFirefox` calls `lib.getBin` on each of the packages in
`nativeMessagingHosts`. I checked `plasma-browser-integration` and
`tridactyl-native`, they both don't have a `bin` output defined, nor any
other specific output, which IIRC means that everything that is built is
put in the default `out` output that a derivation normally gives you.
The native messenger files actually live in the `lib` directory.

What this means is that the wrapper script cannot find the native
messaging files in the packages because it's looking in the wrong
output.

By instead looking in the default output of the derivations, the native
messaging JSON files are found correctly.

Fixes NixOS#281710.
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild labels Mar 11, 2024
@zeorin zeorin closed this Mar 11, 2024
@getreu
Copy link
Contributor

getreu commented Apr 2, 2024

@zeorin You closed this issue although the problem still persists (#281710).

What is wrong about your solution?

@zeorin
Copy link
Contributor Author

zeorin commented Apr 2, 2024

I had misunderstood what lib.getBin actually does. I didn't realise that in the absence of a bin output, it would fall back to the out output. None of the packaged native messengers actually have any output other than out and so the use of lib.getBin is not the cause of the issue. Using lib.getLib is more appropriate, since the native messenger JSON files are in $out/lib/mozilla/native-messaging-hosts/, but, again, it's not currently an issue. This PR wouldn't actually have fixed the issue.

I do want to create a PR to use lib.getLib instead of lib.getBin, but I currently have this PR open, and I'm waiting for that to be handled before opening another one. Not because it'd be dependent on the open one, but just because it's my first nixpkgs PR and I wanted to wait for it to be merged.

@getreu
Copy link
Contributor

getreu commented Apr 2, 2024

Thank you for your detailed explanation. I wish you good luck with your PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild 10.rebuild-linux: 0 This PR does not cause any packages to rebuild
Projects
None yet
Development

Successfully merging this pull request may close these issues.

programs.firefox.nativeMessagingHosts not working
2 participants