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

gnome-passwordsafe: init at 3.99.2 #98044

Merged
merged 1 commit into from Oct 13, 2020
Merged

Conversation

@mvnetbiz
Copy link
Contributor

@mvnetbiz mvnetbiz commented Sep 15, 2020

Motivation for this change

Wanted application to replace keepass
other interest in this application: https://discourse.nixos.org/t/trouble-building-passwordsafe-a-meson-python-gtk3-package/4652

Things done

Added gnome-passwordsafe 3.99.2 application

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Sep 15, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/trouble-building-passwordsafe-a-meson-python-gtk3-package/4652/17

@onny
Copy link
Contributor

@onny onny commented Sep 15, 2020

Wow cool thanks for packaging! Works good for me so far. Building only fails locally because of unknown license licenses.gpl3Only. Changed it to licenses.gpl3 and then it compiles.

@@ -1124,6 +1124,8 @@ in

passExtensions = recurseIntoAttrs pass.extensions;

passwordsafe = callPackage ../applications/misc/passwordsafe { };
Copy link
Member

@ryantm ryantm Sep 15, 2020

Probably should be gnome-passwordsafe since that is what you called it. Looks like there is something else called passwordsafe too: https://repology.org/projects/?search=passwordsafe

Suggested change
passwordsafe = callPackage ../applications/misc/passwordsafe { };
gnome-passwordsafe = callPackage ../applications/misc/gnome-passwordsafe { };

Copy link
Contributor Author

@mvnetbiz mvnetbiz Sep 15, 2020

Okay, I was wondering also if it should not go in gnome3.passwordsafe? I am not sure what goes into deciding that. Should I ping a gnome maintainer?

Copy link
Contributor

@worldofpeace worldofpeace Sep 15, 2020

Hey, a gnome maintainer found this 😁 In the case of any gtk application pinging a gnome maintainer is necessary because gtk packaging can be confusing. We're on the fence with there being gnome3.* at all, so I'd gnome-passwordsafe would be the best attribute for nixpkgs cc @jtojnar

Copy link
Contributor

@worldofpeace worldofpeace Sep 15, 2020

On another topic, the organization of applications/misc isn't optimal. I believe this would be a good chance to add a new directory applications/password-managers. What do you think?

Copy link
Contributor Author

@mvnetbiz mvnetbiz Sep 15, 2020

I found this here, but maybe tools isn't a good directory either? pkgs/tools/security/1password-gui/

Copy link
Contributor Author

@mvnetbiz mvnetbiz Sep 15, 2020

also: pkgs/tools/security/gnome-keysign


python3.pkgs.buildPythonApplication rec {
pname = "passwordsafe";
version = "3.99.2";
Copy link
Member

@ryantm ryantm Sep 15, 2020

3.99.2 is a pre-release/dev version, we have a policy of packaging the latest stable release, or currently 3.32.0

Copy link
Contributor

@worldofpeace worldofpeace Sep 15, 2020

It seems that in latest version there's support implemented for newer versions of pykeepass

3.99.1
* Updated to pykeepass 3.2

3.99
* Implemented compatibility for pykeepass 3.1

It does appear this is a pre-release from looking at other releases (they follow the standard gnome owned pattern). But I'm confused by the implications from the version this will ship in 4.0?

Copy link
Contributor Author

@mvnetbiz mvnetbiz Sep 15, 2020

This is also broken for nix in the 3.32.0 release, we would have to patch it, not sure if that has any bearing
https://gitlab.gnome.org/World/PasswordSafe/-/blob/3.32.0/meson.build#L31-40

Copy link
Contributor

@jtojnar jtojnar Sep 15, 2020

We could apply https://gitlab.gnome.org/World/PasswordSafe/-/commit/73b7aef155ce9ba597ebcadfb4345cbc966565e7 but if the pre-release version works, it might be more convenient to just use that.

Copy link
Contributor Author

@mvnetbiz mvnetbiz Sep 15, 2020

There are 3 other commits to that file after the 3.22.0 tag, so it cant be applied trivially, but anyways I tried with this patch, and it builds, but when I try to open a db it freezes and I have to kill -9 it
https://github.com/mvnetbiz/nixpkgs/blob/gnome-passwordsafe-3-22/pkgs/applications/misc/gnome-passwordsafe/meson.build.patch

pkgs/applications/misc/passwordsafe/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/passwordsafe/default.nix Outdated Show resolved Hide resolved
@worldofpeace worldofpeace requested a review from Sep 15, 2020
@mvnetbiz mvnetbiz force-pushed the gnome-passwordsafe branch from 65fc862 to eb87ab2 Sep 15, 2020
@mvnetbiz mvnetbiz changed the title passwordsafe: init at 3.99.2 gnome-passwordsafe: init at 3.99.2 Sep 15, 2020
@worldofpeace
Copy link
Contributor

@worldofpeace worldofpeace commented Sep 15, 2020

It seems you've done the opposite of #98044 (comment) suggestion.
gobject-introspection belongs in nativeBuildInputs because we need its setup-hook, but it's not a python-modules so it shouldn't be propagated. You will however notice if gobject-introspection is only in nativeBuildInputs it won't work, and that's because the setup-hook is broken with strictDeps in the python application deriver #56943. So adding

strictDeps = false; # https://github.com/NixOS/nixpkgs/issues/56943
will also be necessary (comment also please)

@mvnetbiz mvnetbiz force-pushed the gnome-passwordsafe branch from eb87ab2 to deb4c7e Sep 15, 2020
@mvnetbiz
Copy link
Contributor Author

@mvnetbiz mvnetbiz commented Sep 15, 2020

I think it is less confusing to review if I push fixes and suggested changes individually, and then squash and force-push once someone is ready to commit it, is that a good assumption? This is the most in-depth I've used github... Thanks for being so helpful. 👍

@onny
Copy link
Contributor

@onny onny commented Sep 22, 2020

Using this package already for a while, works really well. Thank you for your work!

@onny
Copy link
Contributor

@onny onny commented Oct 13, 2020

Result of nixpkgs-review pr 98044 1
1 package built: gnome-passwordsafe

Works fine for me

@worldofpeace
Copy link
Contributor

@worldofpeace worldofpeace commented Oct 13, 2020

If you could squash it into one commit I can merge 👍 I think this has been good for awhile

@mvnetbiz mvnetbiz force-pushed the gnome-passwordsafe branch from 4d8b6b1 to 2cebafe Oct 13, 2020
@mvnetbiz
Copy link
Contributor Author

@mvnetbiz mvnetbiz commented Oct 13, 2020

Squashed.

@worldofpeace worldofpeace merged commit 90ce7a4 into NixOS:master Oct 13, 2020
17 checks passed
@mvnetbiz mvnetbiz deleted the gnome-passwordsafe branch Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants