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

Cannot build 3.x releases for php 5.6 in gcc 5.4.0 due to use of "inline" #63

Open
TysonAndre opened this issue Feb 6, 2020 · 4 comments

Comments

@TysonAndre
Copy link
Contributor

TysonAndre commented Feb 6, 2020

The patch 82480df was applied to the latest releases, but never backported to 3.x for php 5.

From #49 , I'm not sure whether or not there are plans to continue supporting the 3.x release line. I didn't find any related issues.

  • Ideally, a 3.0.9 release would be available to fix the uses of inline (extern inline in functions declared in headers?)
  • https://github.com/TysonAndre/pecl-memcache/pull/1/files includes the uses of inline that needed to be changed (extern inline would probably also work, but I didn't test that). mmc_prepare_key_ex is used from 2 different files.

https://gcc.gnu.org/gcc-5/porting_to.html mentions that the way inline was compiled changed, and that extern inline should be used instead (see 82480df)

Motivation: Give projects still using the "memcache" PECL in 5.x a way to update and test in php 5.6 (a newer compiler may be needed on newer OSes) then to php 7.x

@tomassrnka
Copy link
Member

Hi,

I understand your motivation here. If you provide a patch that fixes builds for the extension for 3.x releases (PHP 5.x) with newer GCC, I will be glad to include them and do a new release.

TysonAndre pushed a commit to TysonAndre/pecl-memcache that referenced this issue Feb 7, 2020
Note that the memcache-3.0.8 git branch is a completely different history from
the RELEASE_3.0.6 git tag.

Tests are passing locally with gcc 5.4.0 and php 5.6.40 on NTS non-debug.
(with and without Valgrind)
100b was flaky in valgrind due to timing.

Tests also pass in ZTS debug.

For websupport-sk#63
TysonAndre pushed a commit to TysonAndre/pecl-memcache that referenced this issue Feb 7, 2020
Note that the memcache-3.0.8 git branch is a completely different history from
the RELEASE_3.0.6 git tag.

Tests are passing locally with gcc 5.4.0 and php 5.6.40 on NTS non-debug.
(with and without Valgrind)
100b was flaky in valgrind due to timing.

Tests also pass in ZTS debug with gcc 5.4.0 or gcc 9.2.0.

For websupport-sk#63
@TysonAndre
Copy link
Contributor Author

TysonAndre commented Feb 7, 2020

Hi - I'd suggest creating a 3.0.9 release based on TysonAndre@10dd0df . Feel free to amend the release notes and dates as needed, or take out the test helper files.
(e.g. fetch that commit, push a memcache-3.0.9 or memcache-3 branch, and tag a release based on the branch)

This was created based on the memcache-3.0.8 branch found in https://github.com/websupport-sk/pecl-memcache - the commits included are https://github.com/TysonAndre/pecl-memcache/commits/10dd0df13d496c1cd5e5be49dcfbffccb371c39a

  • I suggest those steps instead of me creating a PR since I don't see a memcache-3 git branch to use as a target branch.

@TysonAndre
Copy link
Contributor Author

@tomassrnka - are there any issues that need to be addressed in the linked patch?

@ryandesign
Copy link

Even with the proposed patch, clang (Apple clang version 12.0.0 (clang-1200.0.32.29)) in its default gnu99 mode emits these warnings when building 3.0.8:

./memcache_pool.h:401:21: warning: inline function 'mmc_prepare_key_ex' is not defined [-Wundefined-inline]
./memcache_queue.h:49:23: warning: inline function 'mmc_queue_push' is not defined [-Wundefined-inline]
./memcache_queue.h:52:23: warning: inline function 'mmc_queue_free' is not defined [-Wundefined-inline]
./memcache_pool.h:145:22: warning: inline function 'mmc_buffer_alloc' is not defined [-Wundefined-inline]
./memcache_queue.h:53:23: warning: inline function 'mmc_queue_copy' is not defined [-Wundefined-inline]
./memcache_queue.h:52:23: warning: inline function 'mmc_queue_free' is not defined [-Wundefined-inline]
./memcache_queue.h:49:23: warning: inline function 'mmc_queue_push' is not defined [-Wundefined-inline]
./memcache_pool.h:145:22: warning: inline function 'mmc_buffer_alloc' is not defined [-Wundefined-inline]
./memcache_pool.h:146:22: warning: inline function 'mmc_buffer_free' is not defined [-Wundefined-inline]
./memcache_queue.h:52:23: warning: inline function 'mmc_queue_free' is not defined [-Wundefined-inline]
./memcache_queue.h:54:23: warning: inline function 'mmc_queue_remove' is not defined [-Wundefined-inline]
./memcache_queue.h:49:23: warning: inline function 'mmc_queue_push' is not defined [-Wundefined-inline]
./memcache_queue.h:50:24: warning: inline function 'mmc_queue_pop' is not defined [-Wundefined-inline]
./memcache_queue.h:51:22: warning: inline function 'mmc_queue_contains' is not defined [-Wundefined-inline]
./memcache_queue.h:53:23: warning: inline function 'mmc_queue_copy' is not defined [-Wundefined-inline]
./memcache_pool.h:401:21: warning: inline function 'mmc_prepare_key_ex' is not defined [-Wundefined-inline]
./memcache_queue.h:49:23: warning: inline function 'mmc_queue_push' is not defined [-Wundefined-inline]
./memcache_pool.h:402:21: warning: inline function 'mmc_prepare_key' is not defined [-Wundefined-inline]

Nevertheless the build succeeds, though I don't know if the module it built works.

If I use -std=gnu89 in CFLAGS the warnings go away.

Looks like the bug was already fixed in memcache 4.x by removing MMC_POOL_INLINE and MMC_QUEUE_INLINE before the function declarations in the headers in c051ca7. Perhaps this and the fix for redefinition of htonll and ntohll can be included in 3.0.9 too.

The remaining changes in that commit relating to constness are not standalone; they depend on other constness changes made in fa69d6a and would only be applicable to php 5.6.x (looks like php 5.5.x and earlier didn't get constified) so perhaps they don't need to be backported. The change from long to zend_long seems to be a php 7 thing that would not be backported.

While we're at it, fc76c47 would be good to backport too.

Here are the patches I've added to MacPorts with 3.0.8:

https://github.com/macports/macports-ports/tree/d447a0707b5a4a6326e74968a2ae996c4d081989/php/php-memcache/files

With those patches, I get no warnings building 3.0.8 for php 5.3.x through 5.5.x and only the constness warnings with 5.6.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants