New directory structure

[Roboron]

An interesting way out (apart from throwing everything in the user directories), would be the creation of a simutrans group, and creating a group writeable folder under /var/lib/simutrans during installation. That would neet root access only once. simutrans could ask the user to start a script creating /var/lib/simutrans and the group if it does not exist and otherwise use "~/simutrans/paksets" for install directory. Simutrans may the use setgid() to change to access this and back.

From the FHS standard:

5.7. /var/games : Variable game data (optional)
5.7.1. Purpose

Any variable data relating to games in /usr should be placed here. /var/games should hold the variable data previously found in /usr; static data, such as help text, level descriptions, and so on, must remain elsewhere, such as /usr/share/games.
Rationale

/var/games has been given a hierarchy of its own, rather than leaving it underneath /var/lib as in release 1.2 of this standard. The separation allows local control of backup strategies, permissions, and disk usage, as well as allowing inter-host sharing and reducing clutter in /var/lib. Additionally, /var/games is the path traditionally used by BSD.

There's also a "games" group, which is present in every Linux install I've tested (but with no users added), which usually have permissions over /var/games (but this folder is not always created, I've found it in Manjaro with correct permissions but not in Ubuntu/Debian). So if Simutrans were to use a global directory for paksets in linux, this is probably the one it should use.

Still, the problem persist that root access is required once to add users to games group and create the directory with correct permissions if it is not already created.

Some games I've found using this directory are GNOME games (for statistics, but they no longer use /var/games but the home directory instead https://sources.debian.org/src/gnome-mahjongg/1%3A3.38.3-1/debian/gnome-mahjongg.postinst/ ) and Minetest / MineOS (for game server administration, I suppose).

[prissi]

Somehow Simutrans as distrocontribution should be able to become a member. Not sure how to enforce this.

The code itself is simple: https://www.gnu.org/software/libc/manual/html_node/Setuid-Program-Example.html

On debian there is only root access there it seems. Simutrans is also owned by root, without setguid set.

[R1dO]

Browsing through this topic it seems that there is a lot of effort on trying to circumvent the security measures of modern linux systems that are in place to prevent arbitrary apps from writing to system folders.

I am wondering if that is really worth the effort?
Using the rationale that putting data in system folders is normally an administrator problem, they are the ones to decide if it is worth to prevent duplication by moving data to a shared location.
Be it:
* The maintainer for a distribution (e.g. simutrans from a package manager installed under "/usr/share" or "/usr/games")
* The local user with sudo credentials (e.g. simutrans install under "/usr/local/.." or "/opt")
* An application bundle (snap, appimage, flatpack).
   Their sandboxing model will make it even more difficult to use shared system folders (if possible at all).

My take on this is that it should be enough for simutrans to recognize some basic path hierarchy (be it data, config, cache or saves) and only deal with the problem what to do when there are duplicates within those paths (using a hypothetical example of pak64v1 under "/usr/share" and pak64v3 under "~/simutrans").
And to make life easier for the administrators, the ability to specify where to put data/config/binary when doing a compile+install or as startup parameter. But this already (mostly?) exist in the current codebase.


Having said that,
If there still is the desire to put files under a system folder there is another option that might be worth exploring.
That is: if it is OK to have a dependency on GVFS (https://en.wikipedia.org/wiki/GVfs & https://wiki.gnome.org/Projects/gvfs).
Within GVFS there exist an URI scheme for admin access, when using this scheme the system should ask the user for admin credentials when trying to access (write to) the specified folder and subfolders, for a limited time and only to those folders.
Usage is simple, given an absolute path you replace the first slash with the scheme (e.g. "/usr/local/share" becomes "admin:///usr/local/share"), integrating it in the codebase might be a bit more difficult though.

[Roboron]

You are right in that we should delegate this problematic to maintainers and system administrators. It is enough that we provide such option. Let the default be the home, that will work in any situation, and move on.

Thank you for your input :-)

In other news, I'm getting the following trying to run simutrans in the same directory as the installation is:

Code Select Expand

Install dir /home/rober/simutrans/simutrans/
Base dir /home/rober/Archivos/Documentos/simutrans/devel/simutrans-svn/trunk/build/simutrans/
dr_fatal_notify: ERROR: The file 'ground.Outside.pak' was not found in
'/home/rober/simutrans/addons/pak64pak64/'

[prissi]

This happens when using addons, and has nothing to do with the place of installation. The PATH_SEPARATOR was simply not appended. Fixed in r10570

[Roboron]

I started simutrans in the build dir. But I have no addons folder. Not there, and not on the user home either. So no idea why it is looking in a non-existent directory and with a duplicated pakset name.

[prissi]

Please try again r10570 or show the last lines of the logfile. I am confused how this can happen.

EDIT: maybe it finds the addon folder only. But that should not be recognized as pakset at all.

[Roboron]

I can give you the full log, but I don't find anything more of value there:

Code Select Expand

Simutrans version 123.0.2 Nightly from Mar 20 2022 r10570
Warning: loadsave_t::rd_open: File 'settings.xml' does not exist or is not accessible
Message: simu_main(): Parsing /home/rober/Archivos/Documentos/simutrans/devel/simutrans-svn/trunk/build/simutrans/config/simuconf.tab
Message: simu_main(): Version:     123.0.2 Nightly  Date: Mar 20 2022
Message: simu_main(): Debuglevel:  4
Message: simu_main(): base_dir:    /home/rober/Archivos/Documentos/simutrans/devel/simutrans-svn/trunk/build/simutrans/
Message: simu_main(): install_dir: /home/rober/simutrans/simutrans/
Message: simu_main(): user_dir:    /home/rober/simutrans/
Message: simu_main(): locale:      es
Message: dr_os_init(SDL2): SDL Driver: x11
Message: SDL_StartTextInput:
Message: dr_query_screen_resolution(SDL2): screen resolution width=1920, height=1080
Message: simu_main(): simgraph_init disp_width=704, disp_height=560, fullscreen=0
Message: dr_query_screen_resolution(SDL2): screen resolution width=1920, height=1080
Message: dr_os_open(): Screen requested 704,560, available max 1920,1080
Message: my_event_filter: 512
Message: internal_create_surfaces(SDL2): Renderer: opengl, Max_w: 0, Max_h: 0, Flags: 14, Formats: 4, SDL_PIXELFORMAT_ARGB8888, SDL_PIXELFORMAT_ABGR8888, SDL_PIXELFORMAT_RGB888, SDL_PIXELFORMAT_BGR888
Message: internal_create_surfaces(SDL2): Renderer: opengles2, Max_w: 0, Max_h: 0, Flags: 14, Formats: 4, SDL_PIXELFORMAT_ARGB8888, SDL_PIXELFORMAT_ABGR8888, SDL_PIXELFORMAT_RGB888, SDL_PIXELFORMAT_BGR888
Message: internal_create_surfaces(SDL2): Renderer: software, Max_w: 0, Max_h: 0, Flags: 13, Formats: 8, SDL_PIXELFORMAT_ARGB8888, SDL_PIXELFORMAT_ABGR8888, SDL_PIXELFORMAT_RGBA8888, SDL_PIXELFORMAT_BGRA8888, SDL_PIXELFORMAT_RGB888, SDL_PIXELFORMAT_BGR888, SDL_PIXELFORMAT_RGB565, SDL_PIXELFORMAT_RGB555
Message: my_event_filter: 512
Message: my_event_filter: 512
Message: internal_create_surfaces(SDL2): Using: Renderer: opengl, Max_w: 16384, Max_h: 16384, Flags: 10, Formats: 8, SDL_PIXELFORMAT_RGB565
Message: dr_os_open(SDL2): SDL realized screen size width=704, height=560 (internal w=704, h=560)
Message: font_t::load_from_fnt: Loading font 'font/prop.fnt'
Message: font_t::load_from_fnt: font/prop.fnt successfully loaded as old format prop font!
Message: simu_main(): .. results in disp_width=704, disp_height=560
Message: simu_main(): Loading colours from /home/rober/Archivos/Documentos/simutrans/devel/simutrans-svn/trunk/build/simutrans/config/simuconf.tab
Message: obj_reader_t::read_file(): filename='modern.pak'
Message: obj_reader_t::read_file(): read 1 blocks, file version is 3eb
dr_fatal_notify: ERROR: The file 'ground.Outside.pak' was not found in
'/home/rober/simutrans/addons/pak64pak64/'.
This file is required for a valid pak set (graphics).
Please install and select a valid pak set.
Message: my_event_filter: 512

[prissi]

There was a separator missing for paks in user path. EDIT: r10575 fixed it for me.

[Roboron]

I'm still getting exactly the same error in r10575.

[prissi]

I tested it in Debian on a virtual machine. It needs a get_pak.sh file in the simutrans directory (and the right installer path, forget to submit that). But it works well for me now.

[Roboron]

Clean build, clean install, installed pak64 with the pakset installer to the correct directory, still the same error. r10583

EDIT: It works when there is more than one pakset installed and I have to select one. It doesn't work when there is only one pakset.

EDIT2: Ok, I found the wrong code- Check r10584.

[Roboron]

Things pending related to this (of the top of my head):

* Adapt flatpak manifest to the new structure
* Adapt MacOS bundle to the new structure. Make it self-contained! -> I'll work on this next
* Investigate if we can extract paksets in a directory different to simutrans with the scripted installer, so we can choose a more appropriate name for the pakset directory.
* When all is done, update documentation on https://simutrans-germany.com/wiki/wiki/en_Installation

[Roboron]

r10632 contains the necessary changes to make MacOS bundle self-contained. I'll test today's nightly later, and if everything goes right, the next revision will remove files related to translocation (since they will be needed no more).

Note that I had to change the install_dir from "/Library/Simutrans/" to "~/Library/Simutrans/" because simutrans can't even chdir to the system one.

[prissi]

Does the install work without homebrew on Mac, or does Mac actually uses the inbuilt installer (so only http and zip?)

[Roboron]

I'm pretty sure it uses the built-in installer, because I tried downloading pak48.Excentrique (github, https), and it did not. However, I believe MacOS comes with curl by default (no homebrew needed), so why it is not using the script installer?

Good news, test succeeded. r10633 got rid of translocation related stuff.

[prissi]

The internal installer should as well call the script, so the user does not see the terminal. Hence, pak48 should work. Please test this, because otherwise we have to bundle the pak with MacOS and use the current code to access them.

[Roboron]

There was a bug with an ifdef. Now working properly. See r10634.

[Roboron]

It is true, but I am not sure how to do this with the scripted installer without first extracting to another directory containing a simutrans folder.

Did you make any progress on this? I think you did some changes in between but I don't know if they are related to this.

If so, is anyone against naming the install folder "paksets"?

[prissi]

Please elaborate more. Do you want all pak folders inside a folder pakset, or only in the addon directory?

The install script cannot handle an install of paks in a folder not named simutrans. It just quits. The easy way out would be naming the addon pak folder simutrans. But this location should be only used as last resort anyway.

[Roboron]

Do you want all pak folders inside a folder pakset, or only in the addon directory?

I didn't say anything about the addon directory. I just want the install directory to be named "paksets" instead of "simutrans", because when the top directory is also named "simutrans" you end up with a "simutrans/simutrans" directory which is confusing, so better if it can be "simutrans/paksets", which is more descriptive, at least in those cases.

Here in this patch I did this for linux/mac and the get_pak.sh, but I don't know if the same can be done for the NSIS script installer (or if it matters at all...).

EDIT: I see it does not matter, because the simutrans install directory used in windows is exclusive for paksets already. Well then, I submited this patch and updated the documentation on https://simutrans-germany.com/wiki/wiki/en_Installation which was also missing the in-game installer.

I mark this as done in my book, finally.

[prissi]

That patch does not work, the pak files still happily ends up inside a directory named simutrans (which is lucky, since the pak loading code will not look for paksets in pakset directory either). This needs more work on the shell script and on the code. Also your script will ignore any settings of simutrans for target directory, which is not what is intended. As it stands now, it is not working at all.

I think there are some essentials:
1) Simutrans should supply the correct installation directory to the script, because it should know best where to put paks.
2) Only if the current dir is read-only, then the script may choose another default.
3) Handling of paksets with and without a simutrans folder needs to change in the script.

Currently the script does not do what you intended it to do and will not work on AmigaOS, Beos, FreeBSD, ...

Thus, I reverted that change for now. I will look into it in the next days to properly fix it.

[Roboron]

the pak files still happily ends up inside a directory named simutrans

No, they don't. They are decompressed into a temporal simutrans directory, but then they are moved to the actual install directory (paksets). Actually this was your idea.

pak loading code will not look for paksets in pakset directory either

I have not looked closely at the code, but it definitely does 

I tried with both, paks with a simutrans folder and without, and everything seemed to work.

Also your script will ignore any settings of simutrans for target directory

Fair point, target directory should be the first to try, only after that look for defaults. Actually, looking at the code, base_dir is used for the target directory and not install_dir (which was the point of this change). That can also be improved.

Attached is a revised version of the patch, with the aforementioned corrections and which always decompress files in the $TEMP directory to simplify logic. I have added the check for the writeable directory but removed the check for the simutrans directory since that's not really true any-more.

get_pak.diff

[prissi]

I have altered get_pak.sh in r10698 to download to any working directory. This means a distribution should have to patch only one path in pak_install.cc to install to any directory. But maybe this should go to simconst.h as it is a more logical place to patch.

[Yona-TYT]

Simutrans directory within simutrans ?.

I apologize for my intrusion, but @Roboron is right, a directory called "paksets" would make more sense, so it would be "user/simutrans/paksets/pak128" .

@Prissi This for me would be the most sensible way, please reconsider.

[prissi]

The directory can NOW be named in any way. It is up to roboron, since I will not find much time until end of August it seems.

[Roboron]

Committed the name change in r10704.