Problem with --homedir command line option in Windows

Bug #1828355 reported by GunChleoc
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
widelands
Won't Fix
Undecided
GunChleoc

Bug Description

See https://wl.widelands.org/forum/topic/4513/

Homedir is fixed, but there is still a problem with the localedir (see #4)

Related branches

GunChleoc (gunchleoc)
Changed in widelands:
milestone: none → build21-rc1
Revision history for this message
kaputtnik (franku) wrote :

With linux the --homedir option works as expected.

Revision history for this message
fuchur (fuchur77) wrote :

The problem is the windows logger which is initialized in WLApplication::setup_homedir(). But there is a call of log() in WLApplication::WLApplication(int const argc, char const* const* const argv) before that happens. See source below

 if (commandline_.count("homedir")) {
  log("Adding home directory: %s\n", commandline_["homedir"].c_str()); // <-- this is it
  homedir_ = commandline_["homedir"];
  commandline_.erase("homedir");
 }
#ifdef REDIRECT_OUTPUT
 if (!redirect_output())
  redirect_output(homedir_);
#endif

 setup_homedir();

If the labeled line is changed to print the output using std::cout (maybe encapsuled by #ifdef _WIN32) or commented it works.

Windows doesn't have a stdout if you run a program using a shortcut. Therefore log() writes to a file stdout.txt which is saved in the homedir folder. I guess in Linux starting widelands from a shell stdout is always accessible -> no problem with log() before setting the homedir.

I assume in Linux one should see two messages about the home directory if one uses the homedir option. One stating "Adding home directory:" and the second one "Set home directory:".

Revision history for this message
GunChleoc (gunchleoc) wrote :

I have replaced the log command with cout in https://bazaar.launchpad.net/~widelands-dev/widelands/trunk/revision/9105

Is it working well enough now, or do we need to do more fixes?

Revision history for this message
fuchur (fuchur77) wrote :

Ok, I made a quick test, the --homedir option is recognised and the given directory is created. A config and stdout.txt are created as well. Starting a game and saving one works. The directories ai, save, replays and temp are then created.

But clicking on options results in an error:

Unexpected error during the game
FileSystem::create: could not find file or directory: F:\widelands\widelands-bzr\hgs-homedir\data/locale

Ok, that's not due to this bug, but a different one (maybe related to it). Unfortunately I won't have time to investigate this further this week. Just found this line in wlapplication.cc

i18n::set_localedir(datadir_ + "/locale");

I think the / is the culprit as it seems that this line in options.cd finally creating the localedir doesn't take care of it.

std::unique_ptr<FileSystem> fs(&FileSystem::create(i18n::get_localedir()));

Maybe it should use something like RealFSImpl on the result of get_localedir(). Or that should be done inside set_localedir().

Sorry for not trying it myself. I don't have time at the moment for further experiments. :-(

GunChleoc (gunchleoc)
description: updated
Revision history for this message
fuchur (fuchur77) wrote :

Now I found a solution concerning the problem with the locale folder. In wlapplication.cc I replaced

i18n::set_localedir(datadir_ + "/locale");

with

i18n::set_localedir(g_fs->canonicalize_name(datadir_ + "/locale"));

I'd have prefered to do the modification inside set_localedir(), but my first attempts weren't successfull as I could not call canonicalize_name() or g_fs->canonicalize_name() from there. So I went on with the easy solution.

The only thing left to do was to copy the locale directory from a running build20 installation to the data directory of my test branch. It seems that (at least here on my machine) the data directory of trunk doesn't contain a locale subdirectory.

GunChleoc (gunchleoc)
Changed in widelands:
assignee: nobody → GunChleoc (gunchleoc)
status: New → In Progress
Revision history for this message
GunChleoc (gunchleoc) wrote :

The fix looks fine. I'll implement it into WLApplication so that we won't get a circular dependency between i18n and the file system.

The locale directory is generated when compiling, so if you compile your test branch with the -t switch, it will have none.

Revision history for this message
GunChleoc (gunchleoc) wrote :

P.S: actually, it should have one, but it will be empty.

Revision history for this message
fuchur (fuchur77) wrote :

Ok, thanks for implementing this. I think I should learn how to upload a branch if I find more to work on.

Concerning the locale directory I configured cmake to build without translations. I think that's the explanation for not having it.

Revision history for this message
kaputtnik (franku) wrote :

fuchur, can you please test the attached branch?

Revision history for this message
GunChleoc (gunchleoc) wrote :
Changed in widelands:
status: In Progress → Won't Fix
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.