-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
browserpass: add module #16
Conversation
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.
Thanks for the contribution! I think this could be a very useful module :-)
I agree that it would be much nicer to do this using home.file
. Looking at the install.sh
file it should be relatively straight forward. The trunk NixOS already has a browserpass module which does something like that.
modules/programs/browserpass.nix
Outdated
enable = mkEnableOption "the browserpass extension host application"; | ||
|
||
browsers = mkOption { | ||
type = types.listOf types.str; |
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 should be type = types.listOf (types.enum (builtins.attrNames browsers));
to allow the man page to properly show the available options. It also will check the values so the assertion in the config section below is unnecessary.
modules/programs/browserpass.nix
Outdated
mv * $out/ | ||
''; | ||
}); | ||
in dagEntryBefore ["writeBoundary"] (builtins.concatStringsSep "" (map (browser: '' |
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.
Should be dagEntryAfter
since the command will write to the home directory.
modules/programs/browserpass.nix
Outdated
]; | ||
|
||
home.activation.browserpass = let | ||
browserpass = pkgs.stdenv.mkDerivation (let |
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 seem to be a browserpass package in Nixpkgs trunk. It might be possible to back-port it to 17.03.
But I guess it cannot be uses as-is because of the permissions issue that you fix below.
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'll try to update the Nixpkgs equivalent so we can use it with home.file
, avoiding the permissions issue.
I already pushed but we should wait for NixOS/nixpkgs#26541 to get merged before squashing. |
Great work! This is a big improvement and I'm happy to merge once the Nixpkgs changes are in place. Are you using unstable Nixpkgs? I'm wondering whether to make a 17.03 branch of home-manager and let the master branch track Nixpkgs master. If so this PR would go into master and not into the 17.03 branch. |
Thanks! 😃 Yes, I use the latest unstable. We should be able to backport to 17.03 though by including copies of the updated packages. |
* browserpass: add module * apply some review requests * browserpass: update to 1.0.5 * browserpass: install from Nixpkgs using `home.file`
A module that installs the host application required by the browserpass extension.
I just hacked this together this evening. A better alternative may be to ditch the install script and use
home.file
instead, but this works as well. I would also prefer to build from source.