[gn] Don't read entire build.ninja into memory in gn gen/clean
This switches ExtractRegenerationCommands to use std::istream rather
than reading the whole file just to use the first ~10 lines.
Change-Id: I2a77c24541edd20d3f5e20130199855148e304fc
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/14321
Reviewed-by: Brett Wilson <brettw@chromium.org>
diff --git a/src/gn/commands.cc b/src/gn/commands.cc
index 5ff7ca8..3840761 100644
--- a/src/gn/commands.cc
+++ b/src/gn/commands.cc
@@ -4,6 +4,7 @@
#include "gn/commands.h"
+#include <fstream>
#include <optional>
#include "base/command_line.h"
@@ -492,25 +493,25 @@
// Write a stripped down build.ninja file with just the commands needed
// for ninja to call GN and regenerate ninja files.
- base::FilePath build_ninja_file(settings->GetFullPath(
+ base::FilePath build_ninja_path(settings->GetFullPath(
SourceFile(settings->build_dir().value() + "build.ninja")));
- std::string build_ninja_contents;
- if (!base::ReadFileToString(build_ninja_file, &build_ninja_contents)) {
- // Couldn't parse the build.ninja file.
- Err(Location(), "Couldn't read build.ninja in this directory.",
+ std::ifstream build_ninja_file(build_ninja_path.value());
+ if (!build_ninja_file) {
+ // Couldn't open the build.ninja file.
+ Err(Location(), "Couldn't open build.ninja in this directory.",
"Try running \"gn gen\" on it and then re-running \"gn clean\".")
.PrintToStdout();
return false;
}
std::string build_commands =
- NinjaBuildWriter::ExtractRegenerationCommands(build_ninja_contents);
+ NinjaBuildWriter::ExtractRegenerationCommands(build_ninja_file);
if (build_commands.empty()) {
Err(Location(), "Unexpected build.ninja contents in this directory.",
"Try running \"gn gen\" on it and then re-running \"gn clean\".")
.PrintToStdout();
return false;
}
- if (util::WriteFileAtomically(build_ninja_file, build_commands.data(),
+ if (util::WriteFileAtomically(build_ninja_path, build_commands.data(),
static_cast<int>(build_commands.size())) ==
-1) {
Err(Location(), std::string("Failed to write build.ninja."))
diff --git a/src/gn/ninja_build_writer.cc b/src/gn/ninja_build_writer.cc
index 90a296b..05db217 100644
--- a/src/gn/ninja_build_writer.cc
+++ b/src/gn/ninja_build_writer.cc
@@ -13,7 +13,6 @@
#include "base/command_line.h"
#include "base/files/file_util.h"
-#include "base/strings/string_split.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "gn/build_settings.h"
@@ -302,13 +301,10 @@
// static
std::string NinjaBuildWriter::ExtractRegenerationCommands(
- const std::string& build_ninja_in) {
- std::vector<std::string_view> lines = base::SplitStringPiece(
- build_ninja_in, "\n", base::KEEP_WHITESPACE, base::SPLIT_WANT_ALL);
-
+ std::istream& build_ninja_in) {
std::ostringstream out;
int num_blank_lines = 0;
- for (const auto& line : lines) {
+ for (std::string line; std::getline(build_ninja_in, line);) {
out << line << '\n';
if (line.empty())
++num_blank_lines;
diff --git a/src/gn/ninja_build_writer.h b/src/gn/ninja_build_writer.h
index 54a80aa..c21769a 100644
--- a/src/gn/ninja_build_writer.h
+++ b/src/gn/ninja_build_writer.h
@@ -56,8 +56,7 @@
// commands::PrepareForRegeneration.
//
// On error, returns an empty string.
- static std::string ExtractRegenerationCommands(
- const std::string& build_ninja_in);
+ static std::string ExtractRegenerationCommands(std::istream& build_ninja_in);
bool Run(Err* err);
diff --git a/src/gn/ninja_build_writer_unittest.cc b/src/gn/ninja_build_writer_unittest.cc
index 453ff03..f8abaf2 100644
--- a/src/gn/ninja_build_writer_unittest.cc
+++ b/src/gn/ninja_build_writer_unittest.cc
@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
+#include <fstream>
#include <sstream>
#include "base/command_line.h"
@@ -211,7 +212,7 @@
std::vector<const Target*> targets = {&target_foo};
- std::ostringstream ninja_out;
+ std::stringstream ninja_out;
std::ostringstream depfile_out;
NinjaBuildWriter writer(setup.build_settings(), used_toolchains, targets,
@@ -242,7 +243,7 @@
EXPECT_SNIPPET(ninja_out_str, expected_default);
std::string commands =
- NinjaBuildWriter::ExtractRegenerationCommands(ninja_out_str);
+ NinjaBuildWriter::ExtractRegenerationCommands(ninja_out);
EXPECT_SNIPPET(commands, expected_rule_gn);
EXPECT_SNIPPET(commands, expected_build_ninja_stamp);
EXPECT_SNIPPET(commands, expected_build_ninja);
@@ -254,6 +255,22 @@
#undef EXPECT_NO_SNIPPET
}
+TEST_F(NinjaBuildWriterTest, ExtractRegenerationCommands_DefaultStream) {
+ std::ifstream ninja_in;
+ EXPECT_EQ(NinjaBuildWriter::ExtractRegenerationCommands(ninja_in), "");
+}
+
+TEST_F(NinjaBuildWriterTest, ExtractRegenerationCommands_StreamError) {
+ std::ifstream ninja_in("/does/not/exist");
+ EXPECT_EQ(NinjaBuildWriter::ExtractRegenerationCommands(ninja_in), "");
+}
+
+TEST_F(NinjaBuildWriterTest, ExtractRegenerationCommands_IncompleteNinja) {
+ std::stringstream ninja_in;
+ ninja_in << "foo\nbar\nbaz\nbif\n";
+ EXPECT_EQ(NinjaBuildWriter::ExtractRegenerationCommands(ninja_in), "");
+}
+
TEST_F(NinjaBuildWriterTest, SpaceInDepfile) {
TestWithScope setup;
Err err;
