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

Building with Ninja #625

Closed
codicodi opened this issue Feb 2, 2017 · 13 comments
Closed

Building with Ninja #625

codicodi opened this issue Feb 2, 2017 · 13 comments
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed

Comments

@codicodi
Copy link
Contributor

codicodi commented Feb 2, 2017

I tried building several packages with Ninja as generator and everything seems to work great. Packaging is noticeably faster.
Since plans to support it popped up already in several discussions and mentions of Ninja are in scripts since initial commit, it seems like this approach has been invetigated before. Are there any hidden issues that need to be solved before vcpkg can use it by default? The way I see it, it's just a matter of tweaking few lines in vcpkg_*_cmake functions and adding Ninja to vcpkg_find_acquire_program.
The benefits would also include guarantee that user-wide integration does not affect packaging in any way. Additionally, adding Ninja as an available tool would make it easier to support third party buildsystems like Meson.

@KindDragon
Copy link
Contributor

It shouldn't be used by default because not all libraries CMake scripts support it. But we can use it for some libraries after testing with Ninja.

@codicodi
Copy link
Contributor Author

codicodi commented Feb 2, 2017

I find that suprising because one of the main points of CMake is to abstract away differences beetween generators. One thing that comes to mind is that buildscript assumes specific layout of build directory, but I'm not sure what benefit it could bring. Could you point me to an example of such problematic library?

@KindDragon
Copy link
Contributor

I noticed that most often doesn't work libraries that call other tools (like code generation). I remember that aws-cpp-sdk with install also produce wrong result (I did not check why).

OpenCV for example cannot be compiled with Ninja opencv/opencv#6372

@codicodi
Copy link
Contributor Author

codicodi commented Feb 2, 2017

I see, thanks. Nonetheless, since vast majority of libraries builds corrently, we may still consider using Ninja by default and simply make problematic ports pass "I don't like ninjas" flag to vcpkg_configure_cmake.
I've tried a few more packages and in addition to ports you mentioned SDL2 also fails due to some Resource Compiler error. On the other hand, bumping version of OpenCV to 3.2.0 may help, as the problematic line was changed there, see opencv/opencv@33ab236

@ras0219-msft ras0219-msft added discussion needed category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed labels Feb 3, 2017
@ras0219-msft
Copy link
Contributor

I'd love to move to Ninja -- in addition to being faster, it also has fewer moving parts (and therefore less to debug when things go wrong). Most CMake scripts are designed with a single-configuration generator in mind, so that's another point in Ninja's favor. It might enable building the Release and Debug flavors in parallel. You can even see evidence of some previous attempts to use Ninja that we rolled back approximately around the time of the release to increase stability [1].

However, there are a few kinks that need to be worked out. If a project uses Precompile Headers, MSBuild will automatically insert the correct dependency tracking whereas Ninja needs special handling [2]. When building for UWP via Ninja, a lot of things aren't added by default, such as /APPCONTAINER and references to the core .winmd files. Inside vcpkg_build_cmake and vcpkg_install_cmake, we pass some MSBuild-specific flags that will also require additional communication between the configure step and the build step.

I would like to get to a point where we can use Ninja by default, however I think the right path for now is to enable it off-by-default, then try to apply it individually to each portfile. We can add it to the built-in template for the create command to encourage adoption by new ports (for desktop triplets).

[1] https://github.com/Microsoft/vcpkg/blob/ce9927f7327bc71ade246108a7d984deda6293fd/scripts/cmake/vcpkg_configure_cmake.cmake#L1
[2] https://github.com/Microsoft/cpprestsdk/blob/c1c673d/Release/src/CMakeLists.txt#L105-L108

@KindDragon
Copy link
Contributor

KindDragon commented Feb 3, 2017

@ras0219-msft
Copy link
Contributor

@KindDragon I think that these issues are all in the CMake generator (Ninja is just running the commands requested with the dependency information given).

On the CMake side, I don't know if these would be considered bugs or not. The PCH issue appears to be quite well known for the last decade [1], for example, and it is simply considered a not-yet-implemented feature afaict. Digging through their bug database for the other issues I've mentioned, I think progress may have been made since I last attempted to use CMake for a WinRT project (a couple years ago). I don't have any good repro's sitting in front of me, so I can't really confirm/deny/file a useful bug.

The point I'd like to make with my past experiences is that it is unfortunately likely that a project currently building with MSBuild may not just work when switched to Ninja. Thus, I believe the prudent course is to switch portfiles over to using Ninja only when they can be tested to still work instead of all at once.

[1] https://cmake.org/Bug/view.php?id=1260

@KindDragon
Copy link
Contributor

Ok, thanks

@KindDragon
Copy link
Contributor

BTW vote for this issue https://gitlab.kitware.com/cmake/cmake/issues/1260

@OlafvdSpek
Copy link
Contributor

Packaging is noticeably faster.

Why is it (significantly) faster?
I understand Ninja is faster in a edit/compile/debug cycle but if a package is only build once it doesn't really matter does it?
And would this allow vcpkg to do it's thing without cmake at all?

@ras0219-msft
Copy link
Contributor

Ninja can be faster than MSBuild because

  1. it has a more fine-grained model for C++ compilation (MSBuild can't simultaneously see inside two different projects) which allows better parallelism.
  2. it is lighter-weight, so the engine/scheduler consumes less CPU/memory/disk, which leaves more resources for the actual compiler/linker.

It has far fewer features than MSBuild as well, but that's fine for us because CMake provides the higher level features and "compiles" down to the lower-level Ninja.

As for Vcpkg working without CMake -- unfortunately that really can't happen in the near future, because a huge chunk of great libraries support windows using CMake. While we could do a bunch of work to remove our internal dependency on CMake (using it for the portfile language, using it for downloads, etc), we would still need it to build the actual library.

As an aside, it is the incredible prevalence of CMake in libraries that support Windows that caused us to take such a large dependency on it in the first place.

@ras0219-msft
Copy link
Contributor

I'm going to close this issue with the merging of #628 since the primary task (add ninja support) has completed, however please feel free to continue asking questions/discussing Ninja here.

@JackSucharee
Copy link

Since PREFER_NINJA was added to some packages, I had some trouble building those packages.

Apparently PREFER_NINJA has caused CMake to choose the wrong compiler for those packages on my machine. In my case I have MinGW-w64's gcc.exe and g++.exe in my PATH, which resulted in CMake to choose those over MSVC's cl.exe. That's because in CMakeDetermineCCompiler.cmake and CMakeDetermineCXXCompiler.cmake it is defined that way. The order is g++ aCC cl bcc xlC clang++ for CXX and gcc cl bcc xlc clang for C.

I could have solve it by removing MinGW-w64 out of my PATH or by changing the order in those two CMakeDetermine*Compiler.cmake files. But a better solution for me was to instead append a fix path to list(APPEND _csc_OPTIONS ... ) in scripts/cmake/vcpkg_configure_cmake.cmake, like:

list(APPEND _csc_OPTIONS
"-DCMAKE_CXX_COMPILER=C:/Program\ Files\ (x86)/Microsoft\ Visual\ Studio\ 14.0/VC/bin/amd64/cl.exe"
"-DCMAKE_C_COMPILER=C:/Program\ Files\ (x86)/Microsoft\ Visual\ Studio\ 14.0/VC/bin/amd64/cl.exe"
...)

Not sure if that's a good solution or not, but it works for me. Maybe there's a better way for PREFER_NINJA to not trigger another compiler.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:vcpkg-feature The issue is a new capability of the tool that doesn’t already exist and we haven’t committed
Projects
None yet
Development

No branches or pull requests

5 participants