Subject: Re: [bug#60003] [PATCH 3/3] gnu: Add pass-import.

From: Jelle Licht <[email-masked]>
Date: Thu, 05 Jan 2023 16:21:09 +0100

Hello Tobias,

Tobias Kortkamp <[email-masked]> writes:

> * gnu/packages/password-utils.scm (pass-import): New variable.
> ---
>  gnu/packages/password-utils.scm | 71 +++++++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
>
> diff --git a/gnu/packages/password-utils.scm b/gnu/packages/password-utils.scm
> index aab0a3f2a4..79781ed9e6 100644
> --- a/gnu/packages/password-utils.scm
> +++ b/gnu/packages/password-utils.scm
> @@ -38,6 +38,7 @@
>  ;;; Copyright © 2022 Maxime Devos <[email-masked]>
>  ;;; Copyright © 2022 ( <[email-masked]>
>  ;;; Copyright © 2022 Nicolas Graves <[email-masked]>
> +;;; Copyright © 2022 Tobias Kortkamp <[email-masked]>
>  ;;;
>  ;;; This file is part of GNU Guix.
>  ;;;
> @@ -101,6 +102,7 @@ (define-module (gnu packages password-utils)
>    #:use-module (gnu packages perl)
>    #:use-module (gnu packages pkg-config)
>    #:use-module (gnu packages python)
> +  #:use-module (gnu packages python-crypto)
>    #:use-module (gnu packages python-web)
>    #:use-module (gnu packages python-xyz)
>    #:use-module (gnu packages readline)
> @@ -1492,3 +1494,72 @@ (define-public xkcdpass
>                     license:lgpl2.0 ;finnish word list
>                     license:lgpl2.1 ;portuguese word list
>                     license:mpl1.1)))) ;portuguese word list
> +
> +(define-public pass-import
> +  (package
> +    (name "pass-import")
> +    (version "3.4")
> +    (source (origin
> +              (method url-fetch)
> +              (uri (string-append "https://github.com/roddhjav/pass-import"
> +                                  "/releases/download/" version "/pass-import-"
You missed a "v" here; could you make it "download/v" instead? That works for
me, with the provided base32 hash.

> +                                  version ".tar.gz"))
> +              (sha256
> +               (base32 "18vh97h3bmspwqhb9xjkapk3p8hy4rxhazrcc06f6pdz8rb6rxxp"))))
> +    (build-system python-build-system)
> +    (arguments
> +     (list #:phases
> +           #~(modify-phases %standard-phases
> +               (add-after 'unpack 'patch-paths
> +                 (lambda* (#:key inputs #:allow-other-keys)
> +                   (substitute* '("import.bash")
> +                     (("python3")
> +                      #$(file-append python-wrapper "/bin/python")))

Perhaps someone more experienced with our python-packages can still
weigh in, but wouldn't the following work here as well?

--8<---------------cut here---------------start------------->8---
                     (("python3")
                      (search-input-file inputs "/bin/python3")))
--8<---------------cut here---------------end--------------->8---

> +                   (substitute* '("setup.py")
> +                     (("base = '/usr'")
> +                      (string-append "base = '" #$output "'"))
> +                     (("sys\\.prefix")
> +                      (string-append "'" #$output "'")))
> +                   (substitute* '("tests/imports/test_parse.py")
> +                     ;; requires running D-Bus session
> +                     (("test_import_gnome_keyring") "disabled_test_import_gnome_keyring")
> +                     ;; requires lastpass
> +                     (("test_import_lastpass") "disabled_test_import_lastpass"))
> +                   (substitute* '("tests/exports/test_lastpass.py")
> +                     (("test_import_lastpass") "disabled_test_import_lastpass")
> +                     (("test_lastpass_exist") "disabled_test_lastpass_exist")
> +                     (("test_lastpass_insert") "disabled_test_lastpass_insert")
> +                     (("test_lastpass_show") "disabled_test_lastpass_show")
> +                     (("test_lastpass_list") "disabled_test_lastpass_list")
> +                     (("test_lastpass_isvalid") "disabled_test_lastpass_isvalid"))))

Nit: in this phase you seem to patch paths and disable tests. Consider splitting
it up into two phases.

I still run into some test failures:
--8<---------------cut here---------------start------------->8---
Ran 135 tests in 12.145s

FAILED (failures=6, errors=1, skipped=10)
Test failed: <unittest.runner.TextTestResult run=135 errors=1 failures=6>
error: Test failed: <unittest.runner.TextTestResult run=135 errors=1 failures=6>
        Password-pwned-1
        Password-pwned-2
        Password-pwned-3
        Password-pwned-4
        Password-pwned-5
        Password-pwned-6
        Password-pwned-7
--8<---------------cut here---------------end--------------->8---

> +               (add-after 'install 'wrap-extension
> +                 (lambda _
> +                   (let ((pylib (string-append #$output "/lib/python"
> +                                               #$(version-major+minor
> +                                                  (package-version python))
> +                                               "/site-packages"))
> +                         (extension (string-append #$output
> +                                                   "/lib/password-store/extensions/import.bash")))

This line seems to make `./pre-inst-env guix lint pass-import' unhappy,
as it is too wide.

> +                     (wrap-program extension
> +                       (list "GUIX_PYTHONPATH" ":"
> +                             'prefix
> +                             (list (getenv "GUIX_PYTHONPATH")
> +                                   pylib)))))))))
> +    (inputs (list gnupg
> +                  password-store
> +                  python-cryptography
> +                  python-defusedxml
> +                  python-magic
> +                  python-pyaml
> +                  python-pykeepass
> +                  python-requests
> +                  python-secretstorage
> +                  python-zxcvbn))
> +    (home-page "https://github.com/roddhjav/pass-import")
> +    (synopsis
> +     "Pass extension for importing data from many password managers")
> +    (description
> +     "This package provides a pass extension for importing data from many password
> +managers. @command{pimport} can be used to export data to other password managers
> +like gopass or keepassxc too.")

Consider removing superfluous 'too'.

Thanks again!
 - Jelle