Subject: Re: [bug#50751] [PATCH] gnu: update Trezor support

From: Sarah Morgensen <[email-masked]>
Date: Thu, 23 Sep 2021 11:35:22 -0700

Hi,

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

Attila Lendvai <[email-masked]> 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