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

pass-secret-service: init at unstable-2020-04-12 #102764

Merged
merged 2 commits into from Nov 6, 2020

Conversation

jluttine
Copy link
Member

@jluttine jluttine commented Nov 4, 2020

Motivation for this change

Add pass-secret-service program. It makes it possible to use pass (Unix password manager) as a backend for libsecret (secret service) D-Bus API.

Things done
  • 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.

I ran nix-review but I don't know what to make out of this output..

$ nix-review rev init-pass-secret-service
$ git -c fetch.prune=false fetch --force https://github.com/NixOS/nixpkgs master:refs/nixpkgs-review/0
remote: Enumerating objects: 12, done.
remote: Counting objects: 100% (12/12), done.
remote: Compressing objects: 100% (6/6), done.
remote: Total 12 (delta 5), reused 7 (delta 5), pack-reused 0
Unpacking objects: 100% (12/12), 44.49 KiB | 555.00 KiB/s, done.
From https://github.com/NixOS/nixpkgs
   f1c65de110f..e33473286ac  master     -> refs/nixpkgs-review/0
$ git worktree add /home/jluttine/.cache/nixpkgs-review/rev-d97f2e6d8ad4b0132f672628a3ed9f982f584a66-1/nixpkgs e33473286ac4165f23c09e5c95c044bfd32cb439
Preparing worktree (detached HEAD e33473286ac)
Updating files: 100% (23093/23093), done.
HEAD is now at e33473286ac Merge pull request #102742 from r-ryantm/auto-update/precice
$ nix-env -f /home/jluttine/.cache/nixpkgs-review/rev-d97f2e6d8ad4b0132f672628a3ed9f982f584a66-1/nixpkgs -qaP --xml --out-path --show-trace
$ git merge --no-commit d97f2e6d8ad4b0132f672628a3ed9f982f584a66
Auto-merging pkgs/top-level/python-packages.nix
Auto-merging pkgs/top-level/all-packages.nix
Automatic merge went well; stopped before committing as requested
$ nix-env -f /home/jluttine/.cache/nixpkgs-review/rev-d97f2e6d8ad4b0132f672628a3ed9f982f584a66-1/nixpkgs -qaP --xml --out-path --show-trace --meta
error: while querying the derivation named 'pass_secret_service-unstable-2020-04-12':
while evaluating the attribute 'out.outPath' at /home/jluttine/.cache/nixpkgs-review/rev-d97f2e6d8ad4b0132f672628a3ed9f982f584a66-1/nixpkgs/lib/customisation.nix:156:13:
while evaluating the attribute 'propagatedBuildInputs' of the derivation 'pass_secret_service-unstable-2020-04-12' at /home/jluttine/.cache/nixpkgs-review/rev-d97f2e6d8ad4b0132f672628a3ed9f982f584a66-1/nixpkgs/pkgs/development/interpreters/python/mk-python-derivation.nix:110:5:
while evaluating the attribute 'out.outPath' at /home/jluttine/.cache/nixpkgs-review/rev-d97f2e6d8ad4b0132f672628a3ed9f982f584a66-1/nixpkgs/lib/customisation.nix:156:13:
while evaluating the attribute 'propagatedBuildInputs' of the derivation 'python3.8-pypass-0.2.1' at /home/jluttine/.cache/nixpkgs-review/rev-d97f2e6d8ad4b0132f672628a3ed9f982f584a66-1/nixpkgs/pkgs/development/interpreters/python/mk-python-derivation.nix:110:5:
while evaluating 'chooseDevOutputs' at /home/jluttine/.cache/nixpkgs-review/rev-d97f2e6d8ad4b0132f672628a3ed9f982f584a66-1/nixpkgs/lib/attrsets.nix:475:22, called from undefined position:
undefined variable 'gnupg' at /home/jluttine/.cache/nixpkgs-review/rev-d97f2e6d8ad4b0132f672628a3ed9f982f584a66-1/nixpkgs/pkgs/development/python-modules/pypass/default.nix:36:5
19260 packages removed:
pastebinit (†1.5) pastel (†0.8.0) pastesrht (†0.11.2) pasystray (†0.7.1) .........clip...........

Nothing changed
$ nix-shell /home/jluttine/.cache/nixpkgs-review/rev-d97f2e6d8ad4b0132f672628a3ed9f982f584a66-1/shell.nix

(I removed gnupg after running that as nix-review complained about it.)

pkgs/development/python-modules/pypass/default.nix Outdated Show resolved Hide resolved
pkgs/applications/misc/pass-secret-service/default.nix Outdated Show resolved Hide resolved

buildPythonApplication rec {
pname = "pass_secret_service";
# Official release in PyPI is old, let's use latest from GitHub
Copy link
Member

Choose a reason for hiding this comment

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

Old is not necessarily bad. Does it miss important bug and security fixes or does it not work with the current versions of other software in nixpkgs?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I agree. I looked into this a bit more carefully now: The release in PyPI seems to me like a semi-random snapshot of the project and its version number is 0.1a0 - so, not a real release anyway, but alpha. Since then the project has switched from using a seemingly abandoned D-Bus package pydbus and started using maintained dbus-next. So, in my opinion, this unstable version makes perfect sense. What do you think? If so, I could write this more detailed reasoning in that comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

I now added a bit more details why it's an unstable version.

@SuperSandro2000
Copy link
Member

Result of nixpkgs-review pr 102764 run on x86_64-linux 1

1 package built:
  • pass-secret-service

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Nov 5, 2020

Result of nixpkgs-review pr 102764 run on x86_64-darwin 1

1 package failed to build:
  • pass-secret-service

A lot of tests fail with:

                self._sock = socket.socket(socket.AF_UNIX,
>                                          socket.SOCK_STREAM | socket.SOCK_NONBLOCK)
E               AttributeError: module 'socket' has no attribute 'SOCK_NONBLOCK'

dbus_next/message_bus.py:508: AttributeError

List of tests:

=========================== short test summary info ============================
FAILED test/test_aio_low_level.py::test_standard_interfaces - AttributeError:...
FAILED test/test_aio_low_level.py::test_sending_messages_between_buses - Attr...
FAILED test/test_aio_low_level.py::test_sending_signals_between_buses - Attri...
FAILED test/test_big_message.py::test_aio_big_message - AttributeError: modul...
FAILED test/test_disconnect.py::test_bus_disconnect_before_reply - AttributeE...
FAILED test/test_request_name.py::test_name_requests - AttributeError: module...
FAILED test/test_tcp_address.py::test_tcp_connection_with_forwarding - Assert...
FAILED test/client/test_methods.py::test_aio_proxy_object - AttributeError: m...
FAILED test/client/test_properties.py::test_aio_properties - AttributeError: ...
FAILED test/client/test_signals.py::test_signals - AttributeError: module 'so...
FAILED test/client/test_signals.py::test_signals_with_changing_owners - Attri...
FAILED test/service/test_export.py::test_export_unexport - AttributeError: mo...
FAILED test/service/test_export.py::test_export_alias - AttributeError: modul...
FAILED test/service/test_export.py::test_export_introspection - AttributeErro...
FAILED test/service/test_methods.py::test_methods[ExampleInterface] - Attribu...
FAILED test/service/test_methods.py::test_methods[AsyncInterface] - Attribute...
FAILED test/service/test_properties.py::test_property_methods - AttributeErro...
FAILED test/service/test_properties.py::test_property_changed_signal - Attrib...
FAILED test/service/test_signals.py::test_signals - AttributeError: module 's...
FAILED test/service/test_signals.py::test_interface_add_remove_signal - Attri...
FAILED test/service/test_standard_interfaces.py::test_introspectable_interface
FAILED test/service/test_standard_interfaces.py::test_object_manager - Attrib...
FAILED test/service/test_standard_interfaces.py::test_standard_interface_properties
============ 23 failed, 25 passed, 7 skipped, 1 deselected in 2.15s ============

@jluttine
Copy link
Member Author

jluttine commented Nov 6, 2020

Result of nixpkgs-review pr 102764 run on x86_64-darwin 1
1 package failed to build:

A lot of tests fail with:

                self._sock = socket.socket(socket.AF_UNIX,
>                                          socket.SOCK_STREAM | socket.SOCK_NONBLOCK)
E               AttributeError: module 'socket' has no attribute 'SOCK_NONBLOCK'

dbus_next/message_bus.py:508: AttributeError

List of tests:

=========================== short test summary info ============================
FAILED test/test_aio_low_level.py::test_standard_interfaces - AttributeError:...
FAILED test/test_aio_low_level.py::test_sending_messages_between_buses - Attr...
FAILED test/test_aio_low_level.py::test_sending_signals_between_buses - Attri...
FAILED test/test_big_message.py::test_aio_big_message - AttributeError: modul...
FAILED test/test_disconnect.py::test_bus_disconnect_before_reply - AttributeE...
FAILED test/test_request_name.py::test_name_requests - AttributeError: module...
FAILED test/test_tcp_address.py::test_tcp_connection_with_forwarding - Assert...
FAILED test/client/test_methods.py::test_aio_proxy_object - AttributeError: m...
FAILED test/client/test_properties.py::test_aio_properties - AttributeError: ...
FAILED test/client/test_signals.py::test_signals - AttributeError: module 'so...
FAILED test/client/test_signals.py::test_signals_with_changing_owners - Attri...
FAILED test/service/test_export.py::test_export_unexport - AttributeError: mo...
FAILED test/service/test_export.py::test_export_alias - AttributeError: modul...
FAILED test/service/test_export.py::test_export_introspection - AttributeErro...
FAILED test/service/test_methods.py::test_methods[ExampleInterface] - Attribu...
FAILED test/service/test_methods.py::test_methods[AsyncInterface] - Attribute...
FAILED test/service/test_properties.py::test_property_methods - AttributeErro...
FAILED test/service/test_properties.py::test_property_changed_signal - Attrib...
FAILED test/service/test_signals.py::test_signals - AttributeError: module 's...
FAILED test/service/test_signals.py::test_interface_add_remove_signal - Attri...
FAILED test/service/test_standard_interfaces.py::test_introspectable_interface
FAILED test/service/test_standard_interfaces.py::test_object_manager - Attrib...
FAILED test/service/test_standard_interfaces.py::test_standard_interface_properties
============ 23 failed, 25 passed, 7 skipped, 1 deselected in 2.15s ============

@SuperSandro2000 These failures are in dbus-next package. This PR doesn't touch that package at all so the issue should be independent of this PR.

@jluttine
Copy link
Member Author

jluttine commented Nov 6, 2020

Thanks for the reviews! I've done all the changes I think were requested. The tests are working now too. Anything else or good to merge?

@jluttine
Copy link
Member Author

jluttine commented Nov 6, 2020

One big change that I made after the reviews: pypass used a lot of command-line tools like gpg, grep, git etc. I now patched these so that the commands are replaced with full nix-store paths to the executables.

@jluttine jluttine changed the title Init pass secret service pass-secret-service: init at unstable-2020-04-12 Nov 6, 2020
Copy link
Contributor

@doronbehar doronbehar left a comment

Choose a reason for hiding this comment

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

Besides that all looks good to me.

pkgs/development/python-modules/pypass/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/pypass/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/pypass/default.nix Outdated Show resolved Hide resolved
@doronbehar
Copy link
Contributor

Also there's a CI error: https://gist.github.com/8c196d662581f9b286124260f14a20a6

pkgs/applications/misc/pass-secret-service/default.nix Outdated Show resolved Hide resolved
pkgs/development/python-modules/pypass/default.nix Outdated Show resolved Hide resolved
click
colorama
pexpect
] ++ stdenv.lib.optionals (pythonOlder "3.4") enum34;
Copy link
Contributor

Choose a reason for hiding this comment

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

It probably is not worth it supporting such old versions.

Copy link
Member Author

Choose a reason for hiding this comment

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

nixpkgs still contains Python 2 support..

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't remove this but I fixed optionals to optional. I don't see any harm in supporting Python 2 as long as it's kept available in nixpkgs.

@jluttine
Copy link
Member Author

jluttine commented Nov 6, 2020

Also there's a CI error: https://gist.github.com/8c196d662581f9b286124260f14a20a6

@doronbehar Thanks, this should be fixed now. I had used optionals when I should have used optional.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 These failures are in dbus-next package. This PR doesn't touch that package at all so the issue should be independent of this PR.

Yeah sure but I can't build this package on darwin without that dependency working.

@doronbehar
Copy link
Contributor

I'm not sure but it seems to me that a dbus related package shouldn't be supported on Darwin, so I guess this is good to go.

@doronbehar doronbehar merged commit 4c84556 into NixOS:master Nov 6, 2020
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