PHPC-2435: Support libmongoc 2.0 by alcaeus · Pull Request #1829 · mongodb/mongo-php-driver

alcaeus · 2025-05-22T13:53:14Z

This fixes the following issues: * PHPC-2581: Bump to libmongoc 2.0.1 * PHPC-2578: Bump to libmongocrypt 1.14.0 * PHPC-2548: Remove MONGOC_WRITE_CONCERN_W_ERRORS_IGNORED * PHPC-2540: Use const for mongoc_host_list_t * PHPC-2547: Remove MONGOC_NO_AUTOMATIC_GLOBALS * PHPC-2549: Remove BSON_EXTRA_ALIGN * PHPC-1548: Add tests for empty authSource URI option * PHPC-2542: Add test coverage for auth mechanism errors

* Add build action to build libmongoc system libraries * Build driver with system libs * Install libmongocrypt as system library * Run tests with system libs * Move system library tests to tests workflow

* PHPC-2545: Drop support for compiling with LibreSSL * Warn when explicitly building with libressl

Copilot

Pull Request Overview

This PR updates the PHP driver to support libmongoc 2.x along with corresponding version bumps for libmongocrypt and the mongo-c-driver, and removes support for LibreSSL.

Updated file paths for version retrieval (e.g. from LIBMONGOC_VERSION_CURRENT to src/libmongoc/VERSION_CURRENT)
Bumped version numbers throughout configuration files, SBOM, CI workflows, and contributor documentation
Removed LibreSSL support from SSL configuration in autotools

Reviewed Changes

Copilot reviewed 47 out of 47 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
scripts/update-sbom.sh	Updated LIBMONGOC_VERSION file location
scripts/autotools/libmongocrypt/CheckSSL.m4	Removed LibreSSL in favor of using only OpenSSL and Darwin
scripts/autotools/libmongoc/Versions.m4	Updated version file path reference
scripts/autotools/libmongoc/CheckSSL.m4	Refined SSL configuration logic
scripts/autotools/libbson/Versions.m4	Updated file path for BSON version file
sbom.json	Bumped versions for libmongocrypt and mongo-c-driver
config.w32	Updated version file path and added extra include path
config.m4	Updated PKG_CHECK_MODULES call and version checks for system libs
CONTRIBUTING.md	Revised instructions for checking out and bumping libmongoc version
.github workflows & Evergreen configs	Updated CI jobs and actions to use new version numbers

config.m4

CONTRIBUTING.md

kevinAlbs

LGTM with minor comments.

kevinAlbs · 2025-05-29T12:52:12Z

bin/prep-release.php


        "src/libmongoc/src/common/src/*.{c,h,h.in}",
+        // Note: src/libmongoc/src/common/src/mlib/ does not contain source files (as of libmongoc 2.0.1)
+        "src/libmongoc/src/common/src/mlib/*.{c,h}",


Suggested change

"src/libmongoc/src/common/src/mlib/*.{c,h}",

"src/libmongoc/src/common/src/mlib/*.h",

Remove unnecessary *.c glob? I expect the purpose of the comment is to indicate there are no *.c source files needed.

Updated - these are indeed not necessary (here and below).

kevinAlbs · 2025-05-29T12:53:34Z

bin/prep-release.php

        "src/libmongocrypt/src/*.{c,h,h.in}",
        "src/libmongocrypt/src/crypto/*.{c,h}",
-        // Note: src/libmongocrypt/src/mlib/ does not contain source files (as of libmongocrypt 1.3.1)
+        "src/libmongocrypt/src/mlib/*.{c,h}",


Suggested change

"src/libmongocrypt/src/mlib/*.{c,h}",

I expect this is redundant with the glob below for src/libmongocrypt/src/mlib/*.h. The only *.c source files are tests.

kevinAlbs · 2025-05-29T13:33:02Z

tests/manager/manager-ctor-write_concern-error-001.phpt

+/* Note: Values of w < 0 are invalid, but libmongoc's URI string parsing only
 * logs a warning instead of raising an error (see: CDRIVER-2234), so we cannot
 * test for this. */


Comment appears incorrect. libmongoc returns an error for unsupported values, e.g. w=-1:

echo throws(function() { create_test_manager("mongodb://localhost:27017/?w=-1", []); }, "MongoDB\Driver\Exception\InvalidArgumentException"), "\n";

Results in output:

Failed to parse MongoDB URI: 'mongodb://localhost:27017/?w=-1'. Error while parsing the 'w' URI option: Unsupported w value [w=-1]. OK: Got MongoDB\Driver\Exception\InvalidArgumentException

Thank you for pointing this out - I guess that comment was never updated after CDRIVER-2234 was fixed.

kevinAlbs · 2025-05-29T13:36:45Z

.github/actions/linux/build-libmongocrypt/action.yml

+    - name: Add repository
+      shell: bash
+      working-directory: /tmp
+      # Note: no packages for Ubuntu 24.04 noble exist, so we use those for 22.04


Suggested change

# Note: no packages for Ubuntu 24.04 noble exist, so we use those for 22.04

# Note: no packages for Ubuntu 24.04 noble exist, so we use those for 22.04. Update after MONGOCRYPT-813 is released.

Aside: MONGOCRYPT-813 is planned for the next 1.14.1 patch release.

Thank you, I'll update this alongside 1.14.1 then :)

* Support building with SSPI support under Windows * Remove support for building with Cyrus SASL on Windows * Apply feedback from Copilot * Apply code review feedback * Fix handling of missing SASL libs when relying on default value for with-mongodb-sasl

jmikola · 2025-07-15T16:03:34Z

bin/prep-release.php

        "src/libmongocrypt/src/*.{c,h,h.in}",
        "src/libmongocrypt/src/crypto/*.{c,h}",
-        "src/libmongocrypt/src/mlib/*.{c,h}",
+        "src/libmongocrypt/src/mlib/*.h",


@alcaeus: This line appears to be redundant with the pattern appearing after the subsequent comment. Perhaps this should be removed entirely?

Indeed. Removed in #1848

jmikola · 2025-07-15T16:06:00Z

.github/actions/linux/build-libmongoc/action.yml

+    description: "Prefix to install libmongoc to"
+    default: "/usr/local"
+  version:
+    description: "Libmongoc version to build"


Noted that the description here is missing "(major.minor)", which you had in the build-libmongocrypt action.

Ah, yes. Added in #1848

jmikola · 2025-07-15T16:08:09Z

.github/actions/linux/build-libmongocrypt/action.yml

+      shell: bash
+      run: sudo sh -c 'curl -s --location https://pgp.mongodb.com/libmongocrypt.asc | gpg --dearmor >/etc/apt/trusted.gpg.d/libmongocrypt.gpg'
+
+    - name: Add repository


Is there a particular reason that libmongocrypt is installed as a package, while libmongoc is compiled for source?

Wouldn't the approach here assume that the build is running on an Ubuntu or Debian OS? I suppose that's OK since runs-on: "ubuntu-latest" is specified for the test-system-libs workflow.

IIRC, this was because I didn't find a 2.0 package for libmongoc. It's also very likely that we'll need to support building from a branch once we start supporting features only included in an upcoming libmongoc version. Thinking about this, the same may apply to libmongocrypt, but I assume it's less likely as libmongocrypt follows a more aggressive release strategy.

OK to cross that bridge when we come to it.

jmikola · 2025-07-15T16:12:17Z

Makefile.frag

 		echo "ERROR: Cannot generate function maps without CLI sapi."; \
 	fi

+show-config: all


Did you write this yourself, or was it pulled from another extension/skeleton?

This was shamelessly copied over from the test target. If there's a way to extract all the setup fluff to a target or similar that then runs whatever command is given, I'd definitely go for that, but my Makefile knowledge doesn't quite extend that far.

jmikola · 2025-07-15T16:13:27Z

bin/prep-release.php

        "src/libmongocrypt/src/mlib/*.h",
        "src/libmongocrypt/src/os_posix/*.{c,h}",
        "src/libmongocrypt/src/os_win/*.{c,h}",
+        "src/libmongocrypt/src/unicode/*.{c,h}",


Noted that this new directory was also included in config.m4 👍

jmikola · 2025-07-15T16:15:05Z

scripts/autotools/libmongoc/CheckSSL.m4

+  PHP_MONGODB_SSL="auto"
+fi
+
+dnl TODO 3.0: Remove libressl from valid options


Is this already tracked in a PHPC ticket?

It is now: PHPC-2605.

alcaeus force-pushed the feature/phpc-2435-libmongoc-2 branch from 9ef16ef to b5f3c8e Compare May 26, 2025 07:17

alcaeus added 2 commits May 26, 2025 09:42

PHPC-2584: Run driver test with system libraries (#1831)

71510e2

* Add build action to build libmongoc system libraries * Build driver with system libs * Install libmongocrypt as system library * Run tests with system libs * Move system library tests to tests workflow

PHPC-2545: Drop support for compiling with LibreSSL (#1836)

adfedd2

* PHPC-2545: Drop support for compiling with LibreSSL * Warn when explicitly building with libressl

alcaeus mentioned this pull request May 27, 2025

Relax assertion when checking exception message mongodb/mongo-php-library#1712

Merged

alcaeus requested a review from Copilot May 27, 2025 11:09

Copilot AI reviewed May 27, 2025

View reviewed changes

config.m4 Outdated Show resolved Hide resolved

CONTRIBUTING.md Show resolved Hide resolved

Fix usage of wrong version variable

6c715e9

alcaeus requested a review from kevinAlbs May 27, 2025 12:11

kevinAlbs approved these changes May 29, 2025

View reviewed changes

alcaeus added 2 commits June 2, 2025 09:32

Apply feedback from code review

9090d84

alcaeus marked this pull request as ready for review June 2, 2025 07:48

alcaeus requested a review from a team as a code owner June 2, 2025 07:48

alcaeus requested review from paulinevos and removed request for a team June 2, 2025 07:48

alcaeus enabled auto-merge (squash) June 3, 2025 11:21

paulinevos approved these changes Jun 3, 2025

View reviewed changes

alcaeus merged commit 4b32208 into v2.x Jun 3, 2025
108 checks passed

alcaeus deleted the feature/phpc-2435-libmongoc-2 branch June 3, 2025 11:22

jmikola reviewed Jul 15, 2025

View reviewed changes

	"src/libmongoc/src/common/src/mlib/*.{c,h}",
	"src/libmongoc/src/common/src/mlib/*.h",

	# Note: no packages for Ubuntu 24.04 noble exist, so we use those for 22.04
	# Note: no packages for Ubuntu 24.04 noble exist, so we use those for 22.04. Update after MONGOCRYPT-813 is released.

Conversation

alcaeus commented May 22, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

kevinAlbs left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants