commit 4b5a2177cb2f246e6cf5fa1316ff4323ece37408
author David Hubbard <dsp@google.com> Wed Sep 21 16:18:07 2022 -0700
committer David Hubbard <dsp@google.com> Wed Sep 21 16:18:07 2022 -0700
tree 03e0a2898e4275b6fcd15d71cf6d19af8445700f
parent 246659993ca5e43621f11711d7935223b6d1f295

Subninja chdir: 'phony' default rule

This fixes the following error in a subninja chdir:

ninja: error: b/build.ninja:8: unknown build rule 'phony'
build all: phony b2.txt
           ^ near here

The bug is caused by the builtin rules (just one rule in this case)
being added to the root scope in the State() constructor. Since
subninja chdir's have a weird scope for their root scope -- one that
both points to its parent and also acts like it has no parent --
the scope gets the builtin rules added just like a root scope.

The subninja chdir scope points to its parent to be able to translate
relative paths to absolute paths.

The subninja chdir scope blocks traversal to its parent when looking
up variables and rules.

This commit fixes the one problem with blocking traversal to its parent:
the default rules (just one rule, the rule for "phony") are not visible
in a chdir.

Change-Id: I42b6011a3ed87924f5c8738c7d049d7d2b311e4f

src/eval_env.cc

diff --git a/src/eval_env.cc b/src/eval_env.cc
index 38517ef..924da07 100644
--- a/src/eval_env.cc
+++ b/src/eval_env.cc
@@ -239,3 +239,8 @@
   }
   return path;
 }
+
+// Add all built-in rules at the top of a root scope.
+void Scope::AddAllBuiltinRules() {
+  AddRule(&State::kPhonyRule);
+}

src/eval_env.h

diff --git a/src/eval_env.h b/src/eval_env.h
index 82e15f7..db7edd6 100644
--- a/src/eval_env.h
+++ b/src/eval_env.h
@@ -196,6 +196,9 @@
     return rules_.insert({ rule->name_hashed(), rule }).second;
   }
 
+  // Add all built-in rules at the top of a root scope.
+  void AddAllBuiltinRules();
+
   static Rule* LookupRuleAtPos(const HashedStrView& rule_name,
                                ScopePosition pos);
 

src/manifest_parser.cc

diff --git a/src/manifest_parser.cc b/src/manifest_parser.cc
index 06b1c82..225e0a2 100644
--- a/src/manifest_parser.cc
+++ b/src/manifest_parser.cc
@@ -184,6 +184,7 @@
 
     // Create scope so that subninja chdir variable lookups cannot see parent_
     *child_scope = new Scope(scope, include.chdir_plus_slash_);
+    (*child_scope)->AddAllBuiltinRules();
   } else if (include.new_scope_) {
     *child_scope = new Scope(scope->GetCurrentEndOfScope());
   } else {

src/state.cc

diff --git a/src/state.cc b/src/state.cc
index d9a3f77..df7966f 100644
--- a/src/state.cc
+++ b/src/state.cc
@@ -73,16 +73,11 @@
   // Reserve scope position (root, 0) for built-in rules.
   root_scope_.AllocDecls(1);
 
-  AddBuiltinRule(&kPhonyRule);
+  root_scope_.AddAllBuiltinRules();
   AddPool(&kDefaultPool);
   AddPool(&kConsolePool);
 }
 
-// Add a built-in rule at the top of the root scope.
-void State::AddBuiltinRule(Rule* rule) {
-  root_scope_.AddRule(rule);
-}
-
 bool State::AddPool(Pool* pool) {
   return pools_.insert({ pool->name_hashed(), pool }).second;
 }