php / pecl-networking-ssh2

Bindings for the libssh2 library
http://pecl.php.net/package/ssh2
Other
51 stars 58 forks source link

PHP 7 Support #7

Closed Sean-Der closed 8 years ago

Sean-Der commented 9 years ago

Hi!

This is a WIP branch for PHP 7 support. Currently all tests pass, but I still have some memory leaks I haven't addressed yet. I also need to mark a few spots that I have questions on. Would love some eyes on this to catch issues early on, thank you!

After that I am going to take swing fixing all the issues that show up with -Wall

langemeijer commented 9 years ago

There is good work in this I can see, but there is one big issue: We need the ssh2 extension to compile to both php 5.6 and php 7.

Sean-Der commented 9 years ago

Hey @langemeijer Sorry for the late reply, not very hip with the new stuff...

So, what do you think about maintaining two branches? There is a fair amount of preprocessor stuff that we could rip out, and could take a chance to clean up the code base (-Wall found some stuff) This extension is also really stable, and hopefully we could just cherry-pick commits back and forth with minor difficulties.

I have been going this route with other extensions, but agreed to explore coming back and doing ZEND_ENGINE 2+3 compatibility with some maintainers.

Is it possible to push releases to pecl and prefix for versions of PHP? Does pickle make things any easier?

thanks!

langemeijer commented 9 years ago

I don't mind maintaining two branches, but the underlying infrastructure for packaging is not really prepared for it. I'm primarily thinking of PECL in this respect, but linux distros will also have trouble doing packaging correctly (although they have it easier, as they pin on a PHP version anyway and just need to be told to use another branch)

Traditionally we've always used the preprocessor stuff, and that has always worked, but has caused massive clutter. We probably still support PHP4 in ssh2, and that's just sad.

Unless someone fixes PECL I see no way to get branches working. I think conditional compilation is the easiest way to go.

langemeijer commented 9 years ago

Have you see https://github.com/bukka/phpc ?

sergeyklay commented 8 years ago

Any updates?

Sean-Der commented 8 years ago

Hey @sergeyklay !

This should work with PHP 7 right now, are you having issues with it?

@langemeijer sorry for missing your follow up. For yaml/msgpack we ended up doing a PHP 7 only release, and it has worked out well.. so far! I feel bad, but I just don't have the time/motivation to make this work for PHP 5. I just use PHP 7 (and have other PHP work that is nagging me).

I am totally happy if you final decision is a single branch, I can open another PR starting that. I think the extension would/should go through major refactors though (drop all the ZTS hackery) so it may not be a small undertaking.

thanks

langemeijer commented 8 years ago

Hi Sean,

I was at my local coding conference, listening to this awesome talk by http://conference.domcode.org/talks.html#phintjens the talk is recorded, should be online soon.

We have to merge. You've done an awfull lot of work.

I think I found a way around the pecl problem: we make the new PHP7 pecl package an alpha release, the current one is a beta. To install the PHP7 package you would need to install ssh2-alpha , php5 ssh2-beta.

I will merge later today branching the current master into a php5 branch, the php7 code should live in master. Any news on the memory leaks you mentioned in the first comment?

Sean-Der commented 8 years ago

@langemeijer The talk sounds really interesting! I am not so great with thinking ahead when it comes to OpenSource, if something excites me/I want it done I do it. However, I have created a fair amount of abandoned code that I could have done a better job of owning/managing. I get pulled away before finishing stuff.

There are some datatype + memory leak issues that I never addressed (and I only realized some errors as I spend more time doing php-src work!) I am going to fix this tonight, and will comment when it is 'good enough to merge'.

I would really like to improve the testing situation. In a perfect world.

Also, are you in any IRC/Slack channels for pecl stuff? I idle on efnet, and if you are waiting for me on anything would love to give you a better way to poke me.

thanks

Sean-Der commented 8 years ago

Hey @langemeijer !

https://github.com/Sean-Der/pecl-networking-ssh2/commit/9e77ed14cb55ceaa5445ef2f4ee6722384a878ba fixed the issues I was referring to. IMO it is good enough for a beta (I would like to hit everything in my TODO list above before I feel comfortable enough with a wide release and I need to cleanup comments+noop stuff from PHP 5)

thanks

sergeyklay commented 8 years ago

:+1:

acasademont commented 8 years ago

Hi @Sean-Der thanks for the work! Could it be possible to realease another PECL tag? Right now we have to compile the extension manually. Thanks!

langemeijer commented 8 years ago

A pecl release will follow in a few weeks.

On Thu, 17 Mar 2016 09:42 Albert Casademont, notifications@github.com wrote:

Hi @Sean-Der https://github.com/Sean-Der thanks for the work! Could it be possible to realease another PECL tag? Right now we have to compile the extension manually. Thanks!

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/php/pecl-networking-ssh2/pull/7#issuecomment-197765453

Jean85 commented 8 years ago

Bump. Still no PECL release? Are there any remaining bugs?

Sohorev commented 8 years ago

wait for pecl +1

peteward commented 8 years ago

indeed, quite desperate for a PECL release too!

gonzalovilaseca commented 8 years ago

Same here!

langemeijer commented 8 years ago

Two PECL releases have been made. 0.13 (stable) is the package as built from the php5 branch 1.0 (alpha) is the package as built from the master branch