Hardcoded USER variable in install.sh

Manicben commented 4 years ago

Happened to notice when running the update.sh script (which calls the install.sh script) that the USER var is hardcoded to giles which results in:

[*] EXECUTE: [chown -R giles:giles /home/pi/Circuit-Shield]
chown: invalid user: ‘giles:giles’
ERROR: Command exited with [1]

Since the USER env var is correctly populated by default (echo $USER results in pi on my system at least, but this may be different for others who have renamed the main user), it should not be overwritten in install.sh otherwise it will fail for almost everyone.

Happy to raise a PR for this and test it locally if you wish.

As a side note, also noticed L50 is hardcoded to PIHOMEDIR="$DEST/home/pi". I know /home/pi is the default Rasbian/Retropie home dir, and not many people would rename it, but I was wondering if using the HOME env var would be safer. Just a suggestion, in case some people not only renamed the user, but also renamed the home dir to match their user name.

kiteretro commented 4 years ago

Nice spot, i'll get this sorted :) thanks!!

kiteretro commented 4 years ago

I've fixed the issue now, while not ideal, it's the same as all the other pieces of build/install/update scripts I have done for these types of projects for I'm going to opt to leave as is for now :) thanks for finding it!

It is now in the image https://github.com/kiteretro/Circuit-Shield/releases/tag/v1.0.1

To use the update script instead of a re-image, edit the install.sh script and change line 52 USER="giles" to USER="pi" :)

Manicben commented 4 years ago

There's 3 ways to do it dynamically that I'm aware of (depending on which method suits your needs).

FOO=$USER will set FOO to the $USER environment variable which is set by the shell. Like the $HOME env var, $USER is set by the shell upon login or ssh and therefore depends on which user you log in as. This approach doesn't take into account who is actually running the script, as you could assume another identity while running the script, like sudo ./install.sh.
FOO=${USER:-pi} also sets FOO to $USER. However, if $USER is not set, it will default to pi.
FOO=$(whoami) will set FOO to the result of whoami, which does take into account who is running the script. However, this may not be desirable as running sudo ./install.sh will result in FOO being set to root.

In most cases, the first method is sufficient as you can at most times assume people will be logging in or ssh'ing in as the main user (which may not be pi, some people may have renamed the user to suit their own preference). It is rare for the $USER env var to not be set, hence why I think the second method is unnecessary. The third method should only be used if you actually care about the user running the script. In this case we don't, as we only care about the main user, due to all the files living in that user's home directory ($HOME).

Just something to keep in mind if you weren't aware already for future releases or projects. Always best to not assume too much about your users' setups. 👍

kiteretro / Circuit-Shield

Hardcoded USER variable in install.sh #1