Fix broken paths in default configuraiton on Unix #657

Unrud · 2022-03-03T20:58:03Z

The default paths for FileSearch.Directories and SoundfontSearch.Directories are somewhat broken.

SHARE_DIR is defined as just /usr/local/share/.
The paths …/games/raze are not added to FileSearch.Directories.
GAME_DIR is defined as .config/raze on Unix. Combining it with the prefix …/share/ is wrong.

Excerpt from the default configuration:

[FileSearch.Directories]
…
Path=/usr/local/share/
…

[SoundfontSearch.Directories]
…
Path=/usr/local/share/.config/raze/soundfonts
Path=/usr/local/share/games/.config/raze/soundfonts
Path=/usr/share/.config/raze/soundfonts
Path=/usr/share/games/.config/raze/soundfonts

The default paths for **FileSearch.Directories** and **SoundfontSearch.Directories** are somewhat broken. `SHARE_DIR` is defined as just `/usr/local/share/`. The paths `…/games/raze` are not added to **FileSearch.Directories**. `GAME_DIR` is defined as `.config/raze` on Unix. Combining it with the prefix `…/share/` is wrong. Excerpt from the default configuration: ```ini [FileSearch.Directories] … Path=/usr/local/share/ … [SoundfontSearch.Directories] … Path=/usr/local/share/.config/raze/soundfonts Path=/usr/local/share/games/.config/raze/soundfonts Path=/usr/share/.config/raze/soundfonts Path=/usr/share/games/.config/raze/soundfonts ```

mjr4077au

@Unrud I've added some comments to a few lines. If you don't mind please looking over them and remediating, we'd be appreciative.

mjr4077au · 2022-07-21T11:53:44Z

source/core/gameconfigfile.cpp

@@ -133,7 +133,8 @@ FGameConfigFile::FGameConfigFile ()
 		SetValueForKey ("Path", "$GAMEDIR", true);
 #else
 		SetValueForKey ("Path", "$HOME/" GAME_DIR, true);
-		SetValueForKey ("Path", SHARE_DIR, true);
+		SetValueForKey ("Path", "/usr/share/games/raze", true);
+		SetValueForKey ("Path", "/usr/local/share/games/raze", true);


@Unrud for the paths that start /usr/local/share/, can you please maintain the usage of the SHARE_DIR macro?

mjr4077au · 2022-07-21T11:54:16Z

source/core/gameconfigfile.cpp

@@ -133,7 +133,8 @@ FGameConfigFile::FGameConfigFile ()
 		SetValueForKey ("Path", "$GAMEDIR", true);
 #else
 		SetValueForKey ("Path", "$HOME/" GAME_DIR, true);
-		SetValueForKey ("Path", SHARE_DIR, true);
+		SetValueForKey ("Path", "/usr/share/games/raze", true);


@Unrud instead of hard coding Raze, can you please use the GAMENAMELOWERCASE macro?

I didn't use the macros for consistency They are not used in any of the other sections. IMHO it should be changed everywhere.

mjr4077au · 2022-07-21T11:55:00Z

source/core/gameconfigfile.cpp

-		SetValueForKey("Path", "/usr/local/share/games/" GAME_DIR "/soundfonts", true);
-		SetValueForKey("Path", "/usr/share/" GAME_DIR "/soundfonts", true);
-		SetValueForKey("Path", "/usr/share/games/" GAME_DIR "/soundfonts", true);
+		SetValueForKey("Path", "/usr/share/games/raze/soundfonts", true);


@Unrud can you please maintain the paths that don't have /games/ within?

I left them out because paths without /games/ do not appear in any of the other sections. IMHO they should also be added everywhere.

madame-rachelle · 2022-08-13T21:49:11Z

What are really the problems here and what are the solutions actually trying to accomplish?

To me this looks more like fixing a problem that doesn't exist by breaking things that already work.

Unrud · 2022-08-14T09:38:28Z

What are really the problems here and what are the solutions actually trying to accomplish?

Change paths from something like /usr/share/games/.config/raze/soundfonts to /usr/share//games/raze/soundfonts.

madame-rachelle · 2022-08-14T14:33:27Z

That isn't the right way to do things, at all. If you want to add a default search path it must be done additively, not take away from the ones that already exist. Please ensure to check mjr's comments as well.

Unrud · 2022-08-14T19:27:00Z

If you want to add a default search path it must be done additively, not take away from the ones that already exist.

I'm not following you. The paths I modified/removed were obviously wrong. I doubt anyone uses the folder in /usr/share/.config/... or stored game data directly in /usr/local/share/.

Please ensure to check mjr's comments as well.

👌

madame-rachelle · 2022-08-14T19:41:49Z

I'm no expert on Unix/Linux systems but I do know one thing: standardizing the paths is an absolute a clusterfuck, even moreso with the absolute sheer number of possible distributions and combos available for each system. "Obviously wrong" is entirely subjective. Someone added them, at some point, and it's likely because there was a need for it at that point. So I am going to reject that argument on that basis alone.

Any removals should go through a very thorough review and should not be arbitrarily removed by one person's pull request.

mrdaemon · 2022-08-14T21:11:25Z

If I may contribute some hopefully helpful insight, having an interest in seeing this improved myself, it's not all that subjective in practice. Specifications and conventions do exist and are defined, and most things chose to honour them. Most of them across unix flavors and Linux are additive on top the basic POSIX spec, which doesn't specify much. So while many things can be valid, some can just be outright against convention.

For instance, by convention, which is codified in the File System Hierarchy Standard (FSH), this tidbit is listed:

User specific configuration files for applications are stored in the user's home directory in a file that starts with the '.' character (a "dot file"). If an application needs to create more than one dot file then they should be placed in a subdirectory with a name starting with a '.' character, (a "dot directory"). In this case the configuration files should not start with the '.' character.

And so, the idea is that configuration files placed in the user's $HOME directory should begin with a ., (dotfiles) in order to hide them by default so they don't clutter the listing, among other reasons.

$HOME/.config itself is an extension of this brought forth by the freedesktop XDG basedir specification, which specifies base directory for Desktop Applications to (hopefully) dump their config in, following a somewhat sane structure. .config/* is normally used by convention, or if not defered to the value of environment variables set (XDG_* etc).

Neither of these specs are Linux-specific, or specific to any flavor of Unix either.

/usr/share and /usr/local/share have defined purposes also, not just on Linux but on every Unix system, usually documented either by LSB hier(8), or the platform's hier(8) manpage, but they are, in this case, usually consistent: it's for shared, architecture independent, read only files and data used by applications.

The LSB is somewhat more specific about it, and less useful for general cross platform support, but still indicates the general convention styles expected.

And thus, in any case .config/* belongs in $HOME, and you just wouldn't have a dotfile anywhere else, so /usr/local/share/.config and /usr/local/.config are definitely wrong here. /usr/share/games/raze or /usr/games/raze, however, would probably be alright.

There is leeway in where you can store the game data system-wide, absolutely, but it should not be in a .config directory unless that is in $HOME, essentially.

Hope this helps somewhat!

mjr4077au self-requested a review July 21, 2022 11:57

mjr4077au self-assigned this Jul 21, 2022

mjr4077au reviewed Jul 21, 2022

View reviewed changes

coelckers force-pushed the master branch from a326dfb to ca11711 Compare August 25, 2022 20:22

coelckers force-pushed the master branch from 83f1b44 to 2f896b8 Compare October 12, 2022 20:12

mjr4077au merged commit 8459ee1 into ZDoom:master Oct 20, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix broken paths in default configuraiton on Unix #657

Fix broken paths in default configuraiton on Unix #657

Unrud commented Mar 3, 2022

mjr4077au left a comment

mjr4077au Jul 21, 2022

Unrud Aug 14, 2022

mjr4077au Jul 21, 2022

Unrud Aug 14, 2022

mjr4077au Jul 21, 2022

Unrud Aug 14, 2022

madame-rachelle commented Aug 13, 2022

Unrud commented Aug 14, 2022

madame-rachelle commented Aug 14, 2022 •

edited

Loading

Unrud commented Aug 14, 2022

madame-rachelle commented Aug 14, 2022

mrdaemon commented Aug 14, 2022

Fix broken paths in default configuraiton on Unix #657

Fix broken paths in default configuraiton on Unix #657

Conversation

Unrud commented Mar 3, 2022

mjr4077au left a comment

Choose a reason for hiding this comment

mjr4077au Jul 21, 2022

Choose a reason for hiding this comment

Unrud Aug 14, 2022

Choose a reason for hiding this comment

mjr4077au Jul 21, 2022

Choose a reason for hiding this comment

Unrud Aug 14, 2022

Choose a reason for hiding this comment

mjr4077au Jul 21, 2022

Choose a reason for hiding this comment

Unrud Aug 14, 2022

Choose a reason for hiding this comment

madame-rachelle commented Aug 13, 2022

Unrud commented Aug 14, 2022

madame-rachelle commented Aug 14, 2022 • edited Loading

Unrud commented Aug 14, 2022

madame-rachelle commented Aug 14, 2022

mrdaemon commented Aug 14, 2022

madame-rachelle commented Aug 14, 2022 •

edited

Loading