epiccurious / jade-diy

Securely custody your bitcoin with Open Source software and generic hardware.
MIT No Attribution
12 stars 1 forks source link

Wrong dependency for venv on debian #74

Open 1-21gigasats opened 3 months ago

1-21gigasats commented 3 months ago

I did a fresh install inside a debian qube (without any dependencies installed). The installer promted me to install python3-virtualenv but imo this is the wrong dependency for the esp-idf as it uses venv. So the required package would be python3-venv.

The problem is that it is a little bit messy to check if its already installed as there is no venv command. My first idea was to run python3 -m venv --help but this is not enough because it works without any errors even if the package is not installed.

So one approach would be to actually try to create a venv with python3 -m venv /tmp/venv_test. This fails with exit code 130 if the required package isnt installed:

The virtual environment was not created successfully because ensurepip is not
available.  On Debian/Ubuntu systems, you need to install the python3-venv
package using the following command.

    apt install python3.11-venv

You may need to use sudo with that command.  After installing the python3-venv
package, recreate your virtual environment.

Failing command: /tmp/venv_test/bin/python3
epiccurious commented 3 months ago

Thanks for reporting this. I'll try reproducing on Debian Proxmox.

1-21gigasats commented 3 months ago

It should be the same on ubuntu and the derivates. My only explanation that the ubuntu ci runner works without errors it that the package must be pre installed.

Check out the requirements from the official esp-idf guide - its's always venv not virtualenv

epiccurious commented 3 months ago

I moved from venv to virtualenv to work around some problem.

Can you submit a PR for this against the diyjade repo?

1-21gigasats commented 3 months ago

replacing virtualenv with venv in depends.txt alone wont work because venv is not a valid command.

venv in normally part of python3.3 and higher, but for some reasons not on debian and derivates. Thats why other distros dont need a venv installation at all.

Jade repo uses virtualenv for the docker image, so acutally i am a little bit confused for what virtualenv is currently needed. I can build succesfully without it on debian.

The simplest solution would be to keep the deps as they are and add an additional test that tries to detect if building a venv really works (python3 -m venv /tmp/venv_test) or with a special test only executed on a debian based distro that checks if the package is installed. Or just accept it and give users some kind of hint that they need the python3-venv package on debian

1-21gigasats commented 3 months ago

Here is one approach to handle it:

https://github.com/bitcoin-tools/diyjade/commit/402406df1997e7b037fa81dd1bdd0f853963000a

epiccurious commented 3 months ago

Thanks for your patience. This looks good. Please make an MR against the new repo https://github.com/bitcoin-tools/nodebuilder

I'd rather use mktemp -d, what do you think?

Can you please add a line to cleanup()? Similar to this if you agree:

[ -d "${path}" ] && rm -r "${path}"
epiccurious commented 3 months ago

special test only executed on a debian based distro

I like this approach. One way would be to grep "^ID=*debian*\| ^ID_LIKE=*debian*| ^ID_LIKE=*ubuntu*" /etc-os-release.

epiccurious commented 3 months ago

venv in normally part of python3.3 and higher, but for some reasons not on debian and derivates. Thats why other distros dont need a venv installation at all.

If non-Debian-based repos don't need venv already have venv installed, why can't we remove virtualenv from the dependencies list?

1-21gigasats commented 3 months ago

Added another simpler approach by just checking if the package is installed on debian an derivates. We should also be able to remove the virtualenv package if jade does not depend on it (esp does not). i will try it without

1-21gigasats commented 3 months ago

I was able to flash successfully without having the python-virtualenv package installed (and no virtualenv command available) so i think it is safe to remove this dependency from all distros.