iiab / iiab-factory

Tools for deploying IIAB, installing IIAB, cloning IIAB, refining Content Packs, etc.
http://internet-in-a-box.org
GNU General Public License v2.0
13 stars 17 forks source link

Script fails if wget not installed [Aside: where should apt packages 'requests' and 'packaging' be installed?] #154

Closed tabbyrobin closed 4 years ago

tabbyrobin commented 4 years ago

Seems like this can be fixed with a simple: $APTPATH/apt -y install wget

However, not immediately clear where best to place this...

user@work ~/git-repos> grep "wget" iiab-factory/iiab -n
140:            wget -O local_vars.yml https://github.com/iiab/iiab/raw/master/vars/local_vars_min.yml
143:            wget -O local_vars.yml https://github.com/iiab/iiab/raw/master/vars/local_vars_big.yml
146:            wget -O local_vars.yml https://github.com/iiab/iiab/raw/master/vars/local_vars_medical.yml
149:            wget -O local_vars.yml https://github.com/iiab/iiab/raw/master/vars/local_vars_medium.yml
304:            wget http://pantry.learningequality.org/downloads/ka-lite/0.17/content/contentpacks/en.zip

git is installed right before it is needed. wget is used in two different areas, both of which have conditionals so it's possible that neither one triggers at all, depending on circumstances.

Maybe just install it towards the very beginning of the script, without any conditions? :/

wget missing is relatively rare but it is the case sometimes (in my case, this happened using a qubes debian-10-minimal template, which is very minimal)

holta commented 4 years ago

@sptankard good question:

Personally I've been trying to keep /usr/sbin/iiab (what runs when folks type sudo iiab) quite minimal — so that newcomers can quickly grasp roughly what bash is doing to their Linux system — whereas software bloat kills many projects as I'm sure you know (-:

But at the same time 1 More Line (TM) can hardly hurt here ;-)

tabbyrobin commented 4 years ago

I understand. And most targets of IIAB will probably not be so minimal as to be missing wget. So it's kind of an edge case / only a problem on non-target systems.

Hmm... maybe it could be added to the line where git is installed? $APTPATH/apt -y install git wget And move that line up towards the top... sort of a 'dependencies' line? Maybe at line 53? https://github.com/iiab/iiab-factory/blob/master/iiab#L53

i notice the git install has no conditions: https://github.com/iiab/iiab-factory/blob/master/iiab#L207

Might not be a bad idea to add grep too for good measure? $APTPATH/apt -y install git wget grep

holta commented 4 years ago

@sptankard let's go with your idea, thanks much.

tim-moody commented 4 years ago

there was a time when we had apt -y install git curl nano gawk wget pastebinit early in the script

most of these are now there by default, but doesn't hurt to install them

holta commented 4 years ago

IIAB's 1-line installer requires curl before it even begins, which thankfully is a part of Ubuntu & RaspiOS. (Debian gurus know what they're doing thankfully, and install it on their own ;)

Let's stay slim and go with these 3 packages:

apt -y install wget grep nano

i.e. things like pastebinit and gawk are nice (and many other things too) but none are required by this script trying to do a better job of declaring its own dependencies. Also FYI extras are installed very promptly afterwards (in Stage 0 or Stage 1 or Stage 2 of iiab-install's 9 stages),

holta commented 4 years ago

I meant 4 packages:

apt -y install git wget grep nano
holta commented 4 years ago

git -> apt typos fixed above. Apologies.

holta commented 4 years ago

Aside:

Beyond the notion of "declaring your own packages...nothing more...nothing less" I'm also reticent to add a wish list of packages (we each have our favorites, for sure) just in case someone later ports IIAB to another OS/distro, where packages might be named differently.

tabbyrobin commented 4 years ago

Btw there is a different missing dependency i encountered, but i'm not sure if it's worth fixing / filing an issue for.

Easy workaround: sudo apt install python3-requests python3-packaging (only requests is mandatory; lack of packaging just gives a warning)

For anyone needing future reference, see also here: https://gist.github.com/sptankard/1dbe2957abd14b2cd96f783235dbc136#file-install-iiab-qubes-debian-10-minimal-txt

The file that triggers the error is: install_menu_defs.py https://github.com/iiab/iiab-admin-console/blob/2286409c7229601ac9fb82db1707944ca63deb55/roles/cmdsrv/templates/scripts/install_menu_defs.py

requests module is used in 3 places: https://github.com/search?q=org%3Aiiab+%22import+requests%22&type=code

holta commented 4 years ago

Easy workaround: sudo apt install python3-requests python3-packaging (only requests is mandatory; lack of packaging just gives a warning)

Thank you @sptankard

Opening a separate issue best?

Or submit a PR (fix) if you prefer!

tabbyrobin commented 4 years ago

I will have a look at fixing it and maybe submit a PR! :)

holta commented 4 years ago

I will have a look at fixing it and maybe submit a PR! :)

Thanks!

These 3 lines seem possibly relevant ? https://github.com/iiab/iiab/blob/master/scripts/ansible#L90-L92 Though adding so many non-Ansible packages here is getting increasingly off-topic (!) given the name of the script (ansible).

So possibly a place like this might be better?? https://github.com/iiab/iiab/blob/master/roles/2-common/tasks/packages.yml

holta commented 4 years ago

Or maybe this belongs in https://github.com/iiab/iiab-admin-console/blob/master/roles/cmdsrv/tasks/packages.yml ?

Or possibly even in https://github.com/iiab/iiab-admin-console/blob/master/roles/common/tasks/main.yml ??

Given that common is installed before cmdsrv here: https://github.com/iiab/iiab-admin-console/blob/master/iiab-admin.yml#L44-L50

jvonau commented 4 years ago

The unused packages.yml is a red-herring, use https://github.com/iiab/iiab-admin-console/blob/master/roles/cmdsrv/tasks/main.yml, as install_menu_defs.py has the requests import, in the same cmdsrv role.

jvonau commented 4 years ago

Btw there is a different missing dependency i encountered, but i'm not sure if it's worth fixing / filing an issue for.

Easy workaround: sudo apt install python3-requests python3-packaging (only requests is mandatory; lack of packaging just gives a warning)

Think 'packaging' one is the pink warning noted here.

requests module is used in 3 places: https://github.com/search?q=org%3Aiiab+%22import+requests%22&type=code

The 'requests' one has some duplicate importing going on between adm_lib and the other 2 python programs, I'm on the fence where the dependency should be installed pending correction of the dup'd imports

shanti-bhardwa commented 4 years ago

There should be section in FAQs about pre-requisites required telling people what they need in terms of various packages they need BEFORE they run the 1-line script.

holta commented 4 years ago

Ladies & Gents: please open new tickets for new issues whenever possible!

jvonau commented 4 years ago

Why bother opening new ticket when existing issues are all but ignored.

holta commented 4 years ago

For anyone needing future reference, see also here: https://gist.github.com/sptankard/1dbe2957abd14b2cd96f783235dbc136#file-install-iiab-qubes-debian-10-minimal-txt

@sptankard thanks again for publishing that!

Can we help you with networking on your QVM install?

Please send us the sprunge.us URL output after you run sudo iiab-diagnostics if you can, Thanks!

jvonau commented 4 years ago

@sptankard The 'error: couldn't restart networking.' noted in your QVM install suggests there are more than one network adapter and/or virtual devices present. Could you forward the output ip a from clean fresh out of the box install of QVM without any other mods to help me understand the network topology you are using or faced with please.