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

SuperTuxKart fails to start when /data exists #2073

Closed
jturner314 opened this issue Apr 4, 2015 · 11 comments
Closed

SuperTuxKart fails to start when /data exists #2073

jturner314 opened this issue Apr 4, 2015 · 11 comments
Assignees

Comments

@jturner314
Copy link

Version: 0.9rc1-1 (Arch Linux package)

Description

When a directory /data exists, STK experiences a fatal error and fails to start.

Steps to reproduce

  1. Install supertuxkart. The Arch Linux package places the supertuxkart binary at /usr/bin/supertuxkart and the data files in /usr/share/supertuxkart/data.
  2. Create a /data directory. (That's a directory named data, immediately under the root directory.)
  3. Run supertuxkart at the command line.

Actual behavior

This results in the following output:

$ supertuxkart
[verbose  ] main: Error messages and other text output will be logged to /home/jim/.config/supertuxkart/0.8.2/stdout.log.
[info   ] [FileManager]: Data files will be fetched from: '../../data/'
[info   ] [FileManager]: User directory is '/home/jim/.config/supertuxkart/0.8.2/'.
[info   ] [FileManager]: Addons files will be stored in '/home/jim/.local/share/supertuxkart/addons/'.
[info   ] [FileManager]: Screenshots will be stored in '/home/jim/.cache/supertuxkart/screenshots/'.
[info   ] [FileManager]: User-defined grand prix will be stored in '/home/jim/.local/share/supertuxkart/grandprix/'.
[warn   ] [FileManager]: Directory 'challenges' not found, aborting.
[warn   ] [FileManager]: Directory 'fonts' not found, aborting.
[warn   ] [FileManager]: Directory 'gfx' not found, aborting.
[warn   ] [FileManager]: Directory 'grandprix' not found, aborting.
[warn   ] [FileManager]: Directory 'gui' not found, aborting.
[warn   ] [FileManager]: Directory 'library' not found, aborting.
[warn   ] [FileManager]: Directory 'models' not found, aborting.
[warn   ] [FileManager]: Directory 'music' not found, aborting.
[warn   ] [FileManager]: Directory 'tracks' not found, aborting.
[warn   ] [FileManager]: Directory 'sfx' not found, aborting.
[warn   ] [FileManager]: Directory 'shaders' not found, aborting.
[warn   ] [FileManager]: Directory 'skins' not found, aborting.
[warn   ] [FileManager]: Directory 'textures' not found, aborting.
[warn   ] [FileManager]: Directory 'po' not found, aborting.
[fatal  ] [FileManager]: Not all assets found - aborting.

Expected behavior

Start SuperTuxKart.

Problem

Looking at the source, it appears that the bug was introduced in b9b2f96. The problem is that STK erroneously assumes that if a directory ../../data exists (in this case /data), it must contain the STK data files. Note that even if the Arch Linux package was built with SUPERTUXKART_DATADIR defined to the correct directory, STK would still check for the problematic path ../../data first.

Temporary workaround

For the time being, I can manually specify the correct data directory with supertuxkart --root=/usr/share/supertuxkart/data.

@deveee
Copy link
Member

deveee commented Apr 4, 2015

You can temporarily workaround it using environment variable:
export SUPERTUXKART_DATADIR=/usr/share/supertuxkart/

I don't see any clear solution atm. We could check first if SUPERTUXKART_DATADIR exists. But then local installation will use /usr/share/supertuxkart instead of ../data and it's not good too...

Even if we check if this directory is our (it contains karts, tracks etc.) we can't be sure that it's proper directory and not totally different STK version installed in strange location.

Maybe we should add file with information about version to stk-assets? In cmake we can use PROJECT_VERSION as compiler definition. Then it could be easily compared in file_manager.

@jturner314
Copy link
Author

What do you think of the proposal above (#2074)? This changes the priority for root_dir path selection to:

  1. The SUPERTUXKART_DATADIR environment variable.
  2. That Mac-specific path (if __APPLE__ is defined).
  3. The SUPERTUXKART_DATADIR preprocessor option (if it's defined).
  4. The various relative paths (data, ../data, ../../data).
  5. Other things...
  6. The last-resort path /usr/local/share/games/supertuxkart.

I think this is more reasonable than the current priorities, because I would expect that a path I specified during compilation would take priority over the existence of the various relative paths. Local installation should still work fine, because this only affects the behavior when the SUPERTUXKART_DATADIR preprocessor option is defined.

Regarding the Arch Linux package, if #2074 is accepted, the package maintainer can just add the option -DSUPERTUXKART_DATADIR="/usr/share/supertuxkart" when building with cmake and everything should work fine.

@deveee
Copy link
Member

deveee commented Apr 4, 2015

I have installed in my system:

  • stable 0.8.1 version in /usr/local/share/supertuxkart
  • static 0.9-rc1 package installed in local files /home/Games/Supetuxkart-0.9-rc1
  • current git version with whole build environment in other local directory
  • some other versions

If you will prioritize SUPERTUXKART_DATADIR, then 0.9-rc1 version will find 0.8.1 data directory in /usr/local/share/supertuxkart and it will crash.

We sould detect if this is our data directory. Ideally we should remember somewhere version number, or at least check if karts and tracks directories exist.

But anyway is ../../data path really needed? @hiker is it needed for Visual Studio? On linux you can just run ./build/bin/supertuxkart from main directory.

@hiker
Copy link
Contributor

hiker commented Apr 4, 2015

/data? Seriously? ;) No other distro ever had /data. How much of a problem is that for 0.9 - I am not exactly keen on changing the startup sequence, since we have basically a code freeze.

../.. was added mainly because many beginners would try to start a self-compiled version in unexpected directories (i.e. in cmake/bin, not cmake), and we got tired of debugging this ;) A very minor reason is that it was convenient once or twice for debugging (though that's not important, obviously you can just type a slightly longer path :P ).

I'd suggest we just add a check if the file stk_config.xml exists in that data directory:

    else if(m_file_system->existFile("../../data") && m_file_system->existFile("../../data/stk_config.xml"))

etc. - but ideally after 0.9.

@jturner314
Copy link
Author

Thank you both for your responsiveness and helpfulness. 😃

If you will prioritize SUPERTUXKART_DATADIR, then 0.9-rc1 version will find 0.8.1 data directory in /usr/local/share/supertuxkart and it will crash.

Wouldn't you define the preprocessor option SUPERTUXKART_DATADIR to the correct directory (/home/Games/Supetuxkart-0.9-rc1) or just leave it undefined when building 0.9-rc1? If you defined it to the correct directory, things would work (because the correct path would take precedence), and if you left it undefined, STK would behave as it does now (because that section would be compiled out).

The only time I can see #2074 being a problem is if SUPERTUXKART_DATADIR was defined to the wrong directory when building 0.9-rc1. I'm not familiar with the STK codebase, so I must be missing something here -- maybe cmake defines SUPERTUXKART_DATADIR by default and uses an incorrect value?

Edit: I just took a look at CMakeLists.txt and saw that it always defines SUPERTUXKART_DATADIR when building on Macs. I would hope that it would define SUPERTUXKART_DATADIR to the correct value, though.

We sould detect if this is our data directory. Ideally we should remember somewhere version number, or at least check if karts and tracks directories exist.

Yeah, that's probably a good idea anyway, and it would fix this issue.

/data? Seriously? ;) No other distro ever had /data.

Yeah, it's not a distro thing. 😉 I have an SSD and an HDD, and most of my HDD is one big partition that I have mounted at /data. It's probably pretty unusual to have a /data directory, so if you don't think it's worth the effort to fix this issue, don't worry about it.

I was just surprised that the existence of a directory seemingly unrelated to STK caused it to crash, so I thought it was appropriate to file a bug report. I'll just specify the --root command line option or the SUPERTUXKART_DATADIR environment variable to workaround the issue.

@konstin
Copy link
Contributor

konstin commented Apr 4, 2015

/data? Seriously? ;) No other distro ever had /data.

I have /data on my smartphone by default (Jolla with Sailfish OS) :P

hiker added a commit that referenced this issue Apr 5, 2015
@hiker
Copy link
Contributor

hiker commented Apr 5, 2015

Mounted /data? That should to go /mnt/data ;)

Can you try the fix_2073 branch? It checks if the file ...../data/stk_config.xml exists (not only ......./data), that should avoid the problem. Not a really complete solution if you should have e.g. 0.8.1 installed in /data it would still detect the incorrect version. We could perhaps add a version number either to the file name of stk_config.xml, or as an xml attribute (which would be more work/slower to check at startup time). But tbh, I doubt that any of those cases merits additional work :)

@konstin
Copy link
Contributor

konstin commented Apr 5, 2015

Why not add a file that has the name of the stk version it belongs to? e.g. for 0.9 you could add a file data/0.9.

@deveee
Copy link
Member

deveee commented Apr 5, 2015

I agree with konstin. In cmake just add:

add_definitions(-DPROJECT_VERSION=\"${PROJECT_VERSION}\")

and check if "data/"PROJECT_VERSION exists.

Btw. /data is important directory on Android ;)

@hiker
Copy link
Contributor

hiker commented Apr 6, 2015

We can discuss adding a version number, but I think we need a separate ticket for that - e.g. instead of PROJECT_VERSION we should probably use supertuxkart_0.9 or so (otherwise someone has /data/0.9 - which could be any not stk related file), see #2077 .

hiker added a commit that referenced this issue Apr 16, 2015
@hiker hiker self-assigned this Jul 31, 2015
hiker added a commit that referenced this issue Jul 31, 2015
…ed by STK

to detect that stk is reading the right data files (and therefore avoids #2073,
in which stk finds the wrong data directory).
@hiker
Copy link
Contributor

hiker commented Jul 31, 2015

As konstin suggested I've added a file 'supertuxkart.git' in 78c592e, which is used by the file manager to detect the right data directory. There are now also proper error messages in place, which is I think as good as it gets.

@hiker hiker closed this as completed Jul 31, 2015
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

No branches or pull requests

4 participants