Skip to content

PHPC-759: Ensure that the driver can be compiled as a built-in PHP extension #830

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.

Sign up for GitHub

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

Merged
merged 10 commits into from
May 15, 2018
Merged

PHPC-759: Ensure that the driver can be compiled as a built-in PHP extension #830

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
94d8c77
PHPC-759: Fix PHP version check for static builds
jmikola May 10, 2018
62894c1
PHPC-759: Use flexible M4 includes for static/shared builds
jmikola May 10, 2018
d01b08b
PHPC-759: Use absolute path for AC_CONFIG_FILES
jmikola May 10, 2018
7d1de0d
Move pthreads CFLAG/LIBS additions to AX_PTHREAD's action-if-found
jmikola May 11, 2018
dd36d74
PHPC-759: Flexible macros for adding sources, includes, and build paths
jmikola May 11, 2018
4500d8d
Remove empty line
jmikola May 11, 2018
c0fc8dd
PHPC-759: Fix pthread linking for static builds
jmikola May 14, 2018
746f92e
Add CFLAGS directly instead of relying on PHP_EVAL_INCLINE
jmikola May 14, 2018
62bb969
Remove redundant PHP_SUBST(EXTRA_LDFLAGS)
jmikola May 14, 2018
e1827ed
PHPC-759: Hide extension config summary for static builds
jmikola May 14, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
170 changes: 101 additions & 69 deletions config.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,23 @@ PHP_ARG_ENABLE([mongodb],
[Enable MongoDB support])])

if test "$PHP_MONGODB" != "no"; then
AC_MSG_CHECKING([Check for supported PHP versions])
PHP_MONGODB_FOUND_VERSION=`${PHP_CONFIG} --version`
PHP_MONGODB_FOUND_VERNUM=`echo "${PHP_MONGODB_FOUND_VERSION}" | $AWK 'BEGIN { FS = "."; } { printf "%d", ([$]1 * 100 + [$]2) * 100 + [$]3;}'`
AC_MSG_RESULT($PHP_MONGODB_FOUND_VERSION)
if test "$PHP_MONGODB_FOUND_VERNUM" -lt "50500"; then
AC_MSG_ERROR([not supported. Need a PHP version >= 5.5.0 (found $PHP_MONGODB_FOUND_VERSION)])
dnl Check PHP version is compatible with this extension
AC_MSG_CHECKING([PHP version])

PHP_MONGODB_PHP_VERSION=$PHP_VERSION
PHP_MONGODB_PHP_VERSION_ID=$PHP_VERSION_ID

if test -z "$PHP_MONGODB_PHP_VERSION"; then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can it sometimes be set, and sometimes not?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test -z checks if the string has non-zero length. For shared builds, PHP_VERSION is never set so PHP_MONGODB_PHP_VERSION will be initialized as an empty string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

if test -z "$PHP_CONFIG"; then
AC_MSG_ERROR([php-config not found])
fi
PHP_MONGODB_PHP_VERSION=`${PHP_CONFIG} --version`
PHP_MONGODB_PHP_VERSION_ID=`echo "${PHP_MONGODB_PHP_VERSION}" | $AWK 'BEGIN { FS = "."; } { printf "%d", ([$]1 * 100 + [$]2) * 100 + [$]3;}'`
fi

AC_MSG_RESULT($PHP_MONGODB_PHP_VERSION)
if test "$PHP_MONGODB_PHP_VERSION_ID" -lt "50500"; then
AC_MSG_ERROR([not supported. Need a PHP version >= 5.5.0 (found $PHP_MONGODB_PHP_VERSION)])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we currently want a check to make sure it's also lower than 7.3?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. In the case of 7.3, we know there's a BC break for iteration that leads to a compile-time error, but jumping between earlier 7.x releases didn't require changes IIRC. I'm fine with that behavior.

Similarly, we don't limit the libbson/libmongoc shared library versions with an upper bound -- provided they satisfy 1.0 ABI. I realize they're more strict about BC than PHP, but it's also possible that behaviors of some of the internal functions we use could change inadvertently :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

fi

PHP_ARG_ENABLE([developer-flags],
Expand Down Expand Up @@ -173,18 +184,18 @@ if test "$PHP_MONGODB" != "no"; then
AC_MSG_CHECKING(for libbson)
if test -x "$PKG_CONFIG" && $PKG_CONFIG --exists libbson-1.0; then
if $PKG_CONFIG libbson-1.0 --atleast-version 1.9.0; then
LIBBSON_INC=`$PKG_CONFIG libbson-1.0 --cflags`
LIBBSON_LIB=`$PKG_CONFIG libbson-1.0 --libs`
LIBBSON_VER=`$PKG_CONFIG libbson-1.0 --modversion`
AC_MSG_RESULT(version $LIBBSON_VER found)
PHP_MONGODB_BSON_CFLAGS=`$PKG_CONFIG libbson-1.0 --cflags`
PHP_MONGODB_BSON_LIBS=`$PKG_CONFIG libbson-1.0 --libs`
PHP_MONGODB_BSON_VERSION=`$PKG_CONFIG libbson-1.0 --modversion`
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These names were changed for consistency with other CheckWhatever.m4 files and the PHP version check up top. I'm all for more "PHP_MONGODB" prefixing.

AC_MSG_RESULT(version $PHP_MONGODB_BSON_VERSION found)
else
AC_MSG_ERROR(system libbson must be upgraded to version >= 1.9.0)
fi
else
AC_MSG_ERROR(pkgconfig and libbson must be installed)
fi
PHP_EVAL_INCLINE($LIBBSON_INC)
PHP_EVAL_LIBLINE($LIBBSON_LIB, MONGODB_SHARED_LIBADD)
PHP_MONGODB_CFLAGS="$PHP_MONGODB_CFLAGS $PHP_MONGODB_BSON_CFLAGS"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the off chance that pkg-config suggests CFLAGS that aren't just include directories, this ensures they get added. I imagine it's unlikely that libbson and the other libraries we use will do so, but I think this is the correct approach given the limitations of PHP_EVAL_INCLINE.

I do think it's OK to continue using PHP_EVAL_LIBLINE, since is less limited. We just require a single work-around for pthreads, which I mention below.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, PHP_EVAL_INCLINE also makes sure there are no duplicates - not sure if that could be a problem here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Not important, if this is broken, people will tell us)

Copy link
Member Author

@jmikola jmikola May 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PHP_EVAL_INCLINE calls PHP_ADD_INCLUDE for each -I path, which calls both PHP_EXPAND_PATH (expanding to an absolute path) and PHP_RUN_ONCE to ensure each path is only added once.

Nearly all of these instances obtain CFLAGS from pkg-config being called for a unique library, so I'm comfortable trusting that their CFLAGS can be added as-is. The only overlap IMO is libmongoc and libbson. Since libmongoc's libmongoc-1.0.pc file depends on libbson-1.0, it ends up including libbson's pkg-config output in its own. To address that, I suppose we could skip merging in libbson's own pkg-config output and just rely on libmongoc. I'd propose making this change today.

One real exception is AX_PTHREAD, which is likely going to return -pthread (ignored by PHP_EVAL_INCLINE entirely) instead of an -I path. For that, I think we certainly need to just append to PHP_MONGODB_CFLAGS.

One other thought. There are more pkg-config options than I realized. Per pkg-config(1), we can request -I CFLAGS and "other" CFLAGS independently, which would let us rely on PHP_EVAL_INCLINE and defer to PHP_MONGODB_CFLAGS when necessary. Similarly, we can requires -L and -l libs independently from "other" linker arguments (which might require manual appends instead of PHP_EVAL_LIBLINE). While this is trivial to add for libbson and libmongoc, since we call pkg-config directly, we also know by looking at their package files that it would have no functional impact. It could be helpful for other libraries (e.g. CheckSSL.m4), but those are all using the pkg.m4 macro at the moment and would require some significant refactoring. I don't think there's a pressing need for this, but it does seem like it'd be the "correct" approach. If it sounds worthwhile, I can make a separate ticket to track the change and we can triage it for some future release.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't worry about it then. I'd defer making a ticket until somebody notices it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created PHPC-1190.

PHP_EVAL_LIBLINE($PHP_MONGODB_BSON_LIBS, MONGODB_SHARED_LIBADD)
AC_DEFINE(HAVE_SYSTEM_LIBBSON, 1, [Use system libbson])
fi

Expand All @@ -197,52 +208,66 @@ if test "$PHP_MONGODB" != "no"; then
AC_MSG_CHECKING(for libmongoc)
if test -x "$PKG_CONFIG" && $PKG_CONFIG --exists libmongoc-1.0; then
if $PKG_CONFIG libmongoc-1.0 --atleast-version 1.9.0; then
LIBMONGOC_INC=`$PKG_CONFIG libmongoc-1.0 --cflags`
LIBMONGOC_LIB=`$PKG_CONFIG libmongoc-1.0 --libs`
LIBMONGOC_VER=`$PKG_CONFIG libmongoc-1.0 --modversion`
AC_MSG_RESULT(version $LIBMONGOC_VER found)

PHP_MONGODB_MONGOC_CFLAGS=`$PKG_CONFIG libmongoc-1.0 --cflags`
PHP_MONGODB_MONGOC_LIBS=`$PKG_CONFIG libmongoc-1.0 --libs`
PHP_MONGODB_MONGOC_VERSION=`$PKG_CONFIG libmongoc-1.0 --modversion`
AC_MSG_RESULT(version $PHP_MONGODB_MONGOC_VERSION found)
else
AC_MSG_ERROR(system libmongoc must be upgraded to version >= 1.9.0)
fi
else
AC_MSG_ERROR(pkgconfig and libmongoc must be installed)
fi
PHP_EVAL_INCLINE($LIBMONGOC_INC)
PHP_EVAL_LIBLINE($LIBMONGOC_LIB, MONGODB_SHARED_LIBADD)
PHP_MONGODB_CFLAGS="$PHP_MONGODB_CFLAGS $PHP_MONGODB_MONGOC_CFLAGS"
PHP_EVAL_LIBLINE($PHP_MONGODB_MONGOC_LIBS, MONGODB_SHARED_LIBADD)
AC_DEFINE(HAVE_SYSTEM_LIBMONGOC, 1, [Use system libmongoc])
fi

if test "$PHP_LIBBSON" = "no" -a "$PHP_LIBMONGOC" = "no"; then
PHP_MONGODB_BSON_CFLAGS="$STD_CFLAGS -DBSON_COMPILATION"
PHP_MONGODB_MONGOC_CFLAGS="$STD_CFLAGS -DMONGOC_COMPILATION -DMONGOC_TRACE"

dnl M4 doesn't know if we're building statically or as a shared module, so
dnl attempt to include both paths while ignoring errors. If neither path
dnl exists, report an error during configure (this is later than M4 parsing
dnl during phpize but better than nothing).
m4_pushdef([_include],[
if test ! \( -f "$1" -o -f "ext/mongodb/$1" \); then
AC_MSG_ERROR([m4 could not include $1: No such file or directory])
fi
m4_builtin([sinclude],[$1])
m4_builtin([sinclude],[ext/mongodb/][$1])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can assume that the extension will be build from this path, since it's the extension's name. That said, users need to manually copy/symlink in order to build statically so I suppose they could inadvertently place us under a different path. If so, the conditional a few lines up isn't going to point out that the ext/FOO path is the problem.

Let me know if you'd like a more helpful error for that case. It may be as simple as adding "ext/mongodb" to the error message, or conditionally printing a special message if $enable_static=yes.

Copy link
Contributor

@derickr derickr May 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should do this, as we don't want to promote compiling the driver as a static extension.

])

dnl Avoid using AC_CONFIG_MACRO_DIR, which might conflict with PHP
m4_include([scripts/build/autotools/m4/ac_compile_check_sizeof.m4])
m4_include([scripts/build/autotools/m4/ac_create_stdint_h.m4])
m4_include([scripts/build/autotools/m4/as_var_copy.m4])
m4_include([scripts/build/autotools/m4/ax_check_compile_flag.m4])
m4_include([scripts/build/autotools/m4/ax_prototype.m4])
m4_include([scripts/build/autotools/m4/ax_pthread.m4])
m4_include([scripts/build/autotools/m4/pkg.m4])

m4_include([scripts/build/autotools/CheckCompiler.m4])
m4_include([scripts/build/autotools/CheckHost.m4])

m4_include([scripts/build/autotools/libbson/CheckAtomics.m4])
m4_include([scripts/build/autotools/libbson/CheckHeaders.m4])
m4_include([scripts/build/autotools/libbson/Endian.m4])
m4_include([scripts/build/autotools/libbson/FindDependencies.m4])
m4_include([scripts/build/autotools/libbson/Versions.m4])

m4_include([scripts/build/autotools/libmongoc/CheckCompression.m4])
m4_include([scripts/build/autotools/libmongoc/CheckResolv.m4])
m4_include([scripts/build/autotools/libmongoc/CheckSasl.m4])
m4_include([scripts/build/autotools/libmongoc/CheckSSL.m4])
m4_include([scripts/build/autotools/libmongoc/FindDependencies.m4])
m4_include([scripts/build/autotools/libmongoc/PlatformFlags.m4])
m4_include([scripts/build/autotools/libmongoc/Versions.m4])
m4_include([scripts/build/autotools/libmongoc/WeakSymbols.m4])
_include([scripts/build/autotools/m4/ac_compile_check_sizeof.m4])
_include([scripts/build/autotools/m4/ac_create_stdint_h.m4])
_include([scripts/build/autotools/m4/as_var_copy.m4])
_include([scripts/build/autotools/m4/ax_check_compile_flag.m4])
_include([scripts/build/autotools/m4/ax_prototype.m4])
_include([scripts/build/autotools/m4/ax_pthread.m4])
_include([scripts/build/autotools/m4/php_mongodb.m4])
_include([scripts/build/autotools/m4/pkg.m4])

_include([scripts/build/autotools/CheckCompiler.m4])
_include([scripts/build/autotools/CheckHost.m4])

_include([scripts/build/autotools/libbson/CheckAtomics.m4])
_include([scripts/build/autotools/libbson/CheckHeaders.m4])
_include([scripts/build/autotools/libbson/Endian.m4])
_include([scripts/build/autotools/libbson/FindDependencies.m4])
_include([scripts/build/autotools/libbson/Versions.m4])

_include([scripts/build/autotools/libmongoc/CheckCompression.m4])
_include([scripts/build/autotools/libmongoc/CheckResolv.m4])
_include([scripts/build/autotools/libmongoc/CheckSasl.m4])
_include([scripts/build/autotools/libmongoc/CheckSSL.m4])
_include([scripts/build/autotools/libmongoc/FindDependencies.m4])
_include([scripts/build/autotools/libmongoc/PlatformFlags.m4])
_include([scripts/build/autotools/libmongoc/Versions.m4])
_include([scripts/build/autotools/libmongoc/WeakSymbols.m4])

m4_popdef([_include])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just cleaning up after ourselves so not to pollute the global M4 namespace. This should also be flexible in the event that some other extension (or PHP) defines an _include macro.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or also prefix it with php_mongodb_ ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently there is no risk of conflicting with an _include macro defined outside of this push/pop scope. The only risk would be if one of the M4 files we include defined its own _include macro without also using push/pop and overwrote our own.

PHP_MONGODB_ADD_SOURCES and friends can't use push/pop (reasonably) since we define them in an included file and call them later on in config.m4. Prefixing makes sense there to be a good citizen, but I don't think prefixing _include would add anything here (apart from verbosity).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK


AC_SUBST(BSON_EXTRA_ALIGN, 0)
AC_SUBST(BSON_OS, 1)
Expand Down Expand Up @@ -271,42 +296,40 @@ if test "$PHP_MONGODB" != "no"; then
dnl Generated with: find src/libmongoc/src/zlib-1.2.11 -maxdepth 1 -name '*.c' -print0 | cut -sz -d / -f 5- | sort -z | tr '\000' ' '
PHP_MONGODB_ZLIB_SOURCES="adler32.c compress.c crc32.c deflate.c gzclose.c gzlib.c gzread.c gzwrite.c infback.c inffast.c inflate.c inftrees.c trees.c uncompr.c zutil.c"

PHP_ADD_SOURCES_X(PHP_EXT_DIR(mongodb)[src/libbson/src/bson], $PHP_MONGODB_BSON_SOURCES, $PHP_MONGODB_BSON_CFLAGS, shared_objects_mongodb, yes)
PHP_ADD_SOURCES_X(PHP_EXT_DIR(mongodb)[src/libbson/src/jsonsl], $PHP_MONGODB_JSONSL_SOURCES, $PHP_MONGODB_BSON_CFLAGS, shared_objects_mongodb, yes)
PHP_ADD_SOURCES_X(PHP_EXT_DIR(mongodb)[src/libmongoc/src/mongoc], $PHP_MONGODB_MONGOC_SOURCES, $PHP_MONGODB_MONGOC_CFLAGS, shared_objects_mongodb, yes)
PHP_MONGODB_ADD_SOURCES([src/libbson/src/bson/], $PHP_MONGODB_BSON_SOURCES, $PHP_MONGODB_BSON_CFLAGS)
PHP_MONGODB_ADD_SOURCES([src/libbson/src/jsonsl/], $PHP_MONGODB_JSONSL_SOURCES, $PHP_MONGODB_BSON_CFLAGS)
PHP_MONGODB_ADD_SOURCES([src/libmongoc/src/mongoc/], $PHP_MONGODB_MONGOC_SOURCES, $PHP_MONGODB_MONGOC_CFLAGS)

PHP_ADD_INCLUDE(PHP_EXT_SRCDIR(mongodb)[/src/libbson/src/])
PHP_ADD_INCLUDE(PHP_EXT_SRCDIR(mongodb)[/src/libbson/src/bson/])
PHP_ADD_INCLUDE(PHP_EXT_SRCDIR(mongodb)[/src/libbson/src/jsonsl/])
PHP_ADD_INCLUDE(PHP_EXT_SRCDIR(mongodb)[/src/libmongoc/src/mongoc/])
PHP_MONGODB_ADD_INCLUDE([src/libbson/src/])
PHP_MONGODB_ADD_INCLUDE([src/libbson/src/bson/])
PHP_MONGODB_ADD_INCLUDE([src/libbson/src/jsonsl/])
PHP_MONGODB_ADD_INCLUDE([src/libmongoc/src/mongoc/])

PHP_ADD_BUILD_DIR(PHP_EXT_BUILDDIR(mongodb)[/src/libbson/src/jsonsl/])
PHP_ADD_BUILD_DIR(PHP_EXT_BUILDDIR(mongodb)[/src/libbson/src/bson/])
PHP_ADD_BUILD_DIR(PHP_EXT_BUILDDIR(mongodb)[/src/libmongoc/src/mongoc/])
PHP_MONGODB_ADD_BUILD_DIR([src/libbson/src/bson/])
PHP_MONGODB_ADD_BUILD_DIR([src/libbson/src/jsonsl/])
PHP_MONGODB_ADD_BUILD_DIR([src/libmongoc/src/mongoc/])

dnl TODO: Use $ext_srcdir if we can move this after PHP_NEW_EXTENSION
ac_config_dir=PHP_EXT_SRCDIR(mongodb)

AC_CONFIG_FILES([
src/libbson/src/bson/bson-config.h
src/libbson/src/bson/bson-version.h
src/libmongoc/src/mongoc/mongoc-config.h
src/libmongoc/src/mongoc/mongoc-version.h
${ac_config_dir}/src/libbson/src/bson/bson-config.h
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my testing, I wasn't able to use PHP_EXT_SRCDIR(mongodb) inside of AC_CONFIG_FILES, as it seems to care about its literal arguments to complain if it's invoked twice with the same parameters.

${ac_config_dir}/src/libbson/src/bson/bson-version.h
${ac_config_dir}/src/libmongoc/src/mongoc/mongoc-config.h
${ac_config_dir}/src/libmongoc/src/mongoc/mongoc-version.h
])

if test "x$bundled_zlib" = "xyes"; then
PHP_ADD_SOURCES_X(PHP_EXT_DIR(mongodb)[src/libmongoc/src/zlib-1.2.11], $PHP_MONGODB_ZLIB_SOURCES, $PHP_MONGODB_MONGOC_CFLAGS, shared_objects_mongodb, yes)
PHP_ADD_INCLUDE(PHP_EXT_SRCDIR(mongodb)[/src/libmongoc/src/zlib-1.2.11/])
PHP_ADD_BUILD_DIR(PHP_EXT_BUILDDIR(mongodb)[/src/libmongoc/src/zlib-1.2.11/])
AC_CONFIG_FILES(src/libmongoc/src/zlib-1.2.11/zconf.h)
PHP_MONGODB_ADD_SOURCES([src/libmongoc/src/zlib-1.2.11/], $PHP_MONGODB_ZLIB_SOURCES, $PHP_MONGODB_MONGOC_CFLAGS)
PHP_MONGODB_ADD_INCLUDE([src/libmongoc/src/zlib-1.2.11/])
PHP_MONGODB_ADD_BUILD_DIR([src/libmongoc/src/zlib-1.2.11/])
AC_CONFIG_FILES([${ac_config_dir}/src/libmongoc/src/zlib-1.2.11/zconf.h])
fi

dnl Apply any CFLAGS and LIBS from libbson calling AX_PTHREAD
PHP_EVAL_INCLINE([$PTHREAD_CFLAGS])
PHP_EVAL_LIBLINE([$PTHREAD_LIBS],[MONGODB_SHARED_LIBADD])
fi

PHP_NEW_EXTENSION(mongodb, $PHP_MONGODB_SOURCES, $ext_shared,, $PHP_MONGODB_CFLAGS)

PHP_SUBST(MONGODB_SHARED_LIBADD)
PHP_SUBST(EXTRA_LDFLAGS)

PHP_ADD_EXTENSION_DEP(mongodb, date)
PHP_ADD_EXTENSION_DEP(mongodb, json)
Expand All @@ -325,10 +348,17 @@ if test "$PHP_MONGODB" != "no"; then
PHP_ADD_BUILD_DIR(PHP_EXT_BUILDDIR(mongodb)[/src/MongoDB/Monitoring/])
PHP_ADD_BUILD_DIR(PHP_EXT_BUILDDIR(mongodb)[/src/contrib/])

dnl Necessary to ensure that static builds include "-pthread" when linking
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you decide that --coverage is a relevant option for static builds, I can amend this comment. Otherwise, I believe this only applies to -pthread.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I said in the other PR: I don't think it's relevant.

if test "$ext_shared" != "yes"; then
EXTRA_LDFLAGS_PROGRAM="$EXTRA_LDFLAGS_PROGRAM $EXTRA_LDFLAGS"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I took a look at the Makefile produced when building PHP statically and found that EXTRA_LDFLAGS was ignored when making the actual SAPI binaries. I'm not sure why that is the case, but EXTRA_LDFLAGS_PROGRAM is actually included in the linking for all SAPI binaries.

We're adding $EXTRA_LDFLAGS here because that's going to be set when detecting pthreads (see my next comment in FindDependencies.m4).

fi

dnl This must come after PHP_NEW_EXTENSION, otherwise the srcdir won't be set
PHP_ADD_MAKEFILE_FRAGMENT

AC_CONFIG_COMMANDS_POST([echo "
AC_CONFIG_COMMANDS_POST([
if test "$enable_static" = "no"; then
echo "
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AC_CONFIG_COMMANDS_POST emits no immediate output in the configure file and instead queues up output for the end of configure generation (after config.status). Therefore, any logic needs to go in its body.

I opted not to use $ext_shared here since this won't actually be executed anywhere near our extension's own configuration and I was worried that the logic might use the $ext_shared value from the very last extension that was configured. $enable_static seemed more reliable as it's always no for phpize shared builds and it appears to always be yes when building PHP itself.

mongodb was configured with the following options:

Build configuration:
Expand All @@ -345,7 +375,9 @@ Build configuration:
Please submit bugreports at:
https://jira.mongodb.org/browse/PHPC

"])
"
fi
])
fi

dnl: vim: et sw=2
12 changes: 10 additions & 2 deletions scripts/build/autotools/libbson/FindDependencies.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,16 @@ AC_CHECK_FUNC(rand_r, [AC_SUBST(BSON_HAVE_RAND_R, 1)], [], [#include <stdlib.h>]
# Check for pthreads. We might need to make this better to handle mingw,
# but I actually think it is okay to just check for it even though we will
# use win32 primatives.
AX_PTHREAD([],
[AC_MSG_ERROR([libbson requires pthreads on non-Windows platforms.])])
AX_PTHREAD([
PHP_MONGODB_BSON_CFLAGS="$PHP_MONGODB_BSON_CFLAGS $PTHREAD_CFLAGS"
PHP_EVAL_LIBLINE([$PTHREAD_LIBS],[MONGODB_SHARED_LIBADD])

# PTHREAD_CFLAGS may come back as "-pthread", which should also be used when
# linking. We can trust PHP_EVAL_LIBLINE to ignore other values.
PHP_EVAL_LIBLINE([$PTHREAD_CFLAGS],[MONGODB_SHARED_LIBADD])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an odd edge case.

PHP_EVAL_LIBLINE has a special case where if it sees -pthread (instead of the usual -lfoo) and the build is static, it adds it to EXTRA_LDFLAGS instead of MONGODB_SHARED_LIBADD. For whatever reason, AX_PTHREAD's own tests doesn't determine that -pthread is required as a link flag, so it only reports it in CFLAGS. Without this, we'll get symbol errors when linking libbson object files in static builds.

Note: this goes hand in hand with our logic in config.m4 to ensure EXTRA_LDFLAGS is merged into EXTRA_LDFLAGS_PROGRAM for static builds.

],[
AC_MSG_ERROR([libbson requires pthreads on non-Windows platforms.])
])


# The following is borrowed from the guile configure script.
Expand Down
4 changes: 2 additions & 2 deletions scripts/build/autotools/libmongoc/CheckCompression.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ found_zlib="no"
bundled_zlib="no"

PKG_CHECK_MODULES([PHP_MONGODB_SNAPPY],[snappy],[
PHP_EVAL_INCLINE([$PHP_MONGODB_SNAPPY_CFLAGS])
PHP_MONGODB_MONGOC_CFLAGS="$PHP_MONGODB_MONGOC_CFLAGS $PHP_MONGODB_SNAPPY_CFLAGS"
PHP_EVAL_LIBLINE([$PHP_MONGODB_SNAPPY_LIBS],[MONGODB_SHARED_LIBADD])
found_snappy="yes"
],[
Expand All @@ -23,7 +23,7 @@ PKG_CHECK_MODULES([PHP_MONGODB_SNAPPY],[snappy],[
])

PKG_CHECK_MODULES([PHP_MONGODB_ZLIB],[zlib],[
PHP_EVAL_INCLINE([$PHP_MONGODB_ZLIB_CFLAGS])
PHP_MONGODB_MONGOC_CFLAGS="$PHP_MONGODB_MONGOC_CFLAGS $PHP_MONGODB_ZLIB_CFLAGS"
PHP_EVAL_LIBLINE([$PHP_MONGODB_ZLIB_LIBS],[MONGODB_SHARED_LIBADD])
found_zlib="yes"
],[
Expand Down
4 changes: 2 additions & 2 deletions scripts/build/autotools/libmongoc/CheckSSL.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ AS_IF([test "$PHP_MONGODB_SSL" = "openssl" -o "$PHP_MONGODB_SSL" = "auto"],[
found_openssl="no"

PKG_CHECK_MODULES([PHP_MONGODB_SSL],[openssl],[
PHP_EVAL_INCLINE([$PHP_MONGODB_SSL_CFLAGS])
PHP_MONGODB_MONGOC_CFLAGS="$PHP_MONGODB_MONGOC_CFLAGS $PHP_MONGODB_SSL_CFLAGS"
PHP_EVAL_LIBLINE([$PHP_MONGODB_SSL_LIBS],[MONGODB_SHARED_LIBADD])
PHP_MONGODB_SSL="openssl"
found_openssl="yes"
Expand Down Expand Up @@ -115,7 +115,7 @@ AS_IF([test "$PHP_MONGODB_SSL" = "libressl" -o "$PHP_MONGODB_SSL" = "auto"],[
found_libressl="no"

PKG_CHECK_MODULES([PHP_MONGODB_SSL],[libtls libcrypto],[
PHP_EVAL_INCLINE([$PHP_MONGODB_SSL_CFLAGS])
PHP_MONGODB_MONGOC_CFLAGS="$PHP_MONGODB_MONGOC_CFLAGS $PHP_MONGODB_SSL_CFLAGS"
PHP_EVAL_LIBLINE([$PHP_MONGODB_SSL_LIBS],[MONGODB_SHARED_LIBADD])
PHP_MONGODB_SSL="libressl"
found_libressl="yes"
Expand Down
4 changes: 2 additions & 2 deletions scripts/build/autotools/libmongoc/CheckSasl.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ AS_IF([test "$PHP_MONGODB_SASL" = "cyrus" -o "$PHP_MONGODB_SASL" = "auto"],[
found_cyrus="no"

PKG_CHECK_MODULES([PHP_MONGODB_SASL],[libsasl2],[
PHP_EVAL_INCLINE([$PHP_MONGODB_SASL_CFLAGS])
PHP_MONGODB_MONGOC_CFLAGS="$PHP_MONGODB_MONGOC_CFLAGS $PHP_MONGODB_SASL_CFLAGS"
PHP_EVAL_LIBLINE([$PHP_MONGODB_SASL_LIBS],[MONGODB_SHARED_LIBADD])
PHP_MONGODB_SASL="cyrus"
found_cyrus="yes"
Expand Down Expand Up @@ -49,7 +49,7 @@ AS_IF([test "$PHP_MONGODB_SASL" = "gssapi" -o "$PHP_MONGODB_SASL" = "auto"],[
found_gssapi="no"

PKG_CHECK_MODULES([PHP_MONGODB_SASL],[krb5-gssapi],[
PHP_EVAL_INCLINE([$PHP_MONGODB_SASL_CFLAGS])
PHP_MONGODB_MONGOC_CFLAGS="$PHP_MONGODB_MONGOC_CFLAGS $PHP_MONGODB_SASL_CFLAGS"
PHP_EVAL_LIBLINE([$PHP_MONGODB_SASL_LIBS],[MONGODB_SHARED_LIBADD])
PHP_MONGODB_SASL="gssapi"
found_gssapi="yes"
Expand Down
15 changes: 13 additions & 2 deletions scripts/build/autotools/libmongoc/FindDependencies.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,16 @@ AC_CHECK_TYPE([socklen_t],
[AC_SUBST(MONGOC_HAVE_SOCKLEN, 0)],
[#include <sys/socket.h>])

dnl libbson already requires pthreads, so skip a redundant, optional check here
dnl AX_PTHREAD
# Check for pthreads. libmongoc's original FindDependencies.m4 script did not
# require pthreads, but it does appear to be necessary on non-Windows platforms
# based on mongoc-openssl.c and mongoc-thread-private.h.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why libmongoc always considered pthreads optional, but they are certainly assumed to exist in these files. It's possible that this was an edge case and the dependency was satisfied by also linking libbson successfully.

In any event, I think this is correct and I'm not going to both libmongoc about it since the M4 file in question no longer exists in their repository.

AX_PTHREAD([
PHP_MONGODB_MONGOC_CFLAGS="$PHP_MONGODB_MONGOC_CFLAGS $PTHREAD_CFLAGS"
PHP_EVAL_LIBLINE([$PTHREAD_LIBS],[MONGODB_SHARED_LIBADD])

# PTHREAD_CFLAGS may come back as "-pthread", which should also be used when
# linking. We can trust PHP_EVAL_LIBLINE to ignore other values.
PHP_EVAL_LIBLINE([$PTHREAD_CFLAGS],[MONGODB_SHARED_LIBADD])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: PHP uses PHP_RUN_ONCE when adding -pthread, so we don't end up with duplicates from the libbson check done earlier. That said, I don't think it'd be problematic even if we did.

],[
AC_MSG_ERROR([libmongoc requires pthreads on non-Windows platforms.])
])
51 changes: 51 additions & 0 deletions scripts/build/autotools/m4/php_mongodb.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
dnl
dnl PHP_MONGODB_ADD_SOURCES(source-path, sources [, special-flags])
dnl
dnl Adds sources which are located relative to source-path. source-path should
dnl be relative to the extension directory (i.e. PHP_EXT_DIR). special-flags
dnl will be passed to the compiler.
dnl
dnl This macro will call PHP_ADD_SOURCES or PHP_ADD_SOURCES_X depending on
dnl whether the extension is being built statically or as a shared module.
dnl
AC_DEFUN([PHP_MONGODB_ADD_SOURCES],[
_src_path=PHP_EXT_DIR(mongodb)

dnl Join extension directory and source path
case $_src_path in
""[)] _src_path="$1" ;;
*/[)] _src_path="$_src_path$1" ;;
*[)] _src_path="$_src_path/$1" ;;
esac

dnl Trim trailing slash from source path
case $_src_path in
*/[)] _src_path=${_src_path%?}
esac

if test "$ext_shared" = "no"; then
PHP_ADD_SOURCES($_src_path, [$2], [$3])
else
PHP_ADD_SOURCES_X($_src_path, [$2], [$3], shared_objects_mongodb, yes)
fi
])

dnl
dnl PHP_MONGODB_ADD_INCLUDE(path)
dnl
dnl Adds an include path relative to the extension source directory (i.e.
dnl PHP_EXT_SRCDIR).
dnl
AC_DEFUN([PHP_MONGODB_ADD_INCLUDE],[
PHP_ADD_INCLUDE(PHP_EXT_SRCDIR(mongodb)[/][$1])
])

dnl
dnl PHP_MONGODB_ADD_BUILD_DIR(path)
dnl
dnl Adds a build directory relative to the extension build directory (i.e.
dnl PHP_EXT_BUILDDIR).
dnl
AC_DEFUN([PHP_MONGODB_ADD_BUILD_DIR],[
PHP_ADD_BUILD_DIR(PHP_EXT_BUILDDIR(mongodb)[/][$1])
])