Miscellaneous Improvements to EVG Config #1049

eramongodb · 2023-10-31T18:02:14Z

Description

This PR aims to make numerous overdue improvements to the EVG config for the CXX Driver. Verified by this patch.

Due to volume of diffs, navigating the changes by-commit is recommended.

EVG Matrix Variants

The "integration task" matrix is currently defined using "matrix variants" (axis: + matrix_*), which have long since been not actively supported. Per documentation:

The matrix syntax is deprecated [...]. Evergreen is unlikely to do further development on matrix variant definitions. [...] please do not add new matrix variant definitions.

Instead of using EVG matrix variants, the "integration matrix" variants have been expanded and defined explicitly, identified by the integration- prefix in their names. Due to their names being different from the prior config (even though their display names remain unchanged), EVG considers these variants distinct from the prior config and thus does not preserve variant/task history.

YAML "merge keys" (<<:) are used to reduce duplication across the "integration tasks" as much as possible. These keys are deliberately limited to the specification of tasks and preserving existing platform-specific behavior. This is part of a greater effort to reduce the complexity of the EVG variant/task definitions by reducing the use of EVG YAML anchors. The only YAML anchors now in use is mongoc_minimum_required and integration_matrix_*.

These changes also addresses ongoing task failures by resolving CXX-2772 (Ubuntu 18.04 -> Ubuntu 20.04 for 7.0 and Latest ). The "Ubuntu 18.04 Release (MongoDB Latest)" variant is thus obsoleted and removed, reducing the total task count by 8 (235 -> 227). The lint task was moved into its own variant.

YAML Anchors

YAML anchors defined under variables: have been almost entirely inlined and moved into related scripts. This is to reduce the complexity of variant/task definitions. These changes include:

tar_options: removed: obsolete (unused?)
poly_mnmlstc: removed: obsolete (unused?)
mongoc_version_default: replaced with mongoc_version_minimum (little/no reason for them not to be synced).
version_*: inlined into mongodb_version: expansion definitions (no significant benefit to their use).
example_projects_*: inlined into the just 5 variants it is used by (asan/ubsan + experimental std).
poly_flags: inlined into the just 4 variants it is used by (MacOS + experimental std).
extra_path: inlined into the "compile" and "test" task definition (specified by platform detection).
*_test_params: inlined into the "test" task definition (specified by TEST_WITH_(ASAN|UBSAN|VALGRIND)).
*_cmake_flags: inlined into the "compile" task definition (specified by platform detection, TEST_WITH_, and CMake generator).

Therefore, the complexity of specifying CMake configuration and compile flags has been moved into the compile.sh and test.sh scripts, where Bash arrays can be used to much more easily and readably specify the flags required per task and platform (especially the CC/CXX compile flags).

Compile and Test

These two EVG functions were occupying a significant portion of the EVG config file. Due to their importance and length, their entire definitions have been moved into the compile.sh and test.sh scripts. This significantly reduces the size of the EVG config file, offsetting the increased line count due to the integration matrix expansion. This also allows applying tools such as formatting and ShellCheck to the compile and task scripts.

To further facilitate maintainability, rather than dumping all expansions via add_expansions_to_env, the specific expansions required by individual functions/scripts are explicitly documented via include_expansions_in_env: and env:. Although a bit verbose, this should help keep track of what expansions are required by what functions/scripts in the future.

Miscellaneous

The -Wno-aligned-new flag was dropped due to conflicts with Clang tasks + appearing to cause no trouble for existing tasks (not entirely clear what motivated its addition or if it still applies; can revert if necessary).
Remove redundant run_on: in variant definitions. Moved into task definition if able.
Although auto-downloaded C Driver's CMake config might find ccache, the CXX Driver's CMake config does not due to variable scoping. The CXX Driver CMake config now opts into use of ccache via the CMAKE_(C|CXX)_COMPILER_LAUNCHER environment variables.
The compiler expansion intended to select GCC vs. Clang was completely unused (bug since its introduction?), meaning the Clang-specific tasks were not using Clang at all. The new cc_compiler and cxx_compiler expansions are used to define CC and CXX environment variables to direct CMake to use the specified compiler(s), fixing the GCC and Clang tasks. Note: these tasks are moved from Ubuntu 22.04 -> Ubuntu 20.04 due to Ubuntu 22.04 apparently not providing a Clang compiler.
Move towards using nproc and CMAKE_BUILD_PARALLEL_LEVEL to enable build parallelism at the CMake level (more consistently and better forwarded to underlying generator without depending on compile flags).
The distcheck command was not properly selecting the polyfill library consistently with the parent build-and-install routines due to CMake config variables not being forwarded properly in cmake -P commands. Additionally, the distcheck commands were unnecessarily specific to the GNU Make generator. The required "polyfill flags" are now properly forwarded, and cmake --build is used instead of make. These changes now allow the distcheck test to be run on Windows, but this addition was deliberately omitted (see: _RUN_DISTCHECK in compile.sh) to avoid increasing the already-lengthy run time of Windows tasks.

kevinAlbs

The removal of most YAML anchors, and movement of scripts from YAML to separate files is a much appreciated simplification.

LGTM with minor suggestions.

.mci.yml

.evergreen/compile.sh

.evergreen/test.sh

.evergreen/compile.sh

.evergreen/test.sh

eramongodb · 2023-11-02T19:53:40Z

Removed the minimum libmongoc variant. Latest changes verified by this patch.

vector-of-bool · 2023-11-06T19:57:47Z

cmake/make_dist/MakeDistCheck.cmake

   separate_arguments (build_opts)
   execute_process_and_check_result (
-      COMMAND ${MY_CMAKE_COMMAND} ${MAKE_COMMAND} ${build_opts}
+      COMMAND ${CMAKE_COMMAND} --build .


Suggested change

COMMAND ${CMAKE_COMMAND} --build .

COMMAND ${CMAKE_COMMAND} --build . --parallel

If --parallel is given with no argument value, CMake will infer the parallelism level automatically (may be useful in other contexts, but is a useful shorthand in general).

Confirming, is inferring parallelism via --parallel preferable/different to using the value of the CMAKE_BUILD_PARALLEL_LEVEL environment variable (which AFAIK should be visible/inherited by the execute_process() commands given the changes in this PR)?

Confirmed: CMAKE_BUILD_PARALLEL_LEVEL suffices.

"Ubuntu 18.04 with minimum libmongoc" was removed in PR mongodb#1049. Testing with minimum libmongoc is now the default.

* bump required C driver to 1.25.0 * remove reference to removed EVG variant "Ubuntu 18.04 with minimum libmongoc" was removed in PR #1049. Testing with minimum libmongoc is now the default. * remove step for updating docker versions This step is listed in later in `Docker Image Build and Publish`. Building the Docker images requires downloading the C++ release tarball. Bump after tagging. * remove `check_docker_file_versions` The Docker versions are bumped after tagging the release. This check may not be expected to succeed during a release. * fix documented introduction of the auto C driver download It is introduced in 3.9.0 as part of CXX-2695. * add `-pthread` to example project build scripts This is a workaround until CDRIVER-4776 is resolved.

eramongodb added 24 commits October 31, 2023 12:38

Remove obsolete tar_options anchor

29263b7

Remove obsolete poly_mnmlstc anchor

6bd6d03

Replace use of mongoc_version_default with minimum

6742672

Expand the integration test matrix into explicit variants

1d7924c

Substitute use of mongodb_version anchors

de0c171

Substitute use of example project anchors

8ae08ce

Substitute use of polyfill library flags

5ff43bf

Substitute CMake path anchors with in-script search

c2ff76e

Substitute use of test param anchors with in-script variables

3ed5c9a

Substitute use of code coverage anchor with in-script variables

8f83716

Remove Make-dependent commands in distcheck routines

fca7aef

Ensure distcheck is configured with correct polyfill library

81d67f6

CXX-2772 Move ubuntu1804 MongoDB Latest and 7.0 tasks to ubuntu2004

1199f1d

Remove redundant use of distros field in variant task list

7c147d8

Remove obsolete -Wno-aligned-new warning from compile flags

1791da7

Update clang-tidy task

ceb686d

Move clang-tidy task to Clang variant

e07fabf

Fix GCC vs. Clang compiler selection via env vars

3d74e3f

Move GCC vs. Clang variants to Ubuntu 20.04

40dcdeb

Move test commands into .evergreen/test.sh

cf1fcd6

Tidy test.sh

5e3e26c

Substitute use of CMake flag anchors with in-script variables

8e2bd99

Move compile commands into .evergreen/compile.sh

ce5e145

Tidy compile.sh

8d13623

eramongodb requested a review from kevinAlbs October 31, 2023 18:02

eramongodb self-assigned this Oct 31, 2023

kevinAlbs requested a review from vector-of-bool October 31, 2023 19:03

kevinAlbs approved these changes Nov 2, 2023

View reviewed changes

eramongodb added 2 commits November 2, 2023 14:39

Remove minimum libmongoc variant

c65d3dc

Add error handling for MSBuild path selection

2c302fb

eramongodb added 2 commits November 2, 2023 14:43

Add note for indirect requirement of distro_id env var

83aa493

Merge else branches for CSFLE test setup commands

a056418

Merge remote-tracking branch 'upstream/master' into cxx-evg

2f71c5e

vector-of-bool approved these changes Nov 6, 2023

View reviewed changes

eramongodb merged commit 1afba63 into mongodb:master Nov 7, 2023

eramongodb deleted the cxx-evg branch November 7, 2023 21:26

kevinAlbs added a commit to kevinAlbs/mongo-cxx-driver that referenced this pull request Nov 8, 2023

remove reference to removed EVG variant

2515109

"Ubuntu 18.04 with minimum libmongoc" was removed in PR mongodb#1049. Testing with minimum libmongoc is now the default.

eramongodb mentioned this pull request Oct 29, 2024

CXX-3126 Refactor EVG config to use config_generator #1244

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Miscellaneous Improvements to EVG Config #1049

Miscellaneous Improvements to EVG Config #1049

Uh oh!

eramongodb commented Oct 31, 2023

Uh oh!

kevinAlbs left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eramongodb commented Nov 2, 2023

Uh oh!

vector-of-bool Nov 6, 2023

Uh oh!

eramongodb Nov 6, 2023

Uh oh!

eramongodb Nov 7, 2023

Uh oh!

Uh oh!

	COMMAND ${CMAKE_COMMAND} --build .
	COMMAND ${CMAKE_COMMAND} --build . --parallel

Miscellaneous Improvements to EVG Config #1049

Miscellaneous Improvements to EVG Config #1049

Uh oh!

Conversation

eramongodb commented Oct 31, 2023

Description

EVG Matrix Variants

YAML Anchors

Compile and Test

Miscellaneous

Uh oh!

kevinAlbs left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eramongodb commented Nov 2, 2023

Uh oh!

vector-of-bool Nov 6, 2023

Choose a reason for hiding this comment

Uh oh!

eramongodb Nov 6, 2023

Choose a reason for hiding this comment

Uh oh!

eramongodb Nov 7, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!