diff mbox series

[bug#50751,1/2] gnu: python-trezor-agent: Fix key generation with trezor-gpg init

Message ID 20210924110506.4676-1-attila@lendvai.name
State Accepted
Headers show
Series [bug#50751,1/2] gnu: python-trezor-agent: Fix key generation with trezor-gpg init | 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. 24, 2021, 11:05 a.m. UTC
Someone with a better understanding of the python infrastructure may be able
to fix it nicer, but it works.

* gnu/packages/finance.scm (trezor-agent): Add comment on why the
undoing of the wrapping is done, and also delete the irrelevant sideffect of
the now undone wrapping.
(python-trezor-agent): Add a patch that changes the python code to handle the
argv[0] changed by the wrapping.
* gnu/packages/patches/trezor-agent-fix-argv0.patch: New file.
---
 gnu/packages/finance.scm                      | 13 +++++++--
 .../patches/trezor-agent-fix-argv0.patch      | 27 +++++++++++++++++++
 2 files changed, 38 insertions(+), 2 deletions(-)
 create mode 100644 gnu/packages/patches/trezor-agent-fix-argv0.patch

Comments

Nicolas Goaziou Nov. 28, 2021, 8:34 p.m. UTC | #1
Hello,

Attila Lendvai <attila@lendvai.name> writes:

> Someone with a better understanding of the python infrastructure may be able
> to fix it nicer, but it works.
>
> * gnu/packages/finance.scm (trezor-agent): Add comment on why the
> undoing of the wrapping is done, and also delete the irrelevant sideffect of
> the now undone wrapping.
> (python-trezor-agent): Add a patch that changes the python code to handle the
> argv[0] changed by the wrapping.
> * gnu/packages/patches/trezor-agent-fix-argv0.patch: New file.

I prefixed the patch name with python-... and referenced it in local.mk.
Then I applied it.

Thank you.

Regards,
Attila Lendvai Nov. 30, 2021, 8:51 p.m. UTC | #2
ouch, i already had a newer version of the patchset locally. i reported an issue upstream, and they got rid of the argv[0] usage, and also made a new release.

i was already testing it locally but then life happened before i could send it. i have sent the new patchset now: https://issues.guix.gnu.org/52207

re splitting the commit: i don't think it's a good idea, mostly due to git bisect: pushing commits that may not even compile can make someone's life much harder down the road. i think updating tightly interdependent packages should happen in one commit, so that each git commit leaves the repo in a functional state, and so that it can compile and run the test suite.

but in spite of the above, i have sent the new patchset as separate commits.

- attila
PGP: 5D5F 45C7 DFCD 0A39
Nicolas Goaziou Dec. 1, 2021, 3:34 p.m. UTC | #3
Hello,

Attila Lendvai <attila@lendvai.name> writes:

> ouch, i already had a newer version of the patchset locally.
> i reported an issue upstream, and they got rid of the argv[0] usage,
> and also made a new release.

It happens.
>
> i was already testing it locally but then life happened before i could
> send it. i have sent the new patchset now:
> https://issues.guix.gnu.org/52207

Great.

> re splitting the commit: i don't think it's a good idea, mostly due to
> git bisect: pushing commits that may not even compile can make
> someone's life much harder down the road. i think updating tightly
> interdependent packages should happen in one commit, so that each git
> commit leaves the repo in a functional state, and so that it can
> compile and run the test suite.
>
> but in spite of the above, i have sent the new patchset as separate
> commits.

I overlooked the fact that packages were interdependent, which is pretty
obvious now you mention it. In that case, I agree splitting the commit
was not a good idea.

So... could you send the new patchset again in a single commit?

Thanks,

Regards,
diff mbox series

Patch

diff --git a/gnu/packages/finance.scm b/gnu/packages/finance.scm
index 9b073541de..a123efe207 100644
--- a/gnu/packages/finance.scm
+++ b/gnu/packages/finance.scm
@@ -844,7 +844,8 @@  the Monero GUI client.")
              (commit (string-append "v" version))))
        (file-name (git-file-name name version))
        (sha256
-        (base32 "0q99vbfd3h85s8rnjipnmldixabqmmlk5w9karv6f0rhyi54f4zv"))))
+        (base32 "0q99vbfd3h85s8rnjipnmldixabqmmlk5w9karv6f0rhyi54f4zv"))
+       (patches (search-patches "trezor-agent-fix-argv0.patch"))))
     (build-system python-build-system)
     (arguments
      `(#:phases
@@ -1089,7 +1090,15 @@  the KeepKey Hardware Wallet.")
          (add-after 'wrap 'fixup-agent-py
            (lambda* (#:key inputs outputs #:allow-other-keys)
              (let* ((out (assoc-ref outputs "out")))
-               ;; overwrite the wrapper with the real thing.
+               ;; The wrap phase also wraps trezor_agent.py (besides the
+               ;; public facing executable called trezor-agent). We need to
+               ;; undo that wrapping. The reason this is needed is that the
+               ;; python easy install generates a toplevel script (?) that
+               ;; messes with argv[0] and then re-opens the python
+               ;; module. This fails when the wrapped file is actually a shell
+               ;; script, not a python file.
+               (delete-file (string-append out "/bin/.trezor_agent.py-real"))
+               ;; Overwrite the wrapped one with the real thing.
                (install-file "./trezor_agent.py"
                              (string-append out "/bin"))
              #t))))))
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)