Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for the XDG base directory specification #336

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mcz
Copy link

@mcz mcz commented Sep 10, 2019

Uses "$XDG_CONFIG_DIR"/ezquake instead of "$HOME"/.ezquake if $XDG_CONFIG_DIR is set.

@tcsabina
Copy link
Collaborator

tcsabina commented Oct 7, 2022

Hello @mcz ,

If you still out there and willing to resolve the merge conflicts with this PR, it could be merge.

Thanks,
Toma

@mcz
Copy link
Author

mcz commented Oct 7, 2022

Might take me some time because the original code was kinda half-assed, and I'd like to do it correctly this time.

com_homedir is used for many things, while $XDG_CONFIG_HOME, should be used only for configuration. The rest belongs in $XDG_DATA_HOME. For that reason I think it would make more sense for Sys_HomeDirectory to return XDG_DATA_HOME if set, meaning com_homedir = $XDG_DATA_HOME/ezquake. The fallback would still be $HOME/.ezquake in order to not break existing setups.
Additionally, config_manager.c should be edited so that it uses $XDG_CONFIG_HOME/ezquake if possible and $HOME/.ezquake as a fallback. Also, the functionallity in Cfg_GetConfigPath() is duplicated in LoadConfig_f() and, curiously, in MOpt_ImportConfig() in menu_options.c, so I think those shoud l be changed to just use Cfg_GetConfigPath() instead.

I also noticed some places using com_homedir without checking if it is set or not first. That means the following functions act as if com_homedir was the root directory if -nohome is set:

  • SB_Serverlist_Serialize_f()
  • SB_Serverlist_Serialize_f() (only tries to read)
  • MT_TempDirectory()
  • SB_PingTree_ScanProxies()
  • SB_PingTree_ScanProxies() (only tries to read)
  • FS_SaveGameDirectory()
  • FS_LegacyDir() (the documentation says, that if cl_mediaroot == 1, paths get interpreted as relative to the user home directory. I think this function should use $HOME (or CSIDL_PERSONAL on win32) instead of com_homedir, similar to how FS_AddUserDirectory() works with userdir_type == 5
  • The 3 functions getting the location of the config file(s) (will be changed anyway)

I can't imagine this being the intentional behaviour, but since I can't find anything documenting how that command line argument is supposed to work, I can't be sure. I think that those should be changed to one of 3 things:

  1. check if com_homedir exists before using it. The problem is that some of the functions don't have a fallback.
  2. set com_homedir to be equal to com_basedir if it is still unset at the end of FS_InitFilesystemEx(). That way, the fallbacks that do exists could be removed.
  3. like 2., but use the current direcotry instead

Finally, Sys_HomeDirectory() is also used in Sys_RegisterQWURLProtocol_f() to write a .desktop file. According to the specification:

If an application is intended to be installed by an unprivileged user for exclusive use by that user only then $XDG_DATA_HOME should be used as value for datadir [...]. If $XDG_DATA_HOME is not set, the default value of $HOME/.local/share should be used for it.

So just blindly using $HOME/.local/share in Sys_RegisterQWURLProtocol_f() is wrong.

Would be great if you could comment on the ideas I've described (especially when it comes to the -nohome stuff), so I don't spend time changing something that's working as intended.

@tcsabina
Copy link
Collaborator

Hi @mcz ,

Thanks for the detailed reply!

The truth of the matter is that we just recently got the ownership of this repo. None of the main developers are active at this moment, so we don't really have a serious direction. Our main goal currently is just to make the contributions get going on again, and to solve serious bugs.
So this means that if you have a clear vision about how the directory structure should look like, and you are willing to work on it and contribute, that probably won't be in conflict with anything or anyone.

What do you think?

@mcz
Copy link
Author

mcz commented Oct 25, 2022

if you have a clear vision about how the directory structure should look like

I have a more or less good idea of what the directory structure should look like when it comes to GNU/Linux (and other platform inplementing the freedesktiop.org standards).

I have basically no idea how things should look like on other platform (WIndows, MacOS, ...) and if there are legacy aspects we should respect. For that I think it would be great to have some input from someone who is familliar with the history of (ez)quake. It's hard to know if the current behaviour is intentional when there is no documentation and the relevant commit message (1562985) doesn't even mention it.

you are willing to work on it and contribute

Definitely. It will probably take a week or so.

@tcsabina
Copy link
Collaborator

Is the directory layout handling not already separated between windows and linux? Are these not handled in sys_posix.c and sys_win.c source files?
If they are separate and guarded by ifdefs, theoretically you can rework the linux part only, while leaving the windows part intact. Not ideal, but at least it would be a start...

I think it will be impossible to extract the "idea" behind the original authors about directory layout. What can be done is to check how is it done currently in a windows environment. But for detailed and correct information I think we should dive deep into the directory handling of windows, including elevated rights/access, virtual store (if that is still a thing on Win10/11) and all that stuff. Combined with the (legacy?) concept of a QuakeWorld client's file- and directory dependencies.
Well, this doesn't trigger me that much, to be honest :D.

@mcz
Copy link
Author

mcz commented Oct 25, 2022

The way they are separated is by having different methods of getting com_basedir and com_homedir (and only com_homedir is set usiing sys_posix.c or sys_win.c. com_basedir is done with an #ifdef)
Those variables are then used in the same way, regardless of what OS is being used, which means that the thing about not checking whether com_homedir is set before using it applies to Windows as well. And since what I'm planning to do is split config and data, i.e have configmanager.c not use com_homedir, but rather a separate location, there has to be a Windows equivalent of that location.
And then there are the other posix platforms (mac and bsd) that might do things even differently.

you can rework the linux part only, while leaving the windows part intact.

I'll have to add some separation in getting the config dir, but it shouldn't be too hard.

Fall back on $HOME/.local/share, if necessary. Return "\0" on fail, just
like the WIndows version.

Use system("mkdir -p") to crate the directory structure in
Sys_RegisterQWURLProtocol_f()
Fixes a bug where having an executable called for example
'/path/to/quake/weird\name' (with a backslash in the filename) would lead
to FS_InitFilesystemEx(true) using '/path/to/quake/test' as com_basedir
instead of '/path/to/quake'.
AddUserDirectory should still use $HOME. Also change the windows
Sys_HomeDirectory to return a pointer to a malloc-ed string that
has to be freed so that it watches the posix version.
@mcz mcz force-pushed the XDG-base-dir-spec branch 2 times, most recently from 3faf681 to 0c606e8 Compare October 29, 2022 01:08
Fallback to com_gamedir. Also detriplicate code for finding the path of
the config file.
Change Signature for Sys_HomeDirectory since it's no longer const.
Fix some mistakes from previous commits.
"Fix" for functions using com_homedir not checking whether it actually
contains a path, leading to paths that should be relative to com_homedir
actually being absolute.
Also remove the now unnecessary checks that did exist in some functions.
@mcz
Copy link
Author

mcz commented Oct 29, 2022

The branch is currently in a state where it accomplishes what I set out to do.
That's however only the user-specific stuff. System-wide settings are still a mess. The problem is historical: The whole file structure is made with the idea in mind that a single user puts a single executable in a directory somewhere and all the files that executable needs are also in that same directory.
That idea just doesn't work with 1) multi-user systems (admttedly rare), 2) systems that split executables and data (like unix), 3) package managers (wanting your users to write their .pak files to /usr, or c:/Program Files/ for that matter, is asking for trouble).

That's why I propose to do a bit of an overhaul of the whole file handling system, including the following points:

  • add an install: to the Makefile, with configurable PREFIX, BINDIR, DATADIR, SYSCONFDIR and DOCDIR (I think those are all the types of files that are needed)
  • search for data in the DATADIR configured at compile time and for configs in SYSCONFDIR (with the user directories having precedence). this means that .pak files will also be searched for in com_homedir

What I still need to figure out is if and how to still leave the option for the current one-directory setup.

@ciscon
Copy link
Collaborator

ciscon commented Oct 29, 2022

this is what debian accomplishes by using -basedir, though it is quite annoying as classically we do just throw a binary into a quake directory and you're done, or symlink it. it could test pwd first and it would accomplish both things.

@mcz
Copy link
Author

mcz commented Oct 29, 2022 via email

@ciscon
Copy link
Collaborator

ciscon commented Oct 29, 2022

you can just put everything in ~/.config/ezquake, or you can put media in /usr/games/quake and use cfg_use_home to save things to ~/.ezquake. that said, i personally have no problem with updating to use xdg directories for things so long as you can absolutely still just use a normal quake directory for everything.

@tcsabina
Copy link
Collaborator

tcsabina commented Nov 1, 2022

@mcz ,
nice work, so far! keep it up!

Could we summarize what does this PR actually contain at this moment? Do we need to create a small how-to for the linux users who are willing to test these changes?

@mcz
Copy link
Author

mcz commented Nov 1, 2022

  • Uses $XDG_DATA_HOME/ezquake (fallback on $HOME/.local/share/ezquake) for everthing that used to use $HOME/.ezquake, except for config files, which go to $XDG_CONFIG_HOME/ezquake (fallback on $HOME/.ezquake)
  • uses com_basedir instead as a fallback when com_homedir isn't set
  • simplifies finding the config file location
  • removes the hardcoded Sys_RegisterQWURLProtocol_f()
  • fixes a bug where a backslash in the executable name would break getting com_basedir on non-win32 systems

I wouldn't release this to testers quite yet, since there are still things I would like to fix. One big thing I noticed is that when working with paths, while functions like strlcat are used to avoid buffer overflows, there are no checks to see if all the data was actually written to *dest. So in the case of very long paths, it is possible that there might be writes to unexpected locations.

And the other thing I would like to add (though that should probably be a separate PR) is differenciating between the directories with the executable and the one with the data (and possibly a third with configs, so systemwide configs can go to /etc). While the xdg base dir thing is for users, this other change would be mainly for packagers.

@tcsabina
Copy link
Collaborator

Hi @mcz,

Give me a status update, please ;)

@mcz
Copy link
Author

mcz commented Dec 15, 2022

I'm sorry. I've been really busy in real life, so I didn't have much time to work on this. Technically the current status of the PR has user-level file management in a good state, while all the system-level stuff should probably be separate.

@tcsabina
Copy link
Collaborator

HI @mcz ,

No worries, no need to apologize ;).
Now, should I merge this? Is this in a state that is good enough as a 1st step?
Could you maybe make a short description/readme about what was actually changed? It will be required in the Release Notes anyhow at some point...

@tcsabina
Copy link
Collaborator

tcsabina commented Jul 3, 2023

Hi @mcz ,

I was wondering about the status of this PR...

There is another PR (#791), which also tackles the XDG_DATA_HOME location. Any comment about that?

@ginzberg
Copy link
Contributor

I've become somewhat interested in this feature. I come from the classic "throw it all in a directory" days, but with the recent maturity of the ezQuake.AppImage builds, I have a grown interest in being able to drop that in ~/.local/bin and being done with it. Being able to use XDG_CONFIG_HOME and XDG_DATA_HOME is super graceful, and I'd love to begin using that. Combination of this structure, the AppImage builds, and a sensible ezQuake.desktop entry makes for a fairly portable and consistent system that could be easily managed across all the distributions, package managers, desktop environments, etc.

@mcz
On the topic of systemwide stuff, I think there's more than just the configs side of it. There's also data that can be considered systemwide (example: id1/pak0.pak, id1/pak1.pak) and data that is clearly user (e.g. screenshots, demos).

Unfortunately, it's not always that clear. There is the messy middle, such as different custom assets in the form of pk3 files or exploded directories. Are those updated ztndm3 textures a user preference or a systemwide configuration? Feels like a user preference to me, but perhaps the QRP high def textures are considered more of a system asset?

I think a potentially interesting approach to that last bucket would be to allow pak.lst to be a configuration that is saved at the user level, while all the pk3 files available to the user could be at the system data level (and/or user level). This would not solve for the exploded directories cases, however.

@ciscon
Copy link
Collaborator

ciscon commented Nov 23, 2023

If you try the latest snapshot of the appimage it should just use wherever you threw it as a quake directory if where you run it from doesn't contain id1, so it pretty much accomplishes the goal. As far as I'm concerned it's still always a bit convoluted/complex to distribute junk around home directory junk instead of just always using a single portable directory for everything, you always end up with some sort of search/save priority and cvars you have to tweak etc etc, making it support both for various things is just a pain and I don't think anybody wants to get rid of just being able to have a quake directory.

@ginzberg
Copy link
Contributor

ginzberg commented Nov 23, 2023 via email

@ciscon
Copy link
Collaborator

ciscon commented Nov 23, 2023

I'm not finding that to be the experience. Latest AppImage in ~ id1 folder within ~/.ezquake ~ ❯ ./ezQuake-x86_64.AppImage executing with native libc Error: Couldn't load gfx/palette.lmp This is typically caused by being unable to locate pak0.pak. Copy pak0.pak into 'id1' folder, in same directory as executable. ~ ❯ tree .ezquake .ezquake ├── id1 │ ├── pak0.pak │ └── pak1.pak └── temp 3 directories, 2 files

On Thu, Nov 23, 2023 at 9:37 AM dustin @.> wrote: If you try the latest snapshot of the appimage it should just use wherever you threw it as a quake directory if where you run it from doesn't contain id1, so it pretty much accomplishes the goal. As far as I'm concerned it's still always a bit convoluted/complex to distribute junk around home directory junk instead of just always using a single portable directory for everything, you always end up with some sort of search/save priority and cvars you have to tweak etc etc, making it support both for various things is just a pain and I don't think anybody wants to get rid of just being able to have a quake directory. — Reply to this email directly, view it on GitHub <#336 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAF5JTDHAVQLKJ5NJYRFQWTYF6CVDAVCNFSM4IVJ2KJ2U5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TCOBSGQ3TMNRVHE3Q . You are receiving this because you commented.Message ID: @.>

(edit: i see you were running it from home, that's not what i meant, it uses the directory the appimage exists in as the basedir as a fallback to cwd)
this is running it from /tmp:

0 | ciscon@herpes10 | /tmp 
% ~/.ezquake/ezQuake-x86_64.AppImage 
executing with native libc
0 | ciscon@herpes10 | /tmp 
% find ~/.ezquake
/home/ciscon/.ezquake
/home/ciscon/.ezquake/id1
/home/ciscon/.ezquake/id1/pak1.pak
/home/ciscon/.ezquake/id1/pak0.pak
/home/ciscon/.ezquake/qw
/home/ciscon/.ezquake/ezQuake-x86_64.AppImage
/home/ciscon/.ezquake/ezquake
/home/ciscon/.ezquake/ezquake/temp
/home/ciscon/.ezquake/ezquake/.ezquake_history

fyi you've always been able to do this if you really wanted, debian's version does something similar by wrapping it in a shell script and just setting basedir and/or symlinking stuff from /usr/share|/usr/games into homedir stuff. again i find all of this a bit ridiculous though, spreading out configs/media/resources amongst multiple homedir directories just over complicates things, especially for something like quake in which we do actually manually edit configs/resources unlike newer stuff where that's the exception and most things are done through menus etc. there will always be times where you set something in a config (or even media) and because that thing is now set in some other directory and is part of the search path, it'll get pulled in even if you're not expecting it to (say you've changed cfg_use_home) because on startup it doesn't know what that's set to yet, and if you've changed it to 0, you're still parsing your home dir config on startup, which may set it back to 1 etc etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants