Skip to content

Comments

wrapFirefox: handle binary wrappers#171985

Merged
vcunat merged 1 commit intoNixOS:staging-nextfrom
ncfavier:fix-firefox
May 9, 2022
Merged

wrapFirefox: handle binary wrappers#171985
vcunat merged 1 commit intoNixOS:staging-nextfrom
ncfavier:fix-firefox

Conversation

@ncfavier
Copy link
Member

@ncfavier ncfavier commented May 7, 2022

Fixes #171497 (comment)

We can't just edit binary wrappers in place because of a length mismatch, so we have to parse the generating makeCWrapper call out of the binary, extract wrapper arguments from it and add them to the Firefox wrapper.

All these contortions are needed because Firefox looks for its runtime in argv0, so the proper argv0 needs to be set by wrappers to always point to the "final" runtime. I think this could be avoided by wrapping /lib/$libName/firefox instead of /bin/firefox, and I'd like to look into that in the future, but for now I'm just fixing the immediate problem.

Tested by running firefox-bin with and without extraPrefs, both on staging-next and nixos-unstable.

@ncfavier ncfavier requested a review from mweinelt as a code owner May 7, 2022 18:16
We can't just edit binary wrappers in place because of a length
mismatch, so we have to parse the generating makeCWrapper call out of
the binary, extract wrapper arguments from it and add them to the
Firefox wrapper.

All these contortions are needed because Firefox looks for its runtime
in argv0, so the proper argv0 needs to be set by wrappers to always
point to the "final" runtime. I think this could be avoided by wrapping
/lib/$libName/firefox instead of /bin/firefox, and I'd like to look into
that in the future, but for now I'm just fixing the immediate problem.
@ofborg ofborg bot added 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. labels May 7, 2022
Copy link
Member

@vcunat vcunat left a comment

Choose a reason for hiding this comment

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

No truly in-depth review, but it seems OK, and we need at least some fix for now.

@vcunat vcunat merged commit 7fc4acf into NixOS:staging-next May 9, 2022
@vcunat
Copy link
Member

vcunat commented May 9, 2022

I think this could be avoided by wrapping /lib/$libName/firefox instead of /bin/firefox, and I'd like to look into that in the future, but for now I'm just fixing the immediate problem.

Beware that wrapFirefox is also used by packages that don't have much to do with Firefox, e.g. midori (webkit-based).

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 on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants