Skip to content

PHP 5.6 RFC default encoding #568

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

Closed
wants to merge 19 commits into from

Conversation

yohgaki
Copy link
Contributor

@yohgaki yohgaki commented Jan 20, 2014

Implementation of this RFC.
https://wiki.php.net/rfc/default_encoding

@smalyshev
Copy link
Contributor

This has a lot of failing tests on Travis that did not fail previously. Could you check that? Moreover, some tests segfault.

@yohgaki yohgaki closed this Jan 27, 2014
@yohgaki yohgaki reopened this Jan 27, 2014
@yohgaki
Copy link
Contributor Author

yohgaki commented Jan 27, 2014

Last commit should fix segfaults. I'll check travis build later.

Using travis/compile.sh configure option under my Fedora19 x86_64 (ja_JP.UTF-8)

My branch
=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped    :   25
Exts tested     :   55
---------------------------------------------------------------------

Number of tests : 13234             11103
Tests skipped   : 2131 ( 16.1%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :   49 (  0.4%) (  0.4%)
Expected fail   :   33 (  0.2%) (  0.3%)
Tests passed    : 11021 ( 83.3%) ( 99.3%)
---------------------------------------------------------------------

PHP-5.6 branch
=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped    :   24
Exts tested     :   54
---------------------------------------------------------------------

Number of tests : 13241             11090
Tests skipped   : 2151 ( 16.2%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :   54 (  0.4%) (  0.5%)
Expected fail   :   33 (  0.2%) (  0.3%)
Tests passed    : 11003 ( 83.1%) ( 99.2%)
---------------------------------------------------------------------

@yohgaki
Copy link
Contributor Author

yohgaki commented Jan 27, 2014

Hmm. php-src's PHP-5.6 branch does not have failed tests, but this does.
There are lots of cli server errors and I get these on my php-src clone also.
It seems I have to check these tests.

@yohgaki
Copy link
Contributor Author

yohgaki commented Jan 28, 2014

@smalyshev
Now it compiles and passes all tests.
Thank you for your comment and patience, Stas!

@yohgaki
Copy link
Contributor Author

yohgaki commented Jan 28, 2014

@smalyshev I'll merge this if you are OK with current code and tests. Please let me know when you finish review.

return FAILURE;
encoding = mbfl_name2encoding(PG(output_encoding));
if (!encoding) {
MBSTRG(http_output_encoding) = &mbfl_encoding_pass;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Raising error might be better, since this code would pass any encoding when invalid encoding name is provided.

; http://php.net/default-charset
;default_charset = "UTF-8"
default_charset = "UTF-8"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Commenting out this speeds up a little. We may be better comment out all INI values if they are the same as compiled defaults

@yohgaki
Copy link
Contributor Author

yohgaki commented Feb 5, 2014

@smalyshev Thank you for your help :)
I think it's good enough to merge now. I'll merge this weekend unless I have more comment.

@yohgaki
Copy link
Contributor Author

yohgaki commented Feb 13, 2014

Committed & Close.
The last compile failure is due to Derick's commit.

@yohgaki yohgaki closed this Feb 13, 2014
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