-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
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
WIP: firefox/wrapper.nix: make firefox wrapper configurable by the user #57554
Conversation
This pull request has been mentioned on Nix community. There might be relevant details there: |
Nice, I too discovered that we could accomplish this with policies, good to see something in the works 👍 |
Thanks for taking this on! |
I am not quite sure what you mean by extensions being "imperative" in this context. |
This was actually my first approach, and I have an implementation using user namespaces and bind mounts in my own nixos config. I did not attempt to upstream that, because its quite "advanced" technology; nixpkgs is more than just linux. I just don't know if that works on all platforms we might want to support. Edit: The logic in my nixos config is overly complicated, because the distribution subdirectory does not exist in the nixpkgs firefox builds. If we changed that, it could be as simple as:
If this solution is preferred, I can also implement that. |
Imperative means it will permanently install these to your home directory instead of being confined to the package, as a package option should be. Maybe I am alone in this concern but that doesn't seem very compatible with the Nix philosophy. |
Ah ok, I see what you mean by imperative vs declarative now. |
For that you would need to calculate the new policy at runtime from the policy of the most recently executed version. I don't even want to begin to imagine the complexity involved. That would be hard in a NixOS option, and it sounds like just asking for trouble in a package option. |
Yes, that does sound rather complicated. Also there are still quite a few bugs in the current implementation of the related policies options. |
Actually, do we have a place for documentation of individual package options? |
Not really, but it would probably fit next to the vim section in the manual: https://nixos.org/nixpkgs/manual/#users-guide-to-vim-pluginsaddonsbundlesscripts-in-nixpkgs |
Ok, I ended up adding the docs to package-notes.xml. But we really need a separate manual for packages / package options. |
doc/package-notes.xml
Outdated
The older | ||
<link xlink:href="https://support.mozilla.org/en-US/kb/customizing-firefox-using-autoconfig"> | ||
AutoConfig </link> method, and the more recent | ||
<link xlink:href="https://github.com/mozilla/policy-templates/blob/master/README.md#extensions"> |
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 hook to #extensions
probably shouldn't be here, as the explanation is not extension-specific.
doc/package-notes.xml
Outdated
disables autoplaying videos, could be specified as follows (the syntax of | ||
AutoConfig is a superset of user.js): | ||
<programlisting> | ||
firefox.override { extraPrefs = '' pref("media.autoplay.default",1); ''; } |
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.
I'd prefer this to be split up over multiple lines for readability:
firefox.override {
extraPrefs = ''
pref("media.autoplay.default",1);
'';
}
doc/package-notes.xml
Outdated
</programlisting> | ||
To disable upload of telemetry data, you could use: | ||
<programlisting> | ||
firefox.override { extraPolicies = { DisableTelemetry = true; }; } |
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.
I don't like that the json options start with an uppercase letter, going against the conventional nix style. I don't really know what to do about it though.
doc/package-notes.xml
Outdated
|
||
<para> | ||
The policies framework can be used to enforce the installation of | ||
extensions as follows: |
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.
Before showing how to install extensions, we should list the caveats:
- installs into user dir, no automatic unintallation
- no upgrades which will be a dealbreaker to most
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.
Sorry, what do you mean by "no upgrades" exactly?
The addon should update as usual, unless you specify an exact version in the url and lock this, I guess.
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.
I'm not sure that is true. First of all I would assume (but don't know for sure), that a link like https://addons.mozilla.org/firefox/downloads/file/1189923/noscript_security_suite-latest-fx.xpi links to a specific version (despite having latest in the filename, as there is also a specific fileid).
And even if that isn't the case, there is this: mozilla/policy-templates#207
Have you tried this with an addon that updated afterwards?
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.
mozilla/policy-templates#158 explains what the "latest" part in the url does. I did not try with local extensions, I will do that.
doc/package-notes.xml
Outdated
|
||
<para> | ||
The firefox (wrapper) package can be configured through several mechanisms. | ||
The older |
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.
I don't think the old/new wording fits here. It suggests that the "new" method is a replacement for the old one, which it is not.
@@ -111,14 +126,55 @@ let | |||
cp -R --no-preserve=mode,ownership ${browser}/Applications/${browserName}.app $out/Applications | |||
rm -f $out${browser.execdir or "/bin"}/${browserName} | |||
'' + '' | |||
if [ ! -x "${browser}${browser.execdir or "/bin"}/${browserName}" ] | |||
|
|||
# Link the runtime. The executable itself has to be copied, |
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.
Have you looked into patching firefox? While this solution works, it is a little hacky.
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.
I have, and I find the code to be surprisingly unreadable for a newcomer. I can spend some more time on that, of course.
If somebody else with more knowledge of the mozilla codebase wants to take a shot at that / point me to the right location in the sources, I am quite happy to do that.
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.
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.
For the autoconfig part, I guess the readConfigFile
and openAndEvaluateJSFile
here are responsible: https://github.com/mozilla/gecko/blob/72f5488046d2d4cc8c0470f071db8f504629f95b/extensions/pref/autoconfig/src/nsReadConfig.cpp#L120
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.
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.
If ld returned 1 exit status
, there may be a more informative error message earlier in the log.
Maybe you ran out of ram or something?
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.
A friend of mine lent me some more capable hardware, it compiles now.
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.
Nice! Is it working?
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.
I got the policies engine working. AutoConfig appears to be more trouble, because of the lock file + obfuscation value + config file proper thing.
Also its cpp instead of javascript, so debugging is harder.
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.
Nice! Yeah AutoConfig does seem a little unnecessary complicated.
This commit changes the way the firefox wrapper works to allow passing system wide configuration to the firefox instance, in the form of an enterprise policies file (policies.json) and/or an AutoConfig file. Since firefox essentially hard codes the paths to these configuration files, and due to the immutable nature of the nix store, we have to copy/symlink the entire runtime. Also since firefox resolves those paths relative to the "true" path of the binary (i.e. following symlinks), the binary itself has to be copied.
This documents configuring firefox by passing options to the wrapper.
a94b0b6
to
3869826
Compare
@xaverdh reading the patches, I wonder, is there any reasons to not copy everything over to the new directory? Knowing that you can uncompress the tarball anywhere you want, I would expect the build result to be position independent of its top-level directory. Making a copy (maybe hard-link) could be a simpler way than selecting the few files which are mandatory. |
The actual runtime is position independent (on linux at least). The wrapper is not (there is already a wrapper coming from the firefox-unwrapped derivation), so that path and the symlink have to be rewritten. I went for symlinks instead of hardlinks, because I did not want to make assumptions on the file system used, etc. Also how would using hard links simplify anything really? Making a recursive copy would simplify the code somewhat I guess, at the cost of using more memory. |
So maybe if we could change firefox-unwrapped to not wrap the binary (e.g. move that code to the actual wrapper), this could be simplified a lot. |
@samueldr So based on the wrapper from @xaverdh I have added extension support and a lot of anti tracking / speed improvements for firefox. If this gets merged I can then open a new pull request. |
Thank you for your contributions.
|
Closing since #91724 was merged |
Motivation for this change
This is a proof of principle for allowing firefox and its variants to be customized and extended more easily.
Could be a first step towards solving #15959.
Very WIP at the moment.