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

Conversation

mvnetbiz
Copy link
Contributor

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

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 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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also: pkgs/tools/security/gnome-keysign


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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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

Choose a reason for hiding this comment

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

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 a team September 15, 2020 14:33
@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

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
Copy link
Contributor Author

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 commented Sep 22, 2020

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

@onny
Copy link
Contributor

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

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

@mvnetbiz
Copy link
Contributor Author

Squashed.

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

Successfully merging this pull request may close these issues.

None yet

6 participants