Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

Commit b56fe80

Browse files
sgrahamevmar
authored andcommitted
if a file is missing in the log, count it as dirty
This could cause overbuilding (if the log is missing an entry and the right file is already in place) but is otherwise necessary for correctness (if a file is already in place but we don't have a log entry for it).
1 parent ac04abe commit b56fe80

File tree

2 files changed

+63
-6
lines changed

2 files changed

+63
-6
lines changed

src/build_test.cc

+54-2
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,31 @@ struct BuildWithLogTest : public BuildTest {
723723
BuildLog build_log_;
724724
};
725725

726+
TEST_F(BuildWithLogTest, NotInLogButOnDisk) {
727+
ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
728+
"rule cc\n"
729+
" command = cc\n"
730+
"build out1: cc in\n"));
731+
732+
// Create input/output that would be considered up to date when
733+
// not considering the command line hash.
734+
fs_.Create("in", now_, "");
735+
fs_.Create("out1", now_, "");
736+
string err;
737+
738+
// Because it's not in the log, it should not be up-to-date until
739+
// we build again.
740+
EXPECT_TRUE(builder_.AddTarget("out1", &err));
741+
EXPECT_FALSE(builder_.AlreadyUpToDate());
742+
743+
commands_ran_.clear();
744+
state_.Reset();
745+
746+
EXPECT_TRUE(builder_.AddTarget("out1", &err));
747+
EXPECT_TRUE(builder_.Build(&err));
748+
EXPECT_TRUE(builder_.AlreadyUpToDate());
749+
}
750+
726751
TEST_F(BuildWithLogTest, RestatTest) {
727752
ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
728753
"rule true\n"
@@ -743,9 +768,22 @@ TEST_F(BuildWithLogTest, RestatTest) {
743768

744769
fs_.Create("in", now_, "");
745770

771+
// Do a pre-build so that there's commands in the log for the outputs,
772+
// otherwise, the lack of an entry in the build log will cause out3 to rebuild
773+
// regardless of restat.
774+
string err;
775+
EXPECT_TRUE(builder_.AddTarget("out3", &err));
776+
ASSERT_EQ("", err);
777+
EXPECT_TRUE(builder_.Build(&err));
778+
ASSERT_EQ("", err);
779+
commands_ran_.clear();
780+
state_.Reset();
781+
782+
now_++;
783+
784+
fs_.Create("in", now_, "");
746785
// "cc" touches out1, so we should build out2. But because "true" does not
747786
// touch out2, we should cancel the build of out3.
748-
string err;
749787
EXPECT_TRUE(builder_.AddTarget("out3", &err));
750788
ASSERT_EQ("", err);
751789
EXPECT_TRUE(builder_.Build(&err));
@@ -790,10 +828,24 @@ TEST_F(BuildWithLogTest, RestatMissingFile) {
790828
fs_.Create("in", now_, "");
791829
fs_.Create("out2", now_, "");
792830

831+
// Do a pre-build so that there's commands in the log for the outputs,
832+
// otherwise, the lack of an entry in the build log will cause out2 to rebuild
833+
// regardless of restat.
834+
string err;
835+
EXPECT_TRUE(builder_.AddTarget("out2", &err));
836+
ASSERT_EQ("", err);
837+
EXPECT_TRUE(builder_.Build(&err));
838+
ASSERT_EQ("", err);
839+
commands_ran_.clear();
840+
state_.Reset();
841+
842+
now_++;
843+
fs_.Create("in", now_, "");
844+
fs_.Create("out2", now_, "");
845+
793846
// Run a build, expect only the first command to run.
794847
// It doesn't touch its output (due to being the "true" command), so
795848
// we shouldn't run the dependent build.
796-
string err;
797849
EXPECT_TRUE(builder_.AddTarget("out2", &err));
798850
ASSERT_EQ("", err);
799851
EXPECT_TRUE(builder_.Build(&err));

src/graph.cc

+9-4
Original file line numberDiff line numberDiff line change
@@ -158,10 +158,15 @@ bool Edge::RecomputeOutputDirty(BuildLog* build_log,
158158
// May also be dirty due to the command changing since the last build.
159159
// But if this is a generator rule, the command changing does not make us
160160
// dirty.
161-
if (!rule_->generator() && build_log &&
162-
(entry || (entry = build_log->LookupByOutput(output->path())))) {
163-
if (BuildLog::LogEntry::HashCommand(command) != entry->command_hash) {
164-
EXPLAIN("command line changed for %s", output->path().c_str());
161+
if (!rule_->generator() && build_log) {
162+
if (entry || (entry = build_log->LookupByOutput(output->path()))) {
163+
if (BuildLog::LogEntry::HashCommand(command) != entry->command_hash) {
164+
EXPLAIN("command line changed for %s", output->path().c_str());
165+
return true;
166+
}
167+
}
168+
if (!entry) {
169+
EXPLAIN("command line not found in log for %s", output->path().c_str());
165170
return true;
166171
}
167172
}

0 commit comments

Comments
 (0)