Infinite re-run loop with CMake 3.5.1 and Ninja 1.10.0

Usage
,
1

I encounter strange behavior when running CMake (3.5.1) in tandem with
Ninja (1.10.0): CMake is constantly re-run until Ninja finally gives up with
the following message:

    ninja: error: manifest 'build.ninja' still dirty after 100 tries

Here is the minimal CMakeLists.txt file to reproduce this behavior:

    add_library(mylib main.cpp)

    # Variable containing a generator expression.
    set(mylib_LIBRARY $<TARGET_FILE_NAME:mylib>)

    # Substitue expression @VAR@ by contents of variables.
    configure_file(mylib-config.cmake.in mylib-config.cmake.tmp)

    # Create final output file, evaluating generator expression(s).
    file(GENERATE OUTPUT mylib-config.cmake INPUT mylib-config.cmake.tmp)

The file mylib-config.cmake.in is just a one-liner:

    set(mylib_LIBRARY "@mylib_LIBRARY@")

The reason seems to be that the temporary intermediate file
mylib-config.cmake.tmp appears in the implicit dependencies controlling
whether to re-run CMake. Ninja complains about that and re-runs CMake:

    ninja explain: output mylib-config.cmake.tmp of phony edge with no inputs doesn't exist

This behavior also happens with newer versions of CMake (e.g. 3.15.3).

Is this a bug in Ninja? It does not appear in earlier versions (e.g. 1.5.3).
However the logic of Ninja seems to be right (i.e. the file does in fact not
exists yet).

Is there a more elegant solution for the original problem (i.e. configuring a
file containing generator expressions)?

2

Hmm. I’ve done things like this before and hadn’t seen this behavior. What filesystem/OS are you on? I wonder if you’re only getting second-level timestamp resolution and it cycles at at least 100 Hz?

Thoughts @brad.king?

3

I am running Ubuntu 16.04.6 on kernel 4.15.0-106.

4

@Holger please try ninja 1.8 and 1.9 to compare the behavior. Starting in Ninja 1.9, file modification times are tracked with nanosecond resolution. That may be what exposed this problem.

5

What filesystem is your build tree using? Any stock filesystem in Linux is going to support nanosecond timestamps. If you’re building on vfat, nfs, or cifs, timestamps may be less accurate (ntfs is probably fine these days, but unknown for sure).

6

@brad.king The ninja versions you mention (1.8 and 1.9) exhibit the same behavior. I went backwards with the versions and found out that the problem was introduced in version 1.6.0 (version 1.5.3 is last working one).

7

@ben.boeckel Running plain old ext4 here.

8

Thanks. Might you be able to build Ninja from source and git bisect to find the exact commit?

9

Here is the result of git bisect:

636034b288777c9f0b7aca1072e0496de94db27c is the first bad commit
commit 636034b288777c9f0b7aca1072e0496de94db27c
Merge: 62458b6 5fbfa4b
Author: Nico Weber <nicolasweber@gmx.de>
Date:   Tue Feb 3 13:01:28 2015 -0800

    Merge pull request #908 from colincross/multipass
    
    Allow manifest rebuild to loop up to 100 times

 src/ninja.cc | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)
10

Thanks. That change to Ninja both raised the number of tries from 2 to 100 and also added the error message. I think this was never fully working but Ninja didn’t diagnose it before.

11

The logic in cmGlobalNinjaGenerator::WriteTargetRebuildManifest needs to be updated to avoid adding to reBuild.ImplicitDeps entries from any local generator’s localGen->GetMakefile()->GetListFiles() that also appear in any local generator’s localGen->GetMakefile()->GetOutputFiles().

12

@brad.king Your suggested fix works! Thank you :slight_smile:

Could you please review the patch?

Index: 3.15.3/src/Source/cmGlobalNinjaGenerator.cxx
===================================================================
--- 3.15.3.orig/src/Source/cmGlobalNinjaGenerator.cxx
+++ 3.15.3/src/Source/cmGlobalNinjaGenerator.cxx
@@ -1341,8 +1341,11 @@ void cmGlobalNinjaGenerator::WriteTarget
   reBuild.Outputs.push_back(this->NinjaOutputPath(NINJA_BUILD_FILE));
 
   for (cmLocalGenerator* localGen : this->LocalGenerators) {
+    const auto& outputFiles = localGen->GetMakefile()->GetOutputFiles();
     for (std::string const& fi : localGen->GetMakefile()->GetListFiles()) {
-      reBuild.ImplicitDeps.push_back(this->ConvertToNinjaPath(fi));
+      if (std::find(outputFiles.begin(), outputFiles.end(), fi) == outputFiles.end()) {
+        reBuild.ImplicitDeps.push_back(this->ConvertToNinjaPath(fi));
+      }
     }
   }
   reBuild.ImplicitDeps.push_back(this->CMakeCacheFile);
13

@brad.king Just re-read your post and found out that the above patch does not take into account multiple local generators. Will rework the patch …

14

@brad.king Next try. Is this what you have in mind?

Index: 3.15.3/src/Source/cmGlobalNinjaGenerator.cxx
===================================================================
--- 3.15.3.orig/src/Source/cmGlobalNinjaGenerator.cxx
+++ 3.15.3/src/Source/cmGlobalNinjaGenerator.cxx
@@ -1342,7 +1342,9 @@ void cmGlobalNinjaGenerator::WriteTarget
 
   for (cmLocalGenerator* localGen : this->LocalGenerators) {
     for (std::string const& fi : localGen->GetMakefile()->GetListFiles()) {
-      reBuild.ImplicitDeps.push_back(this->ConvertToNinjaPath(fi));
+      if (!this->IsContainedInLocalOutput(fi)) {
+        reBuild.ImplicitDeps.push_back(this->ConvertToNinjaPath(fi));
+      }
     }
   }
   reBuild.ImplicitDeps.push_back(this->CMakeCacheFile);
@@ -1564,6 +1566,17 @@ void cmGlobalNinjaGenerator::WriteTarget
   }
 }
 
+bool cmGlobalNinjaGenerator::IsContainedInLocalOutput(const std::string& fn) const
+{
+  for (cmLocalGenerator* localGen : this->LocalGenerators) {
+    const auto& outputFiles = localGen->GetMakefile()->GetOutputFiles();
+    if (std::find(outputFiles.begin(), outputFiles.end(), fn) != outputFiles.end()) {
+      return true;
+    }
+  }
+  return false;
+}
+
 void cmGlobalNinjaGenerator::InitOutputPathPrefix()
 {
   this->OutputPathPrefix =
Index: 3.15.3/src/Source/cmGlobalNinjaGenerator.h
===================================================================
--- 3.15.3.orig/src/Source/cmGlobalNinjaGenerator.h
+++ 3.15.3/src/Source/cmGlobalNinjaGenerator.h
@@ -382,6 +382,9 @@ private:
     cmGeneratorTarget const* target,
     std::set<cmGeneratorTarget const*>& depends);
 
+  /// Check if fn is contained in any LocalGenerator's OutputFiles.
+  bool IsContainedInLocalOutput(const std::string& fn) const;
+
   std::string CMakeCmd() const;
   std::string NinjaCmd() const;
15

@Holger good start, thanks. Please see CONTRIBUTING.rst and open a merge request for that. We can polish the details during review.

16

@brad.king Filed a merge request (https://gitlab.kitware.com/cmake/cmake/-/merge_requests/4939). Looking forward to your input!

17

CMake is re-running because generate.stamp is out-of-date.
Check the system time…