diff mbox series

[bug#50751] gnu: update Trezor support

Message ID 20210923083045.10372-1-attila@lendvai.name
State Accepted
Headers show
Series [bug#50751] gnu: update Trezor support | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Attila Lendvai Sept. 23, 2021, 8:30 a.m. UTC
Fetch everything from their git repos, instead of pypi.

* gnu/packages/finance.scm (python-trezor-agent): Update to 0.14.2.
(python-trezor): Update to 0.12.3.
(trezor-agent): Update to 0.11.0.
(trezord): Update to 2.0.31.
* gnu/packages/patches/trezor-agent-fix-argv0.patch: New file.
---
 gnu/packages/finance.scm                      | 84 ++++++++++++++-----
 .../patches/trezor-agent-fix-argv0.patch      | 27 ++++++
 2 files changed, 92 insertions(+), 19 deletions(-)
 create mode 100644 gnu/packages/patches/trezor-agent-fix-argv0.patch

Comments

Sarah Morgensen Sept. 23, 2021, 6:35 p.m. UTC | #1
Hi,

Thanks for the patch!  Reading over this, I have a few comments.

Attila Lendvai <attila@lendvai.name> writes:

> Fetch everything from their git repos, instead of pypi.
>

Given the resulting version mismatch, and having to pick apart the
sources, is there a compelling reason for this?  It looks like the same
files are included.

> * gnu/packages/finance.scm (python-trezor-agent): Update to 0.14.2.
> (python-trezor): Update to 0.12.3.
> (trezor-agent): Update to 0.11.0.
> (trezord): Update to 2.0.31.
> * gnu/packages/patches/trezor-agent-fix-argv0.patch: New file.

When at all possible, version updates should be separate commits from changes.

> @@ -1091,14 +1137,14 @@ the KeepKey Hardware Wallet.")
>               (let* ((out (assoc-ref outputs "out")))
>                 ;; overwrite the wrapper with the real thing.
>                 (install-file "./trezor_agent.py"
> -                             (string-append out "/bin"))
> -             #t))))))
> +                             (string-append out "/bin")))
> +             #t)))))

This wasn't added by your patch, but it's odd that the reason for
overriding the wrapper wasn't documented....

Because if we didn't overwrite the wrapper, your patch below would be
unnecessary, because the wrapper sets argv[0] to the original name of
the file.

> diff --git a/gnu/packages/patches/trezor-agent-fix-argv0.patch b/gnu/packages/patches/trezor-agent-fix-argv0.patch
> new file mode 100644
> index 0000000000..9462067cd5
> --- /dev/null
> +++ b/gnu/packages/patches/trezor-agent-fix-argv0.patch
> @@ -0,0 +1,27 @@
> +diff --git a/libagent/gpg/__init__.py b/libagent/gpg/__init__.py
> +index 3711bc8..67085de 100644
> +--- a/libagent/gpg/__init__.py
> ++++ b/libagent/gpg/__init__.py
> +@@ -122,15 +122,19 @@ def run_init(device_type, args):
> +     verify_gpg_version()
> + 
> +     # Prepare new GPG home directory for hardware-based identity
> +-    device_name = os.path.basename(sys.argv[0]).rsplit('-', 1)[0]
> +-    log.info('device name: %s', device_name)
> ++    exe_name = os.path.basename(sys.argv[0])
> ++    # drop the Guix wrapper's dot prefix from the name
> ++    if exe_name[0] == '.' and exe_name.endswith('-real'):
> ++        exe_name = exe_name[1:-5:]
> ++    device_name = exe_name.rsplit('-', 1)[0]
> ++    log.info('exe name: %s, device name: %s', exe_name, device_name)
> +     homedir = args.homedir
> +     if not homedir:
> +         homedir = os.path.expanduser('~/.gnupg/{}'.format(device_name))
> + 
> +     log.info('GPG home directory: %s', homedir)
> + 
> +-    if os.path.exists(homedir):
> ++    if os.path.exists(homedir) and not args.subkey:
> +         log.error('GPG home directory %s exists, '
> +                   'remove it manually if required', homedir)
> +         sys.exit(1)

Hope that helps,
--
Sarah
Attila Lendvai Sept. 24, 2021, 11:05 a.m. UTC | #2
> > Fetch everything from their git repos, instead of pypi.
>
> Given the resulting version mismatch, and having to pick apart the
> sources, is there a compelling reason for this? It looks like the same
> files are included.


an objective reason: one of these versions were not in pipy at the
time i was working on this.

a subjective reason: one less organization to rely on. if anything
would happen to pipy, e.g. some random API change, then that wouldn't
affect us anymore. if github went down or went hostile, then we could
just change the git URL and continue with a minor glitch.


> > -   gnu/packages/finance.scm (python-trezor-agent): Update to 0.14.2.
> >
> >     (python-trezor): Update to 0.12.3.
> >     (trezor-agent): Update to 0.11.0.
> >     (trezord): Update to 2.0.31.
> > -   gnu/packages/patches/trezor-agent-fix-argv0.patch: New file.
>
> When at all possible, version updates should be separate commits from changes.


ok, makes sense, because now that i think about it, it was broken with
the prior version, too. i'll resend it in two patches once i have tested them.


> > ;; overwrite the wrapper with the real thing.
> > (install-file "./trezor_agent.py"
> > -                               (string-append out "/bin"))
> > -               #t))))))
> > -                               (string-append out "/bin")))
> > -               #t)))))
>
> This wasn't added by your patch, but it's odd that the reason for
> overriding the wrapper wasn't documented....


i have added a comment explaining the situation.

thank you for the feedback! the updated patches will be coming a bit
later.

- attila
PGP: 5D5F 45C7 DFCD 0A39
diff mbox series

Patch

diff --git a/gnu/packages/finance.scm b/gnu/packages/finance.scm
index 9b073541de..34bb73016c 100644
--- a/gnu/packages/finance.scm
+++ b/gnu/packages/finance.scm
@@ -833,9 +833,11 @@  the Monero GUI client.")
     (license license:bsd-3)))
 
 (define-public python-trezor-agent
+  ;; It is called 'libagent' in pypi; i.e. this is the library as opposed to
+  ;; the toplevel app called trezor-agent.
   (package
     (name "python-trezor-agent")
-    (version "0.13.1")
+    (version "0.14.2")
     (source
      (origin
        (method git-fetch)
@@ -844,7 +846,8 @@  the Monero GUI client.")
              (commit (string-append "v" version))))
        (file-name (git-file-name name version))
        (sha256
-        (base32 "0q99vbfd3h85s8rnjipnmldixabqmmlk5w9karv6f0rhyi54f4zv"))))
+        (base32 "0nl44ldfw9s2v3p7g5bldfw3ds2hz9r28j42bpnp8bj0v5na3ivk"))
+       (patches (search-patches "trezor-agent-fix-argv0.patch"))))
     (build-system python-build-system)
     (arguments
      `(#:phases
@@ -863,11 +866,12 @@  the Monero GUI client.")
              (add-installed-pythonpath inputs outputs)
              (invoke "py.test"))))))
     (propagated-inputs
-     `(("python-configargparse" ,python-configargparse)
+     `(("python-pynacl" ,python-pynacl)
+       ("python-configargparse" ,python-configargparse)
        ("python-daemon" ,python-daemon)
        ("python-docutils" ,python-docutils)
        ("python-ecdsa" ,python-ecdsa)
-       ("python-ed25519" ,python-ed25519)
+       ("python-hidapi" ,python-hidapi)
        ("python-mnemonic" ,python-mnemonic)
        ("python-pymsgbox" ,python-pymsgbox)
        ("python-semver" ,python-semver)
@@ -986,16 +990,35 @@  Nano dongle.")
 (define-public python-trezor
   (package
     (name "python-trezor")
-    (version "0.12.1")
+    (version "0.12.3")
     (source
       (origin
-        (method url-fetch)
-        (uri (pypi-uri "trezor" version))
+        (method git-fetch)
+        (uri (git-reference
+              (url "https://github.com/trezor/trezor-firmware/")
+              (commit (string-append "python/v" version))))
+        (file-name (git-file-name name version))
         (sha256
-          (base32 "1w19m9lws55k9sjhras47hpfpqwq1jm5vy135nj65yhkblygqg19"))))
+         (base32 "0wdm1y5zli6w09zbpjqc6rbcs1b4hjq007mbh7xdr17prbnqprac"))
+        (modules '((guix build utils) (srfi srfi-26) (srfi srfi-1) (ice-9 ftw)))
+        (snippet
+         '(begin
+            ;; Delete everything except ./python/
+            (for-each delete-file-recursively
+                      (scandir "./" (negate (cut member <> '("python" "." "..")
+                                                 string=))))
+            ;; Move ./python/* to the toplevel
+            (for-each (lambda (file-name)
+                        (rename-file (string-append "./python/" file-name)
+                                     (string-append "./" file-name)))
+                      (scandir "./python/"
+                               (negate (cut member <> '("." "..") string=))))
+            (delete-file-recursively "./python")
+            #t))))
     (build-system python-build-system)
     (propagated-inputs
-     `(("python-click" ,python-click)
+     `(("python-attrs" ,python-attrs)
+       ("python-click" ,python-click)
        ("python-construct" ,python-construct)
        ("python-ecdsa" ,python-ecdsa)
        ("python-libusb1" ,python-libusb1)
@@ -1074,16 +1097,39 @@  the KeepKey Hardware Wallet.")
 (define-public trezor-agent
   (package
     (name "trezor-agent")
-    (version "0.10.0")
+    (version "0.11.0")
     (source
      (origin
-       (method url-fetch)
-       (uri (pypi-uri "trezor_agent" version))
+       (method git-fetch)
+       (uri (git-reference
+             (url "https://github.com/romanz/trezor-agent")
+             ;; The version mismatch is not a mistake. Multiple python
+             ;; apps/packages are in the same git repo, and they have
+             ;; different versions. The git tag seems to track libagent,
+             ;; i.e. python-trezor-agent in the Guix namespace.
+             ;; See e.g. ./agents/trezor/setup.py
+             (commit "v0.14.2")))
+       (file-name (git-file-name name version))
        (sha256
         (base32
-         "144657c7bn0a667dq5fv5r6j7iilxf3h9agj29v1m2qpq40g0az8"))))
+         "0nl44ldfw9s2v3p7g5bldfw3ds2hz9r28j42bpnp8bj0v5na3ivk"))
+       (modules '((guix build utils) (srfi srfi-26) (srfi srfi-1) (ice-9 ftw)))
+       (snippet
+        '(begin
+           ;; Delete everything except ./agents/trezor/
+           (for-each delete-file-recursively
+                     (filter (lambda (full-name)
+                               (not (string-prefix? "./agents/trezor/" full-name)))
+                             (find-files ".")))
+           ;; Move ./agents/trezor/* to the toplevel
+           (for-each (lambda (file-name)
+                       (rename-file (string-append "./agents/trezor/" file-name)
+                                    (string-append "./" file-name)))
+                     (scandir "./agents/trezor/"
+                              (negate (cut member <> '("." "..") string=))))
+           (delete-file-recursively "./agents")
+           #t))))
     (arguments
-     ;; Tests fail with "AttributeError: module 'attr' has no attribute 's'".
      `(#:phases
        (modify-phases %standard-phases
          (add-after 'wrap 'fixup-agent-py
@@ -1091,14 +1137,14 @@  the KeepKey Hardware Wallet.")
              (let* ((out (assoc-ref outputs "out")))
                ;; overwrite the wrapper with the real thing.
                (install-file "./trezor_agent.py"
-                             (string-append out "/bin"))
-             #t))))))
+                             (string-append out "/bin")))
+             #t)))))
     (build-system python-build-system)
     (inputs
      `(("python-trezor" ,python-trezor)
        ("python-trezor-agent" ,python-trezor-agent)))
     (native-inputs
-     `(("python-hidapi" ,python-hidapi)))
+     `(("python-attrs" ,python-attrs))) ; for the tests
     (home-page "https://github.com/romanz/trezor-agent")
     (synopsis "Using Trezor as hardware SSH/GPG agent")
     (description "This package allows using Trezor as a hardware SSH/GPG
@@ -1301,7 +1347,7 @@  trezord as a regular user instead of needing to it run as root.")
 (define-public trezord
   (package
     (name "trezord")
-    (version "2.0.30")
+    (version "2.0.31")
     (source
      (origin
        (method git-fetch)
@@ -1310,7 +1356,7 @@  trezord as a regular user instead of needing to it run as root.")
              (commit (string-append "v" version))))
        (sha256
         (base32
-         "1hzvk0wfgg7b4wpqjk3738yqxlv3pj5i7zxwm0jady2h97hmrqrr"))
+         "130nhk1pnr3xx9qkcij81mm3jxrl5zvvdqhvrgvrikqg3zlb6v5b"))
        (file-name (git-file-name name version))))
     (build-system go-build-system)
     (arguments
diff --git a/gnu/packages/patches/trezor-agent-fix-argv0.patch b/gnu/packages/patches/trezor-agent-fix-argv0.patch
new file mode 100644
index 0000000000..9462067cd5
--- /dev/null
+++ b/gnu/packages/patches/trezor-agent-fix-argv0.patch
@@ -0,0 +1,27 @@ 
+diff --git a/libagent/gpg/__init__.py b/libagent/gpg/__init__.py
+index 3711bc8..67085de 100644
+--- a/libagent/gpg/__init__.py
++++ b/libagent/gpg/__init__.py
+@@ -122,15 +122,19 @@ def run_init(device_type, args):
+     verify_gpg_version()
+ 
+     # Prepare new GPG home directory for hardware-based identity
+-    device_name = os.path.basename(sys.argv[0]).rsplit('-', 1)[0]
+-    log.info('device name: %s', device_name)
++    exe_name = os.path.basename(sys.argv[0])
++    # drop the Guix wrapper's dot prefix from the name
++    if exe_name[0] == '.' and exe_name.endswith('-real'):
++        exe_name = exe_name[1:-5:]
++    device_name = exe_name.rsplit('-', 1)[0]
++    log.info('exe name: %s, device name: %s', exe_name, device_name)
+     homedir = args.homedir
+     if not homedir:
+         homedir = os.path.expanduser('~/.gnupg/{}'.format(device_name))
+ 
+     log.info('GPG home directory: %s', homedir)
+ 
+-    if os.path.exists(homedir):
++    if os.path.exists(homedir) and not args.subkey:
+         log.error('GPG home directory %s exists, '
+                   'remove it manually if required', homedir)
+         sys.exit(1)