Skip to content
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

CXX-2039 Add VS2019 support and migrate VS2017 tasks to windows-vsCurrent distro #1082

Merged
merged 7 commits into from
Jan 16, 2024

Conversation

eramongodb
Copy link
Collaborator

Resolves CXX-2039. Verified by this patch.

This PR primarily consists of three steps:

  • Move existing VS 2017 tasks from windows-64-vs2017 to windows-vsCurrent.
  • Rename VS 2017 fields/properties to accomodate multiple VS versions.
  • Extend VS 2017 tasks to VS 2019 (simple duplication).

This should also address the spurious "cygheap base mismatch detected" failures on Evergreen due to BUILD-17907, which were frequently observed on the windows-64-vs2017 distro (VS 2015 tasks thankfully seem to be unaffected?).

As part of this effort, several drive-by improvements are made to the EVG config and scripts:

  • Use CMAKE_GENERATOR_PLATFORM or -A to specify the generator platform rather than the Win64 generator suffix (which only exists for pre-CMake 3.1 compatibility and is not supported by the VS 2019 generator or newer). Documentation and examples were updated accordingly.
  • Drop obsolete MSBuild path handlers (toolchain detection is handled by the "latest" CMake binary, and build system invocation is handled by the toolchain-independent cmake --build command).
  • Ensure CMake generator and platform preferences are also communicated to example projects via CMake environment variables.

This PR initially proposes a simple duplication of the existing VS 2017 tasks with VS 2019 instead. If preferable, the set of tasks and coverage between the VS 2017 and VS 2019 tasks can be adjusted to reduce redundancy.

Note: this PR does not extend C++20 compile coverage to VS 2019, nor does it extend test coverage to VS 2022 (which is available on the new distro).

@eramongodb eramongodb self-assigned this Jan 16, 2024
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 a suggestion for removing all but one VS 2017 integration tasks. Addressing the "cygheap base mismatch detected" failures, and use of newer CMake features is much appreciated.

If preferable, the set of tasks and coverage between the VS 2017 and VS 2019 tasks can be adjusted to reduce redundancy.

I expect there is little value in running tests with multiple server versions with the C++ driver compiled on VS 2017. Testing compile seems valuable, but I expect there is little behavior change between the compiled versions.
I suggest only testing one server version:

# Only test VS 2017 with the latest server and auth. Testing other server versions is covered with newer VS tasks.
- name: integration-auth-vs2017-latest-single
  display_name: "Windows (VS 2017) Debug Latest Auth"
  run_on: windows-vsCurrent-large
  expansions:
      mongodb_version: "latest"
      <<: *integration_matrix_expansions_windows_vs2017
  <<: *integration_matrix_auth_tasks_single

@eramongodb eramongodb merged commit 5b743b1 into mongodb:master Jan 16, 2024
2 of 3 checks passed
@eramongodb eramongodb deleted the cxx-2039 branch January 16, 2024 21:24
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