Installation and writable files

[Ykkrosh]

A number of people have encountered this issue, so it would be good to get it solved, but I'm not sure exactly how, so discussion and/or patches would be good

The problem:

People want system-wide installation of the game, since that's what Linux package managers do (it's installed by root and then executed by (possibly multiple) users). We also want a similar system on Windows, so we can install as Administrator and then run as unprivileged users.

The game wants to write some files at run-time. In particular it wants to write to data/cache/, data/screenshots/, data/profiles/, logs/, probably data/mods/*/*.zip, and maybe some others.

World-writable directories are unacceptably insecure.

So we need to either avoid writing the files at run-time, or we need to write them into per-user directories.

Some thoughts:

It's possible to compute the contents of data/cache/ and data/mods/*/*.zip ahead of time, since they just depend on the contents of data/mods/. They're quite large, so it would be good to share between users to save disk space. If nothing in data/mods/ is changed (and the code itself doesn't change), I believe the game won't even attempt to write into those directories. But when something is changed (e.g. when the user installs a local mod), we need to be able to write new cache files and would like to be able to write new .zip files. (These could probably go in the user's home directory, effectively layered on top of the system files.)

screenshots is initially empty, so it can just be created in the user's home directory. logs initially contains a .jpg and .css file to make the generated logs prettier, so they'd need to be copied(/linked) into the user's home's logs directory. profiles isn't properly implemented yet - it could probably be ignored entirely, except that we save console history in there, and we might want a proper profile system in the future.

The game's file-handling code has been designed with awareness of these issues, so it shouldn't be too hard to implement a solution, but it needs to be configured and maybe tweaked or extended to handle everything we need.

Is there some standard for how current Linux games deal with these issues? I'm not familiar with the 'proper' way of doing these things. Ideas would be appreciated!

[feneur]

Was looking through the Trac Tickets and found something related: http://trac.wildfiregames.com/ticket/81

[bugmenot]

Hello,

I have created a debian package for ubuntu and published on playdeb.

http://www.playdeb.net/updates/#0 A.D.

Before the first start of the game the entire game data and binaries are copied to the user's home directory and the game is then started from there.

The usual approach for games (as far as I can tell) is that the game is able to be started from a non-writable directory and all files that have to be written are stored in the $HOME/.config/GAMENAME directory. Also mods can be installed by putting them in this directory.

[alvinx]

Hello,

exists Freedesktop's standard http://standards.freedesktop.org/basedir-s...r-spec-0.6.html .

Glib has multiplatform implementation:

http://library.gnome.org/devel/glib/unstab...user-config-dir

[olsner]

• cached resources (data/cache and the mod archives): since these (only) depend on the resources themselves, it'd be nice if the installation automatically generated these from the resources installed - unless you actually modify something yourself you shouldn't have to generate them into the user-local directory. I think that when resources are modified we only have to store user-local cached versions of the actually changed resources rather than all of them?
  
• screenshots and logs: should be moved into the user-local directory methinks. We should also have automatic (re-)creation of these directories (including copying the resources for the log html).
  
• profiles: although these are kind of meant to be global (like every user on the computer creates their own profile), the right thing is probably to put the set of profiles entirely inside the user-directory.
  
• user-local mods: while we don't write these - the directory to put them in will depend on all the other stuff. I think if we just mount ~/.0ad as data/ in the VFS, user-local mods and overrides for individual files just go in ~/.0ad/mods/ and that's it. Having this VFS is pretty sweet
  

As janwas mentions in the trac ticket, we should also differentiate between user-local cache and user-local config/data. On unix we could just let the cache and config directories be the same (~/.0ad or ~/.pyrogenesis or whatnot) since there's AFAIK no "application data" distinction there.

If we implement those and set up the VFS to disallow write access to the non-user-local directories, all other things we try to write should be pretty obvious since the VFS should then just fail

[Ykkrosh]

Some more thoughts:

Regardless of whether we're at a point in development where it entirely makes sense, people want to produce Linux packages for the game, and we need to make it work sensibly so the packagers don't have to resort to disgusting hacks. So we do need to solve this problem. But Windows and OS X are largely separate issues, and don't need to be solved now.

The current single-directory approach (i.e. everything is inside binaries/) is better for development (I like to have multiple independent copies of the game), as well as some deployments like running directly from a USB stick. So it should be an option, alongside the new installation approach.

The XDG basedir spec seems like a good idea. (It's already written, so we don't have to make everything up ourselves; some applications already use it; it has some technical benefits (e.g. you can safely blow away the whole of $HOME/.cache); there's a free (MIT) library that implements it). A very brief summary of what it defines:

$XDG_DATA_HOME (default $HOME/.local/share) -- user-specific data files

$XDG_CONFIG_HOME (default $HOME/.config) -- user-specific config files

$XDG_DATA_DIRS (default /usr/local/share/:/usr/share/) -- data files

$XDG_CONFIG_DIRS (default /etc/xdg) -- config files

$XDG_CACHE_HOME (default $HOME/.cache) -- user-specific cached files

*_DIRS are colon-separated, in order of decreasing precedence when searching. *_HOME takes precedence over *_DIRS.

Files go in $XDG_*/subdir/filename.

Missing directories are created with 0700.

We theoretically want to let people install multiple games based on the Pyrogenesis engine. Each game would have a different and separate version of the engine. Therefore the directory names should identify the game rather than the engine. (Also that makes it easier to find for a user who doesn't care about our engine name). Spaces and punctuation are bad, so we should use the name "0ad".

Rearranging the writable contents of binaries/:

data/mods/*/*.zip -> $XDG_CACHE_HOME/0ad/mods/*/*.zip

data/cache/ -> $XDG_CACHE_HOME/0ad/cache/

data/screenshots/ -> $XDG_DATA_HOME/0ad/screenshots/

data/profiles/ -> $XDG_CONFIG_HOME/0ad/profiles/

logs/ -> $XDG_CONFIG_HOME/0ad/logs/

Alternative: Maybe we could skip the mods/*/*.zip thing, by not dynamically creating archives - just do it once during system-wide installation, and never at runtime.

http://www.pathname.com/fhs/pub/fhs-2.3.html is relevant for system-wide installation locations. (I suppose some Linux distributions might have different ideas, so they might need to tweak the defaults, but we should have sensible defaults anyway.) (It suggests /var/games for modifiable game data, but we don't want to do that because of security (someone could write a corrupted file in there, and another user might run the game and it will load that file, and we don't (currently) attempt to guarantee any kind of safety when loading data files).)

Thus, read-only files could be like:

data/config/local.cfg -> $XDG_CONFIG_HOME/0ad/config/local.cfg

data/ -> $XDG_DATA_HOME/0ad/data/:/usr/share/games/0ad/data/ (search in both, install into the latter)

system/pyrogenesis -> /usr/bin/0ad

system/lib*.so -> /usr/lib/0ad/lib*.so

For simplicity, the HTML logs should be changed to standalone files (i.e. get rid of the logo, and use inline CSS).

For efficiency, /usr/share/games/0ad/data/cache/ should be initialised (on installation) with conversions of all the .xml and .dae files.

[janwas]

I'm fine with using XDG, it's reasonably straightforward.

Subdirectory 0ad - makes sense.

mkdir with 0700 - done.

Alternative: Maybe we could skip the mods/*/*.zip thing, by not dynamically creating archives - just do it once during system-wide installation, and never at runtime.

I'm fine with that. This archiving code has long been dormant anyway.

For efficiency, /usr/share/games/0ad/data/cache/ should be initialised (on installation) with conversions of all the .xml and .dae files.

How would that work? Do we really want to create a separate cache-everything routine or executable?

BTW, the VFS was designed for this kind of thing, but the underlying path.cpp's Path presumes that everything will reside in one directory. Am currently removing that logic.

[janwas]

After two coding sessions yesterday and today, the support for user/home directories is in place. Crashlogs and screenshots are written to a subfolder of appdata/0ad or the appropriate XDG folder.

Ticket 81 has been updated.

It works on Windows but is untested on *nix Please drop me a line if there are problems.

[Ykkrosh]

Thanks for doing this

A few things I notice:

http://trac.wildfiregames.com/changeset/7065#file6 -- the intent was that any errors other than file-not-found would be ignored here, and reported normally when trying to open the file normally a few lines later. With the new change, if e.g. your disk accidentally turned into cheese then you'll get a "Cannot find config file" warning instead of the proper disk-is-made-of-cheese error, which would be confusing. So either it should ignore all errors except the legitimate file-not-found/container-directory-not-found/etc ones (so it'll try to open the file and complain about any persistent errors), or else the warning text should be changed. (Or we could add a "don't bring up the ugly assertion-failure dialog if the file merely doesn't exist" flag to g_VFS->LoadFile.)

(Edit: Hmm, actually it should probably use ps/Filesystem.h's FileExists.)

The tests fail:

Running 114 tests.....................................................................vfs.cpp(111): Function call failed: return value was -110101 (Unknown error (-110101, 0xFFFE51EB))
udbg_bfd_init: loading symbols from ./test_dbg.
Function call failed: return value was -110101 (Unknown error (-110101, 0xFFFE51EB))
Location: vfs.cpp:111 (LoadFile)

Call stack:

(0x08539368) ldbg.cpp:101 debug_DumpStack(wchar_t*, unsigned int, void*, char const*)
(0x084f679e) debug.cpp:340 debug_BuildErrorMessage(wchar_t const*, char const*, int, char const*, void*, char const*, ErrorMessageMem*)
(0x084f6c99) debug.cpp:498 debug_DisplayError(wchar_t const*, unsigned int, void*, char const*, char const*, int, char const*, unsigned char*)
(0x084f70af) debug.cpp:552 debug_OnError(long, unsigned char*, char const*, int, char const*)
(0x08514429) vfs.cpp:111 VFS::LoadFile(boost::filesystem::basic_path<std::string, VfsPathTraits> const&, boost::shared_ptr<unsigned char>&, unsigned int&)
(0x081f1e17) test_MeshManager.h:86 TestMeshManager::copyFile(char const*, char const*)
(0x081f28b9) test_MeshManager.h:133 TestMeshManager::test_load_pmd_with_extension()
(0x081f2d01) test_MeshManager.cpp:24 TestDescription_TestMeshManager_test_load_pmd_with_extension::runTest()
...

Directories aren't created with 0700 permissions (which XDG basedir requires) - they seem to be 0755.

There's no tests for the new code, so it would be easy to break it in the future and start putting files in the wrong locations and not notice.

[janwas]

http://trac.wildfiregames.com/changeset/7065#file6 -- the intent was that any errors other than file-not-found would be ignored here, and reported normally when trying to open the file normally a few lines later. With the new change, if e.g. your disk accidentally turned into cheese then you'll get a "Cannot find config file" warning instead of the proper disk-is-made-of-cheese error, which would be confusing. So either it should ignore all errors except the legitimate file-not-found/container-directory-not-found/etc ones (so it'll try to open the file and complain about any persistent errors), or else the warning text should be changed.

Ah, that's a good point, but it looks like philosophical territory. The original intent was that all (low-level) errors be reported immediately when they are generated (i.e. by using WARN_RETURN ERR::xxx instead of return ERR::xxx), so the cheese error (hehe) wouldn't be ignored.

Also, there is something to be said for the main game code saying "cannot find config file" regardless of whether it's just because the file wasn't found or because the disk is made of cheese - the result is the same.

Finally, how can the game code (or for that matter, LoadFile) know what errors to ignore and which ones should actually reported? I'm pretty happy so far with the policy of reporting everything unless a particular error happens often/legitimately, but am happy to be convinced otherwise.

The tests fail:

Uhoh Unfortunately I'm out of time today and probably won't be able to look into the issue until the weekend due to an upcoming trip.

Directories aren't created with 0700 permissions (which XDG basedir requires) - they seem to be 0755.

Yep, I couldn't see a way to do so with the boost filesystem API - create_directory lacks a mode_t parameter:

Attempts to create the directory dp resolves to, as if by POSIX mkdir() with a second argument of S_IRWXU|S_IRWXG|S_IRWXO.

We should revert to calling mkdir then, eh?

There's no tests for the new code, so it would be easy to break it in the future and start putting files in the wrong locations and not notice.

Yeah, I'm kind of worried about confusion regarding profiles etc., especially because the old location is still supported (depending on cmdline args). Do you have any ideas?

Edited August 3, 2009 by janwas

[Ykkrosh]

looks like philosophical territory

I don't care much about philosophy, I just want things to work properly . I'm happy as long as the output is sensible.

... how can the game code (or for that matter, LoadFile) know what errors to ignore and which ones should actually reported?

Surely we can just tell it? We know that most low-level errors are always bad and should be reported in ugly dialog boxes. We also know that if config/default.cfg doesn't exist then it's a serious error and should be reported seriously, but if config/local.cfg doesn't exist then it's harmless and intentional and we should do nothing more than print a note in the log file, but if e.g. we get ENOMEM when looking at local.cfg then it's unexpected and should be reported seriously. So we need to encode that knowledge in the game code, and make sure the filesystem code acts consistently with the desired behaviour.

The GetFileInfo(...) == ERR::VFS_FILE_NOT_FOUND thing was a hack to achieve that behaviour (by using a function that doesn't immediately raise ugly dialog boxes when the file is not found, before using the file-opening function that does). If GetFileInfo will always either return FILE_NOT_FOUND or raise an ugly dialog box itself, then that's okay, but I don't want it to silently ignore any other errors; and it looks like GetFileInfo doesn't do that, and the new code looks like it will silently ignore the other errors.

(Actually it should be changed to only do the file-not-found check on config files that are meant to be optional, like local.cfg, but that's a separate issue.)

Directories aren't created with 0700 permissions (which XDG basedir requires) - they seem to be 0755.

Yep, I couldn't see a way to do so with the boost filesystem API - create_directory lacks a mode_t parameter ... We should revert to calling mkdir then, eh?

That Boost behaviour sounds like a bit of a pain, and I don't see any way around it. (umask could work but it's seemingly not thread-safe so it's probably too dangerous.)

There's no tests for the new code, so it would be easy to break it in the future and start putting files in the wrong locations and not notice.

Yeah, I'm kind of worried about confusion regarding profiles etc., especially because the old location is still supported (depending on cmdline args). Do you have any ideas?

Write a test which creates an empty directory, sets $HOME to it, then runs InitVfs (suitably refactored to be testable) and writes some files and then checks the test directory contains all the right files in the right locations with the right permissions? (and then repeat for the other cmdline-dependent mode, and then ideally test edge cases and error handling)

[janwas]

Surely we can just tell it? We know that most low-level errors are always bad and should be reported in ugly dialog boxes. We also know that if config/default.cfg doesn't exist then it's a serious error and should be reported seriously

hehe, it's not quite that simple. Sure, we can check if the error is "file not found".

However, what was happening in this case is "dir not found" (files and directories are tracked separately). I think the capability of differentiating them is potentially useful, but it'd be burdensome if user code had to check for both of them. (Worse yet, delayed population of directories can postpone file enumeration errors - e.g. due to antivirus - until the first file lookup.)

If GetFileInfo will always either return FILE_NOT_FOUND or raise an ugly dialog box itself, then that's okay, but I don't want it to silently ignore any other errors; and it looks like GetFileInfo doesn't do that, and the new code looks like it will silently ignore the other errors.

GetFileInfo only avoids an *additional* error dialog from CHECK_ERR if you "pass in a flag" (i.e. set pfileInfo to 0), independent of the actual error. However, the new code doesn't silently ignore other errors because they are still reported when first returned from a function.

That Boost behaviour sounds like a bit of a pain, and I don't see any way around it. (umask could work but it's seemingly not thread-safe so it's probably too dangerous.)

*nod* I've implemented our own CreateDirectories with a mode_t parameter.

Write a test which creates an empty directory, sets $HOME to it, then runs InitVfs (suitably refactored to be testable) and writes some files and then checks the test directory contains all the right files in the right locations with the right permissions? (and then repeat for the other cmdline-dependent mode, and then ideally test edge cases and error handling)

Sounds good

Have noted this in the never-ending todo list.

[Ykkrosh]

Hmm, looks like local.cfg isn't being handled properly on Linux. Ideally it should be in ~/.config/0ad/config/local.cfg, but as far as I can see the code just does

g_VFS->Mount("config/", paths.RData()/"config");

and so it only ever looks in the game's root data directory, not the per-user directory. Is that intentional, or a bug, or am I misinterpreting it?

[janwas]

Oops, that logic is incomplete. I think the intent was to grab the system.cfg from paths.RData() (i.e. svn) and local.cfg from paths.Config(), which coincide iff -writableRoot was specified.

How about we add a second Mount there unless the two directories are equal? To make saving config files more likely to work, the writable directory (i.e. Config()) should be mounted last.

[bugmenot]

Ok, what is the status of this bug ?

Trying to run the game from a non-writable directory gives me this output:

korn@ubuntu:/usr/share/games/0ad/system$ ./pyrogenesis_dbg 
mkdir failed with errno=13
vfs_lookup.cpp(86): Assertion failed: "0"
udbg_bfd_init: loading symbols from /usr/share/games/0ad/system/pyrogenesis_dbg.
debug.cpp(216): Unable to open crashlog.txt for writing (please ensure the log directory is writable)
Unable to open crashlog.txt for writing (please ensure the log directory is writable)
Location: debug.cpp:216 (debug_WriteCrashlog)

Call stack:

(0x6c5c1e) none:0 std::vector<CacheRelations::SharedCache, std::allocator<CacheRelations::SharedCache> >::_M_insert_aux(__gnu_cxx::__normal_iterator<CacheRelations::SharedCache*, std::vector<CacheRelations::SharedCache, std::allocator<CacheRelations::SharedCache> > >, CacheRelations::SharedCache const&)
(0x699afb) none:0 std::list<CInput::SRow, std::allocator<CInput::SRow> >::insert(std::_List_iterator<CInput::SRow>, CInput::SRow const&)
(0x69a1ea) none:0 std::list<CInput::SRow, std::allocator<CInput::SRow> >::insert(std::_List_iterator<CInput::SRow>, CInput::SRow const&)
(0x69a02b) none:0 std::list<CInput::SRow, std::allocator<CInput::SRow> >::insert(std::_List_iterator<CInput::SRow>, CInput::SRow const&)
(0x69a1f5) none:0 std::list<CInput::SRow, std::allocator<CInput::SRow> >::insert(std::_List_iterator<CInput::SRow>, CInput::SRow const&)
(0x69a382) none:0 std::list<CInput::SRow, std::allocator<CInput::SRow> >::insert(std::_List_iterator<CInput::SRow>, CInput::SRow const&)
(0x6ae4a2) none:0 std::vector<VfsFile const*, std::allocator<VfsFile const*> >::~vector()
(0x6a76ca) none:0 FileInfo* std::__uninitialized_copy_a<FileInfo*, FileInfo*, FileInfo>(FileInfo*, FileInfo*, FileInfo*, std::allocator<FileInfo>&)
(0x4e6791) none:0 std::_Rb_tree<CStrW, std::pair<CStrW const, long>, std::_Select1st<std::pair<CStrW const, long> >, std::less<CStrW>, std::allocator<std::pair<CStrW const, long> > >::_M_insert_unique_(std::_Rb_tree_const_iterator<std::pair<CStrW const, long> >, std::pair<CStrW const, long> const&)
(0x4e7f4a) none:0 std::_Rb_tree<CStrW, std::pair<CStrW const, long>, std::_Select1st<std::pair<CStrW const, long> >, std::less<CStrW>, std::allocator<std::pair<CStrW const, long> > >::_M_insert_unique_(std::_Rb_tree_const_iterator<std::pair<CStrW const, long> >, std::pair<CStrW const, long> const&)
(0x43e6ae) none:0 _start
(0x7f0d36590abd) /lib/libc.so.6:0 __libc_start_main
(0x43d7d9) none:0 _start

errno = 0 (?)
OS error = ?


(C)ontinue, (B)reak, Launch (D)ebugger, or (E)xit?

These are my build instructions:

	cd libraries/fcollada/src && $(MAKE)

	cd build/workspaces; \
	./update-workspaces.sh; \
	cd gcc; \
	$(MAKE)

[Ykkrosh]

Now filed as #458, #459.

[janwas]

hm, I'm not able to study the VFS lookup internals today (#459 definitely sounds like a bug there), but a quick workaround for #458 exists. We skip touching mods/internal in any way if -onlyPublicFiles is passed on the command line. That switch made sense from our perspective before OSing (allowing testing of the public version without breaking most artist's files by default). However, in the current situation it'd make sense to flip that around, not using internal at all unless the internal mod is explicitly requested. What do you think?

[Ykkrosh]

Hmm, that seems likely to cause minor pain and confusion for people who currently expect it to load the internal mod by default. A quicker workaround is to simply create mods/internal/ so it always exists with the right permissions - it's a bit of a pain to make that happen via SVN (need to fiddle with the authorization files), but anyone packaging the game for distribution can make the directory manually, as a workaround until we fix it properly.