Skip to content

Commit 1984303

Browse files
[CDRIVER-4153] Beginning of Header Hygiene & Verification (#2053)
* Prepare some core header files for public use. This change starts a new pattern for public API headers: Headers that omit the "bson-" or "mongoc-" prefix are intended for public `#include` directives, allowing users to selectively import the APIs that they want. This change required minimal modification of the existing code, as the few headers that have been modified are very basic and have very few dependencies. * Fix C++ and C compatibility in some header files * Add CMake code for header verification. This adds a CMake utility based around VERIFY_INTERFACE_HEADER_SETS to check that certain headers can be included directly in a translation unit without any prerequesits, in either C or C++ code. This verification only concerns itself with headers that we have already made part of the public API. Private header files and header files that are not-yet-sanitized are not checked by this process. The verification does not yet happen automatically. It is only enabled for CMake 3.24 or newer, and requires the dev/CI to build the special linting target the CMake generates. * Earthly target and EVG tasks to run header verification * Remove unused old script * Rename bson.h -> bson-bcon.h
1 parent 912209d commit 1984303

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

77 files changed

+552
-382
lines changed

.evergreen/config_generator/components/earthly.py

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ def os_split(env: EnvKey) -> tuple[str, None | str]:
6868
case "centos7":
6969
return "CentOS", "7.0"
7070
case _:
71-
raise ValueError(f"Failed to split OS env key {env=} into a name+version pair (unrecognized)")
71+
raise ValueError(
72+
f"Failed to split OS env key {env=} into a name+version pair (unrecognized)"
73+
)
7274

7375

7476
class EarthlyVariant(NamedTuple):
@@ -147,11 +149,15 @@ def suffix(self) -> str:
147149

148150
# Authenticate with DevProd-provided Amazon ECR instance to use as pull-through cache for DockerHub.
149151
class DockerLoginAmazonECR(Function):
150-
name = 'docker-login-amazon-ecr'
152+
name = "docker-login-amazon-ecr"
151153
commands = [
152154
# Avoid inadvertently using a pre-existing and potentially conflicting Docker config.
153-
expansions_update(updates=[KeyValueParam(key='DOCKER_CONFIG', value='${workdir}/.docker')]),
154-
ec2_assume_role(role_arn="arn:aws:iam::901841024863:role/ecr-role-evergreen-ro"),
155+
expansions_update(
156+
updates=[KeyValueParam(key="DOCKER_CONFIG", value="${workdir}/.docker")]
157+
),
158+
ec2_assume_role(
159+
role_arn="arn:aws:iam::901841024863:role/ecr-role-evergreen-ro"
160+
),
155161
subprocess_exec(
156162
binary="bash",
157163
command_type=EvgCommandType.SETUP,
@@ -163,7 +169,7 @@ class DockerLoginAmazonECR(Function):
163169
],
164170
args=[
165171
"-c",
166-
'aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 901841024863.dkr.ecr.us-east-1.amazonaws.com',
172+
"aws ecr get-login-password --region us-east-1 | docker login --username AWS --password-stdin 901841024863.dkr.ecr.us-east-1.amazonaws.com",
167173
],
168174
),
169175
]
@@ -176,7 +182,9 @@ def task_filter(env: EarthlyVariant, conf: Configuration) -> bool:
176182
"""
177183
match env, conf:
178184
# u16/u18/centos7 are not capable of building mongocxx
179-
case e, (_sasl, _tls, cxx) if re.match(r"^Ubuntu 16|^Ubuntu 18|^CentOS 7", e.display_name):
185+
case e, (_sasl, _tls, cxx) if re.match(
186+
r"^Ubuntu 16|^Ubuntu 18|^CentOS 7", e.display_name
187+
):
180188
# Only build if C++ driver is test is disabled
181189
return cxx == "none"
182190
# Anything else: Allow it to run:
@@ -300,6 +308,13 @@ def tasks() -> Iterable[EvgTask]:
300308
if task is not None:
301309
yield task
302310

311+
yield EvgTask(
312+
name="verify-headers",
313+
commands=[earthly_exec(kind="test", target="verify-headers")],
314+
tags=["pr-merge-gate"],
315+
run_on=CONTAINER_RUN_DISTROS,
316+
)
317+
303318

304319
def variants() -> Iterable[BuildVariant]:
305320
yield from (ev.as_evg_variant() for ev in all_possible(EarthlyVariant))

.evergreen/generated_configs/legacy-config.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16250,6 +16250,7 @@ buildvariants:
1625016250
- link-with-cmake
1625116251
- link-with-cmake-ssl
1625216252
- link-with-cmake-snappy
16253+
- verify-headers
1625316254
- name: link-with-cmake-mac
1625416255
distros:
1625516256
- macos-14-arm64

.evergreen/generated_configs/tasks.yml

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6772,3 +6772,19 @@ tasks:
67726772
- func: bootstrap-mongo-orchestration
67736773
- func: run-simple-http-server
67746774
- func: run-tests
6775+
- name: verify-headers
6776+
run_on:
6777+
- amazon2
6778+
- debian11-large
6779+
- debian12-large
6780+
- ubuntu2204-large
6781+
- ubuntu2404-large
6782+
tags: [pr-merge-gate]
6783+
commands:
6784+
- command: subprocess.exec
6785+
type: test
6786+
params:
6787+
binary: ./tools/earthly.sh
6788+
working_dir: mongoc
6789+
args:
6790+
- +verify-headers

.evergreen/legacy_config_generator/evergreen_config_lib/variants.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ def days(n: int) -> int:
5050
"link-with-cmake",
5151
"link-with-cmake-ssl",
5252
"link-with-cmake-snappy",
53+
"verify-headers",
5354
OD([("name", "link-with-cmake-mac"), ("distros", ["macos-14-arm64"])]),
5455
OD([("name", "link-with-cmake-windows"), ("distros", ["windows-vsCurrent-large"])]),
5556
OD([("name", "link-with-cmake-windows-ssl"), ("distros", ["windows-vsCurrent-large"])]),
@@ -84,7 +85,7 @@ def days(n: int) -> int:
8485
"build-and-run-authentication-tests-openssl-1.0.1",
8586
"build-and-run-authentication-tests-openssl-1.0.2",
8687
"build-and-run-authentication-tests-openssl-1.1.0",
87-
"build-and-run-authentication-tests-openssl-1.0.1-fips"
88+
"build-and-run-authentication-tests-openssl-1.0.1-fips",
8889
],
8990
{},
9091
),
@@ -356,7 +357,7 @@ def days(n: int) -> int:
356357
"name": "ocsp-openssl-1.0.1",
357358
"execution_tasks": [".ocsp-openssl-1.0.1"],
358359
},
359-
]
360+
],
360361
),
361362
Variant(
362363
"packaging",
@@ -380,7 +381,7 @@ def days(n: int) -> int:
380381
".versioned-api .5.0",
381382
".versioned-api .6.0",
382383
".versioned-api .7.0",
383-
".versioned-api .8.0"
384+
".versioned-api .8.0",
384385
],
385386
{},
386387
),

.evergreen/scripts/check-files.py

Lines changed: 0 additions & 67 deletions
This file was deleted.

.evergreen/scripts/check-preludes.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
},
4040
{
4141
"name": "libbson",
42-
"headers": list(BSON_PREFIX.glob("*.h")),
42+
"headers": list(BSON_PREFIX.glob("bson-*.h")),
4343
"exclusions": [
4444
BSON_PREFIX / "bson-prelude.h",
4545
BSON_PREFIX / "bson.h",

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ include(FeatureSummary)
2828
include (MongoSettings)
2929
include (MongoPlatform)
3030
include (GeneratePkgConfig)
31+
include (VerifyHeaders)
3132

3233
# Subcomponents:
3334
mongo_bool_setting(ENABLE_MONGOC "Enable the build of libmongoc libraries (The MongoDB C database driver)")

Earthfile

Lines changed: 66 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -9,25 +9,11 @@ IMPORT ./tools/ AS tools
99
# For target names, descriptions, and build parameters, run the "doc" Earthly subcommand.
1010
# Example use: <earthly> +build --env=u22 --sasl=off --tls=OpenSSL --c_compiler=gcc
1111

12-
# build :
13-
# Build libmongoc and libbson using the specified environment.
14-
#
15-
# The --env argument specifies the build environment among the `+env.xyz` environment
16-
# targets, using --purpose=build for the build environment. Refer to the target
17-
# list for a list of available build environments.
18-
build:
19-
# env is an argument
20-
ARG --required env
21-
FROM --pass-args +env.$env --purpose=build
22-
# The configuration to be built
23-
ARG config=RelWithDebInfo
24-
# The prefix at which to install the built result
25-
ARG install_prefix=/opt/mongo-c-driver
26-
# Build configuration parameters. Will be case-normalized for CMake usage.
27-
ARG --required sasl
28-
ARG --required tls
29-
LET source_dir=/opt/mongoc/source
30-
LET build_dir=/opt/mongoc/build
12+
# COPY_SOURCE :
13+
# Copy source files required for the build into the specified "--into" directory
14+
COPY_SOURCE:
15+
FUNCTION
16+
ARG --required into
3117
COPY --dir \
3218
build/ \
3319
CMakeLists.txt \
@@ -37,8 +23,16 @@ build:
3723
src/ \
3824
THIRD_PARTY_NOTICES \
3925
VERSION_CURRENT \
40-
"$source_dir"
41-
ENV CCACHE_HOME=/root/.cache/ccache
26+
"$into"
27+
28+
# CONFIGURE :
29+
# Configure the project in $source_dir into $build_dir with a common set of configuration options
30+
CONFIGURE:
31+
FUNCTION
32+
ARG --required source_dir
33+
ARG --required build_dir
34+
ARG --required tls
35+
ARG --required sasl
4236
RUN cmake -S "$source_dir" -B "$build_dir" -G "Ninja Multi-Config" \
4337
-D ENABLE_MAINTAINER_FLAGS=ON \
4438
-D ENABLE_SHM_COUNTERS=ON \
@@ -50,6 +44,29 @@ build:
5044
-D ENABLE_COVERAGE=ON \
5145
-D ENABLE_DEBUG_ASSERTIONS=ON \
5246
-Werror
47+
48+
# build :
49+
# Build libmongoc and libbson using the specified environment.
50+
#
51+
# The --env argument specifies the build environment among the `+env.xyz` environment
52+
# targets, using --purpose=build for the build environment. Refer to the target
53+
# list for a list of available build environments.
54+
build:
55+
# env is an argument
56+
ARG --required env
57+
FROM --pass-args +env.$env --purpose=build
58+
# The configuration to be built
59+
ARG config=RelWithDebInfo
60+
# The prefix at which to install the built result
61+
ARG install_prefix=/opt/mongo-c-driver
62+
# Build configuration parameters. Will be case-normalized for CMake usage.
63+
ARG --required sasl
64+
ARG --required tls
65+
LET source_dir=/opt/mongoc/source
66+
LET build_dir=/opt/mongoc/build
67+
DO +COPY_SOURCE --into=$source_dir
68+
ENV CCACHE_HOME=/root/.cache/ccache
69+
DO --pass-args +CONFIGURE --source_dir=$source_dir --build_dir=$build_dir
5370
RUN --mount=type=cache,target=$CCACHE_HOME \
5471
env CCACHE_BASE="$source_dir" \
5572
cmake --build $build_dir --config $config
@@ -399,6 +416,34 @@ vcpkg-base:
399416
COPY src/libmongoc/examples/cmake/vcpkg/ $src_dir
400417
WORKDIR $src_dir
401418

419+
# verify-headers :
420+
# Execute CMake header verification on the sources
421+
#
422+
# See `earthly.rst` for more details.
423+
verify-headers:
424+
# We test against multiple different platforms, because glibc/musl versions may
425+
# rearrange their header contents and requirements, so we want to check against as
426+
# many as possible.
427+
BUILD +do-verify-headers-impl \
428+
--from +env.alpine3.19 \
429+
--from +env.u22 \
430+
--from +env.centos7 \
431+
--sasl=off --tls=off --cxx_compiler=gcc --c_compiler=gcc
432+
433+
do-verify-headers-impl:
434+
ARG --required from
435+
# We don't really care about the specifics of the build env/settings, so set some
436+
# reasonable defaults so the caller doesn't need to specify. In the future, it is
437+
# possible that we will need to test other environments and build settings.
438+
FROM --pass-args "$from" --purpose=build
439+
# Add C++ so we can test as C++ headers
440+
DO --pass-args tools+ADD_CXX_COMPILER
441+
DO +COPY_SOURCE --into=/s
442+
DO --pass-args +CONFIGURE --source_dir /s --build_dir /s/_build
443+
# The "all_verify_interface_header_sets" target is created automatically
444+
# by CMake for the VERIFY_INTERFACE_HEADER_SETS target property.
445+
RUN cmake --build /s/_build --target all_verify_interface_header_sets
446+
402447
# run :
403448
# Run one or more targets simultaneously.
404449
#

0 commit comments

Comments
 (0)