PYTHON-6050 Bump the bundled libbson to 2.5.0 - #112
Draft
blink1073 wants to merge 27 commits into
Draft
Conversation
libbson 2.x's mlib/time_point.h needs POSIX feature-test macros defined directly rather than detected, and bson_as_json() was removed in favor of bson_as_json_with_opts().
- vendor.sh now diffs and reverts bsonjs/common/common-config.h the same way it already does for bsonjs/bson/config.h, instead of silently overwriting the hand-checked-in copy. - MANIFEST.in now includes *.th files so the new mlib/vec.th template ships in the sdist. - bsonjs.c uses bson_as_legacy_extended_json() instead of manually building bson_json_opts_t, matching the RELAXED/CANONICAL calls already in this function.
Uses drivers-github-tools' reusable codeql action, scanning our own
C code (bsonjs.c/bsonjs.h), the Python build/test glue, and the
workflow files themselves. Excludes bsonjs/{bson,jsonsl,common},
which are vendored libbson/jsonsl sources synced by vendor.sh rather
than code we maintain.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed portability/correctness issues in newly introduced/modified macros (clang deprecation pragma pop/push mismatch and overly broad BSON_HAVE_ALIGNED_ALLOC enabling) that can break builds or silently alter diagnostic behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates the project’s bundled (vendored) libbson sources to 2.5.0 and adjusts the build, packaging, and wrapper code so the Python extension continues to compile and expose the same BSON↔Extended JSON functionality.
Changes:
- Bump vendored libbson from 1.27.2 to 2.5.0 and update vendoring script paths for the 2.x source layout.
- Update build configuration (feature-test macros) and packaging to accommodate new libbson/common/mlib headers and templates.
- Update the Python wrapper’s JSON conversion call and refresh documentation/changelog to reflect the new bundled version and benchmark results.
File summaries
| File | Description |
|---|---|
| vendor.sh | Update vendoring workflow for libbson 2.x layout and renamed generated headers. |
| setup.py | Add POSIX feature-test macros required by libbson 2.x clock/time code; refactor define_macros handling. |
| README.rst | Update libbson version link and benchmark claims/numbers. |
| MANIFEST.in | Include *.th template headers needed by mlib. |
| CHANGELOG.rst | Add 0.8.0 entry describing libbson bump and links to upstream changelog. |
| bsonjs/jsonsl/jsonsl.h | Update jsonsl header to include new libbson/common/mlib headers and adjust API/export details. |
| bsonjs/common/mlib/timer.h | Add vendored mlib timer utilities used by libbson 2.x. |
| bsonjs/common/mlib/str_vec.h | Add vendored mlib string-vector wrapper using vec.th. |
| bsonjs/common/mlib/platform.h | Add vendored mlib platform header for OS-specific includes/defines. |
| bsonjs/common/mlib/loop.h | Add vendored mlib loop/foreach macros. |
| bsonjs/common/mlib/intutil.h | Add vendored mlib integer utility macros/helpers. |
| bsonjs/common/mlib/cmp.h | Add vendored mlib safe integer comparison/range utilities. |
| bsonjs/common/common-utf8-private.h | Add vendored common UTF-8 helpers extracted from older bson-utf8 logic. |
| bsonjs/common/common-thread.c | Update vendored thread helpers to new header naming/includes and optional-param macro usage. |
| bsonjs/common/common-thread-private.h | Update vendored thread private header to new include structure and macro naming. |
| bsonjs/common/common-prelude.h | Update vendored prelude naming macros formatting and copyright. |
| bsonjs/common/common-oid.c | Add vendored OID helper implementation (zero OID helpers). |
| bsonjs/common/common-oid-private.h | Add vendored OID helper declarations. |
| bsonjs/common/common-md5-private.h | Update vendored md5 private header and move md5 struct typedef here. |
| bsonjs/common/common-macros-private.h | Expand vendored private macros for diagnostics/warnings control. |
| bsonjs/common/common-bits-private.h | Add vendored bit-manipulation helper(s). |
| bsonjs/common/common-b64-private.h | Update vendored base64 private header naming and macro formatting. |
| bsonjs/common/common-atomic.c | Add vendored atomic emulation/helpers replacing prior bson atomic impl. |
| bsonjs/bsonjs.c | Switch legacy-mode JSON serialization to libbson 2.x legacy JSON API. |
| bsonjs/bson/version.h | Bump BSON version macros/strings to 2.5.0. |
| bsonjs/bson/validate-private.h | Add vendored validate private API/constants for new validation implementation. |
| bsonjs/bson/memory.h | Add vendored memory API header (replaces removed bson-memory.h). |
| bsonjs/bson/memory.c | Update vendored memory implementation (aligned alloc selection, array alloc helpers, vtable). |
| bsonjs/bson/error.h | Add vendored public error API header (replaces older bson-error header layout). |
| bsonjs/bson/error.c | Update vendored error implementation (category field, strerror portability paths). |
| bsonjs/bson/config.h | Update hand-maintained config header; add BSON_HAVE_ALIGNED_ALLOC knob. |
| bsonjs/bson/compat.h | Update vendored compat header includes/exports and platform defines. |
| bsonjs/bson/bson-writer.h | Update writer header includes and formatting; use new memory/bson_t headers. |
| bsonjs/bson/bson-writer.c | Update writer implementation to new bson_t private layout and flags. |
| bsonjs/bson/bson-version-functions.h | Update version-functions header formatting. |
| bsonjs/bson/bson-version-functions.c | Update version-functions implementation includes and formatting. |
| bsonjs/bson/bson-vector-private.h | Add vendored private helpers for BSON vector element header bytes. |
| bsonjs/bson/bson-value.h | Update bson-value header includes/formatting. |
| bsonjs/bson/bson-value.c | Update value copy/destroy implementation (range checks, allocations, includes). |
| bsonjs/bson/bson-utf8.h | Update utf8 header includes/formatting. |
| bsonjs/bson/bson-utf8.c | Refactor utf8 implementation to use common UTF-8 and common JSON escaping helpers. |
| bsonjs/bson/bson-timegm-private.h | Update timegm private header includes/formatting. |
| bsonjs/bson/bson-string.h | Simplify public string header to core string APIs; remove bson_string_t API from public header. |
| bsonjs/bson/bson-reader.h | Update reader header to use BSON_CALL function pointer typedefs and compat include. |
| bsonjs/bson/bson-private.h | Remove old private bson implementation header (replaced by bson_t-private.h). |
| bsonjs/bson/bson-prelude.h | Update vendored prelude header copyright. |
| bsonjs/bson/bson-oid.h | Update OID header includes/formatting and unsafe-buffer-usage warning macros. |
| bsonjs/bson/bson-oid.c | Update OID implementation (init logic, includes/formatting). |
| bsonjs/bson/bson-memory.h | Remove old bson-memory public header (replaced by bson/memory.h). |
| bsonjs/bson/bson-md5.h | Remove deprecated md5 public header. |
| bsonjs/bson/bson-md5.c | Remove deprecated md5 shim implementation. |
| bsonjs/bson/bson-keys.h | Update keys header includes/formatting. |
| bsonjs/bson/bson-keys.c | Update uint32-to-string implementation to assert/return size_t safely. |
| bsonjs/bson/bson-json.h | Update JSON API header includes and BSON_CALL typedefs/formatting. |
| bsonjs/bson/bson-json-private.h | Ensure JSON private header exports public JSON header. |
| bsonjs/bson/bson-iso8601.c | Update ISO8601 parse implementation includes/formatting; remove date format helper implementation. |
| bsonjs/bson/bson-iso8601-private.h | Remove ISO8601 date format helper declaration; update includes. |
| bsonjs/bson/bson-error-private.h | Update error private header to export bson/error.h and add category setter helper. |
| bsonjs/bson/bson-endian.h | Update endian header; add float endian-swap support and modernize include structure. |
| bsonjs/bson/bson-decimal128.h | Update decimal128 header includes/formatting. |
| bsonjs/bson/bson-context.h | Update context header includes/formatting. |
| bsonjs/bson/bson-context-private.h | Update context-private header includes; remove deprecated seq64 helper declaration. |
| bsonjs/bson/bson-cmp.h | Remove old bson-cmp helpers (replaced by mlib cmp utilities). |
| bsonjs/bson/bson-clock.h | Update clock header includes/formatting. |
| bsonjs/bson/bson-clock.c | Update monotonic time implementation to use mlib time_point/duration utilities. |
| bsonjs/bson/bson-bcon.h | Add vendored BCON declarations header in new location/name. |
| bsonjs/bson/bson-atomic.c | Remove old bson atomic implementation (replaced by common-atomic.c). |
| bsonjs/bson/bson_t.h | Add new public bson_t definition header. |
| bsonjs/bson/bson_t-private.h | Add new private bson_t implementation details header (replaces bson-private.h). |
| bsonjs/bson/bcon.h | Remove old bcon.h location (replaced by bson-bcon.h). |
Review details
Suppressed comments (1)
bsonjs/common/common-macros-private.h:78
- In the clang branch, END_IGNORE_DEPRECATIONS expands to a diagnostic push instead of a pop, so the push/pop pair is unbalanced and could leave warnings suppressed (or cause pragma-stack warnings) if these macros are used.
- Files reviewed: 104/105 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The workflow's paths-ignore config didn't keep two alerts in vendored libbson files (bsonjs/bson/bson-decimal128.c, bson-timegm.c) out of the c-cpp analysis, despite those paths being listed there and confirmed present in CodeQL's own augmented config. .codeqlignore is applied before extraction and is the mechanism GitHub documents as reliable for compiled languages.
…g them
paths-ignore only filters results for interpreted languages or
build-mode: none — it doesn't apply when build-mode is manual/autobuild,
per GitHub's own docs, so it never excluded bsonjs/bson/*.c from the
c-cpp analysis despite listing them. .codeqlignore is likewise a no-op
here since it's only read by GitHub's Default Setup, which this repo
doesn't use.
CodeQL only extracts files the compiler is actually invoked on, so the
c-cpp job now compiles just bsonjs.c directly instead of running the
full `pip install -e .`, which also built every vendored .c file under
bsonjs/{bson,jsonsl,common}.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new CodeQL workflow’s permissions are likely insufficient for repository checkout/analysis, and there is a confirmed Clang diagnostic macro bug (END_IGNORE_DEPRECATIONS) that should be fixed to avoid unbalanced diagnostic state.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
bsonjs/common/common-macros-private.h:79
- For the Clang branch, END_IGNORE_DEPRECATIONS expands to a diagnostic push instead of a pop, which will unbalance the diagnostic stack and can affect subsequent warning state in any TU that uses these macros.
- Files reviewed: 104/105 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new CodeQL workflow YAML is invalid (matrix/strategy mis-indented) and there is a confirmed uninitialized-memory copy in bsonjs/bson/error.c that can leak stack data into bson_error_t.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
bsonjs/common/common-macros-private.h:79
- For Clang builds,
END_IGNORE_DEPRECATIONSis defined asMC_PRAGMA_DIAGNOSTIC_PUSH, which leaves the diagnostic state unbalanced and defeats the purpose of the push/pop pair. It should pop.
bsonjs/common/mlib/loop.h:92 - Spelling typo in comment: "iteraiton" should be "iteration".
- Files reviewed: 104/105 changed files
- Comments generated: 2
- Review effort level: Lite
fail-fast and matrix had ended up nested under permissions instead of strategy, so the workflow failed to parse.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
bsonjs/common/common-macros-private.h defines END_IGNORE_DEPRECATIONS as a diagnostic push under __clang__, which is an objective macro bug that can leave diagnostic state unbalanced and should be corrected before merging.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
bsonjs/common/common-macros-private.h:78
- In the clang branch, END_IGNORE_DEPRECATIONS is defined as MC_PRAGMA_DIAGNOSTIC_PUSH, which does not restore the prior diagnostic state and can leave warning state permanently altered (and unbalanced pushes) in translation units that use these macros. It should pop the diagnostic state instead.
- Files reviewed: 104/105 changed files
- Comments generated: 0 new
- Review effort level: Lite
bson-bcon.c (BCON), common-md5.c, and common-thread.c implement libbson features bsonjs never calls, and nothing else in the vendored tree references their symbols either. Verified via nm against the rest of the built extension and confirmed by import + the test suite. bson-vector.c looked like another candidate by the same criteria, but bson.c's array builder actually calls into it (bson_append_array_from_vector), so it stays.
vendor.sh now deletes bson-bcon.c, common-md5.c, and common-thread.c right after syncing them, so future vendor bumps keep them out automatically instead of needing setup.py to filter them out of the build every time. Their headers stay: bson.h includes bson-bcon.h, and common-thread-private.h is a real dependency of bson-context.c and common-b64.c. setup.py's sources glob goes back to unfiltered, now that the files it would have excluded no longer exist in the tree.
…implementation-9cba3c # Conflicts: # setup.py
The manual-build-command's standalone gcc invocation didn't define BSONJS_VERSION, so it failed once the merge from main brought in bsonjs.c's __version__ macro. Read it from pyproject.toml the same way setup.py does.
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
The PR is a large vendored libbson upgrade with broad C-level surface area changes, so it warrants final human review despite only minor actionable findings identified here.
Review details
Suppressed comments (3)
bsonjs/common/common-macros-private.h:78
- END_IGNORE_DEPRECATIONS is defined as MC_PRAGMA_DIAGNOSTIC_PUSH for the clang branch, which creates an unbalanced push/pop pair and fails to restore warning state. It should pop the diagnostic state.
bsonjs/common/mlib/loop.h:92 - Typo in comment: “iteraiton” should be “iteration”.
bsonjs/common/common-utf8-private.h:175 - Grammar in comment: “prior to it's beginning” should be “prior to its beginning”.
- Files reviewed: 100/105 changed files
- Comments generated: 0 new
- Review effort level: Lite
| - language: actions | ||
| build-mode: none | ||
| steps: | ||
| - uses: mongodb-labs/drivers-github-tools/codeql@d518d2c7d04fdec10266c4218c36791a4fcf98d8 # v3 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bumps the vendored libbson in
bsonjs/from 1.27.2 to 2.5.0 (the current maintained tag), per PYTHON-6050. 98 of the 105 changed files, and ~16.7k of the ~16.8k added lines, are in the vendored files.Changes
vendor.shfor libbson 2.x's renamedconfig.hand itscommon/mlibsources moving undersrc/common/src.bsonjs/bson/config.hto stay a portable, CPython-macro-driven header instead of the machine-specific one CMake generates.setup.pythat libbson 2.x now requires for its clock functions.bson_as_jsoncall in ourbsonjs/bsonjs.cwithbson_as_legacy_extended_json, matching the existing RELAXED/CANONICAL calls in that function.vendor.shnow deletesbson-bcon.c,common-md5.c, andcommon-thread.cright after syncing: nothing else in the vendored tree calls into them (confirmed vianm), so they're dead weight in the build and the sdist. Their headers stay, since other compiled files still need them.README.rst, and added aCHANGELOG.rstentry.c-cpp/python/actions), scoped to compile only our ownbsonjs.cfor thec-cpplanguage so vendored libbson/jsonsl sources underbsonjs/{bson,jsonsl,common}are never analyzed.None of the above are patches that need to be carried forward.
config.his a permanent hand-maintained file thatvendor.shalready knew to skip before this PR (same mechanism as the oldbson-config.h, just renamed);setup.pyandbsonjs.care ordinary application code outside the vendored tree, so future vendor bumps won't touch or need to reconcile against them.Benchmarks
Re-ran
benchmark.pyon a clean host and updatedREADME.rst's numbers: bsonjs is now ~16.5x faster thanjson_utilfordumpsand ~5.3x faster forloads, up from the old "3-4x" claim, consistent with libbson 2.x's performance work.Testing
python setup.py build_ext --inplacesucceeds.pytest test/— 23 passed.buildacross Linux/macOS/Windows x Python 3.9-3.14,Build CPython-*wheel jobs,Make SDist, and CodeQL (Analyze (actions),Analyze (c-cpp),Analyze (python)).