Skip to content
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

Change old hiredis functions to make it work with hired is 1.0.0, Part II #52

Closed
wants to merge 1 commit into from

Conversation

skyzyx
Copy link

@skyzyx skyzyx commented Aug 6, 2016

Identical to #47 by @jbbrunsveld, but with the Git conflicts resolved.

Performed a PCRE regex search and replace in phpiredis.c of:

(redis)Reply([^\(|^\s]+) → $1$2

Compiled with hiredis v0.13.3 and PHP 7.0.9. Without this patch, phpiredis failed to function (see #46) when compiled against hiredis v0.13.3.


#46 This removes the following old function aliases, use the new name now: (See hiredis Changelog)

redisReplyReaderCreate => redisReaderCreate
redisReplyReaderCreate => redisReaderCreate
redisReplyReaderFree => redisReaderFree
redisReplyReaderFeed => redisReaderFeed
redisReplyReaderGetReply => redisReaderGetReply
redisReplyReaderSetPrivdata => redisReaderSetPrivdata
redisReplyReaderGetObject => redisReaderGetObject
redisReplyReaderGetError => redisReaderGetError


This change is Reviewable

@skyzyx
Copy link
Author

skyzyx commented Aug 6, 2016

Looks like the Travis test runner failed. This is what I get when I run make test on the machine that I compiled this on:

$ make test

Build complete.
Don't forget to run 'make test'.


=====================================================================
PHP         : /opt/remi/php70/root/usr/bin/php 
PHP_SAPI    : cli
PHP_VERSION : 7.0.9
ZEND_VERSION: 3.0.0
PHP_OS      : Linux - Linux instapop-vm 3.18.34-20.el7.x86_64 #1 SMP Fri May 27 12:12:30 UTC 2016 x86_64
INI actual  : /root/package-phpiredis/phpiredis/tmp-php.ini
More .INIs  :   
CWD         : /root/package-phpiredis/phpiredis
Extra dirs  : 
VALGRIND    : Not used
=====================================================================
TIME START 2016-08-06 03:38:07
=====================================================================
PASS [CLIENT] Connect to Redis [tests/client_001.phpt] 
PASS [CLIENT] Persistent and non-persistent connections [tests/client_002.phpt] 
SKIP [CLIENT] Persistent and non-persistent connections (UNIX socket) [tests/client_003.phpt] reason: Cannot find UNIX domain socket file at /tmp/redis.sock
PASS [CLIENT] Execute commands [tests/client_004.phpt] 
PASS [CLIENT] Execute pipelined commands [tests/client_005.phpt] 
PASS [CLIENT] Execute pipelined commands returning nested multibulk responses [tests/client_006.phpt] 
PASS [CLIENT] Execute commands (binary-safe) [tests/client_007.phpt] 
PASS [CLIENT] Execute commands with binary data (binary-safe) [tests/client_008.phpt] 
PASS [CLIENT] Execute pipelined commands (binary safe) [tests/client_009.phpt] 
PASS [CLIENT] Do not attempt to reconnect upon disconnection [tests/client_010.phpt] 
PASS [CLIENT] Do not attempt to reconnect upon disconnection (pipeline) [tests/client_011.phpt] 
PASS [READER] Create reader resource [tests/reader_001.phpt] 
PASS [READER] Feed reader and parse responses [tests/reader_002.phpt] 
PASS [READER] Check reader state [tests/reader_003.phpt] 
PASS [READER] Reset reader [tests/reader_004.phpt] 
PASS [READER] Response types [tests/reader_005.phpt] 
PASS [READER] Set custom error handler [tests/reader_006.phpt] 
PASS [READER] Set custom status handler [tests/reader_007.phpt] 
PASS [READER] Custom error handler throwing exceptions or returning objects [tests/reader_008.phpt] 
PASS [READER] Properly handle NULL responses [tests/reader_009.phpt] 
PASS [READER] Keep multibulk responses types [tests/reader_010.phpt] 
PASS [READER] Test regression for segfaults [tests/reader_011.phpt] 
PASS [READER] Parameters formatting should not modify input argument. [tests/reader_012.phpt] 
PASS [READER] Parameters formatting should not break on many arguments [tests/reader_013.phpt] 
PASS [SERIALIZER] Command serialization [tests/serializer_001.phpt] 
PASS [SERIALIZER] Command serialization with non-string arguments [tests/serializer_002.phpt] 
PASS [UTILS] Calculate the CRC16 of strings [tests/utils_001.phpt] 
=====================================================================
TIME END 2016-08-06 03:38:07

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

Number of tests :   27                26
Tests skipped   :    1 (  3.7%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    0 (  0.0%) (  0.0%)
Tests passed    :   26 ( 96.3%) (100.0%)
---------------------------------------------------------------------
Time taken      :    0 seconds
=====================================================================

This report can be automatically sent to the PHP QA team at
http://qa.php.net/reports and http://news.php.net/php.qa.reports
This gives us a better understanding of PHP's behavior.
If you don't want to send the report immediately you can choose
option "s" to save it.  You can then email it to [email protected] later.
Do you want to send this report now? [Yns]: 

Please enter your email address.
(Your address will be mangled so that it will not go out on any
mailinglist in plain text): 

Posting to http://qa.php.net/buildtest-process.php

Thank you for helping to make PHP better.

@skyzyx
Copy link
Author

skyzyx commented Aug 15, 2016

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

@cwhsu1984
Copy link

Hope this commit get merged as soon as possbile because phpiredis does not pass make test without these modifications!

runphp added a commit to runphp/phpiredis that referenced this pull request Nov 16, 2017
This was referenced Nov 16, 2017
@devrck
Copy link

devrck commented Jun 11, 2019

Any updates on this PR ?

@nrk nrk added the hiredis label Aug 26, 2020
@nrk
Copy link
Owner

nrk commented Aug 26, 2020

Better late than never, they say 😅
Thank you @skyzyx, will merge your PR in the next few days!

@gearhead
Copy link
Contributor

FWIW, Arch just updated hiredis to 1.0.0 and this no longer builds a functional package even with the minor name change to 0.14.
x.

@nrk
Copy link
Owner

nrk commented Sep 20, 2020

@gearhead I merged a more recent PR and it builds fine with Hiredis 1.0, I don't have an ArchLinux installation at hand though so could you try building phpiredis from the new v1.1 branch and see if it builds and runs fine?

phpiredis

phpiredis => enabled
phpiredis version => 1.1.0-dev
hiredis version => 1.0.1

EDIT @gearhead I just noticed that the other PR I merged (#65) was actually yours 😃

@nrk nrk closed this Sep 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants