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

Add support for the Ninja CMake generator #8

Closed
Manouchehri opened this issue Dec 13, 2017 · 6 comments · Fixed by #830
Closed

Add support for the Ninja CMake generator #8

Manouchehri opened this issue Dec 13, 2017 · 6 comments · Fixed by #830

Comments

@Manouchehri
Copy link
Contributor

Using Ninja would significantly speed up the process of small rebuilds, and make parallelism work easily out of the box. This would help with #2 and #7.

Tried to do this myself, but wasn't able to figure out some of the CMakeLists.txt dependencies. Usually once I/we figure out what the build dependency issues are, it's as simple as:

cmake .. -GNinja -DCMAKE_INSTALL_PREFIX=/local/bin
ninja
ninja install
@stek29
Copy link
Contributor

stek29 commented Dec 13, 2017

I doubt it would help much with #7 tbh :)

@tpruzina
Copy link

tpruzina commented Dec 13, 2017

Hm, build seems to be able to saturate 16 threads (make -j16), don't think there is too much to gain there.
Switching to meson could somewhat speed up "./configure" phase, but considering the number of dependencies I don't think this is gonna be high on priority list.

As far as small rebuilds are concerned, I don't find them to be that terrible, just touched a few files in src/ and stuff builds in ~3-4 seconds.

make -j16 4.70s user 1.33s system 149% cpu 4.026 total

s3rvac added a commit to avast/llvm that referenced this issue Dec 24, 2017
Reasons:
1. The build rules for Linux are now more similar to those for Windows. Indeed,
   on Windows, all tools were built.
2. The use of "$(MAKE)" was incompatible with the Ninja CMake generator (Ninja
   uses `ninja`, not `make`). This brings us closer towards support the Ninja
   generator in RetDec (avast/retdec#8)
@s3rvac s3rvac changed the title Ninja for faster builds Add support for the Ninja CMake generator Dec 25, 2017
@s3rvac
Copy link
Member

s3rvac commented Dec 25, 2017

I tried to tackle this as I wanted to get more familiar with the Ninja build system. Some of our repositories are already buildable with Ninja, but there is a general problem with the way we link libraries built via external projects.

For each external project, we create a target that we can later use in target_link_libraries(). Both GNU Make and Visual Studio are fine with the fact that libraries coming from external projects do not exist when the build starts (they will exist once the external project is built). However, ninja fails with

ninja: error: '/path/to/libfoo.a', needed by 'target', missing and no known rule to make it

From what I have found, I believe that we will need to specify all the libraries in the BUILD_BYPRODUCTS option of ExternalProject_Add so that Ninja knows they will be built later.

@Manouchehri
Copy link
Contributor Author

Manouchehri commented Dec 26, 2017

Thanks for looking into it. =)

I ran into the same library linking issues, but I thought it was my fault. (I did a "fix" like avast/llvm@294bfb1 too, but ended up breaking the normal build.)

Is add_dependencies alone not enough?

@s3rvac
Copy link
Member

s3rvac commented Dec 26, 2017

No, add_depedencies() only says "build this after you have built that". It does not say anything about the origin of /path/to/libfoo.a (that it is a product of a particular external project). Only after I added the library into BUILD_BYPRODUCTS, the Ninja build started working. I guess the Ninja generator is more strict than other generators.

@PeterMatula
Copy link
Collaborator

Should be working on Linux now. I changed Linux Debug configuration in TeamCity to use Ninja and test this. I'm not sure if Ninja can be used with MSVC. I tried, but was unable to force Ninja on Windows to use MSVC compiler.

PeterMatula added a commit that referenced this issue Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants