[pull] master from ninja-build:master

.github/workflows/linux-musl.yml

@@ -36,7 +36,7 @@ jobs:
 
       - name: Configure ninja build
         if: matrix.build_method == 'cmake'
-        run: cmake -B build -D CMAKE_BUILD_TYPE="Release"
+        run: cmake -B build -D CMAKE_BUILD_TYPE="Release" -D CMAKE_COMPILE_WARNING_AS_ERROR="ON"
 
       - name: Cmake Build ninja
         if: matrix.build_method == 'cmake'
@@ -48,7 +48,7 @@ jobs:
 
       - name: Python Build ninja
         if: matrix.build_method == 'python'
-        run: python3 configure.py --bootstrap --verbose
+        run: python3 configure.py --warnings-as-errors --bootstrap --verbose
 
       - name: Python test ninja
         if: matrix.build_method == 'python'

.github/workflows/linux.yml

@@ -69,7 +69,7 @@ jobs:
       - name: Build release ninja
         shell: bash
         run: |
-          cmake -GNinja -DCMAKE_BUILD_TYPE=Release -B release-build
+          cmake -GNinja -DCMAKE_BUILD_TYPE=Release -B release-build -DCMAKE_COMPILE_WARNING_AS_ERROR=1
           cmake --build release-build --parallel --config Release
           strip release-build/ninja
 
@@ -164,6 +164,8 @@ jobs:
           apt install -y g++ python3
       - name: ${{ matrix.image }}
         run: |
+          # Do not set --warnings-as-errors here as that triggers an irrelevant
+          # compiler warnings in <stdio.h> with ubuntu:24.04. See issue #2615
           python3 configure.py --bootstrap
           ./ninja all
           python3 misc/ninja_syntax_test.py
@@ -184,7 +186,7 @@ jobs:
 
       - run: |
           # BUILD
-          cmake -DCMAKE_BUILD_TYPE=Release -B release-build
+          cmake -DCMAKE_BUILD_TYPE=Release -B release-build -DCMAKE_COMPILE_WARNING_AS_ERROR=1
           cmake --build release-build --parallel --config Release
           strip release-build/ninja
           file release-build/ninja

.github/workflows/macos.yml

@@ -21,7 +21,7 @@ jobs:
       env:
         MACOSX_DEPLOYMENT_TARGET: 10.15
       run: |
-        cmake -Bbuild -GXcode '-DCMAKE_OSX_ARCHITECTURES=arm64;x86_64'
+        cmake -Bbuild -GXcode '-DCMAKE_OSX_ARCHITECTURES=arm64;x86_64' -DCMAKE_COMPILE_WARNING_AS_ERROR=1
         cmake --build build --config Release
 
     - name: Test ninja (Release)

.github/workflows/windows.yml

@@ -28,7 +28,7 @@ jobs:
     - name: Build ninja
       shell: bash
       run: |
-        cmake -Bbuild -A ${{ matrix.arch }}
+        cmake -Bbuild -A ${{ matrix.arch }} -DCMAKE_COMPILE_WARNING_AS_ERROR=1
         cmake --build build --parallel --config Debug
         cmake --build build --parallel --config Release
 

configure.py

@@ -220,6 +220,8 @@ def _run_command(self, cmdline: str) -> None:
                   choices=Platform.known_platforms())
 parser.add_option('--debug', action='store_true',
                   help='enable debugging extras',)
+parser.add_option('--warnings-as-errors', action='store_true',
+                  help='convert warnings into errors during build',)
 parser.add_option('--profile', metavar='TYPE',
                   choices=profilers,
                   help='enable profiling (' + '/'.join(profilers) + ')',)
@@ -349,6 +351,8 @@ def search_system_path(file_name: str) -> Optional[str]:  # type: ignore # Missi
               '/DNOMINMAX', '/D_CRT_SECURE_NO_WARNINGS',
               '/D_HAS_EXCEPTIONS=0',
               '/DNINJA_PYTHON="%s"' % options.with_python]
+    if options.warnings_as_errors:
+        cflags.append('/WX')
     if platform.msvc_needs_fs():
         cflags.append('/FS')
     ldflags = ['/DEBUG', '/libpath:$builddir']
@@ -365,6 +369,8 @@ def search_system_path(file_name: str) -> Optional[str]:  # type: ignore # Missi
               '-std=c++14',
               '-fvisibility=hidden', '-pipe',
               '-DNINJA_PYTHON="%s"' % options.with_python]
+    if options.warnings_as_errors:
+        cflags += [ "-Werror" ]
     if options.debug:
         cflags += ['-D_GLIBCXX_DEBUG', '-D_GLIBCXX_DEBUG_PEDANTIC']
         cflags.remove('-fno-rtti')  # Needed for above pedanticness.

src/build.cc

@@ -951,8 +951,9 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) {
     disk_interface_->RemoveFile(rspfile);
 
   if (scan_.build_log()) {
-    if (!scan_.build_log()->RecordCommand(edge, start_time_millis,
-                                          end_time_millis, record_mtime)) {
+    if (!scan_.build_log()->RecordCommand(
+            edge, static_cast<int>(start_time_millis),
+            static_cast<int>(end_time_millis), record_mtime)) {
       *err = string("Error writing to build log: ") + strerror(errno);
       return false;
     }

src/build_test.cc

@@ -1024,7 +1024,7 @@ TEST_F(BuildTest, DepFileMissing) {
 
 TEST_F(BuildTest, DepFileOK) {
   string err;
-  int orig_edges = state_.edges_.size();
+  int orig_edges = static_cast<int>(state_.edges_.size());
   ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
 "rule cc\n  command = cc $in\n  depfile = $out.d\n"
 "build foo.o: cc foo.c\n"));

src/deps_log.cc

@@ -62,7 +62,7 @@ bool DepsLog::OpenForWrite(const string& path, string* err) {
 
 bool DepsLog::RecordDeps(Node* node, TimeStamp mtime,
                          const vector<Node*>& nodes) {
-  return RecordDeps(node, mtime, nodes.size(), nodes.data());
+  return RecordDeps(node, mtime, static_cast<int>(nodes.size()), nodes.data());
 }
 
 bool DepsLog::RecordDeps(Node* node, TimeStamp mtime, int node_count,
@@ -260,7 +260,7 @@ LoadStatus DepsLog::Load(const string& path, State* state, string* err) {
       // dependency record entry.)
       unsigned checksum = *reinterpret_cast<unsigned*>(buf + size - 4);
       int expected_id = ~checksum;
-      int id = nodes_.size();
+      int id = static_cast<int>(nodes_.size());
       if (id != expected_id || node->id() >= 0) {
         read_failed = true;
         break;
@@ -398,7 +398,7 @@ bool DepsLog::UpdateDeps(int out_id, Deps* deps) {
 }
 
 bool DepsLog::RecordId(Node* node) {
-  int path_size = node->path().size();
+  int path_size = static_cast<int>(node->path().size());
   assert(path_size > 0 && "Trying to record empty path Node!");
   int padding = (4 - path_size % 4) % 4;  // Pad path to 4 byte boundary.
 
@@ -418,7 +418,7 @@ bool DepsLog::RecordId(Node* node) {
   }
   if (padding && fwrite("\0\0", padding, 1, file_) < 1)
     return false;
-  int id = nodes_.size();
+  int id = static_cast<int>(nodes_.size());
   unsigned checksum = ~(unsigned)id;
   if (fwrite(&checksum, 4, 1, file_) < 1)
     return false;

src/deps_log_test.cc

@@ -426,7 +426,7 @@ TEST_F(DepsLogTest, Truncated) {
     }
 
     ASSERT_GE(node_count, (int)log.nodes().size());
-    node_count = log.nodes().size();
+    node_count = static_cast<int>(log.nodes().size());
 
     // Count how many non-NULL deps entries there are.
     int new_deps_count = 0;

src/edit_distance.cc

@@ -17,8 +17,6 @@
 #include <algorithm>
 #include <vector>
 
-using namespace std;
-
 int EditDistance(const StringPiece& s1,
                  const StringPiece& s2,
                  bool allow_replacements,
@@ -35,10 +33,10 @@ int EditDistance(const StringPiece& s1,
   // only the entries to the left, top, and top-left are needed.  The left
   // entry is in row[x-1], the top entry is what's in row[x] from the last
   // iteration, and the top-left entry is stored in previous.
-  int m = s1.len_;
-  int n = s2.len_;
+  int m = static_cast<int>(s1.len_);
+  int n = static_cast<int>(s2.len_);
 
-  vector<int> row(n + 1);
+  std::vector<int> row(n + 1);
   for (int i = 1; i <= n; ++i)
     row[i] = i;
 
@@ -50,17 +48,17 @@ int EditDistance(const StringPiece& s1,
     for (int x = 1; x <= n; ++x) {
       int old_row = row[x];
       if (allow_replacements) {
-        row[x] = min(previous + (s1.str_[y - 1] == s2.str_[x - 1] ? 0 : 1),
-                     min(row[x - 1], row[x]) + 1);
+        row[x] = std::min(previous + (s1.str_[y - 1] == s2.str_[x - 1] ? 0 : 1),
+                          std::min(row[x - 1], row[x]) + 1);
       }
       else {
         if (s1.str_[y - 1] == s2.str_[x - 1])
           row[x] = previous;
         else
-          row[x] = min(row[x - 1], row[x]) + 1;
+          row[x] = std::min(row[x - 1], row[x]) + 1;
       }
       previous = old_row;
-      best_this_row = min(best_this_row, row[x]);
+      best_this_row = std::min(best_this_row, row[x]);
     }
 
     if (max_edit_distance && best_this_row > max_edit_distance)

src/graph.cc

@@ -408,12 +408,14 @@ struct EdgeEnv : public Env {
 
 string EdgeEnv::LookupVariable(const string& var) {
   if (var == "in" || var == "in_newline") {
-    int explicit_deps_count = edge_->inputs_.size() - edge_->implicit_deps_ -
-      edge_->order_only_deps_;
+    int explicit_deps_count =
+        static_cast<int>(edge_->inputs_.size() - edge_->implicit_deps_ -
+                         edge_->order_only_deps_);
     return MakePathList(edge_->inputs_.data(), explicit_deps_count,
                         var == "in" ? ' ' : '\n');
   } else if (var == "out") {
-    int explicit_outs_count = edge_->outputs_.size() - edge_->implicit_outs_;
+    int explicit_outs_count =
+        static_cast<int>(edge_->outputs_.size() - edge_->implicit_outs_);
     return MakePathList(&edge_->outputs_[0], explicit_outs_count, ' ');
   }
 
@@ -706,7 +708,7 @@ bool ImplicitDepLoader::ProcessDepfileDeps(
     Edge* edge, std::vector<StringPiece>* depfile_ins, std::string* err) {
   // Preallocate space in edge->inputs_ to be filled in below.
   vector<Node*>::iterator implicit_dep =
-      PreallocateSpace(edge, depfile_ins->size());
+      PreallocateSpace(edge, static_cast<int>(depfile_ins->size()));
 
   // Add all its in-edges.
   for (std::vector<StringPiece>::iterator i = depfile_ins->begin();

src/jobserver-posix.cc

@@ -49,7 +49,7 @@ class PosixJobserverClient : public Jobserver::Client {
       return Jobserver::Slot::CreateImplicit();
     }
     uint8_t slot_char = '\0';
-    int ret;
+    ssize_t ret;
     do {
       ret = ::read(read_fd_, &slot_char, 1);
     } while (ret < 0 && errno == EINTR);
@@ -70,7 +70,7 @@ class PosixJobserverClient : public Jobserver::Client {
     }
 
     uint8_t slot_char = slot.GetExplicitValue();
-    int ret;
+    ssize_t ret;
     do {
       ret = ::write(write_fd_, &slot_char, 1);
     } while (ret < 0 && errno == EINTR);

src/jobserver_test.cc

@@ -331,7 +331,8 @@ TEST(Jobserver, PosixFifoClient) {
   ASSERT_TRUE(write_fd.IsValid()) << "Cannot open FIFO at: " << strerror(errno);
   for (size_t n = 0; n < kSlotCount; ++n) {
     uint8_t slot_byte = static_cast<uint8_t>('0' + n);
-    ::write(write_fd.fd_, &slot_byte, 1);
+    ssize_t ret = ::write(write_fd.fd_, &slot_byte, 1);
+    (void)ret;  // make compiler happy
   }
   // Keep the file descriptor opened to ensure the fifo's content
   // persists in kernel memory.

src/manifest_parser.cc

@@ -117,7 +117,7 @@ bool ManifestParser::ParsePool(string* err) {
 
     if (key == "depth") {
       string depth_string = value.Evaluate(env_);
-      depth = atol(depth_string.c_str());
+      depth = atoi(depth_string.c_str());
       if (depth < 0)
         return lexer_.Error("invalid pool depth", err);
     } else {

src/missing_deps.cc

@@ -103,7 +103,8 @@ void MissingDependencyScanner::ProcessNode(Node* node) {
     std::string err;
     dep_loader.LoadDeps(edge, &err);
     if (!depfile_deps.empty())
-      ProcessNodeDeps(node, &depfile_deps[0], depfile_deps.size());
+      ProcessNodeDeps(node, &depfile_deps[0],
+                      static_cast<int>(depfile_deps.size()));
   }
 }
 

src/ninja.cc

@@ -1713,27 +1713,29 @@ int ReadFlags(int* argc, char*** argv,
         break;
       case 'j': {
         char* end;
-        int value = strtol(optarg, &end, 10);
+        long value = strtol(optarg, &end, 10);
         if (*end != 0 || value < 0)
           Fatal("invalid -j parameter");
 
         // We want to run N jobs in parallel. For N = 0, INT_MAX
         // is close enough to infinite for most sane builds.
-        config->parallelism = value > 0 ? value : INT_MAX;
+        config->parallelism =
+            static_cast<int>((value > 0 && value < INT_MAX) ? value : INT_MAX);
         config->disable_jobserver_client = true;
         deferGuessParallelism.needGuess = false;
         break;
       }
       case 'k': {
         char* end;
-        int value = strtol(optarg, &end, 10);
+        long value = strtol(optarg, &end, 10);
         if (*end != 0)
           Fatal("-k parameter not numeric; did you mean -k 0?");
 
         // We want to go until N jobs fail, which means we should allow
         // N failures and then stop.  For N <= 0, INT_MAX is close enough
         // to infinite for most sane builds.
-        config->failures_allowed = value > 0 ? value : INT_MAX;
+        config->failures_allowed =
+            static_cast<int>((value > 0 && value < INT_MAX) ? value : INT_MAX);
         break;
       }
       case 'l': {

src/subprocess_test.cc

@@ -230,7 +230,7 @@ TEST_F(SubprocessTest, SetWithLots) {
   ASSERT_EQ(0, getrlimit(RLIMIT_NOFILE, &rlim));
   if (rlim.rlim_cur < kNumProcs) {
     printf("Raise [ulimit -n] above %u (currently %lu) to make this test go\n",
-           kNumProcs, rlim.rlim_cur);
+           kNumProcs, static_cast<unsigned long>(rlim.rlim_cur));
     return;
   }
 

src/util.cc

@@ -798,11 +798,13 @@ int ParseCPUFromCGroup() {
   auto cgroups = ParseMountInfo(subsystems);
 
   // Prefer cgroup v2 if both v1 and v2 should be present
-  if (const auto cgroup2 = cgroups.find("cgroup2"); cgroup2 != cgroups.end()) {
+  const auto cgroup2 = cgroups.find("cgroup2");
+  if (cgroup2 != cgroups.end()) {
     return ParseCgroupV2(cgroup2->second);
   }
 
-  if (const auto cpu = cgroups.find("cpu"); cpu != cgroups.end()) {
+  const auto cpu = cgroups.find("cpu");
+  if (cpu != cgroups.end()) {
     return ParseCgroupV1(cpu->second);
   }
   return -1;
@@ -878,7 +880,8 @@ int GetProcessorCount() {
   }
 #endif
   if (cgroupCount >= 0 && schedCount >= 0) return std::min(cgroupCount, schedCount);
-  if (cgroupCount < 0 && schedCount < 0) return sysconf(_SC_NPROCESSORS_ONLN);
+  if (cgroupCount < 0 && schedCount < 0)
+    return static_cast<int>(sysconf(_SC_NPROCESSORS_ONLN));
   return std::max(cgroupCount, schedCount);
 #endif
 }