Remove symlink_output support
This was added so that bazel could run ninja files, but we abandoned
that approach.
Bug: 160568334
Test: OUT_DIR=out ./prebuilts/build-tools/build-prebuilts.sh
Change-Id: I2c1b4571dedce203527c84387976b13ca6c01111
diff --git a/src/build.cc b/src/build.cc
index a682bcd..d179418 100644
--- a/src/build.cc
+++ b/src/build.cc
@@ -977,32 +977,6 @@
}
}
- set<string> declared_symlinks;
- if (config_.uses_symlink_outputs) {
- string symlink_outputs = edge->GetSymlinkOutputs();
- if (symlink_outputs.length() > 0) {
- stringstream ss(symlink_outputs);
- string path;
- /// Naively split symlink_outputs path by the empty ' ' space character.
- /// because the '$ ' escape doesn't exist at this stage. In experimentation
- /// and practice across a number of AOSP configurations, this is OK.
- ///
- /// We could modify the GetBindingImpl/GetSymlinkOutputs API to support lists,
- /// but it'd be an invasive change that'll require a little bit more designing.
- /// For example, how do we expand "${out}.d" if ${out} is a list?
- ///
- /// That said, keep in mind that this is a simple string split that could
- /// fail with paths containing spaces.
- while (getline(ss, path, ' ')) {
- uint64_t slash_bits;
- if (!CanonicalizePath(&path, &slash_bits, err)) {
- return false;
- }
- declared_symlinks.insert(move(path));
- }
- }
- }
-
for (vector<Node*>::iterator o = edge->outputs_.begin();
o != edge->outputs_.end(); ++o) {
bool is_dir = false;
@@ -1013,32 +987,6 @@
TimeStamp new_mtime = (*o)->mtime();
- if (config_.uses_symlink_outputs) {
- /// Warn or error if created symlinks aren't declared in symlink_outputs,
- /// or if created files are declared in symlink_outputs.
- if (is_symlink) {
- if (declared_symlinks.find((*o)->path()) == declared_symlinks.end()) {
- // Not in declared_symlinks
- if (!result->output.empty())
- result->output.append("\n");
- result->output.append("ninja: " + (*o)->path() + " is a symlink, but it was not declared in symlink_outputs");
- if (config_.undeclared_symlink_outputs_should_err) {
- result->status = ExitFailure;
- }
- } else {
- declared_symlinks.erase((*o)->path());
- }
- } else if (!is_symlink && declared_symlinks.find((*o)->path()) != declared_symlinks.end()) {
- if (!result->output.empty())
- result->output.append("\n");
- result->output.append("ninja: " + (*o)->path() + " is not a symlink, but it was declared in symlink_outputs");
- declared_symlinks.erase((*o)->path());
- if (config_.undeclared_symlink_outputs_should_err) {
- result->status = ExitFailure;
- }
- }
- }
-
if (config_.uses_phony_outputs) {
if (new_mtime == 0) {
if (!result->output.empty())
@@ -1077,21 +1025,6 @@
}
}
- /// Ensure that declared_symlinks is empty after verifying that symlink outputs
- /// were declared in the edge. A non-empty declared_symlinks set indicates that
- /// not all declared symlinks were created by the edge itself (over-specification).
- if (config_.uses_symlink_outputs && declared_symlinks.size() > 0) {
- string missing_outputs;
- for (string symlink : declared_symlinks) {
- missing_outputs = missing_outputs + " " + symlink;
- }
- result->output.append(
- "ninja: not all symlink_outputs were created for this edge:" + missing_outputs);
- if (config_.undeclared_symlink_outputs_should_err) {
- result->status = ExitFailure;
- }
- }
-
status_->BuildEdgeFinished(edge, end_time_millis, result);
if (result->success() && !nodes_cleaned.empty()) {
diff --git a/src/build.h b/src/build.h
index 41afa33..b630f2d 100644
--- a/src/build.h
+++ b/src/build.h
@@ -167,8 +167,6 @@
failures_allowed(1), max_load_average(-0.0f),
frontend(NULL), frontend_file(NULL),
missing_depfile_should_err(false),
- uses_symlink_outputs(false),
- undeclared_symlink_outputs_should_err(false),
uses_phony_outputs(false),
output_directory_should_err(false),
missing_output_file_should_err(false),
@@ -200,13 +198,6 @@
/// Whether a missing depfile should warn or print an error.
bool missing_depfile_should_err;
- /// Whether Ninja should check that symlink outputs are declared in the
- /// symlink_outputs variable
- bool uses_symlink_outputs;
-
- /// Whether undeclared symlink outputs should print a warning or error out
- bool undeclared_symlink_outputs_should_err;
-
/// Whether the generator uses 'phony_output's
/// Controls the warnings below
bool uses_phony_outputs;
diff --git a/src/build_test.cc b/src/build_test.cc
index c8814ab..54e0334 100644
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -857,232 +857,6 @@
EXPECT_EQ("touch out out.imp", command_runner_.commands_ran_[0]);
}
-TEST_F(BuildTest, SymlinkOutputsIsValidVariable) {
- string err;
- ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
-"rule dangling_symlink\n"
-" command = ln -sf nil $out\n"
-" symlink_outputs = $out\n"
-"build l1: dangling_symlink\n"
-"rule symlink\n"
-" command = ln -sf $in $out\n"
-" symlink_outputs = $out\n"
-"build l2: symlink file\n"
-))
-
- fs_.Create("file", "content");
- /// Disabled, but symlink_outputs is still a valid variable.
- config_.uses_symlink_outputs = false;
-
- EXPECT_TRUE(builder_.AddTarget("l1", &err));
- EXPECT_TRUE(builder_.AddTarget("l2", &err));
- EXPECT_TRUE(builder_.Build(&err));
- EXPECT_EQ("", err);
- EXPECT_EQ(2u, command_runner_.commands_ran_.size());
-}
-
-TEST_F(BuildTest, SymlinkOutputsOKWithDeclaration) {
- string err;
- ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
-"rule dangling_symlink\n"
-" command = ln -sf nil $out\n"
-" symlink_outputs = $out\n"
-"build l1: dangling_symlink\n"
-"rule symlink\n"
-" command = ln -sf $in $out\n"
-" symlink_outputs = $out\n"
-"build l2: symlink file\n"
-))
-
- config_.uses_symlink_outputs = true;
- config_.undeclared_symlink_outputs_should_err = true;
-
- fs_.Create("file", "content");
-
- EXPECT_TRUE(builder_.AddTarget("l1", &err));
- EXPECT_TRUE(builder_.AddTarget("l2", &err));
- EXPECT_TRUE(builder_.Build(&err));
- EXPECT_EQ("", err);
- EXPECT_EQ(2u, command_runner_.commands_ran_.size());
-}
-
-TEST_F(BuildTest, SymlinkOutputsOKWithUncanonicalizedDeclaration) {
- string err;
- ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
-"rule dangling_symlink\n"
-" command = ln -sf nil .//$out\n"
-" symlink_outputs = ././$out\n"
-"build l1: dangling_symlink\n"
-"rule symlink\n"
-" command = ln -sf $in ././$out\n"
-" symlink_outputs = .//$out\n"
-"build l2: symlink file\n"
-))
-
- config_.uses_symlink_outputs = true;
- config_.undeclared_symlink_outputs_should_err = true;
-
- fs_.Create("file", "content");
-
- EXPECT_TRUE(builder_.AddTarget("l1", &err));
- EXPECT_TRUE(builder_.AddTarget("l2", &err));
- EXPECT_TRUE(builder_.Build(&err));
- EXPECT_EQ("", err);
- EXPECT_EQ(2u, command_runner_.commands_ran_.size());
-}
-
-TEST_F(BuildTest, FileOutputsWarnWithSymlinkOutputsDeclaration) {
- ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
-"rule touch\n"
-" command = touch $out\n"
-" symlink_outputs = $out\n"
-"build f1: touch\n"));
-
- config_.uses_symlink_outputs = true;
- config_.undeclared_symlink_outputs_should_err = false;
-
- string err;
- EXPECT_TRUE(builder_.AddTarget("f1", &err));
- EXPECT_EQ("", err);
-
- EXPECT_TRUE(builder_.Build(&err));
- EXPECT_EQ("ninja: f1 is not a symlink, but it was declared in symlink_outputs", status_.last_output_);
-}
-
-TEST_F(BuildTest, FileOutputsErrorWithSymlinkOutputsDeclaration) {
- ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
-"rule touch\n"
-" command = touch $out\n"
-" symlink_outputs = $out\n"
-"build f1: touch\n"));
-
- config_.uses_symlink_outputs = true;
- config_.undeclared_symlink_outputs_should_err = true;
-
- string err;
- EXPECT_TRUE(builder_.AddTarget("f1", &err));
- EXPECT_EQ("", err);
-
- EXPECT_FALSE(builder_.Build(&err));
- EXPECT_EQ("subcommand failed", err);
- EXPECT_EQ("ninja: f1 is not a symlink, but it was declared in symlink_outputs", status_.last_output_);
-}
-
-TEST_F(BuildTest, DanglingSymlinkOutputsWarnWithoutDeclaration) {
- ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
-"rule dangling_symlink\n"
-" command = ln -sf nil $out\n"
-"build l1: dangling_symlink\n"
-))
-
- config_.uses_symlink_outputs = true;
- config_.undeclared_symlink_outputs_should_err = false;
-
- string err;
- EXPECT_TRUE(builder_.AddTarget("l1", &err));
- EXPECT_EQ("", err);
-
- EXPECT_TRUE(builder_.Build(&err));
- EXPECT_EQ("ninja: l1 is a symlink, but it was not declared in symlink_outputs", status_.last_output_);
-}
-
-TEST_F(BuildTest, RegularSymlinkOutputsWarnWithoutDeclaration) {
- ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
-"rule symlink\n"
-" command = ln -sf $in $out\n"
-"build l1: symlink file\n"
-))
- fs_.Create("file", "content");
-
- config_.uses_symlink_outputs = true;
- config_.undeclared_symlink_outputs_should_err = false;
-
- string err;
- EXPECT_TRUE(builder_.AddTarget("l1", &err));
- EXPECT_EQ("", err);
-
- EXPECT_TRUE(builder_.Build(&err));
- EXPECT_EQ("ninja: l1 is a symlink, but it was not declared in symlink_outputs", status_.last_output_);
-}
-
-TEST_F(BuildTest, DanglingSymlinkOutputsErrorWithoutDeclaration) {
- ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
-"rule dangling_symlink\n"
-" command = ln -sf nil $out\n"
-"build l1: dangling_symlink\n"
-))
-
- config_.uses_symlink_outputs = true;
- config_.undeclared_symlink_outputs_should_err = true;
-
- string err;
- EXPECT_TRUE(builder_.AddTarget("l1", &err));
- EXPECT_EQ("", err);
-
- EXPECT_FALSE(builder_.Build(&err));
- EXPECT_EQ("subcommand failed", err);
- EXPECT_EQ("ninja: l1 is a symlink, but it was not declared in symlink_outputs", status_.last_output_);
-}
-
-TEST_F(BuildTest, RegularSymlinkOutputsErrorWithoutDeclaration) {
- ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
-"rule symlink\n"
-" command = ln -sf $in $out\n"
-"build l1: symlink file\n"
-))
- fs_.Create("file", "content");
-
- config_.uses_symlink_outputs = true;
- config_.undeclared_symlink_outputs_should_err = true;
-
- string err;
- EXPECT_TRUE(builder_.AddTarget("l1", &err));
- EXPECT_EQ("", err);
-
- EXPECT_FALSE(builder_.Build(&err));
- EXPECT_EQ("subcommand failed", err);
- EXPECT_EQ("ninja: l1 is a symlink, but it was not declared in symlink_outputs", status_.last_output_);
-}
-
-TEST_F(BuildTest, ExtraSymlinkOutputsPrintsWarning) {
- ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
-"rule dangling_symlink\n"
-" command = ln -sf nil $out\n"
-"build l1: dangling_symlink\n"
-" symlink_outputs = l1 l2\n"
-))
-
- config_.uses_symlink_outputs = true;
- config_.undeclared_symlink_outputs_should_err = false;
-
- string err;
- EXPECT_TRUE(builder_.AddTarget("l1", &err));
- EXPECT_EQ("", err);
-
- EXPECT_TRUE(builder_.Build(&err));
- EXPECT_EQ("ninja: not all symlink_outputs were created for this edge: l2", status_.last_output_);
-}
-
-TEST_F(BuildTest, ExtraSymlinkOutputsRaisesError) {
- ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
-"rule dangling_symlink\n"
-" command = ln -sf nil $out\n"
-"build l1: dangling_symlink\n"
-" symlink_outputs = l1 l2\n"
-))
-
- config_.uses_symlink_outputs = true;
- config_.undeclared_symlink_outputs_should_err = true;
-
- string err;
- EXPECT_TRUE(builder_.AddTarget("l1", &err));
- EXPECT_EQ("", err);
-
- EXPECT_FALSE(builder_.Build(&err));
- EXPECT_EQ("subcommand failed", err);
- EXPECT_EQ("ninja: not all symlink_outputs were created for this edge: l2", status_.last_output_);
-}
-
// Test case from
// https://github.com/ninja-build/ninja/issues/148
TEST_F(BuildTest, MultiOutIn) {
diff --git a/src/eval_env.cc b/src/eval_env.cc
index 507fb6d..6a02abb 100644
--- a/src/eval_env.cc
+++ b/src/eval_env.cc
@@ -41,7 +41,6 @@
var == "rspfile" ||
var == "rspfile_content" ||
var == "phony_output" ||
- var == "symlink_outputs" ||
var == "msvc_deps_prefix";
}
diff --git a/src/graph.cc b/src/graph.cc
index b1ed28b..e4ca090 100644
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -620,7 +620,6 @@
static const HashedStrView kDyndep { "dyndep" };
static const HashedStrView kRspfile { "rspfile" };
static const HashedStrView kRspFileContent { "rspfile_content" };
-static const HashedStrView kSymlinkOutputs { "symlink_outputs" };
bool Edge::EvaluateCommand(std::string* out_append, bool incl_rsp_file,
std::string* err) {
@@ -738,10 +737,6 @@
return GetBindingImpl(key, EdgeEval::kFinalScope, EdgeEval::kShellEscape);
}
-std::string Edge::GetSymlinkOutputs() {
- return GetBindingImpl(kSymlinkOutputs, EdgeEval::kFinalScope, EdgeEval::kDoNotEscape);
-}
-
std::string Edge::GetUnescapedDepfile() {
return GetBindingImpl(kDepfile, EdgeEval::kFinalScope, EdgeEval::kDoNotEscape);
}
diff --git a/src/graph.h b/src/graph.h
index 8e15b27..c4dd3e4 100644
--- a/src/graph.h
+++ b/src/graph.h
@@ -426,8 +426,6 @@
/// Like GetBinding("rspfile"), but without shell escaping.
string GetUnescapedRspfile();
- string GetSymlinkOutputs();
-
void Dump(const char* prefix="") const;
/// Temporary fields used only during manifest parsing.
diff --git a/src/ninja.cc b/src/ninja.cc
index 75da99c..ffcd121 100644
--- a/src/ninja.cc
+++ b/src/ninja.cc
@@ -1221,11 +1221,7 @@
" requires -o usesphonyoutputs=yes\n"
" outputdir={err,warn} how to treat outputs that are directories\n"
" missingoutfile={err,warn} how to treat missing output files\n"
-" oldoutput={err,warn} how to treat output files older than their inputs\n"
-"\n"
-" requires -o usessymlinkoutputs=yes\n"
-" undeclaredsymlinkoutputs={err,warn} build statements creating symlink outputs must "
-"declare them in symlink_outputs\n");
+" oldoutput={err,warn} how to treat output files older than their inputs\n");
return false;
} else if (name == "dupbuild=err") {
options->dupe_edges_should_err = true;
@@ -1269,12 +1265,6 @@
} else if (name == "oldoutput=warn") {
config->old_output_should_err = false;
return true;
- } else if (name == "undeclaredsymlinkoutputs=err") {
- config->undeclared_symlink_outputs_should_err = true;
- return true;
- } else if (name == "undeclaredsymlinkoutputs=warn") {
- config->undeclared_symlink_outputs_should_err = false;
- return true;
} else {
const char* suggestion =
SpellcheckString(name.c_str(), "dupbuild=err", "dupbuild=warn",
@@ -1282,8 +1272,7 @@
"missingdepfile=err", "missingdepfile=warn",
"outputdir=err", "outputdir=warn",
"missingoutfile=err", "missingoutfile=warn",
- "oldoutput=err", "oldoutput=warn",
- "undeclaredsymlinkoutputs=err", "undeclaredsymlinkoutputs=warn", NULL);
+ "oldoutput=err", "oldoutput=warn", NULL);
if (suggestion) {
Error("unknown warning flag '%s', did you mean '%s'?",
name.c_str(), suggestion);
@@ -1304,9 +1293,6 @@
" outputdir\n"
" missingoutfile\n"
" oldoutput\n"
-" usessymlinkoutputs={yes,no} whether the generate uses 'symlink_outputs' so \n"
-" that these warnings work:\n"
-" undeclaredsymlinkoutputs\n"
" preremoveoutputs={yes,no} whether to remove outputs before running rule\n"
" usesninjalogasweightlist={yes,no} whether to use ninja log as source of weight list\n"
" usesweightlist={<file path>,no} whether to prioritize some rules based on weight list from file\n");
@@ -1317,12 +1303,6 @@
} else if (name == "usesphonyoutputs=no") {
config->uses_phony_outputs = false;
return true;
- } else if (name == "usessymlinkoutputs=yes") {
- config->uses_symlink_outputs = true;
- return true;
- } else if (name == "usessymlinkoutputs=no") {
- config->uses_symlink_outputs = false;
- return true;
} else if (name == "preremoveoutputs=yes") {
config->pre_remove_output_files = true;
return true;
