longxinH / xhprof

PHP7/PHP8 support
http://pecl.php.net/package/xhprof
Apache License 2.0
1.04k stars 165 forks source link

Add nightly to travis testing matrix and remove unsupported #43

Closed andypost closed 3 years ago

andypost commented 3 years ago

7.0 and 7.1 is unsupported https://www.php.net/supported-versions.php

andypost commented 3 years ago

There's one failed test for 8.x

FAILED TEST SUMMARY
---------------------------------------------------------------------
XHProf: Test Curl Additional Info
Author: longxinhui [tests/xhprof_012.phpt]
andypost commented 3 years ago

Copied the test looks for php 8 curl integration needs change

 main()                                  : ct=       1; wt=*;
 main()==>curl_close                     : ct=       1; wt=*;
-main()==>curl_exec#https://www.php.net/ : ct=       1; wt=*;
-main()==>curl_getinfo                   : ct=       1; wt=*;
+main()==>curl_exec                      : ct=       1; wt=*;
 main()==>curl_init                      : ct=       1; wt=*;
 main()==>curl_setopt                    : ct=       2; wt=*;
 main()==>xhprof_disable                 : ct=       1; wt=*;

also few build warnings

cc -I. -I/home/travis/build/longxinH/xhprof/extension -DPHP_ATOM_INC -I/home/travis/build/longxinH/xhprof/extension/include -I/home/travis/build/longxinH/xhprof/extension/main -I/home/travis/build/longxinH/xhprof/extension -I/home/travis/.phpenv/versions/master/include/php -I/home/travis/.phpenv/versions/master/include/php/main -I/home/travis/.phpenv/versions/master/include/php/TSRM -I/home/travis/.phpenv/versions/master/include/php/Zend -I/home/travis/.phpenv/versions/master/include/php/ext -I/home/travis/.phpenv/versions/master/include/php/ext/date/lib -DHAVE_CONFIG_H -g -O2 -c /home/travis/build/longxinH/xhprof/extension/xhprof.c  -fPIC -DPIC -o .libs/xhprof.o

/home/travis/build/longxinH/xhprof/extension/xhprof.c: In function ‘zm_startup_xhprof’:

/home/travis/build/longxinH/xhprof/extension/xhprof.c:266:26: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]

     _zend_compile_string = zend_compile_string;
                          ^
/home/travis/build/longxinH/xhprof/extension/xhprof.c:267:25: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]

     zend_compile_string = hp_compile_string;
                         ^
/home/travis/build/longxinH/xhprof/extension/xhprof.c: In function ‘zm_shutdown_xhprof’:

/home/travis/build/longxinH/xhprof/extension/xhprof.c:300:27: warning: assignment from incompatible pointer type [-Wincompatible-pointer-types]

     zend_compile_string   = _zend_compile_string;
                           ^
/home/travis/build/longxinH/xhprof/extension/xhprof.c: In function ‘hp_trace_callback_curl_exec’:

/home/travis/build/longxinH/xhprof/extension/xhprof.c:1536:13: warning: initialization makes pointer from integer without a cast [-Wint-conversion]
             1
             ^
/home/travis/build/longxinH/xhprof/extension/xhprof.c:1536:13: note: (near initialization for ‘fci.named_params’)
andypost commented 3 years ago

Fixes for

andypost commented 3 years ago

Curl changed return type to return object instead of resource, that's why test failed

https://github.com/php/php-src/blob/master/UPGRADING#L948

andypost commented 3 years ago

@longxinH thank you for merging, I still found no way to fix curl

andypost commented 3 years ago

The related change is https://github.com/php/php-src/pull/5402

longxinH commented 3 years ago

@andypost Thank you, is there any other solution I try

andypost commented 3 years ago

I used to run the test to try get object but stuck at this line https://github.com/longxinH/xhprof/blob/master/extension/xhprof.c#L1519-L1520

make NO_INTERACTION=1 REPORT_EXIT_STATUS=1 test \
    PHP_TEST_SHARED_EXTENSIONS="-d extension=/usr/lib/php8/modules/curl.so -d extension=modules/xhprof.so" \
    TESTS=tests/xhprof_012_8.phpt
andypost commented 3 years ago

It just type 8 (object) in 8+ so after https://github.com/longxinH/xhprof/pull/44 makes sense to create new release)