Prerequisites

[pkutzner]

I found some small errors in the script regarding missing 'sudo's at the beginning of some commands, as well as a missing target for the git pull command for xo-server.

I added a check for both the 'sudo' command being present, as well as the ability for the user to sudo. Sudo doesn't appeared to be installed by default on a minimal Debian 9 installation, hence the addition. I also added checks for 'curl' and 'git' for the same reason. It will now check if they're installed and install them if they're not. Otherwise it just skips to your original script.

This script will now complete successfully on a minimal Debian install as of 2018-04-26

[Jarli01]

How are you running the installation if you don't have the prerequisites? On a minimal installation one can't run sudo bash and then sudo curl before both sudo and curl are installed.

I'm not declining the changes, just want to understand what I am missing here

[Jarli01]

Ah!! I see. You're using wget to download the file. But you still need sudo to execute it.

[pkutzner]

Hi Dustin,

I have to install sudo manually first as root, but I do use wget to grab the script from the raw.githubusercontent.com url as the nonprivileged user. From there, as long as the user running the script has 'sudo' permissions, it'll run.

On Thu, Apr 26, 2018 at 5:16 PM, Dustin B notifications@github.com wrote:

Ah!! I see. You're using wget

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Jarli01/xenorchestra_installer/pull/14#issuecomment-384805847, or mute the thread https://github.com/notifications/unsubscribe-auth/AEe75_vp58SHX6Id5BMp1-exvD9gZCGeks5tskdEgaJpZM4ToELM .

-- Preston

[Danp2]

One of the script's requirement is to be run with elevated privileges, so are the "missing" sudos really needed?

[pkutzner]

Well, I saw the sudos in the original script, which would be unnecessary if the script was run as root or run via 'sudo'. In this configuration, the script can be run as a normal user with sudo privileges. This was just a quick hack to get it working.

I do plan on adding some logic in my fork to determine if the script has been run EUID=0 and, if not, place the sudos in front of the commands at execute, that way it doesn't matter if you forget to run it as root or sudo, it'll just figure it out and run appropriately.

[Jarli01]

I believe that is why I removed the original "sudo's" it was redundant and just bloated the code.

I'm not against people downloading the installation script, but it might be wasteful to go down that approach. The script can be run with very minimal software added to almost any distro. . . hrm

[Danp2]

Right... I think someone else added the a bunch of the sudo prefixes that now exist. Also, I was thinking that there was already a check to ensure the script was running under the correct user, but I guess that is in the updater script.

[Danp2]

Can you explain the benefit for some of your modifications, specifically,

• Git Clone change
• Sudo Tee addition
• Chmod removal

[pkutzner]

It might be easier to just kill this pull request and I can re-implement my check for 'git' and 'curl' and probably clean out the sudo's as they'd be unnecessary if run as root or sudo anyhow. If that's the intent, they aren't necessary in the script. I see that the majority of changes surrounded the confusion with the sudos in the first place.

With regards to the chmod removal however, systemd unit files don't need to be marked as executable. I had changed it to the proper 'systemd daemon-reload' command that should be run after adding/removing/changing unit files to get systemctl to recognize them.

I can re-submit a new pull request with only the git and wget change (or just add them to the first apt-get install command) and the change to the unit file executable bit.

[Jarli01]

I like the changes and do see the value, the sudo portions are redundant though.

I'll close the PR for now, please make the discussed changes and we can test them out.