Use standard configuration directories

[M374LX]

With this PR, the "lmmsrc.xml" file is stored in the platform's standard directory for configuration files, but the file at the legacy location is checked first for backwards compatibility.

According to the documentation at https://doc.qt.io/qt-5/qstandardpaths.html#StandardLocation-enum, the location is ~/.config/<APPNAME> for Linux, C:/Users/<USER>/AppData/Local/<APPNAME> for Windows, and ~/Library/Preferences/<APPNAME> for macOS.

Closes #5869.

Note: tested only on Linux.

Without eb37ee6, the directory is created even when LMMS runs in development mode, which is not desired, as "lmmsrc.xml" is stored on the same directory LMMS runs from in this case.

Some method names in ConfigManager have problems: initDevelopmentWorkingDir() never touches m_workingDir, while initPortableWorkingDir() and initInstalledWorkingDir() set the RC file in addition to m_workingDir. It looks like ConfigManager needs some reorganization (which 38d67b2 does).

[LmmsBot]

🤖 Hey, I'm @LmmsBot from github.com/lmms/bot and I made downloads for this pull request, click me to make them magically appear! 🎩

Windows

• Windows 32-bit: lmms-1.3.0-alpha-msvc2017-win32.exe (build link)
• Windows 64-bit: lmms-1.3.0-alpha-msvc2017-win64.exe (build link)

🤖

{"platform_name_to_artifacts": {"Windows": [{"artifact": {"title": {"title": "32-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/mm958rwvy1wcr6k2/artifacts/build/lmms-1.3.0-alpha-msvc2017-win32.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/42909767"}, {"artifact": {"title": {"title": "64-bit", "platform_name": "Windows"}, "link": {"link": "https://ci.appveyor.com/api/buildjobs/ki7xr0ieb48bsn0w/artifacts/build/lmms-1.3.0-alpha-msvc2017-win64.exe"}}, "build_link": "https://ci.appveyor.com/project/Lukas-W/lmms/builds/42909767"}]}, "commit_sha": "7413c8c4fce8a9b16e2174b50d118ad52efcd0ac"}

[superpaik]

It works ok with windows (64) as well. My user folder is in D: (not as usual in C:) and it works ok.
Some things I've noticed.

• Before, the file stored at drive:\users<username> the filename begun with a dot ".lmmsrc.xml" so it was a hide file, now it's a normal file (without dot at the beginning) It's not a problem to me and I think that's the usual behaviour for other apps, but I don't know if this is something wanted or a sideeffect.
• As you say in the PR description, there is backwards compatibility so the legacy location is checked. That is ok, but it is stored there as well. I don't know if the config file should be stored in the new location, otherwise, if a person has the old version and change to the new, the lmmsrc.xml location won't change. I don't have a solid opinion on what the correct behaviour should be.

[M374LX]

@superpaik
The absence of the dot at the beginning of the file name is intentional, as most Linux applications that use the .config directory have no dot in their files.

[MRAAGH]

This would be great to see happen! Putting configuration files in the right places seems more professional. I don't know about those additional commits though, does that really belong in the same PR?

[M374LX]

@MRAAGH
The additional commits in the same PR are helpful in preventing code rot and bugs.

[RiedleroD]

what happens if files in both locations exist? I haven't checked, but imo LMMS should either give a notice or parse the newest config.

[MRAAGH]

what happens if files in both locations exist?

Usually you would have a predefined order in which candidate configuration files are checked. When a configuration file is found, no additional paths are checked. So, if both locations exist, one overrides the other. Don't rely on modification time with config files, that's just asking for weird accidents

[M374LX]

what happens if files in both locations exist? I haven't checked, but imo LMMS should either give a notice or parse the newest config.

The legacy location (the user's home directory) is checked first and used if the .lmmsrc.xml file is found there. Otherwise, the new location is checked.

[Spekular]

Wouldn't it be better to prefer the new location, and only use the old one as a fallback?

[M374LX]

492fcac makes the new location preferable, as suggested by @Spekular.

[Spekular]

Is there a reason for all the manual inlining? It makes the diff a lot harder to follow and I can't tell if it's necessary somehow.

[M374LX]

@Spekular No specific reason for the manual inlining. Should methods like initPortableWorkingDir() and initInstalledWorkingDir() be kept?

[Spekular]

Should methods like initPortableWorkingDir() and initInstalledWorkingDir() be kept?

At a glance I'd say it seemed easier to tell what was being done. I would keep them, but since it's a matter of taste I think the second reviewer can decide (serving as tiebreaker).

[M374LX]

b49e64d restores the directory init methods in case the maintainers prefer this way.

[M374LX]

@Spekular I forgot, but the actual reason for the manual inlining is in the description of this PR. It is because initPortableWorkingDir() and initInstalledWorkingDir() are not good names due to the fact that the methods also set the RC file. For this reason, I think b49e64d should be reverted.

[Spekular]

I think initPortablePaths and initInstalledPaths would be fairly descriptive alternatives, perhaps?

[DomClark]

There are a couple of issues here:

• standardLocations isn't guaranteed to return any locations at all, so this may index the result out of bounds.
• We use the same directory both to read and to write the config file. The paths in standardLocations aren't guaranteed to be writable: they may include system-wide config locations, for example.
• In the case of XDG directories, relative paths are considered invalid. However, Qt makes no effort to omit these paths from the result.

I think the easiest way to address these is to use the writable location for configuration files instead. (It's possible for the writable location to be an empty string, or a relative path, in which case I'd just fall back to the legacy location.)

If you want to support reading configuration files from all possible locations, you could consider copying any such file found to the writable location first. You could also include the legacy location as one of these possible locations, which would offer a migration mechanism instead of a fallback mechanism.

[M374LX]

So, should we just fall back to the legacy path if standardLocations returns an empty or unwritable path?

[M374LX]

Knowing of the plans to reduce the dependency of the core on Qt and eventually removing it (as suggested by #6426 and #6472), a possible better idea is to hard code each platform's standard path and use stat() and similar C functions, as well as the Windows equivalents.

Does everyone agree?

[BeatLink]

Are there any updates with this?

Knowing of the plans to reduce the dependency of the core on Qt and eventually removing it (as suggested by #6426 and #6472), a possible better idea is to hard code each platform's standard path and use stat() and similar C functions, as well as the Windows equivalents.

Does everyone agree?

I think it would be a safer bet to use platform agnostic means to determine the configuration folder. Less code maintenance for you guys and better compatibility with non standard OSes, environments and distros. There must be a library out there that handles this for you already.

[Rossmaxx]

Drafted because this is no longer being worked on.