Add support for MINETEST_USERDATA environment variable

[vilhelmgray]

The MINETEST_USERDATA environment variable can be used to defined a
custom path for Minetest user data. If MINETEST_USERDATA is empty or
unset, the HOME environment variable is used as the default user data
path; this ensures backwards compatibility with existing user setups.

[SmallJoker]

Please document this feature here: https://github.com/minetest/minetest/blob/master/doc/minetest.6

Also a notice in "Paths" (https://github.com/minetest/minetest/blob/master/README.md#paths) might be helpful - either a general one, or mentioning the command line and environment variable methods.

EDIT: Also what about Windows users?

[vilhelmgray]

I decided to change the variable name to MINETEST_USER_PATH to match the naming convention of the existing Minetest environment variables listed in the minetest.6 file. I also added support for an equivalent %MINETEST_USER_PATH% Windows environment variable so users on Windows can also set this path.

[SmallJoker]

LGTM

[ghost]

I find it a little odd that $MINETEST_USER_PATH is not actually the path to minetest's user directory, but rather the path to the directory where minetest's user directory is created. My first reaction upon seeing this was "oh cool there's now a way to put my tests the data in .local/share" but you can only kinda do that... because it'll still make a hidden directory. For that reason, my first instinct when testing was to try ~/.local/share/minetest... but in fact this will fail, because MT will only try to create the .minetest dir and not any of its parents. This yields the not very useful error of

Cannot create user data directory

I feel like if we're going to have a feature like this, we should

• Allow minetest to create the full path, not just the final directory
• Let the user specify the full path to the data, not the path to .minetest's parent.

If we don't do this, I think the documentation should be updated. "Path to Minetest user data directory." doesn't read as the parent dir, whereas something like "Path to search for the Minetest user data directory" might be clearer. I will leave that determination to the PR author and/or any other coredevs who want to chime in.

EDIT One other benefit of pointing to the actual dir is the possibility that someone can make a folder of various MT setups, ie minetest-1, minetest-2, minetest-3, etc. They can do that in its current state, but it's going to require a useless hidden secondary folder which makes the management of those configs more of a pain.

[vilhelmgray]

I agree that specifying the actual directory would be better. The reason for the current behavior of Minetest creating a .minetest folder is because the user's home directory is assumed and thus we don't want to pollute the user's home directory with various Minetest data. If the user can now specify the directory they want, there's no need to create a .minetest directory under that because the user has already chosen where they want Minetest data to go.

However before I update this, I'll wait for other devs to chime in and confirm this is a good approach to take for this PR.

[SmallJoker]

I totally missed that part about nested directories. Indeed, I agree with @Df458 that the variable should point directly to the user data, and not its parent directory.

[vilhelmgray]

So I adjusted the code to have path_user set to the MINETEST_USER_DATA environment variable directory (if it is set).

In separate commit, I adjusted create_userdata_path() to create the user data directory using std::filesystem::create_directories(); this will automatically create any missing parent directories. The problem is that this function was added in C++17, but I think the Minetest CMakeLists.txt specifies C++14. Does it make sense to bump up the C++ standard for Minetest, or does another part of the codebase prevent that?

[SmallJoker]

You could recursively call fs::RemoveLastPathComponent with fs::IsDir to determine which directories to create. The switch to C++14 was done recently, and it will again take a while until most of the supported distributions have GCC with C++17 support.

[vilhelmgray]

Okay, I understand. I'll utilize fs::RemoveLastPathComponent instead then.

[vilhelmgray]

As I was implementing this, I discovered fs::CreateAllDirs which does what we want so I've adjusted the commit to call that instead.

[SmallJoker]

Tested on Linux with a trivial change in initializePaths() to enforce system paths due to RUN_IN_PLACE configuration/build.

Works. The environment variable now points exactly to the desired location.

[sfan5]

If you look at CI you will also see that the build is broken

[sfan5]

Looks good.