Skip to content

Conversation

@eramongodb
Copy link
Contributor

@eramongodb eramongodb commented Aug 5, 2025

Resolves CXX-2812. Following #1412, which defines and exports the first v1 ABI symbol bsoncxx::v1::exception::~exception(), the abi-compliance-check can now support comparing v1 symbols without errors (as followup to #1083). Unlike what was stated in #1083, it seems sufficient for the "new" library to contain at least one exported symbol; both the "old" and "new" libraries containing at least one exported symbol does not seem to be necessary.

Drive-by improvements include:

  • Numerically comparing the exit code to distinguish ABI incompatibility (== 1) from tool errors (> 1).
  • Exclude mongocxx/v_noabi/config/* headers for consistency with bsoncxx and to avoid triggering the new v1 macro guard inclusion checks which are inherited by mongocxx.
  • Exclude the detail namespace for both bsoncxx and mongocxx regardless of nesting.

@eramongodb eramongodb requested a review from kevinAlbs August 5, 2025 19:49
@eramongodb eramongodb self-assigned this Aug 5, 2025
@eramongodb eramongodb requested a review from a team as a code owner August 5, 2025 19:49
Copy link
Collaborator

@kevinAlbs kevinAlbs left a comment

Choose a reason for hiding this comment

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

LGTM with swapping the if conditions.

# curl -sS -d "${status:?}" -H "Content-Type: application/json" -X POST localhost:2285/task_status || true
declare ret
abi-compliance-checker -lib mongo-cxx-driver -old old.xml -new new.xml 2>&1 && ret="$?" || ret="$?"
if [[ "${ret:?}" -gt 0 ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

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

Swap if blocks. I expect the -gt 1 block will never be entered (since the -gt 0 will be true too).

@eramongodb eramongodb merged commit de19710 into mongodb:master Aug 6, 2025
20 of 21 checks passed
@eramongodb eramongodb deleted the cxx-2812 branch August 6, 2025 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants