Navigation Menu

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

Build on Windows using VS 2019 + LLVM/Clang #4258

Merged
merged 7 commits into from Jan 31, 2021

Conversation

KrzysFR
Copy link
Contributor

@KrzysFR KrzysFR commented Jan 30, 2021

As talked in https://forums.foundationdb.org/t/looking-for-community-support-for-the-windows-build/2531, this PR attempts to build on Windows using Clang-CL instead of MSVC.

DISCLAIMER: I'm not a C++ developer, and I'm not familiar at all with clang/cl, and only tested on Windows. Please review the changes and tell me if I'm doing something stupid/broke other platforms!

Required dependencies to build (ie: what's installed on my machine)

  • Visuals Studio 2019 16.8 (I'm using 16.9-preview3)
  • CMAKE 3.12+ (3.9 will NOT work)
  • LLVM/CLANG 11.x

This PR is based on the 6.3 branch

I run CMAKE with the following arguments:

cmake -G "Visual Studio 16 2019" -A x64 -T"ClangCL" -DCMAKE_BUILD_TYPE=Release %PATH_TO_FDB_REPO%

  • -T"ClangCL" switches from MSVC to clang-cl
  • -A x64 ensures that the 64-bit platform toolset is selected, so that clang-cl.exe does not run out of address space when reticulating splines^Hc++ templates
  • I'm not sure about -DCMAKE_BUILD_TYPE=Release ? doc told me to add it, never tried without...

note: adding -DOPEN_FOR_IDE currently breaks the build for me!

Then I build with the following arguments:

msbuild /p:CL_MPCount=XX /m /p:UseMultiToolTask=true /p:Configuration=Release foundationdb.sln

  • /p:UseMultiToolTask=true helps reduce the build time from more than one hour to about 5 minutes.
  • /p:CL_MPCount=XX helps me tune the max number of cores used during the build. Replace XX with the max number of logical threads of your cpu minus 1 or 2 to keep your host usable during the build...

I tried installing the resulting MSI package and got it to start the service and fdbcli was able to connect to it.

- renamed variable so as not to use a reserved keyword.
- fix compile error requiring first arg of main() to be of type int
…X to fix various macro expansions issues

- NOMINMAX to fix confusion between std::max(..) and max(..) macro redefinition
- BOOST_USE_WINDOWS_H to prevent boost from redefining win32 APIs
- WIN32_LEAN_AND_MEAN to fix include ordering issues with winsock.h
@sfc-gh-mpilman
Copy link
Collaborator

@fdb-build test this please

…unpack in Debug build

- defined twice in FileBackupAgent.actor.cpp and BackupAgent.actor.h
- only fails when building in Debug ?
@KrzysFR
Copy link
Contributor Author

KrzysFR commented Jan 30, 2021

I found a new issue with template duplication when building for Debug, that does happen in Release (why??)

I have to move the definition of Codec<ERestoreState>::pack and ::unpack in a central location. I'm not sure what's the best place to put them, so I kinda put close to the enum definition... lmk if that's not the correct spot.

@mpilman
Copy link
Contributor

mpilman commented Jan 31, 2021

I looked at the template change you made and I don't quite understand why this would cause problems (though having this code duplicated across two compilation units seems bad style). But the change imho improves on the previous state.

@KrzysFR
Copy link
Contributor Author

KrzysFR commented Jan 31, 2021

I'm tracking two issues with -DCMAKE_EXPORT_COMPILE_COMMANDS=ON not working.

  1. On my home windows machine, I only had python 2.7 installed, but the script gen_compile_db.py is for python 3. For some reason, FDBComponents.cmake tests the presence of Python 2 to enable WITH_PYTHON, while only testing Python 3 to enable WITH_DOCUMENTATION. It then uses WITH_PYTHON (which did not explicitely checked for python 3) to run a python 3 script.
  • I fixed the issue by installing python 3.9.1 on my machine, but it seems the detection logic is outdated.
  • Do you know if there are ANY script that still requires python 2? Could we simply change the cmake file so that it enable WITH_PYTHON by looking specifically for python3 interpreter? That way it would not attempt to run this step if the correct dependency is missing.
  1. the gen_compile_db.py attempts to modify the content of compile_commands.json. The issue is that this file is present in .gitignore so is missing from the repository when checked out, and the python script fails with a File not Found when it attempts to open it (line 35).
  • The documentation talks about this file, but does not explicitly tell how to create it (or it was not created on my machine for some reason?)
  • Either the CMAKE should test for this file, or the python script should handle the missing file, and either do nothing, or ouptut a message "File is missing, here is how to create it ...." ?

Anyway, before I spend too much time on this, does this compile_commands.json file would in anyway help someone using either Visual Studio 2019 or Visual Studo Code work on fdb's code base on windows? or not ?

@sfc-gh-mpilman
Copy link
Collaborator

I am merging this -- I believe that everything that doesn't work now didn't work before, so this PR is already a strict improvement (it also helps my work on the docker stuff).

For the two problems:

  1. I think we require python 3 these days. For a while (I believe FDB 6.2) we needed Python 2.7 for the documentation and Python 3 for other stuff, so this is a bit messy. But I think we should now change cmake to reject python2 and enforce python3 in find_package.
  2. gen_compile_db.py is a weird hack that I wrote a while ago... It reads a compile_commands.json from the build directory and writes a compile_commands.json to the source directory. The idea is that this way a user can just open the source dir and tools like clangd will work out of the box. The problem you run into is that CMAKE_EXPORT_COMPILE_COMMANDS doesn't work if you generate a visual studio project. Instead on Windows you have to create two projects, one with OPEN_FOR_IDE which you then can use to edit code and one without that flag (for building). Unless we can somehow use Ninja to build on Windows (I don't know how we could do that), there's not a lot we can do... I guess the best solution would be to print a better error message if a user attempts to set DCMAKE_EXPORT_COMPILE_COMMANDS to ON.

@sfc-gh-mpilman sfc-gh-mpilman merged commit 824145a into apple:release-6.3 Jan 31, 2021
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.

None yet

3 participants