non portable build does not show all available languages

[Hans Joachim Desserud]

A while back, a patch was merged to make the language selection work properly [1]. This was tested across various operating systems and everyone agreed it worked as expected.
However, I may have discovered a flaw in our testing. For my part at least, I normally compile with compile.sh and run ./widelands. In this case, all possible languages are shown and everything works fine. However, earlier today I realized that if you compile with cmake [2] instead and run 'make install', only English and current system languages are shown as alternatives. I guess the installation is more in line with what actually happens when WL is packaged and installed on a system, rather than being run from the same folder it was compiled in.

I have only tried this on Arch Linux so far, but I fear other systems are affected by this as well. I realize I should have been more thorough in my initial tests, but at least I discovered it before the final release. What do we do now?

[1] https://code.launchpad.net/~widelands-dev/widelands/martin_i18n/+merge/49202
[2] http://wl.widelands.org/wiki/WidelandsGoingCMake/

[Hans Joachim Desserud (hjd)]

>I have only tried this on Arch Linux so far
Same result on Ubuntu. I guess that means it's universal.

[David Allwicher (aber)]

Don't Panic. That sound like perfect normal behavior. We do not build locale per default. Did you build the locale using make lang && make install?
Does invoking cmake -DWL_PORTABLE=true make a difference?

[Hans Joachim Desserud (hjd)]

>Don't Panic.
"it said in big, friendly letters on the cover" :)

>We do not build locale per default.
Oh, I thought so. Wouldn't that be useful?

>Did you build the locale using make lang && make install?
Tried it now, but it didn't seem to have any effect.

>Does invoking cmake -DWL_PORTABLE=true make a difference?
Yes, that gets all the various languages in the options. Also fills up /usr/local/games, though I guess that would be the point of a portable version.

[Nicolai Hähnle (nha)]

The reason that building locales was disabled by default is that it took quite a long time away from the build process and so made the modify-compile-test cycle of regular development less efficient. Unless somebody can figure out a way to avoid this waste of time, please do not enable it by default.

[SirVer (sirver)]

It is enabled by default for Release builds which is just the way it should be in my opinion. I agree with nicolai that it should be off for debug builds for various reasons.

[Hans Joachim Desserud (hjd)]

From what I can see locales are built when RELEASE is specified, however they don't show up when running the game. I have also tested with build16-rc1 packaged in Arch Linux and Debian unstable, and they both have the same problem, current and English are the only available options.

This is by the way, how the package is built/installed in Arch, see the methods build() and install() :
http://projects.archlinux.org/svntogit/community.git/tree/widelands/repos/community-i686/PKGBUILD

[SirVer (sirver)]

Why is this targeted to build 16? It is not really critical, is it? Packagers can get languages to work with the correct compile time flags, afaik.m There have been numerous patches and bug reports for this, but we agreed on them to be all fixed. Please clarify.

[Hans Joachim Desserud (hjd)]

>Why is this targeted to build 16? It is not really critical, is it?
I guess that can be discussed, but I liked being able to switch between the various languages. While probably not critical, I still see it as a serious regression.

> Packagers can get languages to work with the correct compile time flags, afaik.
As I mentioned in #6, I have tried with the packages available from Debian and Arch Linux. Either they are missing some flags, or it doesn't work.

>There have been numerous patches and bug reports for this, but we agreed on them to be all fixed.
I tested the patch which was merged earlier this year a bit, and that worked/works fine when I compile it with compile.sh and run it in the same directory. When I started experimenting with cmake, and installing it properly it doesn't seem to list all possible locales anymore. I'm not sure if this was looked at when it was tested earlier.

To be specific:
Only current locale and english are available. Switching between both works, and the work as intended, but the rest of the languages are not available.

>Please clarify.
Hope that helps. I can probably answer additional questions and/or test things if that's required...

[Tino (tino79)]

Please try the following:
- delete ~/.widelands/config

I've experienced problems on my system, because the path to the
language files gets written to the config-file. Now with severals
widelands versions on my system (installed, dev build...) this leads
to a conflict if you start different widelands binaries...

Also try to use the language dir parameter of the wl binary if you
have the compiled language files not in the standard relative path.

Regards,
Tino

2011/4/11 Hans Joachim Desserud <email address hidden>:
>>Why is this targeted to build 16? It is not really critical, is it?
> I guess that can be discussed, but I liked being able to switch between the various languages. While probably not critical, I still see it as a serious regression.
>
>> Packagers can get languages to work with the correct compile time flags, afaik.
> As I mentioned in #6, I have tried with the packages available from Debian and Arch Linux. Either they are missing some flags, or it doesn't work.
>
>>There have been numerous patches and bug reports for this, but we agreed on them to be all fixed.
> I tested the patch which was merged earlier this year a bit, and that worked/works fine when I compile it with compile.sh and run it in the same directory. When I started experimenting with cmake, and installing it properly it doesn't seem to list all possible locales anymore. I'm not sure if this was looked at when it was tested earlier.
>
> To be specific:
> Only current locale and english are available. Switching between both works, and the work as intended, but the rest of the languages are not available.
>
>>Please clarify.
> Hope that helps. I can probably answer additional questions and/or test things if that's required...
>
> ** Attachment added: "Language selection"
>   https://bugs.launchpad.net/widelands/+bug/752436/+attachment/2015733/+files/languages.png
>
> --
> You received this bug notification because you are subscribed to
> widelands.
> https://bugs.launchpad.net/bugs/752436
>
> Title:
>  non portable build does not show all available languages
>

[Hans Joachim Desserud (hjd)]

> - delete ~/.widelands/config
has no effect, as far as I can see. When testing the debian package, I did so on a freshly installed system, and it had the same problem there.

> Also try to use the language dir parameter of the wl binary if
> you have the compiled language files not in the standard
> relative path.

Hm, when running
'[~] $ widelands --localedir=/usr/share/widelands/locale'
from anywhere on the system, nothing seems to change. When actually going to the directory and from there running
'[/usr/share/widelands/locale] $ widelands --localedir=.'
all languages (plus stuff like campaigns, maps) appear in the list. Still nothing changes when selecting any of these.

Explanation above [current directory], $=normal user (also tried as root).

[Hans Joachim Desserud (hjd)]

Also noticed something. Attached a log from starting up widelands (no parameters, no config). And I notice the line starting with "localedir:" is missing.

[David Allwicher (aber)]

Hans Joachim did a lot of work to show the problem.
I. As we can see the the current Linux build16 packages are broken.
II. This will not be fixable using "proper" compile time flags.
III. The problem is, we attach INSTALL_LOCALEDIR to the base directory. Which in the case that INSTALL_LOCALEDIR is absolut is nonsense.

[SirVer (sirver)]

David, the patch that you provide: will it fix the problem? I am aware that I am not of much help tracking this down, but I have no access to linux systems atm and I am in terrible fear that we break stuff for other oses.

However, if there is a problem with build 16rc1 here, we need to fix it for build 16.

[SirVer (sirver)]

Again to clarify: if the packages we ship for windows, linux and ./compile.sh produce a working and valid widelands executable with all languages, I am happy with that. If we can get it to work with cmake as well that is fine but we MUST ABSOLUTELY NOT break the other things. If unsure, delay till after build 16.

[David Allwicher (aber)]

Our filesystem is not robust at all. The Patch fixes on issue, but looking at the debian package this will not work. They use a prefix and then append datadir and localedir. I did work with absolute paths. I have no idea how this is supposed to work.

I think this is something that needs fixing before we ship the release.

[Hans Joachim Desserud (hjd)]

Hm, I got David's patch working in an initial attempt, but when I tried a bit more thorough (dowloading build16-rc1.bz2 and compiling from scratch) didn't work for some reason. Might be my fault though, and I'll look more into it.

Have we actually gotten confirmation that all languages are listed and working in the rc on other platforms (windows, osx)? I'm not asking this to be mean, just curious. :)

Worst case scenario: I'd really like to see this fixed before release, but if the actual issue goes deeper that may not be feasible. Though I think if we release with this, it should be clearly documented as a known issue for Linux-distros. I'm not really sure how much people will miss the other languages, but from the discussion around the patch earlier this year, it seemed like others were also interested in this. Though I guess the main part is covered, as the current locale and a fallback to English is available.

[SirVer (sirver)]

So, this is the current state:
- the win-build 16rc1 packages work as expected, show all languages and one can
  select all and they work
- the Mac OS X universal 16rc1 packages work as expected, show all languages
  and one can select all and they work
- The linux packages that are installed system wide only show english and the
  current selection as local, but the LOCAL variables are respected, so
  widelands starts up in the local language

So windows and mac are optimal, linux is suboptimal but better than build 15 was. I deem this uncritical even though it is not as we expected; i am very fearful of touching the locales code again and maybe break things for windows and mac that we do not have the time to test anymore.

I suggest pushing this to after build 16.

[SirVer (sirver)]

Oh, btw, i tested the windows and mac versions for #17... damn, i always forget to mention something :)

[David Allwicher (aber)]

As far as I can see, the debian package is completely broken. Showing only the selected language, but not working. So I'm in favor of fixing this issue. If you are in fear, let's just mask the patch and apply it for linux only.

[SirVer (sirver)]

How do you propose to only apply it to linux? Should we apply it to the source releases as well? That creates more confusion than it's worth. Propose a patch and we'll see to which margin it is dangerous.

[Jari Hautio (jarih)]

I took a look at the code and the problem seems to be that RealFSImpl:FindFiles does not handle absolute paths correctly. It always prepends some root directory like data dir or home dir.

I'll attach a patch that gets locales shown when run with command:
./widelands --localedir=/home/jari/code/build16/locale/

This should also fix the issue with packages. Note that this still requires that locale directory resides under home dir, data dir or working directory.

[Jari Hautio (jarih)]

And when I looked that patch again, I seem to have missed a slash there. So here again with proper separator in place.

[SirVer (sirver)]

I committed jaris patch in trunk 5903. Jari, is this patch save or does it open the possiblitiy to read outside of the g_fs paradigm?

People: please test with this patch and --localedir. If this fixes the problems, packagers might have to add a widelands.sh script that calls widelands with the correct localedir, but that would be better than nothing.

[SirVer (sirver)]

And test fast: If we delay build 16 by one day, we effectivly delay it by 2 weeks because we will not have a windows pacakge for this. So we only have today and tomorrow for testing.

[SirVer (sirver)]

This is the results I got from Enrico Tassi who pacakged the debian build 16-rc1 package. His tests were likely done before adding jari's patch:

I had locate set to it_IT in .widelands/config
It was not working
I passed --localedir=/usr/share/games/widelands/locale/ and it worked
 even if the list of translations was made of 3 items (EN, IT, system).
I removed the config file
Now the list shows only (EN, system).

Of course I ship all the locales:

 dpkg -L widelands-data |grep LC| wc -l
 526

My guess is that some paths are screwed up, and the flag --localedir=
fixes only the loading of the locales but not the listing of available
locales.

This is how I compile WL:

       cd build-debian; cmake \
               -DWL_INSTALL_PREFIX=/usr \
               -DWL_INSTALL_DATADIR=share/games/widelands \
               -DWL_INSTALL_BINDIR=games \
               -DWL_INSTALL_LOCALEDIR=share/games/widelands/locale \
               -DCMAKE_INSTALL_PREFIX=/usr \
               -DCMAKE_INSTALL_DATADIR=share/games/widelands \
               -DCMAKE_INSTALL_BINDIR=games \
               -DCMAKE_INSTALL_LOCALEDIR=share/games/widelands/locale \
               -DCMAKE_BUILD_TYPE=Release \
               ../

Hope it helps, keep me posted.

[Jari Hautio (jarih)]

The patch is safe, it's not used in windows build and if path is not absolute, the behaviour is identical to old one. But please review/test the change in case I missed something.

WL_INSTALL_LOCALEDIR must be absolute path. In the code prefix is prepended to datadir, but not to localedir. See David's patch in #19.

[SirVer (sirver)]

Some good news from enrico:

I've just tested WL blindly applying the patch on the thread:

 https://launchpadlibrarian.net/69308276/fixfindfiles.diff

It works for me, without any --flag at launch runtime.

Can someone test on windows and on mac os?

[Tino (tino79)]

Test on win32. The patch doesn't seem to change anything, all findings apply to both b16rc1 and bzr5903:

- launch without any flag and /locale directly under the executable: Ok. List shows all Languages
- launch with --localedir=. : Language List shows directory listening, see attached screenshot
- launch with --localedir=./locale : Only English and System language available
- launch with --localedir=h:\build_widelands\locale (referencing a localedir absolut in windows style) : Only English and System language available

At least i was not able to get the language list display a directory listening outside the dir the executable resides in, only that and nested dirs....

[Jari Hautio (jarih)]

I did not fix the underlying bug in the windows version of the code as it is not critical for the release. There are also other bugs in the file system code that I have fixed in unit test related branch, and which will go to trunk after build 16.

I think some of the issues in the Tinos's list are fixed there. So I think Tino's findings should go to another bug report related to --localedir or path handling under windows. By the way current logic requires that locale dir resides under datadir, homedir, or executable dir (on windows). This is enforced by the filesystem code because locale dir is not added to directory list, which is used for accessing files.

[David Allwicher (aber)]

I can load locale from everywhere. I will put them close to the kernel. Works as expected on OS X.

[SirVer (sirver)]

Alright, so i guess this is fixed enough for build 16. Terrific work everyone and thanks for the speedy testing!

If no one complains, I will tag build 16 saturday in the early morning.

[Hans Joachim Desserud (hjd)]

I have tested a bit with r5903 on Arch and Ubuntu Natty. All languages are listed and switching between them works as expected. Nothing else seems to have been broken.

High fives all around!

[SirVer (sirver)]

Released in build 16.

[Miroslav Ďurian (aasami)]

The Build 16 (Release) doesn't fix the issue. On Lubuntu Oneiric it's still missing languages in menu as dscribed in post #25.

Running widelands with:

widelands --localedir=/usr/share/games/widelands/locale

does fix the issue and display all avialable languages.

[David Allwicher (aber)]

@see https://bugs.launchpad.net/widelands/+bug/872777
@see http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=645127