Move config file out of source control

[mtlynch]

Generally, files intended to be user-editable should not be under source control. Otherwise, a developer who wants to contribute upstream has to always remember to exclude their config file from their git commit.

Additionally, it's difficult to run vdesktop multiple times in a script with different configurations.

For example:

sudo ./vdesktop myimg.img cli-login # Log in as pi user
#TODO: Edit config file to change user setting...
sudo ./vdesktop myimg.img cli-login # Log in as a new user

The TODO is difficult to implement with standard bash utilities.

Proposal 1: Configure vdesktop through environment variables

The simplest approach would be to simply use environment variables. For example, we change the username line to this:

username="${VDESKTOP_USER-pi}"

Then changing users between script execution is straightforward and obvious:

sudo ./vdesktop myimg.img cli-login  # Log in as pi user
VDESKTOP_USER=newuser
sudo ./vdesktop myimg.img cli-login # Log in as a new user

Proposal 2: Specify config file on the command line

If we want to preserve the config file, we could allow the config file as a command-line option, e.g.

sudo ./vdesktop myimg.img cli-login --config=default.conf # Log in as pi user
sudo ./vdesktop myimg.img cli-login --config=newuser.conf # Log in as a new user

We could rename config to default.conf.example and instruct users to cp default.conf.example default.conf to use it. If no --config is specified, vdesktop uses default options.

The downside of this approach is that the existing config file format is very brittle. Options have to appear in a rigid order or everything breaks silently, as there's no rigid validation. It's also not friendly to changes over time, as additions or deletions of config file options will break all previous config scripts.

---

I'm willing to implement these changes if you agree with one of these proposals.

[Botspot]

Generally, files intended to be user-editable should not be under source control. Otherwise, a developer who wants to contribute upstream has to always remember to exclude their config file from their git commit.

Additionally, it's difficult to run vdesktop multiple times in a script with different configurations.

You make a very good point right there.

The simplest approach would be to simply use environment variables.

username="${VDESKTOP_USER-pi}"

Environment variables is a great idea. I think we'll go with that.

The downside of this approach is that the existing config file format is very brittle. Options have to appear in a rigid order or everything breaks silently, as there's no rigid validation. It's also not friendly to changes over time, as additions or deletions of config file options will break all previous config scripts.

The config file is a bit fragile. It gets its config options based on line numbers. Line 1 enables/disables the sleep infinity, line 2 specifies the username, etc.
If you were to set line 1 to "" (nothing, just a newline), vdesktop wouldn't care. But if you completely removed line 1, including the newline, then things would get messed up. But as long as you don't do that you're fine.
Adding new entries to the end of the file doesn't break anything either.

[mtlynch]

Environment variables is a great idea. I think we'll go with that.

Okay, cool. Would you like me to create a PR or is this something you'd prefer to do yourself?

[Botspot]

I already did it. :)
Locally, at least. Haven't pushed the changes yet.

[Botspot]

Having a small issue here. This does not work:

export VDESKTOP_USER=1234
sudo /home/pi/vdesktop/vdesktop /path/to/my.img

This does not work:

VDESKTOP_USER=1234 sudo /home/pi/vdesktop/vdesktop /path/to/my.img

In both above cases vdesktop thought the variable was empty.

But this does work:

sudo VDESKTOP_USER=1234 /home/pi/vdesktop/vdesktop /path/to/my.img

Any ideas?

[mtlynch]

Can you push your changes to a branch so I can take a look?

[mtlynch]

I forgot sudo needs to have --preserve-env to inherit env variables that are exported. The sudo VDESKTOP_USER=1234 /home/pi/vdesktop/vdesktop should work, though.

[Botspot]

Not necessary to push changes. The same issue is evident in this simple script named vartest.sh:

#!/bin/bash
echo "This should not be blank: $VDESKTOP_USER"

Tests:

$ export VDESKTOP_USER=pi
$ ./vartest.sh
This should not be blank: pi
$ sudo ./vartest.sh
This should not be blank:

Looks like sudo is breaking it somehow. Maybe if I run export with sudo the variable will be visible to root processes:

$ sudo export VDESKTOP_USER=pi
sudo: export: command not found

[Botspot]

The sudo VDESKTOP_USER=1234 /home/pi/vdesktop/vdesktop should work, though.

Yes, it did work.

[mtlynch]

Right, it needs to be:

sudo --preserve-env ./vartest.sh

or the shorthand:

sudo -E ./vartest.sh

[Botspot]

Right, it needs to be:

sudo -E ./vartest.sh

That doesn't seem very user friendly. 100% of vdesktop users are used to sudo ./vdesktop/vdesktop img.img, but to do environment variable config all those users will have to remember to run sudo -E from now on.
Seems hard to remember, and impossible for vdesktop to detect and warn about.
Maybe it would be better to have a separate, non-sudo script which takes the environment vars and uses them to generate a config file, which vdesktop then uses.
So to change the settings, you'd run something like:

export VDESKTOP_USER=foobar
./vdesktop/refresh-config.sh
sudo ./vdesktop/vdesktop /path/to/my.img

An added benefit with that would be that customized settings persist between reboots, instead of forcing the user to redo their export commands every time.

[mtlynch]

Maybe it would be better to have a separate, non-sudo script which takes the environment vars and uses them to generate a config file, which vdesktop then uses.
So to change the settings, you'd run something like:

export VDESKTOP_USER=foobar
./vdesktop/refresh-config.sh
sudo ./vdesktop/vdesktop /path/to/my.img

At that point, you might as well just have vdesktop source a hardcoded path of environment variables like:

#!/bin/bash

SETTINGS_FILE=settings.env
if [ -f "$SETTINGS_FILE" ]; then
  source "$SETTINGS_FILE"
fi

echo "user=$VDESKTOP_USER"

And then you'd change settings like:

$ echo 'VDESKTOP_USER=otheruser' > settings.env
$ sudo ./vdesktop
user=otheruser

But it would be more flexible if the user could specify the extra .env file from the command line like:

sudo ./vdesktop/vdesktop /path/to/my.img --env-file settings.env

Although, to me, that's more work than just doing sudo -E.

[Botspot]

Problems with sudo -E:

• It's annoying to remember to type every time.
• Problem with sudo -E is that if a user forgets the -E, vdesktop won't have any way to detect it and warn the user.
  If the user sets the environment vars, but vdesktop appears to ignore them, that would be very confusing.
• Additionally, the custom settings aren't preserved across reboots.

So I'm trying to find a way which is as easy as env-vars, permanent as config files, and flexible as CLI flags.

I'll check out the .env idea.

[Botspot]

Okay, env file implemented. I have yet to update the docs, but it's hopefully self-explanatory.

Note that if you export an environment variable, the env file will take priority. To prevent the env file from setting a certain var, simply comment out the line in the env file.

[mtlynch]

Okay, env file implemented. I have yet to update the docs, but it's hopefully self-explanatory.

Unfortunately, I've decided to disengage from this repo at this point.

I run this script in production as part of my business, so I can't inspect the source to reverse engineer undocumented breaking changes, especially when they're bundled with tons of other changes that are not explained in the source history.

Introducing uninspectable binaries into source control is also a dealbreaker for me.

Thanks for your work! I'll check back in on the project in the future to see if it's stabilized, and maybe I can merge my fork back in.

[Botspot]

Unfortunately, I've decided to disengage from this repo at this point.

Understand. This version includes many advancements for GUI usage. (like HW acceleration & support for other desktop environments)
For CLI usage, not much has changed at all.

Key differences:

• Compiled versions of systemd-nspawn. Turns out the repo's version of systemd-nspawn is outdated and has caused all the Chromium "Aw, snap!" messages.
  • If you aren't comfortable with those binaries, I could add a variable/flag to disable their usage and instead use the installed version of systemd-nspawn.
  • Or, I could have vdesktop compile systemd-nspawn natively on your device.
• Addition of an .env file as you suggested. It's just another way to set your variables and if you don't like it, you can

I run this script in production as part of my business, so I can't inspect the source to reverse engineer undocumented breaking changes, especially when they're bundled with tons of other changes that are not explained in the source history.

I tried hard to not introduce any breaking changes for you, however if you encounter any please let me know.
The README will be updated in a few days.

Thanks for your work! I'll check back in on the project in the future to see if it's stabilized, and maybe I can merge my fork back in.

Thank you too, for your help, your suggestions, and your donations!
Vdesktop has exciting plans for the future, and I'm glad you've been a part of it.