Add arginfo to functions by andypost · Pull Request #8 · php/pecl-php-uploadprogress

andypost · 2020-08-21T19:14:06Z

Extracted arginfo from #5

Jan-E · 2020-10-29T19:41:14Z

@ramsey @remicollet This seems to be the last step to PHP 8 compatibility. Can somebody merge it?

andypost · 2020-12-05T05:56:08Z

I bet it needs follow-up to create stubs to generate this arginfo

PHP         : /usr/bin/php8 
PHP_SAPI    : cli
PHP_VERSION : 8.0.0
ZEND_VERSION: 4.0.0-dev
PHP_OS      : Linux - Linux andy-HP 5.8.0-32-generic #34-Ubuntu SMP Fri Nov 27 15:10:41 UTC 2020 x86_64
INI actual  : /mnt/testing/php8-pecl-uploadprogress/src/pecl-php-uploadprogress-uploadprogress-1.1.3/tmp-php.ini
More .INIs  :   
CWD         : /mnt/testing/php8-pecl-uploadprogress/src/pecl-php-uploadprogress-uploadprogress-1.1.3
Extra dirs  : 
VALGRIND    : Not used
=====================================================================
TIME START 2020-12-05 06:03:30
=====================================================================
PASS uploadprogress_get_info - Filename containing spaces (bug #58318) [tests/bug58318.phpt] 
PASS uploadprogress_get_contents - Returns contents of uploading file [tests/uploadprogress_get_contents_basic.phpt] 
PASS uploadprogress_get_contents - Warns when function is disabled [tests/uploadprogress_get_contents_error001.phpt] 
SKIP uploadprogress_get_contents - Warns when too few arguments [tests/uploadprogress_get_contents_error002.phpt] reason: test not valid on PHP 8
PASS uploadprogress_get_contents - Warns when maxlength is less than zero [tests/uploadprogress_get_contents_error003.phpt] 
TEST 6/21 [tests/uploadprogress_get_contents_error004.phpt]
========DIFF========
001+ Fatal error: Uncaught ArgumentCountError: uploadprogress_get_contents() expects at least 2 arguments, 1 given in /mnt/testing/php8-pecl-uploadprogress/src/pecl-php-uploadprogress-uploadprogress-1.1.3/tests/uploadprogress_get_contents_error004.php:2
001- Fatal error: Uncaught ArgumentCountError: uploadprogress_get_contents() expects at least 2 parameters, 1 given in %s/uploadprogress_get_contents_error004.php:%d
     Stack trace:
     #0 %s/uploadprogress_get_contents_error004.php(%d): uploadprogress_get_contents('fileIdentifier')
     #1 {main}
--
========DONE========
FAIL uploadprogress_get_contents - Warns when too few arguments [tests/uploadprogress_get_contents_error004.phpt] 
PASS uploadprogress_get_contents - When contents template is empty [tests/uploadprogress_get_contents_variation001.phpt] 
PASS uploadprogress_get_contents - File contents not found [tests/uploadprogress_get_contents_variation002.phpt] 
PASS uploadprogress_get_contents - Returns contents of file up to maxlen [tests/uploadprogress_get_contents_variation003.phpt] 
PASS uploadprogress_get_contents - Returns empty string for empty file [tests/uploadprogress_get_contents_variation004.phpt] 
PASS uploadprogress_get_contents - Template without replacement assumes directory [tests/uploadprogress_get_contents_variation005.phpt] 
PASS uploadprogress_get_contents - When contents file cannot be read [tests/uploadprogress_get_contents_variation006.phpt] 
PASS uploadprogress_get_info - Returns info for uploading file [tests/uploadprogress_get_info_basic.phpt] 
SKIP uploadprogress_get_info - Warns when too few arguments [tests/uploadprogress_get_info_error001.phpt] reason: test not valid on PHP 8
PASS uploadprogress_get_info - File info not found [tests/uploadprogress_get_info_error002.phpt] 
PASS uploadprogress_get_info - When filename template is empty [tests/uploadprogress_get_info_error003.phpt] 
TEST 17/21 [tests/uploadprogress_get_info_error004.phpt]
========DIFF========
001+ Fatal error: Uncaught ArgumentCountError: uploadprogress_get_info() expects exactly 1 argument, 0 given in /mnt/testing/php8-pecl-uploadprogress/src/pecl-php-uploadprogress-uploadprogress-1.1.3/tests/uploadprogress_get_info_error004.php:2
001- Fatal error: Uncaught ArgumentCountError: uploadprogress_get_info() expects exactly 1 parameter, 0 given in %s/uploadprogress_get_info_error004.php:%d
     Stack trace:
     #0 %s/uploadprogress_get_info_error004.php(%d): uploadprogress_get_info()
     #1 {main}
--
========DONE========
FAIL uploadprogress_get_info - Warns when too few arguments [tests/uploadprogress_get_info_error004.phpt] 
PASS uploadprogress_get_info - Template without replacement assumes directory [tests/uploadprogress_get_info_variation001.phpt] 
PASS uploadprogress_get_info - When empty template and identifier [tests/uploadprogress_get_info_variation002.phpt] 
PASS uploadprogress_get_info - When info file cannot be read [tests/uploadprogress_get_info_variation003.phpt] 
PASS uploadprogress_get_info - Trims whitespace around keys and values [tests/uploadprogress_get_info_variation004.phpt] 
=====================================================================
TIME END 2020-12-05 06:03:30

=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped    :    0
Exts tested     :   13
---------------------------------------------------------------------

Number of tests :   21                19
Tests skipped   :    2 (  9.5%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    2 (  9.5%) ( 10.5%)
Tests passed    :   17 ( 81.0%) ( 89.5%)
---------------------------------------------------------------------
Time taken      :    0 seconds
=====================================================================

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
uploadprogress_get_contents - Warns when too few arguments [tests/uploadprogress_get_contents_error004.phpt]
uploadprogress_get_info - Warns when too few arguments [tests/uploadprogress_get_info_error004.phpt]

Jan-E · 2020-12-05T09:26:51Z

I bet it needs follow-up to create stubs to generate this arginfo

@cmb6 @andypost Is there any benefit in creating the arginfo usng stubs, in stead of directly adding them in the code of the extension?

andypost · 2020-12-09T02:01:53Z

@Jan-E I think it will be easy to manage in long run, for example if extension using Z_PARAM_STRICT_LONG details in https://externals.io/message/105970

Also there's existing stubs to re-use in IDE https://github.com/JetBrains/phpstorm-stubs so only one definition to maintain and which is easier to read

Girgias · 2020-12-09T12:07:47Z

Z_PARAM_STRICT_LONG

This doesn't exist in PHP 8.0 any more, as it wasn't as strict as the normal LONG ZPP check.

Advantages of stubs is that you define the function as you would in PHP and get type information for parameters and return value and correct default value iniatialition, with all of the functions being declared for the extension instead of doing it manually.

There is also work to generate class infos and such from stubs.

remicollet · 2020-12-10T07:44:43Z

I bet it needs follow-up to create stubs to generate this arginfo

@cmb6 @andypost Is there any benefit in creating the arginfo usng stubs, in stead of directly adding them in the code of the extension?

Pros:

easier to maintain a stub
can generate both PHP 7 and 8 arginfo (with type hintings)

Cons:

requires PHP 8 to generate

This PR add arginfo without type hintings, which is fine, but "minimal" for PHP 8

andypost · 2021-02-22T00:55:43Z

Is it really hard blocker to use quick fix and release 1.1.4?

wallon-ines · 2021-02-22T16:02:46Z

I can't use my PHP 8 image because of this error

eg with cli command:

[16804@d7836afb7859 project]$ drush cr

Warning: Missing arginfo for uploadprogress_get_info() in Unknown on line 0

Warning: Missing arginfo for uploadprogress_get_contents() in Unknown on line 0
 [success] Cache rebuild complete.

williamdes · 2021-04-29T23:33:33Z

Hi,
I am sorry to use this PR as a channel, but could anyone have a look to this bug I found for this extension please ?
https://bugs.php.net/bug.php?id=80979

tfr-readonly · 2021-05-15T18:39:01Z

I can't use my PHP 8 image because of this error

Me neither. I had to git clone -b arginfo https://github.com/skilld-labs/pecl-php-uploadprogress uploadprogress to have a working extension for PHP8. Could anybody at least merge this PR?

rfay · 2021-09-04T14:36:38Z

Looking forward to having this in there.

ramsey · 2021-09-28T18:35:28Z

I merged master into your arginfo branch, fixed the conflict, and tried to push, but it looks like I don't have permissions to make changes to your branch. I've pushed my changes to a branch in my repo here: https://github.com/ramsey/pecl-php-uploadprogress/tree/arginfo

Please pull these changes into your branch and push again so that it kicks off the CI build on GitHub Actions.

Thanks!

ramsey · 2021-09-28T18:37:10Z

BTW, as you can see here: https://github.com/ramsey/pecl-php-uploadprogress/actions/runs/1283850036

This does make the tests pass on 8.0 and 8.1! 🎉

ramsey · 2021-09-28T18:39:18Z

I don't know why I was asking you to merge my changes to this branch. I was able to handle it on my own and merge to master. 🤦🏻‍♂️

rfay · 2021-09-28T18:59:38Z

Can we get a release with this in there soon? Thanks!

ramsey · 2021-09-28T19:00:17Z

Yes

Add arginfo to functions

f14654d

relax tests for PHP 8

9148790

andypost mentioned this pull request Apr 29, 2021

Incompatible PHP 8.0 pecl extensions wodby/php#129

Closed

andypost force-pushed the arginfo branch from 38ac9fb to 749d4df Compare April 29, 2021 23:22

llaville mentioned this pull request Jul 13, 2021

Feature - add new extension uploadprogress mlocati/docker-php-extension-installer#388

Merged

Added testing for 8.0 and nightly, disable xdebug

4ab96a5

andypost force-pushed the arginfo branch from 749d4df to 4ab96a5 Compare July 16, 2021 17:43

ramsey merged commit 2424369 into php:master Sep 28, 2021

andypost deleted the arginfo branch September 29, 2021 02:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add arginfo to functions - #8

Add arginfo to functions#8
ramsey merged 3 commits into
php:masterfrom
skilld-labs:arginfo

andypost commented Aug 21, 2020

Jan-E commented Oct 29, 2020

andypost commented Dec 5, 2020 •

edited

Loading

Jan-E commented Dec 5, 2020

andypost commented Dec 9, 2020 •

edited

Loading

Girgias commented Dec 9, 2020

remicollet commented Dec 10, 2020

andypost commented Feb 22, 2021

wallon-ines commented Feb 22, 2021

williamdes commented Apr 29, 2021

tfr-readonly commented May 15, 2021

rfay commented Sep 4, 2021

ramsey commented Sep 28, 2021

ramsey commented Sep 28, 2021

ramsey commented Sep 28, 2021

rfay commented Sep 28, 2021

ramsey commented Sep 28, 2021

Labels

9 participants

Uh oh!

Conversation

andypost commented Aug 21, 2020

Jan-E commented Oct 29, 2020

andypost commented Dec 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Jan-E commented Dec 5, 2020

andypost commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Girgias commented Dec 9, 2020

remicollet commented Dec 10, 2020

andypost commented Feb 22, 2021

wallon-ines commented Feb 22, 2021

williamdes commented Apr 29, 2021

tfr-readonly commented May 15, 2021

rfay commented Sep 4, 2021

ramsey commented Sep 28, 2021

ramsey commented Sep 28, 2021

ramsey commented Sep 28, 2021

rfay commented Sep 28, 2021

ramsey commented Sep 28, 2021

Labels

9 participants

andypost commented Dec 5, 2020 •

edited

Loading

andypost commented Dec 9, 2020 •

edited

Loading