report if libmaxminddb is bundled or system by remicollet · Pull Request #289 · maxmind/MaxMind-DB-Reader-php

remicollet · 2026-09-11T08:15:06Z

As version 1.14.0 now has an option to use the system or bundled library, it could be useful to report it.

$ php -n -d extension=modules/maxminddb.so --ri maxminddb

maxminddb

MaxMind DB Reader => enabled
maxminddb extension version => 1.14.0
libmaxminddb bundled library version => 1.14.0

Summary by CodeRabbit

New Features
- PHP information output now identifies whether the libmaxminddb library is bundled with the extension or provided by the system.

coderabbitai · 2026-09-11T08:15:30Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 770d0786-f4cd-4a87-a804-59e1e84adeb7

📥 Commits

Reviewing files that changed from the base of the PR and between 30ddfe8 and a97f383.

📒 Files selected for processing (3)

ext/config.m4
ext/config.w32
ext/maxminddb.c

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📝 Walkthrough

Walkthrough

Bundled Unix and Windows builds now define HAVE_LIBMAXMINDDB_BUNDLED. PHP info output labels libmaxminddb as bundled or system-provided based on this macro.

Changes

libmaxminddb labeling

Layer / File(s)	Summary
Bundled library detection and reporting `ext/config.m4`, `ext/config.w32`, `ext/maxminddb.c`	Bundled builds define `HAVE_LIBMAXMINDDB_BUNDLED`. PHP info output uses the macro to select the bundled or system-provided libmaxminddb label.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Feature

Suggested reviewers: oschwald

Merge Risk: ⚪ Minimal · up to a97f3

The change accurately reports whether the loaded libmaxminddb is bundled or system-provided without affecting disabled builds.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (2 skipped: 2 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states the main change: reporting whether libmaxminddb is bundled or system-provided.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. (2 skipped: 2 unsupported.)

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{A rabbit checks the library sign
Bundled branches now align
Windows joins the build parade
PHP info shows the choice displayed
System or bundled, clear and bright
Hop through one small change tonight}

_{Comment @coderabbitai help to get the list of available commands.}

remicollet · 2026-09-11T08:30:16Z

Perhaps version is irrelevant when bundled, so an alternative report is

libmaxminddb library version => bundled

Or

libmaxminddb library version => 1.14.0

horgh

Good idea, thank you! I have a few comments.

horgh

Great, thank you!

php_info_print_table_row() emits one cell per argument and no colspan, so the three-argument row added in maxmind#289 was wrong output rather than an unusual layout. In text mode it read libmaxminddb library version => 1.14.0 => (bundled) which has the Directive => Local Value => Master Value shape of an ini table, so the version row stopped being key => value. In HTML it was a three-cell <tr> in a table whose other rows have two, leaving a ragged empty column. The row keeps the shape maxmind#289 intended, with the source as a suffix in the same cell: libmaxminddb library version => 1.14.0 (bundled) Selecting the string ahead of the call also removes the `);` that was duplicated across both #ifdef arms, and the hunk now formats cleanly under .clang-format, whose BinPackArguments: false would otherwise have rewritten it. lint.yml does not run clang-format, so CI had not noticed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The two build files are the most heavily commented in the repository and the AC_DEFINE added in maxmind#289 was the one line in them with no rationale. Worse, config.w32 says a few lines above it that "the Windows build never defines HAVE_CONFIG_H" and that defining it "would make our own maxminddb.c include a config.h that does not exist". A maintainer trusting that prose could reasonably conclude the AC_DEFINE is dead and delete it. It is not: under phpize the value is written to <devpack>\include\main\config.pickle.h and reaches maxminddb.c through php.h, zend_portability.h, zend_config.w32.h and main\config.w32.h, a route that has nothing to do with HAVE_CONFIG_H. Both files now say what the macro is for, why it is defined as 0 rather than left absent on the system path, and, in config.w32, the include chain it travels and the stale-pickle failure that the 0 exists to overwrite. The HAVE_CONFIG_H paragraph points at that comment so the two are not read as contradicting each other. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

The new row is user-visible output and maxmind#289 shipped without noting it. 1.14.0 is already released at the top of the file, so this opens the next section, undated until release the way 1.14.0 was. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

horgh reviewed Sep 11, 2026

View reviewed changes

Comment thread ext/config.m4 Outdated

Comment thread ext/config.m4

Comment thread ext/config.m4

Comment thread ext/maxminddb.c Outdated

report if libmaxminddb is bundled or system

a97f383

remicollet force-pushed the issue-info-bundled branch from 30ddfe8 to a97f383 Compare September 14, 2026 05:14

remicollet requested a review from horgh September 14, 2026 05:18

horgh approved these changes Sep 14, 2026

View reviewed changes

horgh merged commit f873503 into maxmind:main Sep 14, 2026
65 checks passed

remicollet deleted the issue-info-bundled branch September 16, 2026 08:23

Navigation Menu

report if libmaxminddb is bundled or system - #289

report if libmaxminddb is bundled or system#289
horgh merged 1 commit into
maxmind:mainfrom
remicollet:issue-info-bundled

remicollet commented Sep 11, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Sep 11, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

remicollet commented Sep 11, 2026

Uh oh!

horgh left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

horgh left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Conversation

remicollet commented Sep 11, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

remicollet commented Sep 11, 2026

Uh oh!

horgh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

horgh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

remicollet commented Sep 11, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Sep 11, 2026 •

edited

Loading