Skip to content

Use own definition of msgpack_unserialize_data_t #104

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 4 commits into from

Conversation

sodabrew
Copy link
Contributor

@sodabrew sodabrew commented Nov 1, 2016

PHP changes its struct php_unserialize_data, so we need our own. Closes #100.

@sodabrew
Copy link
Contributor Author

sodabrew commented Nov 1, 2016

Test failures on PHP 7.0 (pre-existing, not caused by this PR):

TEST 41/122 [tests/041.phpt]
========DIFF========
022+ float(-0)
023+ float(-0)
024+ cb8000000000000000
022- float(0)
023- float(0)
024- cb0000000000000000
========DONE========
FAIL Check for double NaN, Inf, -Inf, 0, and -0 [tests/041.phpt] 

Test failures on PHP 7.1 (don't appear to be caused by this PR):

TEST 37/122 [tests/040.phpt]
========DIFF========
001+ ec
001- 
002+ int(-20)
========DONE========
FAIL broken random data test [tests/040.phpt] 
TEST 38/122 [tests/040b.phpt]
========DIFF========
001+ ec
001- 
002+ int(-20)
========DONE========
FAIL broken random data test : MessagePack class [tests/040b.phpt] 
TEST 39/122 [tests/040c.phpt]
========DIFF========
001+ ec
001- 
002+ int(-20)
========DONE========
FAIL broken random data test : MessagePackUnpacker::feed [tests/040c.phpt] 
TEST 40/122 [tests/040d.phpt]
========DIFF========
001+ ec
001- 
002+ int(-20)
========DONE========
FAIL broken random data test : MessagePackUnpacker::execute [tests/040d.phpt] 
TEST 41/122 [tests/041.phpt]
========DIFF========
022+ float(-0)
023+ float(-0)
024+ cb8000000000000000
022- float(0)
023- float(0)
024- cb0000000000000000
========DONE========
FAIL Check for double NaN, Inf, -Inf, 0, and -0 [tests/041.phpt] 

@sodabrew
Copy link
Contributor Author

sodabrew commented Nov 1, 2016

@laruence @remicollet This PR based on your suggestion: #100 (comment)

@sodabrew sodabrew force-pushed the struct_unserialize_data branch from 2b314d3 to 3ba3abe Compare November 11, 2016 20:14
@sodabrew sodabrew force-pushed the struct_unserialize_data branch from 3ba3abe to 4bb4fab Compare November 11, 2016 20:15
@codecov-io
Copy link

codecov-io commented Nov 11, 2016

Current coverage is 86.87% (diff: 100%)

Merging #104 into master will increase coverage by <.01%

@@             master       #104   diff @@
==========================================
  Files             8          8          
  Lines          1249       1234    -15   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
- Hits           1085       1072    -13   
+ Misses          164        162     -2   
  Partials          0          0          

Powered by Codecov. Last update 3411376...4bb4fab

@sodabrew
Copy link
Contributor Author

Ping @laruence

@sodabrew
Copy link
Contributor Author

Annoying ping for attention because PHP 7.1 is coming soon.

@jjsaunier
Copy link

jjsaunier commented Dec 2, 2016

Hello,

We had a similar issue during migrating our container to php 7.1,

I think it's related :

running: make
/bin/bash /tmp/pear/temp/pear-build-defaultuserJehL4h/msgpack-2.0.1/libtool --mode=compile cc  -I. -I/tmp/pear/temp/msgpack -DPHP_ATOM_INC -I/tmp/pear/temp/pear-build-defaultuserJehL4h/msgpack-2.0.1/include -I/tmp/pear/temp/pear-build-defaultuserJehL4h/msgpack-2.0.1/main -I/tmp/pear/temp/msgpack -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/TSRM -I/usr/local/include/php/Zend -I/usr/local/include/php/ext -I/usr/local/include/php/ext/date/lib  -DHAVE_CONFIG_H  -g -O2   -c /tmp/pear/temp/msgpack/msgpack.c -o msgpack.lo
mkdir .libs
 cc -I. -I/tmp/pear/temp/msgpack -DPHP_ATOM_INC -I/tmp/pear/temp/pear-build-defaultuserJehL4h/msgpack-2.0.1/include -I/tmp/pear/temp/pear-build-defaultuserJehL4h/msgpack-2.0.1/main -I/tmp/pear/temp/msgpack -I/usr/local/include/php -I/usr/local/include/php/main -I/usr/local/include/php/TSRM -I/usr/local/include/php/Zend -I/usr/local/include/php/ext -I/usr/local/include/php/ext/date/lib -DHAVE_CONFIG_H -g -O2 -c /tmp/pear/temp/msgpack/msgpack.c  -fPIC -DPIC -o .libs/msgpack.o
/tmp/pear/temp/msgpack/msgpack.c: In function 'ps_srlzr_decode_msgpack':
/tmp/pear/temp/msgpack/msgpack.c:160:29: error: storage size of 'var_hash' isn't known
  msgpack_unserialize_data_t var_hash;
                             ^
/tmp/pear/temp/msgpack/msgpack.c: In function 'php_msgpack_unserialize':
/tmp/pear/temp/msgpack/msgpack.c:210:29: error: storage size of 'var_hash' isn't known
  msgpack_unserialize_data_t var_hash;
                             ^
make: *** [msgpack.lo] Error 1
ERROR: `make' failed

What is missing to merge and released? Because this issue is a blocker and résolve this issue as well.

@RochesterinNYC
Copy link

The Cloud Foundry Buildpacks team is coming across the issue that this PR resolves as well.

@sodabrew
Copy link
Contributor Author

sodabrew commented Dec 2, 2016

Annoying ping for attention because PHP 7.1.0 was released yesterday.

@jchesterpivotal
Copy link

Is there some way that Buildpacks team can help?

@Sean-Der
Copy link
Member

Sean-Der commented Dec 7, 2016

Merged!

Sorry that this took so long, I am not very active with PHP anymore. I don't have access to release on PECL, but I will get master back in a good state for 7.1

thanks

@Sean-Der Sean-Der closed this Dec 7, 2016
@laruence
Copy link
Member

laruence commented Dec 7, 2016

I will make a release when it's done, thanks :)

@Sean-Der
Copy link
Member

Sean-Der commented Dec 7, 2016

awesome, thank you @laruence !

@jjsaunier
Copy link

jjsaunier commented Dec 7, 2016 via email

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.

7 participants