Client settings are not stored in a user writable location
Opened this issue · 13 comments
I was packaging frostbite for a linux distribution, but ran into a blocking issue due to the way that client settings are hardcoded to be written to client.ini inside the application dir path (i.e. the directory where the executable is).
When a user downloads and extracts a release tar/zip/archive locally and runs the binary from that dir, that works fine. However, if one wants to install the application to the system, this presents an issue. On unixes, this simply doesn't work without running the application as root. On windows, I believe you'd similarly have to run the application as an admin.
If you want to allow people to distribute and package the software to be installed on users' systems, for client.ini, consider using AppDataLocation instead of applicationDirPath, which you already do a few lines down for ClientSettings::profilePath() (or perhaps something else? I'm not that familiar with qt best practices).
frostbite/gui/clientsettings.cpp
Line 10 in 0091853
This is all correct but i'm not sure what do you mean by installing to the system? There are no installers for any of the distributions. It's as is, extract and run only. You need all of it to be there, not just client.ini.
You can find the release packaging scripts in the gui.pro file. All it does is pick up the libs and plugins, then tar/gz it all up and that's about it.
https://github.com/matoom/frostbite/blob/master/gui/gui.pro#L227
They recommend using scripts to link libs for linux but there are ways around that.
https://doc.qt.io/qt-5/linux-deployment.html#creating-the-application-package
I'm not using static linking because that requires to rebuild the entire qt using static flags, i think it's just better to package everything with the release. As you can see this approach does not require any elevated privileges (The disadvantage of the first approach is that the user must have super user privileges).
The QApplication::applicationDirPath() actually allows you to run it outside the installed location (where you extracted it) with no elevated privileges required.
i'm not sure what do you mean by installing to the system? There are no installers for any of the distributions.
Right, because nobody can package it. I was trying to do so when I ran into this issue. :)
I was packaging frostbite for a linux distribution
To clarify, this issue is to address an issue blocking packaging of this software for linux distributions.
If you have no intention of allowing this to occur, feel free to close the issue.
I'm not sure if it can be done both ways but i need to look into this more closely to understand how the packaging process actually works.
I'm not sure if it can be done both ways but i need to look into this more closely to understand how the packaging process actually works.
By packaging it for linux, one compiles it and puts it into the filesystem hierarchy essentially. How exactly one does that varies drastically based on the distro / package manager.
To simply things, for frostbite it would would look something like
/usr/bin/Frostbite
/usr/share/frostbite/(everything else)
In Qt, /usr/share/frostbite corresponds to the non-writeable AppDataLocation, which I mentioned in my first post. You're already using the writeable AppDataLocation for some things as mentioned above.
I'm not sure if it can be done both ways
The two common ways to do this are a compile time flag or runtime detection.
The compile time flag would switch between applicationDirPath (the current behavior and what you'd want when making releases as they are done now) and AppDataLocation (a standard build that you can qmake, make, and make install).
Runtime detection would first check applicationDirPath and fallback to AppDataLocation.
After using Frostbite for a bit, I've noticed that client settings are saved in the config files (e.g. client.ini) that are bundled with the binary, rather than the more traditional approach of saving to the user's home directory (i.e. writeable AppDataLocation in Qt terminology).
Qt may very well have a standard solution to handling user configs, but generally how applications handle this is to either 1) read from writeable data location if it exists, otherwise fallback to non-writeable, and always write to the writeable location (of course), or 2) copy the non-writeable data location contents to the writeable one if it is empty, then always use the writeable location after that.
Ok, well right now it's consistent between all platforms with no installers to keep it simple for me to distribute and maintaining the ability to run anywhere (even if user has no install privileges on the system). The standardized qt deployers make everything a lot easier on windows and osx while it's true that linux doesn't have one. For the most part i want to keep it that way. For this to work as you described there needs to be a lot more changes than just moving the client.ini but i think it's technically still viable for packaging.
I did some searching around and i would say user/bin is probably not the right fit for this type of software.
On the other hand, /opt is a directory for installing unbundled packages (i.e. packages not part of the Operating System distribution, but provided by an independent source), each one in its own subdirectory. They are already built whole packages provided by an independent third party software distributor.
https://unix.stackexchange.com/questions/11544/what-is-the-difference-between-opt-and-usr-local
As a basic proof of concept for the package:
- /frostbite/DEBIAN
- /frostbite/opt/frostbite
~/Desktop/frostbite$ ls
DEBIAN opt
~/Desktop/frostbite/DEBIAN$ ls
control postinst preinst prerm
~/Desktop/frostbite/DEBIAN$ cat postinst
sudo ln -s /opt/frostbite/Frostbite /usr/bin/frostbite
exit 0
~/Desktop/frostbite/DEBIAN$ cat prerm
rm /usr/bin/frostbite
exit 0
~/Desktop/frostbite/opt/frostbite$ ls
api.ini Frostbite log.ini maps profiles scripts sounds
client.ini lib logs plugins qt.conf snapshots
~/Desktop$ dpkg-deb --build frostbite
This gets you the frostbite.deb file that can be installed using the package manager.
I was able to run without any super user privileges.
The installation scripts probably need to be more fleshed out but this should more or less work?
Ok, well right now it's consistent between all platforms with no installers to keep it simple for me to distribute and maintaining the ability to run anywhere (even if user has no install privileges on the system).
There is no need to remove this. That isn't necessary to address this issue. Either the compile-time or runtime approaches in #61 (comment) would allow both.
Also, if it wasn't clear, I'm not asking you to maintain a package in any package manager repos. Addressing this issue would allow me and others to do that. I like to package software I use so that others can easily install/update said software via their package manager (if someone hasn't done it already).
If it helps clarify, in terms of Qt concepts, the issue fundamentally boils down to following the semantics of writeable and non-writeable AppDataLocation and applicationDirPath (see https://doc.qt.io/qt-5/qstandardpaths.html ). https://doc.qt.io/qt-5/qmake-tutorial.html might also help explain how qmake, make, and make install interact to install software to a system.
I did some searching around and i would say user/bin is probably not the right fit for this type of software.
re: /opt, /opt is for pre-packaged software. Think mozilla-branded firefox (not firefox you compile yourself), discord (you download their binary), chrome (not chromium), slack, unity editor, etc.
The way you currently distribute frostbite is in a sense in line with them, yes; however, note how every one of them still writes user config data into some user-writeable location in $HOME. That's why they can be installed in /opt, which is not user-writeable. In your deb example, when the user goes to save config settings, the user will not have permissions to modify /opt/frostbite/client.ini, and if he did somehow, a second user could come by and overwrite those same settings.
That said, it's very unusual to see open source software that's unencumbered by binary redistribution limitations bundled like this rather than using the traditional way to install binaries (into /usr/bin generally. It varies between distros) and its data files (into /usr/share generally. It varies between distros) via make and make install. It could work if not for issue #61.
To reiterate what I said before, if you don't want to enable others to package the software differently and only want to release the software as you already do, that's fine. My sense is that the user experience is worse to download an archive from a website / github releases, extract it, cd, run it from there, & then copy some (all?) ini files when they want to update to a new version (maybe other files too? what about my custom maps and scripts?). It's a non-standard approach that tends to be more work for end users than a simple apt-get install frostbite and automatically saving config files, custom scripts, etc in $HOME. I don't want to come across like this is a high priority issue, as I don't expect a large number of your users are going to even switch over at this point. Plus, as you said, you already have an existing way of getting the software into users' hands (and the dr userbase probably won't grow in size considerably).
As i was saying it's not changing the config paths that concerns me as much as everything else. There's literally a dialog in the client that lets you put config files into the application path as an intended feature. There's a locking mechanism that limits to one process per installation. There's some work done towards allowing full binary distributions that i can't recall how it exactly functions but maybe it won't be a problem. etc.
I don't exactly know if anything will break or what future changes could result in failure but right now i don't think anything catastrophic will happen. Pretty much you can just "not press that button" and it will be fine. It might work as a half measure hack.
If you only want to relocate the client.ini i think that would work as a runtime check but if it's everything then it has to be a compile time decision. Is client.ini sufficient or change all paths?
If you only want to relocate the client.ini i think that would work as a runtime check but if it's everything then it has to be a compile time decision. Is client.ini sufficient or change all paths?
No, I don't think client.ini alone would be sufficient. The issue applies to all writeable application data. client.ini was the first obvious example that jumped out to me. I probably should have looked closer at the code before opening the issue, as there does appear to be more (some of which you just explained above e.g. locking mechanism). Examples would be scripts dir, log dir, maps dir. Any writeable application data falls into this category. That's what I was trying to capture here:
If it helps clarify, in terms of Qt concepts, the issue fundamentally boils down to following the semantics of writeable and non-writeable AppDataLocation and applicationDirPath (see https://doc.qt.io/qt-5/qstandardpaths.html ).
I suspect 80% of solving this is simply using Qt's writeable AppDataLocation instead of applicationDirPath for reading/writing app data. Thankfully, you have that well encapsulated into ClientSettings::ClientSettings(), so I'm expecting fixing the majority of this issue lies in using the writeable AppDataLocation within there.
I might look into fixing this myself in the next couple weeks, if I'm still playing DR. I'll start hacking away at the client sooner or later I continue using it. I'm already building up a list of other features I might send as PRs, e.g. assess window, ways to make the UI a little more minimal, adjusting some of the bars/indicators so they are more readable/functional. :)
Feel free to make changes but if it's a PR i want to review it. I'm doing things a certain way and there are some design decisions based on my own personal preference or about keeping things simple/maintainable. The server-side protocol is a mess and it's sometimes just about working around bugs or plain broken mechanics. As there's no official specs, it's all reverse engineered from what i've been able to gather over the years and lets just say there's a reason for a genie v3 (i'm locked into some of the early design decisions).
The assess window is a good example of what i don't want to do. It's not supported by the protocol and i don't really do client side scripting or business data processing (adds a lot of complexity, needs to be isolated from ui threads etc). The client side is mostly about presenting only and the rest is all delegated to scripting, whatever that may be. The native scripting api and lich both support making custom windows and can be used exactly for this purpose.
I recently gave a simple example for both that can be found here: #59 (comment)
Quite frankly i don't know why you would want to add the assess window in the first place? Multi-opponent hasn't been a thing for at least 5 years now, it's kind of useless now and only adds more clutter.
Thanks for that info about your design direction. Yea, anything I'd change and want upstreamed I'd put in a fork, and then send a pull request for you to review.
Quite frankly i don't know why you would want to add the assess window in the first place?
I figured it would help me keep track when things get very fast/chaotic during events like that one the other day. I was effectively blind during it all. Although, given your criticism of the feature, I suspect there are better ways to solve this that I just haven't discovered.
Anyhow, thanks for that custom window example link. That should do nicely.
I figured it would help me keep track when things get very fast/chaotic during events like that one the other day.
What bothers me is that there's no way to position yourself to fight only one mob given that retreat has a pretty hefty penalty now. That leaves you with no options no matter what kind of bearings assess is giving you and really assess doesn't tell you any more than "begins advancing you" does. If you're edging between engagement and you only want to fight a certain number of mobs there's not much you can do about it. At least in my experience you should be prepared to tank four or avoid engagement altogether. Maybe i'm missing something but the whole assess mechanic seems like a relic of past times when combat had different rules.
I don't want to say that it's completely useless but i don't see it as absolutely essential either. It would make sense the way genie handles it as an addon or otherwise you would need to use regular expressions to find assesses from all text and that would have to be hard-coded into the core application. This is why i prefer the scripting approach.
I guess in short as you can't position yourself using retreat, "begins to advance" is where i make my decisions about engagement.