Check if applied locale correctly lowers chars and fallback

[basilgello]

.. to default region if it does not.

Fixes #19883.

[basilgello]

Got an ACK from @hbayindir in the Debian bug report thread:

Thanks a lot for your intense effort. The new version you uploaded is working 
as intended without any changes to locale.

I can confirm that the problem is now solved.

Let's start the review?

[basilgello]

@fuzzard @lrusak @Rechi @phunkyfish @DaveTBlake Sorry I dont see who is actually code owner of LangInfo.cpp - it was written so long ago… :)

[basilgello]

@lrusak Fixed, thanks! Not sure if m_defaultRegion.m_strName is meaningful though, but at least the failing region name should be displayed correctly and the latter messages from SetGlobalLocale show the locale is set to C

[basilgello]

Not sure if m_defaultRegion.m_strName is meaningful though

Yes, default region is marked as 'N/A' but the locale info is displayed as INFO:

2021-06-30 05:11:15.500 T:8514     INFO <general>: CAddonMgr::FindAddon: resource.language.tr_tr v9.0.22 installed
2021-06-30 05:11:15.551 T:8514     INFO <general>: CLangInfo: loading resource.language.tr_tr language information...
2021-06-30 05:11:15.552 T:8514     INFO <general>: global locale set to tr_TR.UTF-8
2021-06-30 05:11:15.552 T:8514    ERROR <general>: region 'Türkiye' is affected by #19883 - falling back to default region 'N/A'
2021-06-30 05:11:15.552 T:8514     INFO <general>: global locale set to C
2021-06-30 05:11:15.552 T:8514     INFO <general>: CLangInfo: loading resource.language.tr_tr language strings...
2021-06-30 05:11:15.597 T:8514     INFO <general>: global locale set to tr_TR.UTF-8
2021-06-30 05:11:15.597 T:8514    ERROR <general>: region 'Türkiye' is affected by #19883 - falling back to default region 'N/A'
2021-06-30 05:11:15.597 T:8514     INFO <general>: global locale set to C

I think there's no need to move ifdeffery from https://github.com/xbmc/xbmc/blob/master/xbmc/LangInfo.cpp#L222 into a separate function std::string CLangInfo::GetLocaleName(CLangInfo::CRegion& region); just to print exact locale names?

[lrusak]

I think it's fine as is. The only other thing I can say is maybe change to LOGWARNING rather than LOGERROR but I'm fine either way.

[phunkyfish]

jenkins build this please

[howie-f]

i did not see this bug on windows 10 with turkish language (system and kodi) installed.
unfortunately can't test android and osx... android would be interesting though

[basilgello]

android would be interesting though

I just tested 19.1 from F-Droid in my LineageOS tablet and it seems Android is not vulnerable due to https://github.com/xbmc/xbmc/blob/19.1-Matrix/xbmc/LangInfo.cpp#L297

In fact, C.UTF-8 becomes the default region applied by this fix if Turkish I is detected.

do we need to unset those in further files - example https://github.com/xbmc/xbmc/blob/master/tools/Linux/kodi-standalone.sh.in ? and what are consequences of unsetting as LC_ALL

No, we need to unset LC_* variables only once, just before kodi.bin is invoked. Unsetting and setting back LC_ALL in X11 code alters the already-unset value so it should not interfere. The proper way avoiding kodi.sh.in edit is to understand what Kodi dependency breaks rendering before GetLocaleName is first invoked, but I did not manage to dig that out.

[basilgello]

@phunkyfish can you please run unpatched 19.1 on OSX with Turkish system and Kodi language and check whether the bug manifests?

[phunkyfish]

@phunkyfish can you please run unpatched 19.1 on OSX with Turkish system and Kodi language and check whether the bug manifests?

How exactly do I do that? Being a native English speaker I never deal with other languages!!!

[basilgello]

Temporary set system language to Turkish (you can leave region intact!) and start Kodi, then install Turkish language via Settings - Interface - Region and language then restart Kodi and check if movie windows, poweroff prompt etc are readable.

[phunkyfish]

How will I know if they are readable? If there is a screenshot of what they should look like?

[basilgello]

Screenshots of affected Kodi are in #19883. Settings pane has buttons with captions and images if not affected but is empty if it is.

[fuzzard]

Appears to be ok (if ive done what you have said properly. Was just a stock 19.1 release build, MacOS 11.5.2

[basilgello]

Yes, thanks! So it is Linux-only issue!

[howie-f]

should be ready after it got a bit love and polishing 😉
EDIT: please also fix the warning message (nobody will know what issue 19883 was back then)

[basilgello]

Yeah, I paused this after I tried to move kodi.sh.in part into xbmc/platform/posix/main.cpp and the fix failed to address an issue :) Gonna try it out once more & fix the rest + backport!

[basilgello]

@howie-f I fixed the points you brought on the last review! Unfortunately I could not get rid of un-setting environment variables in kodi.sh.in because something does not honor even the thermonuclear locale setting:

   std::setlocale(LC_ALL, "C");
   std::locale::global(std::locale("C"));

I think we should go with battle-tested code here, and the fact that kodi.sh.in is used only by Linux platform suggest it is a correct place to alter LC_* stuff.

[ksooo]

Sorry, I don't want to be PITA, but shouldn't we try to find and fix the root cause of this problem instead of polluting Kodi code base with workarounds?

[basilgello]

Sorry, I don't want to be PITA, but shouldn't we try to find and fix the root cause of this problem instead of polluting Kodi code base with workarounds?

One of root cases of this problem is that https://github.com/xbmc/xbmc/blob/19.1-Matrix/xbmc/filesystem/SpecialProtocol.cpp#L207 uses lower/upper functions to find files in a case-insensitive manner. There are others + some that even I failed to figure out by today :(

I am OK to solve that issue completely, but I am really afraid (see my comments in 19883) that the 'true' fix will be greater than CDateTime one :(

[ksooo]

but I am really afraid (see my comments in 19883) that the 'true' fix will be greater than CDateTime one :(

Let's do the job right and bite that bullet.

Kodi "survived" without this fix for many many years. I don't see the need for this kinda special case to hurry and go with a workaround.

[basilgello]

Kodi "survived" without this fix for many many years. I don't see the need for this kinda special case to hurry and go with a workaround.

Matrix users from Turkey already reported this on r/kodi and in Debian. I am OK to hold this for Nexus, but Matrix should be worked around. I do ship the fix already in Debian, but Manjaro / Arch / Gentoo users still lack it. Should we leave it as-is in such case?

[ksooo]

Sorry, I don't see this in master. This is only a workaround and we know that the root cause is in Kodi code base. So, let's fix it properly.

[ksooo]

Matrix users from Turkey already reported this on r/kodi and in Debian. I am OK to hold this for Nexus, but Matrix should be worked around.

I do agree.

[jenkins4kodi]

I've made some formatting changes to meet the current code style. The diffs are available in the following links:

• PR19905-0001-01fc80cb96-Check-if-applied-locale-correctly-.diff

For more information please see our current code style guidelines.