Skip to content

How to build FAST with Visual Studio 2019+CMake+Ninja on Windows#755

Merged
lalitb merged 16 commits into
mainfrom
maxgolov/winbuild_with_ninja
May 20, 2021
Merged

How to build FAST with Visual Studio 2019+CMake+Ninja on Windows#755
lalitb merged 16 commits into
mainfrom
maxgolov/winbuild_with_ninja

Conversation

@maxgolov
Copy link
Copy Markdown
Contributor

@maxgolov maxgolov commented May 14, 2021

Changes

This is a set of changes + documentation needed to switch from MSBuild to Ninja for Windows build.

What it gives:

  • it is dramatically improving your productivity: gives you back 4 minutes of your lifetime for every clean build. Timing details are in the markdown doc.
  • your incremental builds now are seconds (5s per build configuration).
  • if you use Visual Studio IDE, now your test run time for selective tests are also seconds. Markdown doc shows how.

This is rather trivial change. No code changes. Just build infra. See building-with-vs2019.md markdown for all the details.

I'll send a separate follow-up PR that adds this process ( setup-buildtools.cmd+build.cmd) as part of our CI/CD loop, with a goal in mind to continuously validate both nostd and stdlib build flavor on Windows. This process is flexible, as it supports any Visual Studio version and any compiler. In fact, it even works with CLang-LLVM if Visual Studio is not installed (verified that). I have not tested if similar process works for MinGW or Emscripten. But from previous personal experience, same build script should largely work with minor adjustments for other compilers. Linux process doc comes after in a separate PR.

@maxgolov maxgolov requested a review from a team May 14, 2021 20:21
@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2021

Codecov Report

Merging #755 (96bdb90) into main (d406934) will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #755      +/-   ##
==========================================
- Coverage   95.97%   95.95%   -0.02%     
==========================================
  Files         176      176              
  Lines        7172     7172              
==========================================
- Hits         6883     6882       -1     
- Misses        289      290       +1     
Impacted Files Coverage Δ
sdk/src/logs/batch_log_processor.cc 93.75% <0.00%> (-1.25%) ⬇️

Comment thread CMakeLists.txt
if(BUILD_TESTING)
if(MSVC)
# GTest bug: https://github.com/google/googletest/issues/860
add_compile_options(/wd4275)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we scope this option to only test code compilation?

Copy link
Copy Markdown
Contributor Author

@maxgolov maxgolov May 14, 2021

Choose a reason for hiding this comment

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

Unfortunately the issue is that we have so many different test projects, and it appears like it would have to be applied to all projects that use Google Test. The warning comes from Google Test header. I wonder if there is a trick that can be used to selectively add compile options if target name matches *test*, but it may not be sufficient.

The root cause fix is in the latest Google Test. But that latest Google Test is not yet getting pulled via vcpkg, so I see this error. It'd have to be added to too many files, pretty much to all test project files. I can do that if you insist.

My logical reasoning here as follows:

  • warning is related to DLL exports and is only seen in Google Test.
  • we know our own interfaces are all fine. Only Google Test header has the issue, not the SDK itself.
  • and we do not support publishing the SDK as DLL yet in v1.0, not at least until v1.1 of SDK.

So suppressing this DLL export warning globally does not anyhow regress any of our other code.

Comment thread CMakeLists.txt Outdated
Copy link
Copy Markdown
Member

@lalitb lalitb left a comment

Choose a reason for hiding this comment

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

LGTM

@lalitb lalitb merged commit b706993 into main May 20, 2021
@lalitb lalitb deleted the maxgolov/winbuild_with_ninja branch May 20, 2021 18:00
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.

3 participants