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

trezorctl: fix Click 8.1 compatibility #2199

Closed
lestephane opened this issue Mar 30, 2022 · 7 comments
Closed

trezorctl: fix Click 8.1 compatibility #2199

lestephane opened this issue Mar 30, 2022 · 7 comments
Assignees
Labels
bug Something isn't working as expected low hanging fruit Simple, quick task. trezorlib Python library and the command line trezorctl tool.

Comments

@lestephane
Copy link

$ python3 --version
Python 3.8.10
$ python3 -m venv trezor
$ source trezor/bin/activate
$ pip3 install setuptools wheel
$ pip3 install trezor
$ trezorctl
Traceback (most recent call last):
  File "/home/elstephane/VirtualEnvironments/trezor/bin/trezorctl", line 5, in <module>
    from trezorlib.cli.trezorctl import cli
  File "/home/elstephane/VirtualEnvironments/trezor/lib/python3.8/site-packages/trezorlib/cli/trezorctl.py", line 191, in <module>
    @cli.resultcallback()
AttributeError: 'TrezorctlGroup' object has no attribute 'resultcallback'
@trezor-ci trezor-ci added this to 📥 Inbox in Backlog 🗂 via automation Mar 30, 2022
@hynek-jina hynek-jina added the code Code improvements label Mar 30, 2022
@hynek-jina hynek-jina moved this from 📥 Inbox to 💻 Code in Backlog 🗂 Mar 30, 2022
@sime
Copy link
Member

sime commented Mar 30, 2022

@lestephane What operating system? Have you managed to install the trezor package previously/?

@matejcik matejcik changed the title trezorctl: AttributeError: 'TrezorctlGroup' object has no attribute 'resultcallback' trezorctl: fix Click 8.1 compatibility Mar 30, 2022
@matejcik matejcik added trezorlib Python library and the command line trezorctl tool. bug Something isn't working as expected low hanging fruit Simple, quick task. and removed code Code improvements labels Mar 30, 2022
@matejcik matejcik moved this from 💻 Code to 🐛 Bug in Backlog 🗂 Mar 30, 2022
@matejcik
Copy link
Contributor

We are currently incompatible with click 8.1. As a workaround, pip3 install "click<8.1"

The current code in master has the right versioning set, but the published package does not.

Maybe a better approach is to fix the compatibility.

@jonathancross
Copy link
Contributor

@matejcik the fix you suggested isn't working for me when using the PyPi version of trezorctl:

$ pip3 install "click<8.1"
Requirement already satisfied: click<8.1 in /usr/lib/python3/dist-packages (7.1.2)

$ trezorctl
Traceback (most recent call last):
  File "/home/████/.local/bin/trezorctl", line 5, in <module>
    from trezorlib.cli.trezorctl import cli
  File "/home/████/.local/pipx/venvs/trezor/lib/python3.9/site-packages/trezorlib/cli/trezorctl.py", line 191, in <module>
    @cli.resultcallback()
AttributeError: 'TrezorctlGroup' object has no attribute 'resultcallback'

@jonathancross
Copy link
Contributor

Nevermind... because I was using pipx environment, it was still using the incorrect 8.1 version instead of the one in /usr/lib/python3/dist-packages 😬

@prusnak
Copy link
Member

prusnak commented May 27, 2022

This bit me again today. I think we should make trezorctl compatible with click 7.x, 8.0.x and 8.1.x and publish a new release on PyPI.

@sime
Copy link
Member

sime commented May 31, 2022

Can also confirm on OS X 12.2.1 with a new user the following is required to make trezorctl work. (purposefully tried a new user to get the virgin environment variables).

pip3 install trezor
pip3 install "click<8.1"
$HOME/Library/Python/3.8/bin/trezorctl version # Returns 0.13.0

@jackinloadup
Copy link

I was able to use trezorctl after making the following simple modification.

diff --git a/python/src/trezorlib/cli/trezorctl.py b/python/src/trezorlib/cli/trezorctl.py
index 8e133d85c..a1f7a86fe 100755
--- a/python/src/trezorlib/cli/trezorctl.py
+++ b/python/src/trezorlib/cli/trezorctl.py
@@ -189,7 +189,7 @@ def cli_main(
 cli = cast(TrezorctlGroup, cli_main)
 
 
-@cli.resultcallback()
+@cli.result_callback()
 def print_result(res: Any, is_json: bool, script: bool, **kwargs: Any) -> None:
     if is_json:
         if isinstance(res, protobuf.MessageType):

Here are steps to reproduce the fix.

git clone https://github.com/trezor/trezor-firmware.git
cd trezor-firmware
patch -p1 < patch_file_above.patch
nix-shell
poetry install
poetry shell
trezorctl list

This fix only works inside the nested nix and poetry shell. I didn't see any issues but haven't done any extensive testing. I didn't find instruction on running a test suite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected low hanging fruit Simple, quick task. trezorlib Python library and the command line trezorctl tool.
Projects
Archived in project
Development

No branches or pull requests

8 participants