Anonymous | Login | Signup for a new account | 2024-03-29 00:35 UTC |
My View | View Issues | Change Log | Roadmap | Doomseeker Issue Support Ranking | Rules | My Account |
View Issue Details [ Jump to Notes ] | [ Issue History ] [ Print ] | ||||||||
ID | Project | Category | View Status | Date Submitted | Last Update | ||||
0003233 | Doomseeker | [All Projects] Bug | public | 2017-09-01 10:58 | 2018-10-27 22:55 | ||||
Reporter | WubTheCaptain | ||||||||
Assigned To | Zalewa | ||||||||
Priority | normal | Severity | feature | Reproducibility | always | ||||
Status | closed | Resolution | fixed | ||||||
Platform | x86_64 (really cross-platform) | OS | Debian GNU/Linux | OS Version | buster/sid | ||||
Product Version | 1.1 | ||||||||
Target Version | 1.2 | Fixed in Version | 1.2 | ||||||
Summary | 0003233: Add support for the XDG Base Directory Specification and default to it | ||||||||
Description | Doomseeker creates a config directory to /home/<username>/.doomseeker (Note: Not $HOME, username hard-written in config) and sets default Wadseeker and IWAD/PWAD paths to it. To avoid polluting $HOME, the XDG Base Directory Specification should be implemented. User-specific config files should be written to $XDG_CONFIG_HOME (default: $HOME/.config), e.g. $HOME/.config/doomseeker. User-specific Wadseeker and IWAD/PWAD paths should default to $XDG_DATA_HOME (default: $HOME/.local/share), e.g. $HOME/.local/share/doomseeker. | ||||||||
Steps To Reproduce |
| ||||||||
Additional Information | 'https://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html [^]' /home/<username>/ may be hardcoded to many config files, including server browser and server configuration may fail to load if the username is changed. | ||||||||
Attached Files | |||||||||
Relationships | |||||||||||
|
Notes | |
(0018250) Zalewa (developer) 2017-09-09 18:53 edited on: 2017-09-09 18:58 |
If we're about to recognize $HOME as an expression expanding to the user, well, $HOME, should we also allow any env variable to be used? Someone might have $MY_SECRET_DOOM_FOLDER, after all. On Windows env variables are used by wrapping them in percent signs, like this: %HOMEDRIVE%%HOMEPATH%:
Do we consider $HOME a special variable that will expand to $HOME on Linux and %USERPROFILE% on Windows or do we disable this feature for Windows builds? (by this I mean that we should probably be using QStandardPaths) QDir::homePath() documentation presents how the path should be seeked on Windows:'http://doc.qt.io/qt-5/qdir.html#homePath [^]' I haven't checked if Qt will expand '~' automatically - maybe that's the correct venue here? |
(0018381) WubTheCaptain (reporter) 2017-09-25 14:13 |
In example: "If $XDG_CONFIG_HOME is either not set or empty, a default equal to $HOME/.config should be used." It is safe to assume QStandardPaths::HomeLocation or QDir::homePath() to match $HOME. I guess $XDG_CONFIG_HOME may indeed have any other environment variable in it. Stricly speaking this would have to be supported in the specification with my interpretation, but I wouldn't mind partial support emulating only the defaults in $HOME. XDG basedirs don't seem to concern with Windows, it is undefined there and only an Unix-like thing. I'm not familiar how Windows handles configuration, probably the AppData folder. Quote from Zalewa Not automatically, to my awareness. There's also use cases where you may have a path "/home/wub/foo/~bar/baz", which shouldn't expand to /home/bar/baz, but without quotes or escaping it would from a shell expand to /home/bar/baz. The mad men. |
(0018382) WubTheCaptain (reporter) 2017-09-25 14:16 |
Do note the $PATH variable doesn't have $HOME in it, but a hardwritten home path like /home/wub/.local/bin. |
(0018383) WubTheCaptain (reporter) 2017-09-25 14:20 |
Seems like you want to use QStandardPaths::AppConfigLocation. |
(0018384) WubTheCaptain (reporter) 2017-09-25 14:27 |
'https://wiki.debian.org/XDGBaseDirectorySpecification [^]' (see: libqtxdg) |
(0018934) Blzut3 (administrator) 2017-12-03 23:38 |
'https://bitbucket.org/Doomseeker/doomseeker/commits/908ed1778e0a76b21f2be680b30386b6620f10c2 [^]' This is the first (and most impactful) part. Haven't checked behavior on Windows and not sure if we want to go forward with correcting some of the paths there so reassigning to Zalewa. Probably will punt on the variable resolution half of this ticket. |
(0018935) Zalewa (developer) 2017-12-04 17:49 |
I'm going on a 3.5-week long holiday leave in 2 weeks so hopefully I will have some more time to focus on the tickets again. I've been busy with some (positive) personal stuff lately. I'll try to take a look at your changes soon. |
(0018941) WubTheCaptain (reporter) 2017-12-08 18:19 |
I've came here now to say the same as Zalewa. |
(0018945) Zalewa (developer) 2017-12-10 14:27 edited on: 2017-12-10 14:30 |
If we were to migrate paths for Windows, just using the same enums that have just been used for Mac and Linux, we would get these results:
The left path (of "vs.") is the current one, the right path is the QStandardPaths path. I find it quite interesting that AppConfigLocation is actually in Local and not in Roaming, I guess someone at Qt decided that program configuration is workstation-specific rather than profile-specific. In our case I suppose that the Local path is the more correct one, given that we store both workstation-agnostic and workstation-specific information in both config files. I have checked what other programs installed on my PC store in Roaming to see how others do it. ActivePerl is happy to dump 80 Megabytes of "install" dir including a "lib/" subdir full of Perl scripts. Microsoft's Visual Studio Code will dump 110 MB into Roaming, out of which 64 MB is in directories named "Cache" (so, even Microsoft doesn't care??). Daemon Tools Lite has icon cache, even though the directory size is tiny. FiraxisLive (XCOM2 stuff?) stores 10 MB of logs and cache. So, if we want to drop one '#ifdef Q_OS_WINDOWS', which is always a plus, we can migrate Roaming to Local. One thing to remember here is that the current migration routine relies on configDir and dataDir to be different, which they are on Linux and Mac, but on Windows they are the same. Another thing is that before the commit, DataPaths::localDataLocationPath() did actually return a path to AppData/Local, where we used to store update packages and it was also the default storage location for IRC chat logs. |
(0018947) Blzut3 (administrator) 2017-12-10 20:52 |
I'm not sure I'd call our configurations workstation-specific since in an environment where you'd have roaming profiles you'd generally have your software installed in the same places (managed) so the paths should generally work out. That is quite strange that it looks like Qt has AppDataLocation and AppConfigLocation backwards on Windows? That said I can't really argue with reducing OS specific code branches so local only does seem like a pretty good way to go. |
(0018948) Zalewa (developer) 2017-12-10 21:16 |
But your resolution can be different, hence a setting like MainWindowGeometry doesn't really fit in Roaming. To be fair, I think that this is really the only one that really doesn't fit, as you're naturally 100% correct with maintaining same paths on all workstations. |
(0018949) Blzut3 (administrator) 2017-12-10 21:23 |
Fair point, although that makes me wonder if we should be doing anything to validate that the stored geometry is still valid? Even locally the resolution and monitors can change between runs. But that's another ticket if it is an issue. |
(0018950) Zalewa (developer) 2017-12-11 14:56 |
I wondered the same thing some time ago when I experimented with multi-screen setup at work. Qt already does all the necessary sorcery to try to recover from resolution change and will try to move the window into the visible viewport. I just reverified that by move()-ing the MainWindow to x,y = (10000, 10000) and checking that these values are saved in MainWindowGeometry. Upon subsequent run, the window hugs the bottom-right corner of the screen. |
(0018952) Zalewa (developer) 2017-12-11 17:04 |
Windows migration implemented:'https://bitbucket.org/Doomseeker/doomseeker/commits/1f368d48bd757b23ef532da9e125b95581d3f11f [^]' |
(0018953) Blzut3 (administrator) 2017-12-12 04:26 |
Since 0003354 and 0003262 cover the variable resolution and relative path support, I think that completes this ticket. Anything else you wanted to do before marking this needs testing? |
(0018954) Zalewa (developer) 2017-12-12 09:56 |
No, not really. |
(0019424) WubTheCaptain (reporter) 2018-08-27 03:21 |
I think this is mostly correct now, sans $HOME environment variable. Fine for me to close this ticket, I guess. |
This issue is already marked as resolved. If you feel that is not the case, please reopen it and explain why. |
|
Supporters: | No one explicitly supports this issue yet. |
Opponents: | No one explicitly opposes this issue yet. |
Issue History | |||
Date Modified | Username | Field | Change |
2017-09-01 10:58 | WubTheCaptain | New Issue | |
2017-09-01 16:29 | Zalewa | Relationship added | child of 0003246 |
2017-09-06 02:34 | Blzut3 | Assigned To | => Blzut3 |
2017-09-06 02:34 | Blzut3 | Status | new => assigned |
2017-09-06 02:34 | Blzut3 | Target Version | => 1.2 |
2017-09-09 18:53 | Zalewa | Note Added: 0018250 | |
2017-09-09 18:58 | Zalewa | Note Edited: 0018250 | View Revisions |
2017-09-09 18:58 | Zalewa | Note Edited: 0018250 | View Revisions |
2017-09-25 14:13 | WubTheCaptain | Note Added: 0018381 | |
2017-09-25 14:16 | WubTheCaptain | Note Added: 0018382 | |
2017-09-25 14:20 | WubTheCaptain | Note Added: 0018383 | |
2017-09-25 14:27 | WubTheCaptain | Note Added: 0018384 | |
2017-09-27 21:59 | WubTheCaptain | Relationship added | child of 0003279 |
2017-09-27 22:00 | WubTheCaptain | Relationship deleted | child of 0003246 |
2017-10-07 08:55 | WubTheCaptain | Relationship added | related to 0003247 |
2017-12-03 23:38 | Blzut3 | Note Added: 0018934 | |
2017-12-03 23:38 | Blzut3 | Assigned To | Blzut3 => Zalewa |
2017-12-03 23:38 | Blzut3 | Status | assigned => needs review |
2017-12-04 17:49 | Zalewa | Note Added: 0018935 | |
2017-12-08 18:19 | WubTheCaptain | Note Added: 0018941 | |
2017-12-10 14:27 | Zalewa | Note Added: 0018945 | |
2017-12-10 14:30 | Zalewa | Note Edited: 0018945 | View Revisions |
2017-12-10 20:52 | Blzut3 | Note Added: 0018947 | |
2017-12-10 21:16 | Zalewa | Note Added: 0018948 | |
2017-12-10 21:23 | Blzut3 | Note Added: 0018949 | |
2017-12-11 14:56 | Zalewa | Note Added: 0018950 | |
2017-12-11 17:04 | Zalewa | Note Added: 0018952 | |
2017-12-12 04:26 | Blzut3 | Note Added: 0018953 | |
2017-12-12 09:56 | Zalewa | Note Added: 0018954 | |
2017-12-12 09:56 | Zalewa | Status | needs review => needs testing |
2018-08-27 03:21 | WubTheCaptain | Note Added: 0019424 | |
2018-08-27 03:21 | WubTheCaptain | Status | needs testing => resolved |
2018-08-27 03:21 | WubTheCaptain | Fixed in Version | => 1.2 |
2018-08-27 03:21 | WubTheCaptain | Resolution | open => fixed |
2018-10-27 22:55 | WubTheCaptain | Status | resolved => closed |
Copyright © 2000 - 2024 MantisBT Team |