Skip to content

Conversation

@realFlowControl
Copy link
Contributor

Hey there 🖖

this fixes the failing tests on Solaris as stated in https://bugs.php.net/bug.php?id=70574

Kind regards
Florian

Copy link
Member

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like these tests now fail in CI:

Gettext basic test with en_US locale that should be on nearly every system [ext/gettext/tests/gettext_basic-enus.phpt]
Test if bindtextdomain() returns string id if no directory path is set(if directory path is 'null') [ext/gettext/tests/gettext_bindtextdomain-cwd.phpt]
Test dcgettext() functionality [ext/gettext/tests/gettext_dcgettext.phpt]
Test dgettext() functionality [ext/gettext/tests/gettext_dgettext.phpt]
Test if dngettext() returns the correct translations (optionally plural). [ext/gettext/tests/gettext_dngettext-plural.phpt]
Test ngettext() functionality [ext/gettext/tests/gettext_ngettext.phpt]

@realFlowControl
Copy link
Contributor Author

That's interesting, locally everything was green 😉
I'll take a look later

@realFlowControl realFlowControl changed the title Move message catalog to proper locale directory to fix #70574 Fix #70574 - Move message catalog to proper locale directory Aug 5, 2020
@realFlowControl realFlowControl force-pushed the 70574-fix-failing-gettext-tests-on-solaris branch from 78138e9 to b8f7658 Compare August 5, 2020 19:14
@realFlowControl
Copy link
Contributor Author

As it turns out, those six tests were actually failing also locally, I don't know what I saw there ... 😥
Problem is with the directory the locales have to reside, it is not locale/en_US.UTF-8 but locale/en_US.utf8.
Fixed it and pushed

@realFlowControl realFlowControl marked this pull request as draft August 5, 2020 20:22
@realFlowControl
Copy link
Contributor Author

Converted to draft as I would like to clarify on the charset in the locale... Digging deeper I found that UTF-8 should actually work and utf8 might be another fallback. I have to double check this

@realFlowControl realFlowControl force-pushed the 70574-fix-failing-gettext-tests-on-solaris branch from b8f7658 to e42a0a3 Compare August 6, 2020 18:22
@realFlowControl
Copy link
Contributor Author

Hey there 🖖

I checked how things go and found out the following. When locale is set to en_US.UTF-8 the gettext() C functions tries to find the message catalog in the following order:

  • locale/en_US.UTF-8/
  • locale/en_US.utf8/
  • locale/en_US/
  • locale/en.UTF-8/
  • locale/en.utf8/
  • locale/en/

Having the catalog in the en_US.UTF-8 folder therefore is correct, everything else is fallback behavior.

Sorry for the inconvenience.

Florian

@realFlowControl realFlowControl marked this pull request as ready for review August 6, 2020 18:42
@realFlowControl realFlowControl requested a review from nikic August 6, 2020 18:42
@nikic
Copy link
Member

nikic commented Aug 6, 2020

So the problem in the original PR was just that it used UTF8 rather than UTF-8 in the path, right?

@realFlowControl
Copy link
Contributor Author

Exactly

@php-pulls php-pulls closed this in 5be6702 Aug 7, 2020
@realFlowControl realFlowControl deleted the 70574-fix-failing-gettext-tests-on-solaris branch August 12, 2020 16:53
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

Successfully merging this pull request may close these issues.

2 participants