-
-
Notifications
You must be signed in to change notification settings - Fork 15k
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
Fix firefox wrapper #105796
Fix firefox wrapper #105796
Conversation
Empty |
We also set |
Fixed both. I introduced a new flag called |
Would you mind rebasing and squashing the commits to help clean up the history? You seem to have a ton of merge master-into-master commits, as well as commits with duplicate summaries. |
37c7e4f
to
b8b26c1
Compare
After considering I changed the flag |
LGTM |
Please add a proper commit message with reasoning and a why. Currently it doesn't even conform with the contribution guidelines. |
question: I have tried to use your MR yesterday without this fix and my manual extensions were gone. I've tried to rebuild with this fix and they are still gone. Are they gone forever (i.e., do I need to reinstall them) or they are just hidden and I can recover them ? |
Yes they are gone, but the addon config data is still there so after reinstalling them manually it should just work |
b8b26c1
to
7c4b7a4
Compare
I changed the flag |
Please refer CONTRIBUTING.md for the convention for commit messages in nixpkgs. For example, |
f39bb83
to
81f2019
Compare
a6a9e4e
to
2d667f4
Compare
2d667f4
to
b63de18
Compare
b63de18
to
d3a6a12
Compare
DisableAppUpdate = true; | ||
} // | ||
{ | ||
lib.optionalAttrs usesNixExtensions { | ||
ExtensionSettings = { | ||
"*" = { | ||
blocked_install_message = "You can't have manual extension mixed with nix 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.
This message should explain how to disable this enforcement.
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.
@kevincox can you make a PR?
Any chance we can roll these changes back till its tested properly. This has had a massive impact on most of the user base. For example it uninstalled of my extensions then when firefox sync ran it uninstalled all of them on all of my machines. Not only that, because i use container tabs it wiped all of my cookies. This feature should only be opt in. |
TBH at this point after a sync that removed my extensions on all my machines a rollback won't help much. Now, what should this config look like if we were to use nix to manage common extensions like ublock-origin and maybe an obscure one that's not packaged yet and we'll need to provide the urls and hashes?
|
You can use something like {
environment.systemPackages =
let noscript = pkgs.fetchFirefoxAddon {
name = "noscript";
url = "https://addons.mozilla.org/firefox/downloads/latest/noscript/latest.xpi";
sha256 = "sha256-Pj4AAm22plzV7rvkosIU/X70N8j00X+lkchfYeq50eU=";
};
in [ (firefox.override { nixExtensions = [ noscript ]; }) ];
} The hash can be obtained by not e.g. specifying one, and trying to build. |
That's a pretty bad solution, though. This means that everyone has to maintain a package set of browser extensions (versions, hashes, etc.) by themselves, duplicating each other efforts. If those extensions were available to choose from in Nixpkgs, this might make sense, but as-is this just adds maintenance effort for the users to duplicate a perfectly good package manager that's already included in Firefox anyway. |
Yes this should ideally be managed by a concerted effort of the community to provide a package set for addons. Ideally it would also be seeded automatically from AMO. There are still valid use cases for manually packaging addons, e.g. if you develop your own addon and don't want to have it on AMO (yet). |
We discuss having them in a nix-community maintained repo here: #105783 |
Motivation for this change
Fixing #91724 (comment)
Sorry @Mic92 I missed that detail.
Things done
Only enforce nix extensions if at least one is defined
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)