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

Symmetry: Accept 'foreign' build artifacts without .ninja_log entry #1

Merged
merged 1 commit into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ else()
add_compile_options(-fdiagnostics-color)
endif()
endif()
add_compile_definitions(SYMMETRY)

# --- optional re2c
find_program(RE2C re2c)
Expand Down
23 changes: 23 additions & 0 deletions src/build_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1652,6 +1652,9 @@ TEST_F(BuildWithLogTest, NotInLogButOnDisk) {
// Because it's not in the log, it should not be up-to-date until
// we build again.
EXPECT_TRUE(builder_.AddTarget("out1", &err));
#ifdef SYMMETRY
EXPECT_TRUE(builder_.AlreadyUpToDate());
#else
EXPECT_FALSE(builder_.AlreadyUpToDate());

command_runner_.commands_ran_.clear();
Expand All @@ -1660,6 +1663,7 @@ TEST_F(BuildWithLogTest, NotInLogButOnDisk) {
EXPECT_TRUE(builder_.AddTarget("out1", &err));
EXPECT_TRUE(builder_.Build(&err));
EXPECT_TRUE(builder_.AlreadyUpToDate());
#endif
}

TEST_F(BuildWithLogTest, RebuildAfterFailure) {
Expand Down Expand Up @@ -1701,10 +1705,14 @@ TEST_F(BuildWithLogTest, RebuildAfterFailure) {

// Run again, should rerun even though the output file is up to date on disk
EXPECT_TRUE(builder_.AddTarget("out1", &err));
#ifdef SYMMETRY
EXPECT_TRUE(builder_.AlreadyUpToDate());
#else
EXPECT_FALSE(builder_.AlreadyUpToDate());
EXPECT_TRUE(builder_.Build(&err));
EXPECT_EQ(1u, command_runner_.commands_ran_.size());
EXPECT_EQ("", err);
#endif
}

TEST_F(BuildWithLogTest, RebuildWithNoInputs) {
Expand Down Expand Up @@ -1758,6 +1766,9 @@ TEST_F(BuildWithLogTest, RestatTest) {

fs_.Create("in", "");

#ifdef SYMMETRY
string err;
#else
// Do a pre-build so that there's commands in the log for the outputs,
// otherwise, the lack of an entry in the build log will cause out3 to rebuild
// regardless of restat.
Expand All @@ -1774,6 +1785,8 @@ TEST_F(BuildWithLogTest, RestatTest) {
fs_.Tick();

fs_.Create("in", "");
#endif

// "cc" touches out1, so we should build out2. But because "true" does not
// touch out2, we should cancel the build of out3.
EXPECT_TRUE(builder_.AddTarget("out3", &err));
Expand Down Expand Up @@ -3001,7 +3014,11 @@ TEST_F(BuildWithDepsLogTest, RestatMissingDepfileDepslog) {
fs_.Create("header.h", "");

RebuildTarget("out", manifest, "build_log", "ninja_deps");
#ifdef SYMMETRY
ASSERT_EQ(1u, command_runner_.commands_ran_.size());
#else
ASSERT_EQ(2u, command_runner_.commands_ran_.size());
#endif

// Sanity: this rebuild should be NOOP
RebuildTarget("out", manifest, "build_log", "ninja_deps");
Expand Down Expand Up @@ -3639,10 +3656,16 @@ TEST_F(BuildWithLogTest, DyndepBuildDiscoverRestat) {
ASSERT_EQ("", err);
EXPECT_TRUE(builder_.Build(&err));
ASSERT_EQ("", err);
#ifdef SYMMETRY
ASSERT_EQ(2u, command_runner_.commands_ran_.size());
#else
ASSERT_EQ(3u, command_runner_.commands_ran_.size());
#endif
EXPECT_EQ("cp dd-in dd", command_runner_.commands_ran_[0]);
EXPECT_EQ("true", command_runner_.commands_ran_[1]);
#ifndef SYMMETRY
EXPECT_EQ("cat out1 > out2", command_runner_.commands_ran_[2]);
#endif

command_runner_.commands_ran_.clear();
state_.Reset();
Expand Down
10 changes: 10 additions & 0 deletions src/graph.cc
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,10 @@ bool DependencyScan::RecomputeOutputDirty(const Edge* edge,
EXPLAIN("command line changed for %s", output->path().c_str());
return true;
}
#ifdef SYMMETRY
// ignore mtime of an existing .ninja_log entry
// (for reggae's --dub-objs-dir and re-using 'foreign' artifacts)
#else
if (most_recent_input && entry->mtime < most_recent_input->mtime()) {
// May also be dirty due to the mtime in the log being older than the
// mtime of the most recent input. This can occur even when the mtime
Expand All @@ -352,11 +356,17 @@ bool DependencyScan::RecomputeOutputDirty(const Edge* edge,
entry->mtime, most_recent_input->mtime());
return true;
}
#endif
}
#ifdef SYMMETRY
// don't require an existing .ninja_log entry
// (for reggae's --dub-objs-dir and re-using 'foreign' artifacts)
#else
if (!entry && !generator) {
EXPLAIN("command line not found in log for %s", output->path().c_str());
return true;
}
#endif
}

return false;
Expand Down