[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension
Github user RobberPhex commented on the issue: https://github.com/apache/thrift/pull/1391 @jeking3 Currently, I just remove PHP5 support at PHP extension(thrift_protocol), PHP library is still support PHP5. So, I think this PR is good. --- If PHP5 support for extension is needed, I will add it back. ---
[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 I posted a message on the developer mailing list about removing php5 support. We'll see what comes of it. php5 is currently still shipping so I have some reservations about removing support for it, but I want to hear if there are objections. If there are, can this fix be modified to deal with both? ---
[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 I already discussed the possibility of retiring the trusty build environment to reduce project maintenance. Maybw now is the time to do it. That said, I would really like to get input from someone else who uses php regularly. @Jens-G perhaps... to ensure this change is acceptable (removing php5 support). I haven't seen any objections in the developer email list but it hasn't been expressly mentioned. ---
[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension
Github user RobberPhex commented on the issue: https://github.com/apache/thrift/pull/1391 at job `Autotools (Ubuntu Trusty)`, it just run `make check`, so php extension cannot be compiled. at job `Cross Language Tests`, it needn't php extension. So, I think there already are some CI problems at php part. ---
[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 Hmm, I'm just curious about this - given support for php5 was dropped, any idea why the ubuntu-trusty "make check" job succeeded (because it has PHP 5.5.9 in it)? Is the build job not actually running any unit tests or anything that would parse (and fail, if it really is incompatible) the code? ---
[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension
Github user RobberPhex commented on the issue: https://github.com/apache/thrift/pull/1391 Hi, about license. `Copyright (C) 2009 Facebook` at file `lib/php/src/ext/thrift_protocol/config.m4` was add by commit f82aee5087bd62989482f5c532cbd80f97a39b7f. But before this commit, the license at [`php_thrift_protocol.cpp`](https://github.com/apache/thrift/blob/f82aee5087bd62989482f5c532cbd80f97a39b7f^/lib/php/src/ext/thrift_protocol/php_thrift_protocol.cpp), [`php_thrift_protocol.h`](https://github.com/apache/thrift/blob/f82aee5087bd62989482f5c532cbd80f97a39b7f^/lib/php/src/ext/thrift_protocol/php_thrift_protocol.h) was Apache License. ---
[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 @jfarrell I see licensing changes in this pull request, can you review? ---
[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 I saw cross test pass before, so I think we're good. ---
[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension
Github user RobberPhex commented on the issue: https://github.com/apache/thrift/pull/1391 build job 4 failed due to https://issues.apache.org/jira/browse/THRIFT-2913. And, appveyor build queued. but, same commit at my repo CI is success: https://ci.appveyor.com/project/RobberPhex/thrift/build/1.0.0-dev.58 ---
[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 UBsan build failed due to https://issues.apache.org/jira/browse/THRIFT-2913. ---
[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension
Github user RobberPhex commented on the issue: https://github.com/apache/thrift/pull/1391 this pr fix [`THRIFT-4356`](https://issues.apache.org/jira/browse/THRIFT-4356) and [`THRIFT-4353`](https://issues.apache.org/jira/browse/THRIFT-4353). --- And, this work is besed `è£è² `'s work. so I just squashed commits by author(not all-in-one commit). ---
[GitHub] thrift issue #1391: Fix segment fault at thrift_protocol extension
Github user jeking3 commented on the issue: https://github.com/apache/thrift/pull/1391 Is this for THRIFT-4353? Can you squash it? Rebase it on master to fix the CI issues. ---
