Skip to content

Follow ups to bundled/system libmaxminddb reporting #291

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
oschwald merged 6 commits into from
Sep 14, 2026
Merged

Follow ups to bundled/system libmaxminddb reporting #291

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
382ad9d
Append the libmaxminddb source to the version row in one cell
horgh Sep 14, 2026
845eb09
Describe the bundled macro rather than the option that sets it
horgh Sep 14, 2026
7052ab8
Define the bundled macro on every path and test it with #if
horgh Sep 14, 2026
58f2bda
Say why the bundled macro exists and how it reaches maxminddb.c
horgh Sep 14, 2026
c151c7e
Add a changelog entry for the phpinfo() libmaxminddb source row
horgh Sep 14, 2026
8ad6b04
Correct the claim that no Windows workflow checks config.w32
horgh Sep 14, 2026
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
CHANGELOG
=========

1.15.0
-------------------

* `phpinfo()` and `php --ri maxminddb` now show whether the extension was built
with the bundled libmaxminddb or links a system library. The
`libmaxminddb library version` row reads, for example, `1.14.0 (bundled)` or
`1.9.1 (system)`. Pull request by Remi Collet. GitHub #289.

1.14.0 (2026-09-10)
-------------------

Expand Down
9 changes: 8 additions & 1 deletion ext/config.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,15 @@ if test $PHP_MAXMINDDB != "no"; then

maxminddb_sources="$maxminddb_sources libmaxminddb/src/maxminddb.c libmaxminddb/src/data-pool.c"

AC_DEFINE([HAVE_LIBMAXMINDDB_BUNDLED], [1], [Use bundled or system libmaxminddb])
dnl phpinfo() reports this. It is 1 here and 0 in the system branch,
dnl never absent, so maxminddb.c can tell a system build from a
dnl definition that never arrived and refuse to compile. config.w32
dnl says why that is a real risk on Windows.
AC_DEFINE([HAVE_LIBMAXMINDDB_BUNDLED], [1], [1 if the bundled libmaxminddb is compiled into the extension, 0 if a system libmaxminddb is linked])
else
dnl See the bundled branch above for why this is 0 rather than absent.
AC_DEFINE([HAVE_LIBMAXMINDDB_BUNDLED], [0], [1 if the bundled libmaxminddb is compiled into the extension, 0 if a system libmaxminddb is linked])

AC_PATH_PROG(PKG_CONFIG, pkg-config, no)

AC_MSG_CHECKING(for libmaxminddb)
Expand Down
31 changes: 24 additions & 7 deletions ext/config.w32
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ if (PHP_MAXMINDDB == "yes") {
ERROR("--with-maxminddb-bundled needs the bundled libmaxminddb sources; run \"git submodule update --init\"");
}

/* Read the version out of the submodule rather than repeating it here.
* Nothing else checks this file -- there is no Windows workflow in this
* repository -- so a bump that updated config.m4 and forgot this line
* would ship a PACKAGE_VERSION that lies, with nothing to notice it.
* config.m4 keeps a literal because test-bundled.yml compares
* MMDB_LIB_VERSION against configure.ac on every build. */
/* Read the version out of the submodule so a bump cannot leave this
* line behind. The Unix build keeps a literal in
* ext/bundled-include/maxminddb_config.h instead; that header says
* why. The bundled and windows-bundled jobs in test-bundled.yml check
* both by comparing MMDB_LIB_VERSION against configure.ac on every
* build. */
var maxminddb_ac = file_get_contents(maxminddb_bundled + "\\configure.ac");
/* The match is captured rather than read back out of RegExp.$1, which is
* engine-global state shared with every other config.w32 evaluated in
Expand Down Expand Up @@ -57,6 +57,7 @@ if (PHP_MAXMINDDB == "yes") {
* The Windows build never defines HAVE_CONFIG_H, so libmaxminddb
* already does the right thing, and defining it here would instead
* make our own maxminddb.c include a config.h that does not exist.
* The AC_DEFINE below reaches maxminddb.c by another route; see there.
*
* MSVC has no unsigned __int128, so the byte array is the only option
* for uint128 values. Both this file and config.m4 have to define
Expand All @@ -82,7 +83,20 @@ if (PHP_MAXMINDDB == "yes") {
* ask for it explicitly too, and the dependency is ours now. */
ADD_FLAG("LIBS_MAXMINDDB", "ws2_32.lib");

AC_DEFINE("HAVE_LIBMAXMINDDB_BUNDLED", 1, "Use bundled or system libmaxminddb");
/* phpinfo() reports this. It reaches maxminddb.c through php.h, not
* through any config header of ours: as of PHP 8.4, under phpize,
* configure.js writes AC_DEFINE results to
* <devpack>\include\main\config.pickle.h, which main\config.w32.h
* includes (in a php-src tree they land in that file directly).
*
* It is 1 here and 0 in the system branch below, never absent,
* because config.pickle.h is shared state in the PHP install and
* generate_config_pickle_h() removes only the lines whose keys the
* current run defines. A bundled configure followed by a system one
* would leave this 1 in place and the system build would report
* "bundled". The 0 overwrites it, and maxminddb.c refuses to compile
* if neither arrives. */
AC_DEFINE("HAVE_LIBMAXMINDDB_BUNDLED", 1, "1 if the bundled libmaxminddb is compiled into the extension, 0 if a system libmaxminddb is linked");

EXTENSION("maxminddb", "maxminddb.c");

Expand Down Expand Up @@ -112,6 +126,9 @@ if (PHP_MAXMINDDB == "yes") {
"maxminddb", "libmaxminddb");
} else if (CHECK_HEADER_ADD_INCLUDE("maxminddb.h", "CFLAGS_MAXMINDDB", PHP_MAXMINDDB + ";" + PHP_PHP_BUILD + "\\include\\maxminddb") &&
CHECK_LIB("libmaxminddb.lib;maxminddb.lib", "maxminddb", PHP_MAXMINDDB)) {
/* See the bundled branch above for why this is 0 rather than absent. */
AC_DEFINE("HAVE_LIBMAXMINDDB_BUNDLED", 0, "1 if the bundled libmaxminddb is compiled into the extension, 0 if a system libmaxminddb is linked");

EXTENSION("maxminddb", "maxminddb.c");
} else {
WARNING('Could not find maxminddb.h, libmaxminddb.lib, or maxminddb.lib; skipping');
Expand Down
28 changes: 21 additions & 7 deletions ext/maxminddb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,15 @@
#include "ext/standard/info.h"
#include <maxminddb.h>

/* Both build files define this as 1 or 0 on every path, so a missing
* definition is a build-file mistake, not a system build. Fail rather than
* report "system" for a bundled object. Unlike php-src's HAVE_GD_BUNDLED,
* which is absent in a system build, this must be tested with #if: #ifdef
* is true for the 0 too. */
#ifndef HAVE_LIBMAXMINDDB_BUNDLED
#error "HAVE_LIBMAXMINDDB_BUNDLED must be defined by the build files"
Comment on lines +34 to +35

@horgh horgh Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The guard sits after #include <php.h> (line 20), not before it. The config.h that config.w32 leaves out is a different header. On Windows the macro reaches this file through php.h → zend_portability.h → zend_config.w32.h → main/config.w32.h → config.pickle.h, which the comment above the AC_DEFINE in config.w32 spells out.

CI confirms it: the Windows bundled job in test-bundled.yml compiled and loaded the DLL on this PR, which it could not have done if this #error fired.

🤖 Comment by Claude (Claude Code) on behalf of Will.

#endif

#ifdef ZTS
#include <TSRM.h>
#endif
Expand Down Expand Up @@ -792,18 +801,23 @@ PHP_MINIT_FUNCTION(maxminddb) {
}

static PHP_MINFO_FUNCTION(maxminddb) {
#if HAVE_LIBMAXMINDDB_BUNDLED
const char *lib_source = "bundled";
#else
const char *lib_source = "system";
#endif
char *lib_version;

php_info_print_table_start();

php_info_print_table_row(2, "MaxMind DB Reader", "enabled");
php_info_print_table_row(
2, "maxminddb extension version", PHP_MAXMINDDB_VERSION);
php_info_print_table_row(
3, "libmaxminddb library version", MMDB_lib_version(),
#ifdef HAVE_LIBMAXMINDDB_BUNDLED
"(bundled)");
#else
"(system)");
#endif

spprintf(&lib_version, 0, "%s (%s)", MMDB_lib_version(), lib_source);
CHECK_ALLOCATED(lib_version);
php_info_print_table_row(2, "libmaxminddb library version", lib_version);
Comment on lines +817 to +819

@horgh horgh Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed that nothing in CI calls phpinfo(), so a wrong label would go unnoticed.

If we add a check, the cheapest place is dev-bin/verify-extension.php. It already receives the expected libmaxminddb version, and every job that runs it is a bundled build, so it could capture phpinfo(INFO_MODULES) with output buffering and assert the row reads libmaxminddb library version => <version> (bundled). That covers the Linux matrix and the Windows job with no package.xml change. The system side would be a one-line grep for (system) in test.yml after the build step. A phpt cannot know which mode it was built in, so it could only check the row's shape, and it would also need a package.xml entry.

Will doesn't want to bother with this right now, though.

🤖 Comment by Claude (Claude Code) on behalf of Will.

efree(lib_version);

php_info_print_table_end();
}
Expand Down
Loading