Skip to content

report if libmaxminddb is bundled or system #289

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

Open
remicollet wants to merge 1 commit into
base: main
Choose a base branch
from
+8 −1
Open

report if libmaxminddb is bundled or system #289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 3 additions & 0 deletions ext/config.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ if test $PHP_MAXMINDDB != "no"; then
CFLAGS="$CFLAGS -fvisibility=hidden -UHAVE_CONFIG_H -DHAVE_CONFIG_H=0 -DMMDB_UINT128_USING_MODE=0 -DMMDB_UINT128_IS_BYTE_ARRAY=1"

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

@horgh horgh Sep 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think we need this in ext/config.w32 too.

@remicollet remicollet Sep 12, 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.

Indeed, but I don't have any idea about how Windows works (I'm Linux only for >20 years)

AC_DEFINE([HAVE_LIBMAXMINDDB_BUNDLED], [1], [Use bundled or system libmaxminddb])

@horgh horgh Sep 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe "Defined when the bundled libmaxminddb is compiled into the extension"?

PHP_SUBST([HAVE_LIBMAXMINDDB_BUNDLED])

@horgh horgh Sep 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this line needed? I think it might not do anything useful.

else
AC_PATH_PROG(PKG_CONFIG, pkg-config, no)

Expand Down
6 changes: 5 additions & 1 deletion ext/maxminddb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -798,7 +798,11 @@ static PHP_MINFO_FUNCTION(maxminddb) {
php_info_print_table_row(
2, "maxminddb extension version", PHP_MAXMINDDB_VERSION);
php_info_print_table_row(
2, "libmaxminddb library version", MMDB_lib_version());
#ifdef HAVE_LIBMAXMINDDB_BUNDLED
2, "libmaxminddb bundled library version", MMDB_lib_version());
#else
2, "libmaxminddb system library version", MMDB_lib_version());

@horgh horgh Sep 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if adding a new row would be a good option, e.g. something like

#ifdef HAVE_LIBMAXMINDDB_BUNDLED
    const char *lib_source = "bundled";
#else
    const char *lib_source = "system";
#endif
    php_info_print_table_row(2, "libmaxminddb library", lib_source);

And we could keep the current version one as is, which is still useful.

Alternatively we could have the current line appear like libmaxminddb library version 1.xxx (bundled) and libmaxminddb library version 1.xxx.

If you think keeping your proposed way is superior, that seems okay too though.

@remicollet remicollet Sep 12, 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.

Feel free to implement an alternative
I wont be able to work on this in the next days

#endif

php_info_print_table_end();
}
Expand Down
Loading