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

Should ninja prevent build.ninja from depending on the build? #2135

Open
smeenai opened this issue May 26, 2022 · 1 comment
Open

Should ninja prevent build.ninja from depending on the build? #2135

smeenai opened this issue May 26, 2022 · 1 comment

Comments

@smeenai
Copy link
Contributor

smeenai commented May 26, 2022

We had a bug in LLVM's build recently where the build.ninja rule ended up depending on part of the build. This was effectively a circular dependency, and meant that incremental builds would spuriously fail on CMake changes, because the build file regeneration step would fail to build before it could perform the regeneration.

https://github.com/smeenai/ninja-regeneration is a simple repro to demonstrate what I mean:

git clone https://github.com/smeenai/ninja-regeneration
cd ninja-regeneration
git checkout HEAD^
mkdir build
cd build
cmake -G Ninja ..
git checkout main
ninja generate

You'll get a spurious link error, because build file regeneration is itself attempting to build generate, which needs the CMake changes to be reflected in the build system in order to work. Manually regenerating the build system (cmake -S .. -B .) will make the build work.

Should ninja error out for this sort of scenario? It never seems desirable and will always lead to broken incremental builds; it's basically a circular dependency in disguise (since all other targets implicitly depend on the build system regeneration). If this seems more like a CMake concern than a ninja concern, let me know and I'll ask there instead.

@digit-google
Copy link
Contributor

Ninja already detects circular dependencies that are found in its build plan (and will error correctly in this case).
It looks like the generated build plan is simply missing the right set of dependencies, and there is no special logic to handle build plan regeneration in the implementation itself. So I would conjecture that this is an issue with what CMake generates here.

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

No branches or pull requests

2 participants