Skip to content

[CDRIVER-4153] Beginning of Header Hygiene & Verification #2053

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion .evergreen/config_generator/components/earthly.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
"Valid options for the SASL configuration parameter"
TLSOption = Literal["OpenSSL", "off"]
"Options for the TLS backend configuration parameter (AKA 'ENABLE_SSL')"
CxxVersion = Literal["none"] # TODO: Once CXX-3103 is released, add latest C++ release tag.
CxxVersion = Literal["none"] # TODO: Once CXX-3103 is released, add latest C++ release tag.
"C++ driver refs that are under CI test"

# A separator character, since we cannot use whitespace
Expand Down Expand Up @@ -266,6 +266,13 @@ def tasks() -> Iterable[EvgTask]:
if task is not None:
yield task

yield EvgTask(
name="verify-headers",
commands=[earthly_exec(kind="test", target="verify-headers")],
tags=["pr-merge-gate"],
run_on=CONTAINER_RUN_DISTROS,
)


def variants() -> Iterable[BuildVariant]:
yield from (ev.as_evg_variant() for ev in all_possible(EarthlyVariant))
1 change: 1 addition & 0 deletions .evergreen/generated_configs/legacy-config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16250,6 +16250,7 @@ buildvariants:
- link-with-cmake
- link-with-cmake-ssl
- link-with-cmake-snappy
- verify-headers
- name: link-with-cmake-mac
distros:
- macos-14-arm64
Expand Down
16 changes: 16 additions & 0 deletions .evergreen/generated_configs/tasks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6420,3 +6420,19 @@ tasks:
- func: bootstrap-mongo-orchestration
- func: run-simple-http-server
- func: run-tests
- name: verify-headers
run_on:
- amazon2
- debian11-large
- debian12-large
- ubuntu2204-large
- ubuntu2404-large
tags: [pr-merge-gate]
commands:
- command: subprocess.exec
type: test
params:
binary: ./tools/earthly.sh
working_dir: mongoc
args:
- +verify-headers
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ def days(n: int) -> int:
"link-with-cmake",
"link-with-cmake-ssl",
"link-with-cmake-snappy",
"verify-headers",
OD([("name", "link-with-cmake-mac"), ("distros", ["macos-14-arm64"])]),
OD([("name", "link-with-cmake-windows"), ("distros", ["windows-vsCurrent-large"])]),
OD([("name", "link-with-cmake-windows-ssl"), ("distros", ["windows-vsCurrent-large"])]),
Expand Down Expand Up @@ -84,7 +85,7 @@ def days(n: int) -> int:
"build-and-run-authentication-tests-openssl-1.0.1",
"build-and-run-authentication-tests-openssl-1.0.2",
"build-and-run-authentication-tests-openssl-1.1.0",
"build-and-run-authentication-tests-openssl-1.0.1-fips"
"build-and-run-authentication-tests-openssl-1.0.1-fips",
],
{},
),
Expand Down Expand Up @@ -363,7 +364,7 @@ def days(n: int) -> int:
"name": "ocsp-openssl-1.0.1",
"execution_tasks": [".ocsp-openssl-1.0.1"],
},
]
],
),
Variant(
"packaging",
Expand All @@ -387,7 +388,7 @@ def days(n: int) -> int:
".versioned-api .5.0",
".versioned-api .6.0",
".versioned-api .7.0",
".versioned-api .8.0"
".versioned-api .8.0",
],
{},
),
Expand Down
2 changes: 1 addition & 1 deletion .evergreen/scripts/check-preludes.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
},
{
"name": "libbson",
"headers": list(BSON_PREFIX.glob("*.h")),
"headers": list(BSON_PREFIX.glob("bson-*.h")),
"exclusions": [
BSON_PREFIX / "bson-prelude.h",
BSON_PREFIX / "bson.h",
Expand Down
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ include(FeatureSummary)
include (MongoSettings)
include (MongoPlatform)
include (GeneratePkgConfig)
include (VerifyHeaders)

# Subcomponents:
mongo_bool_setting(ENABLE_MONGOC "Enable the build of libmongoc libraries (The MongoDB C database driver)")
Expand Down
87 changes: 66 additions & 21 deletions Earthfile
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,11 @@ IMPORT ./tools/ AS tools
# For target names, descriptions, and build parameters, run the "doc" Earthly subcommand.
# Example use: <earthly> +build --env=u22 --sasl=off --tls=OpenSSL --c_compiler=gcc

# build :
# Build libmongoc and libbson using the specified environment.
#
# The --env argument specifies the build environment among the `+env.xyz` environment
# targets, using --purpose=build for the build environment. Refer to the target
# list for a list of available build environments.
build:
# env is an argument
ARG --required env
FROM --pass-args +env.$env --purpose=build
# The configuration to be built
ARG config=RelWithDebInfo
# The prefix at which to install the built result
ARG install_prefix=/opt/mongo-c-driver
# Build configuration parameters. Will be case-normalized for CMake usage.
ARG --required sasl
ARG --required tls
LET source_dir=/opt/mongoc/source
LET build_dir=/opt/mongoc/build
# COPY_SOURCE :
# Copy source files required for the build into the specified "--into" directory
COPY_SOURCE:
COMMAND
ARG --required into
COPY --dir \
build/ \
CMakeLists.txt \
Expand All @@ -34,8 +20,16 @@ build:
src/ \
THIRD_PARTY_NOTICES \
VERSION_CURRENT \
"$source_dir"
ENV CCACHE_HOME=/root/.cache/ccache
"$into"

# CONFIGURE :
# Configure the project in $source_dir into $build_dir with a common set of configuration options
CONFIGURE:
COMMAND
ARG --required source_dir
ARG --required build_dir
ARG --required tls
ARG --required sasl
RUN cmake -S "$source_dir" -B "$build_dir" -G "Ninja Multi-Config" \
-D ENABLE_MAINTAINER_FLAGS=ON \
-D ENABLE_SHM_COUNTERS=ON \
Expand All @@ -47,6 +41,29 @@ build:
-D ENABLE_COVERAGE=ON \
-D ENABLE_DEBUG_ASSERTIONS=ON \
-Werror

# build :
# Build libmongoc and libbson using the specified environment.
#
# The --env argument specifies the build environment among the `+env.xyz` environment
# targets, using --purpose=build for the build environment. Refer to the target
# list for a list of available build environments.
build:
# env is an argument
ARG --required env
FROM --pass-args +env.$env --purpose=build
# The configuration to be built
ARG config=RelWithDebInfo
# The prefix at which to install the built result
ARG install_prefix=/opt/mongo-c-driver
# Build configuration parameters. Will be case-normalized for CMake usage.
ARG --required sasl
ARG --required tls
LET source_dir=/opt/mongoc/source
LET build_dir=/opt/mongoc/build
DO +COPY_SOURCE --into=$source_dir
ENV CCACHE_HOME=/root/.cache/ccache
DO --pass-args +CONFIGURE --source_dir=$source_dir --build_dir=$build_dir
RUN --mount=type=cache,target=$CCACHE_HOME \
env CCACHE_BASE="$source_dir" \
cmake --build $build_dir --config $config
Expand Down Expand Up @@ -396,6 +413,34 @@ vcpkg-base:
COPY src/libmongoc/examples/cmake/vcpkg/ $src_dir
WORKDIR $src_dir

# verify-headers :
# Execute CMake header verification on the sources
#
# See `earthly.rst` for more details.
verify-headers:
# We test against multiple different platforms, because glibc/musl versions may
# rearrange their header contents and requirements, so we want to check against as
# many as possible.
BUILD +do-verify-headers-impl \
--from +env.alpine3.19 \
--from +env.u22 \
--from +env.centos7 \
--sasl=off --tls=off --cxx_compiler=gcc --c_compiler=gcc

do-verify-headers-impl:
ARG --required from
# We don't really care about the specifics of the build env/settings, so set some
# reasonable defaults so the caller doesn't need to specify. In the future, it is
# possible that we will need to test other environments and build settings.
FROM --pass-args "$from" --purpose=build
# Add C++ so we can test as C++ headers
DO --pass-args tools+ADD_CXX_COMPILER
DO +COPY_SOURCE --into=/s
DO --pass-args +CONFIGURE --source_dir /s --build_dir /s/_build
# The "all_verify_interface_header_sets" target is created automatically
# by CMake for the VERIFY_INTERFACE_HEADER_SETS target property.
RUN cmake --build /s/_build --target all_verify_interface_header_sets

# run :
# Run one or more targets simultaneously.
#
Expand Down
116 changes: 116 additions & 0 deletions build/cmake/VerifyHeaders.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
include_guard(DIRECTORY)

#[==[
Add header verification targets for given headers:

mongo_verify_headers(
<tag>
[USES_LIBRARIES [<library> ...]]
[HEADERS [<glob> ...]]
[EXCLUDE_REGEX [<pattern> ...]]
)

Here `<tag>` is an arbitrary string that is used to qualify the internal target
created for the verification. The `<glob>` expressions are used to automatically
collect sources files (relative to the current source directory). All files
collected by `<glob>` must live within the current source directory.

After collecting sources according to the `<glob>` patterns, sources are
excluded if the filepath contains any substring that matches any regular
expression in the `<pattern>` list. Each `<pattern>` is tested against the
relative path to the header file that was found by `<glob>`, and not the
absolute path to the file.

The header verification targets are compiled according to the usage requirements
from all `<library>` arguments.
]==]
function(mongo_verify_headers tag)
list(APPEND CMAKE_MESSAGE_CONTEXT "${CMAKE_CURRENT_FUNCTION}(${tag})")
cmake_parse_arguments(
PARSE_ARGV 1 arg
"" # No flags
"" # No args
"HEADERS;EXCLUDE_REGEX;USE_LIBRARIES" # List args
)
if(arg_UNPARSED_ARGUMENTS)
message(FATAL_ERROR "Unknown arguments: ${arg_UNPARSED_ARGUMENTS}")
endif()

# Collect headers according to our patterns
set(headers_to_verify)
foreach(pattern IN LISTS arg_HEADERS)
# Use a recursive glob from the current source dir:
file(GLOB_RECURSE more
# Make the paths relative to the calling dir to prevent parent paths
# from interfering with the exclusion regex logic below
RELATIVE "${CMAKE_CURRENT_SOURCE_DIR}"
# We need to re-run configuration if any files are added/removed
CONFIGURE_DEPENDS
"${pattern}"
)
# Warn if this pattern didn't match anything. It is probably a mistake
# in the caller's argument list.
if(NOT more)
message(WARNING "Globbing pattern “${pattern}” did not match any files")
endif()
list(APPEND headers_to_verify ${more})
endforeach()

# Exclude anything that matches any exclusion regex
foreach(pattern IN LISTS arg_EXCLUDE_REGEX)
list(FILTER headers_to_verify EXCLUDE REGEX "${pattern}")
endforeach()

# Drop duplicates since globs may grab a file more than once
list(REMOVE_DUPLICATES headers_to_verify)
list(SORT headers_to_verify)
foreach(file IN LISTS headers_to_verify)
message(DEBUG "Verify header file: ${file}")
endforeach()

# We create two targets: One for C and one for C++
# C target
set(c_target ${tag}-verify-headers-c)
message(DEBUG "Defining header verification target “${c_target}” (C)")
# Create object libraries. They will only have one empty compiled source file.
# The source file language will tell CMake how to verify the associated header files.
add_library(${c_target} OBJECT "${CMAKE_CURRENT_FUNCTION_LIST_DIR}/empty.c")
# Define the file set
target_sources(${c_target} PUBLIC FILE_SET HEADERS)
# Conditionally do the same thing for C++
if(CMAKE_CXX_COMPILER)
# C++ is available. define it
set(cxx_target ${tag}-verify-headers-cxx)
message(DEBUG "Defining header verification targets “${cxx_target}” (C++)")
add_library(${cxx_target} OBJECT "${CMAKE_CURRENT_FUNCTION_LIST_DIR}/empty.cpp")
target_sources(${cxx_target} PUBLIC FILE_SET HEADERS)
else()
message(AUTHOR_WARNING "No C++ compiler is available, so the header-check C++ targets won't be defined")
unset(cxx_target)
endif()
# Populate the properties and file sets.
set_target_properties(${c_target} ${cxx_target} PROPERTIES
# The main header file set:
HEADER_SET "${headers_to_verify}"
# Enable header verification:
VERIFY_INTERFACE_HEADER_SETS TRUE
# Add the usage requirements that propagate to the generated compilation rules:
INTERFACE_LINK_LIBRARIES "${arg_USE_LIBRARIES}"
)
endfunction()

#[[
Variable set to TRUE if-and-only-if CMake supports header verification.
]]
set(MONGO_CAN_VERIFY_HEADERS FALSE)
if(CMAKE_VERSION VERSION_GREATER_EQUAL "3.24")
set(MONGO_CAN_VERIFY_HEADERS TRUE)
endif()

# Try to enable C++, but don't require it. This will be used to conditionally
# define the C++ header-check tests
include(CheckLanguage)
check_language(CXX)
if(CMAKE_CXX_COMPILER)
enable_language(CXX)
endif()
3 changes: 3 additions & 0 deletions build/cmake/empty.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/* This is an empty placeholder file used for miscellaneous build system tasks
*/
#include <stddef.h>
2 changes: 2 additions & 0 deletions build/cmake/empty.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// This is an empty placeholder file used for miscellaneous build system tasks
#include <cstddef>
15 changes: 15 additions & 0 deletions docs/dev/earthly.rst
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,21 @@ enumerated using ``earthly ls`` or ``earthly doc`` in the root of the repository
See: `SNYK_TOKEN`


.. program:: +verify-headers
.. earthly-target:: +verify-headers

This runs `CMake's header verification`__ on the library sources, to ensure
that the public API headers can be ``#include``\ 'd directly in a C++
compiler.

__ https://cmake.org/cmake/help/latest/prop_tgt/VERIFY_INTERFACE_HEADER_SETS.html

This target does not produce any output artifacts. This only checks that our
public API headers are valid. This checks against a variety of environments
to test that we are including the necessary standard library headers in our
public API headers.


.. _earthly.secrets:

Setting Earthly Secrets
Expand Down
2 changes: 1 addition & 1 deletion src/common/src/common-string-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -485,7 +485,7 @@ mcommon_string_append_overflow (mcommon_string_append_t *append)
*/
bool
mcommon_string_append_selected_chars (mcommon_string_append_t *append,
const char *template,
const char *template_,
const char *selector,
size_t selector_len);

Expand Down
8 changes: 3 additions & 5 deletions src/common/src/common-thread-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,9 @@
#ifndef MONGO_C_DRIVER_COMMON_THREAD_PRIVATE_H
#define MONGO_C_DRIVER_COMMON_THREAD_PRIVATE_H

#define BSON_INSIDE
#include <bson/bson-compat.h>
#include <bson/bson-config.h>
#include <bson/bson-macros.h>
#undef BSON_INSIDE
#include <bson/compat.h>
#include <bson/config.h>
#include <bson/macros.h>

BSON_BEGIN_DECLS

Expand Down
Loading