search

neuronalmotion / qtrpi

An easy-to-use environment to cross-compile Qt applications for Raspberry Pi from your desktop.
MIT License
104 stars 40 forks source link

init and deploy scripts would benefit from 'set -e' #73

Closed staniek closed 6 years ago

staniek commented 7 years ago

Maybe the init and deploy scripts are not trivial so they would benefit from using 'set -e' as the first command. Giving up on first error they would behave more nicely than trying to continue when it makes no sense (e.g. when there's no access to the rpi machine, it's locked or there was error downloading files).

GuillaumeLazar commented 7 years ago

I think it's a good idea. But we have to check that no command will accidentally stop the script.

staniek commented 7 years ago

Yes, that's true.

synapticvoid commented 6 years ago

Thanks for you suggestion @staniek , however, right now some commands may fail and that's fine (e.g. the sysroot un-mount command).

I'll close this issue for now, implementing it would imply and very thorough analysis of which command is allowed to fail.

1e7 commented 6 years ago

Thanks fort the info @synapticvoid. Trivial info that you may know: command that is allowed to fail can be executed like if ! command ; then echo "Warning: ..." fi

Or: command || true

If we do not know what commands can fail and what now and decide to ignore failing commands the script can be considered unsafe or even dangerous. Just think of that if we even use the 'rm' command - if executed on a failing scenario it can even remove important files or even entire $HOME...

Your decision but as for the workflow, I am sorry this issue is closed this way since it means for me "closed as invalid / won't do" and the topic would get forgotten. No contributor would come to fix is if it's closed :)

synapticvoid commented 6 years ago

Hi @1e7 ,

Yep, the command || true works like a charm!

About this specific issue, I understand what you mean. However, implementing it would almost require a ground up check of all the sources of qtrpi. Many commands have been used with the idea that they may fail (mkdir, umount, etc). Just look at the content of the utils to get an idea.

About the sanity check of the system, to avoid any big problem, we already did that in common.sh. This script is included in all the other scripts so the commands are rather safe.

IMHO, if we wanted to use set -e, it should have been from the very beginning. Right now, it would demand a lot of time.