From b611e797ad9db9794f4d151f454fa731d12b0bd3 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Wed, 14 Nov 2018 13:08:29 +0200
Subject: Fallback to loading outer buildfile if there isn't one in src_base

This covers the case where the target is defined in the outer buildfile which
is common with non-intrusive project conversions where everything is built
from a single root buildfile.
---
 build2/b.cxx                | 95 +++++++++++++++++++++++++++++++++++----------
 build2/config/operation.cxx |  4 +-
 build2/config/utility.cxx   |  2 +-
 build2/file.cxx             | 16 ++++----
 build2/file.hxx             |  2 +
 build2/filesystem.cxx       |  9 +++--
 build2/filesystem.hxx       |  2 +-
 build2/operation.cxx        | 18 ++++++++-
 build2/operation.hxx        |  2 +
 build2/parser.cxx           |  6 +--
 build2/target.cxx           | 69 +++++++++++++++++++++++++++++++-
 build2/target.hxx           | 11 ++++++
 build2/target.txx           | 29 ++------------
 13 files changed, 199 insertions(+), 66 deletions(-)

diff --git a/build2/b.cxx b/build2/b.cxx
index c1bf07d..5415472 100644
--- a/build2/b.cxx
+++ b/build2/b.cxx
@@ -511,20 +511,30 @@ main (int argc, char* argv[])
     if (bspec.empty ())
       bspec.push_back (metaopspec ()); // Default meta-operation.
 
-    // Check for a buildfile in the specified directory returning empty path
-    // if it does not exist.
+    // Check for a buildfile starting from the specified directory and
+    // continuing in the parent directories until root. Return empty path if
+    // not found.
     //
-    auto find_buildfile = [] (const dir_path& d)
+    auto find_buildfile = [] (const dir_path& d, const dir_path& root)
     {
       const path& n (ops.buildfile ());
 
-      if (n.string () != "-")
+      if (n.string () == "-")
+        return n;
+
+      for (path f (d / n);; )
       {
-        path f (d / n);
-        return exists (f) ? f : path ();
+        if (exists (f))
+          return f;
+
+        dir_path p (f.directory ());
+        if (p == root)
+          break;
+
+        f = p.directory () / n;
       }
-      else
-        return n;
+
+      return path ();
     };
 
     // If not NULL, then lifted points to the operation that has been "lifted"
@@ -814,9 +824,16 @@ main (int argc, char* argv[])
               // then this can only mean a simple project. Which in turn means
               // there should be a buildfile in out_base.
               //
+              // Note that unlike the normal project case below, here we don't
+              // try to look for outer buildfiles since we don't have the root
+              // to stop at. However, this shouldn't be an issue since simple
+              // project won't normally have targets in subdirectories (or, in
+              // other words, we are not very interested "complex simple
+              // projects").
+              //
               if (out_root.empty ())
               {
-                if (find_buildfile (out_base).empty ())
+                if (find_buildfile (out_base, out_base).empty ())
                 {
                   fail << "no buildfile in " << out_base <<
                     info << "consider explicitly specifying its src_base";
@@ -1157,6 +1174,53 @@ main (int argc, char* argv[])
             }
           }
 
+          // If we cannot find the buildfile in this directory, then try our
+          // luck with the nearest outer buildfile, in case our target is
+          // defined there (common with non-intrusive project conversions
+          // where everything is built from a single root buildfile).
+          //
+          // The directory target case is ambigous since it can also be the
+          // implied buildfile. The heuristics that we use is to check whether
+          // the implied buildfile is plausible: there is a subdirectory with
+          // a buildfile. Checking for plausability feels expensive since we
+          // have to recursively traverse the directory tree. Note, however,
+          // that if the answer is positive, then shortly after we will be
+          // traversing this tree anyway and presumably this time getting the
+          // data from the cash (we don't really care about the negative
+          // answer since this is a degenerate case).
+          //
+          path bf (find_buildfile (src_base, src_base));
+          if (bf.empty ())
+          {
+            // If the target is a directory and the implied buildfile is
+            // plausible, then assume that. Otherwise, search for an outer
+            // buildfile.
+            //
+            if ((tn.directory () || tn.type == "dir") &&
+                exists (src_base)                     &&
+                dir::check_implied (src_base))
+              ; // Leave bf empty.
+            else
+            {
+              if (src_base != src_root)
+                bf = find_buildfile (src_base.directory (), src_root);
+
+              if (bf.empty ())
+                fail << "no buildfile in " << src_base << " or parent "
+                     << "directories" <<
+                  info << "consider explicitly specifying src_base for "
+                     << out_base;
+
+              // Adjust bases to match the directory where we found the
+              // buildfile since that's the scope it will be loaded in. Note:
+              // but not the target since it is resolved relative to work; see
+              // below.
+              //
+              src_base = bf.directory ();
+              out_base = out_src (src_base, out_root, src_root);
+            }
+          }
+
           if (verb >= 5)
           {
             trace << "bootstrapped " << tn << ':';
@@ -1169,18 +1233,6 @@ main (int argc, char* argv[])
               trace << "  amalgamation: " << cast<dir_path> (l);
           }
 
-          path bf (find_buildfile (src_base));
-          if (bf.empty ())
-          {
-            // If the target is a directory and src_base exists, then assume
-            // implied buildfile; see dir::search_implied().
-            //
-            if (!((tn.directory () || tn.type == "dir") && exists (src_base)))
-              fail << "no buildfile in " << src_base <<
-                info << "consider explicitly specifying src_base for "
-                   << out_base;
-          }
-
           // Enter project-wide (as opposed to global) variable overrides.
           //
           // The mildly tricky part here is to distinguish the situation where
@@ -1322,6 +1374,7 @@ main (int argc, char* argv[])
 
             mif->search (mparams,
                          rs, bs,
+                         ts.buildfile,
                          target_key {tt, &d, &out, &tn.value, e},
                          l,
                          tgs);
diff --git a/build2/config/operation.cxx b/build2/config/operation.cxx
index 0d256a6..1dc51b6 100644
--- a/build2/config/operation.cxx
+++ b/build2/config/operation.cxx
@@ -471,6 +471,7 @@ namespace build2
     configure_search (const values& params,
                       const scope& root,
                       const scope& base,
+                      const path& bf,
                       const target_key& tk,
                       const location& l,
                       action_targets& ts)
@@ -483,7 +484,7 @@ namespace build2
         ts.push_back (&root);
       }
       else
-        search (params, root, base, tk, l, ts); // Normal search.
+        search (params, root, base, bf, tk, l, ts); // Normal search.
     }
 
     static void
@@ -751,6 +752,7 @@ namespace build2
     disfigure_search (const values&,
                       const scope& root,
                       const scope&,
+                      const path&,
                       const target_key&,
                       const location&,
                       action_targets& ts)
diff --git a/build2/config/utility.cxx b/build2/config/utility.cxx
index 976e46f..005be41 100644
--- a/build2/config/utility.cxx
+++ b/build2/config/utility.cxx
@@ -281,7 +281,7 @@ namespace build2
       //
       if (buildfile)
       {
-        path f (d / "buildfile");
+        path f (d / buildfile_file);
 
         if (verb >= verbosity)
           text << (verb >= 2 ? "cat >" : "save ") << f;
diff --git a/build2/file.cxx b/build2/file.cxx
index 3f0e7b7..51bb810 100644
--- a/build2/file.cxx
+++ b/build2/file.cxx
@@ -24,15 +24,15 @@ using namespace butl;
 
 namespace build2
 {
-  const dir_path build_dir ("build");
-  const dir_path root_dir (dir_path (build_dir) /= "root");
+  const dir_path build_dir     ("build");
+  const dir_path root_dir      (dir_path (build_dir) /= "root");
   const dir_path bootstrap_dir (dir_path (build_dir) /= "bootstrap");
 
-  const path root_file (build_dir / "root.build");
-  const path bootstrap_file (build_dir / "bootstrap.build");
-  const path src_root_file (bootstrap_dir / "src-root.build");
-  const path out_root_file (bootstrap_dir / "out-root.build");
-  const path export_file (build_dir / "export.build");
+  const path root_file      (build_dir     / "root.build");
+  const path bootstrap_file (build_dir     / "bootstrap.build");
+  const path src_root_file  (bootstrap_dir / "src-root.build");
+  const path out_root_file  (bootstrap_dir / "out-root.build");
+  const path export_file    (build_dir     / "export.build");
 
   // While strictly speaking it belongs in, say, config/module.cxx, the static
   // initialization order strikes again. If we ever make the config module
@@ -40,6 +40,8 @@ namespace build2
   //
   const path config_file (build_dir / "config.build");
 
+  const path buildfile_file ("buildfile");
+
   ostream&
   operator<< (ostream& os, const subprojects& sps)
   {
diff --git a/build2/file.hxx b/build2/file.hxx
index 9f93365..a56a61f 100644
--- a/build2/file.hxx
+++ b/build2/file.hxx
@@ -35,6 +35,8 @@ namespace build2
   extern const path export_file;       // build/export.build
   extern const path config_file;       // build/config.build
 
+  extern const path buildfile_file;    // buildfile
+
   bool
   is_src_root (const dir_path&);
 
diff --git a/build2/filesystem.cxx b/build2/filesystem.cxx
index 8b4e3ec..183c3da 100644
--- a/build2/filesystem.cxx
+++ b/build2/filesystem.cxx
@@ -190,7 +190,7 @@ namespace build2
     }
   }
 
-  const path buildignore (".buildignore");
+  const path buildignore_file (".buildignore");
 
   fs_status<mkdir_status>
   mkdir_buildignore (const dir_path& d, uint16_t verbosity)
@@ -200,7 +200,7 @@ namespace build2
     // Create the .buildignore file if the directory was created (and so is
     // empty) or the file doesn't exist.
     //
-    path p (d / buildignore);
+    path p (d / buildignore_file);
     if (r || !exists (p))
       touch (p, true /* create */, verbosity);
 
@@ -217,7 +217,8 @@ namespace build2
         // The .buildignore filesystem entry should be of the regular file
         // type.
         //
-        if (de.path () != buildignore || de.ltype () != entry_type::regular)
+        if (de.path () != buildignore_file ||
+            de.ltype () != entry_type::regular)
           return false;
       }
     }
@@ -238,7 +239,7 @@ namespace build2
     // first check that the directory is otherwise empty and doesn't contain
     // the working directory.
     //
-    path p (d / buildignore);
+    path p (d / buildignore_file);
     if (exists (p) && empty_buildignore (d) && !work.sub (d))
       rmfile (p, verbosity);
 
diff --git a/build2/filesystem.hxx b/build2/filesystem.hxx
index 3bcd807..257a1d9 100644
--- a/build2/filesystem.hxx
+++ b/build2/filesystem.hxx
@@ -133,7 +133,7 @@ namespace build2
   // recursive names patterns. For now the file is just a marker and its
   // contents don't matter.
   //
-  extern const path buildignore; // .buildignore
+  extern const path buildignore_file; // .buildignore
 
   // Create a directory containing an empty .buildignore file.
   //
diff --git a/build2/operation.cxx b/build2/operation.cxx
index aa92756..03d0476 100644
--- a/build2/operation.cxx
+++ b/build2/operation.cxx
@@ -94,6 +94,7 @@ namespace build2
   search (const values&,
           const scope&,
           const scope& bs,
+          const path& bf,
           const target_key& tk,
           const location& l,
           action_targets& ts)
@@ -104,11 +105,23 @@ namespace build2
 
     const target* t (targets.find (tk, trace));
 
-    if (t == nullptr && tk.is_a<dir> ())
+    // Only do the implied buildfile if we haven't loaded one. Failed that we
+    // may try go this route even though we've concluded the implied buildfile
+    // is implausible and have loaded an outer buildfile (see main() for
+    // details).
+    //
+    if (t == nullptr && tk.is_a<dir> () && bf.empty ())
       t = dir::search_implied (bs, tk, trace);
 
     if (t == nullptr)
-      fail (l) << "unknown target " << tk;
+    {
+      diag_record dr (fail (l));
+
+      dr << "unknown target " << tk;
+
+      if (!bf.empty ())
+        dr << " in " << bf;
+    }
 
     ts.push_back (t);
   }
@@ -463,6 +476,7 @@ namespace build2
   info_search (const values&,
                const scope& rs,
                const scope&,
+               const path&,
                const target_key& tk,
                const location& l,
                action_targets& ts)
diff --git a/build2/operation.hxx b/build2/operation.hxx
index 1fc6abf..f79228b 100644
--- a/build2/operation.hxx
+++ b/build2/operation.hxx
@@ -106,6 +106,7 @@ namespace build2
     void (*search) (const values&,
                     const scope& root,
                     const scope& base,
+                    const path& buildfile,
                     const target_key&,
                     const location&,
                     action_targets&);
@@ -166,6 +167,7 @@ namespace build2
   search (const values&,
           const scope&,
           const scope&,
+          const path&,
           const target_key&,
           const location&,
           action_targets&);
diff --git a/build2/parser.cxx b/build2/parser.cxx
index 731ef46..5348fbe 100644
--- a/build2/parser.cxx
+++ b/build2/parser.cxx
@@ -1133,14 +1133,14 @@ namespace build2
       //
       path p (move (n.dir));
       if (n.value.empty ())
-        p /= "buildfile";
+        p /= buildfile_file;
       else
       {
         bool d (path::traits::is_separator (n.value.back ()));
 
         p /= path (move (n.value));
         if (d)
-          p /= "buildfile";
+          p /= buildfile_file;
       }
 
       l6 ([&]{trace (l) << "relative path " << p;});
@@ -3041,7 +3041,7 @@ namespace build2
         //
         const string& s (m.string ());
         if ((p[0] != '.' && s[path::traits::find_leaf (s)] == '.') ||
-            (m.to_directory () && exists (*sp / m / buildignore)))
+            (m.to_directory () && exists (*sp / m / buildignore_file)))
           return !interm;
 
         // Note that we have to make copies of the extension since there will
diff --git a/build2/target.cxx b/build2/target.cxx
index 20b9fd6..e70a386 100644
--- a/build2/target.cxx
+++ b/build2/target.cxx
@@ -791,6 +791,73 @@ namespace build2
     false
   };
 
+  // dir
+  //
+  bool dir::
+  check_implied (const dir_path& d)
+  {
+    try
+    {
+      for (const dir_entry& e: dir_iterator (d, true /* ignore_dangling */))
+      {
+        switch (e.type ())
+        {
+        case entry_type::directory:
+          {
+            if (check_implied (d / path_cast<dir_path> (e.path ())))
+              return true;
+
+            break;
+          }
+        case entry_type::regular:
+          {
+            if (e.path () == buildfile_file)
+              return true;
+
+            break;
+          }
+        default:
+          break;
+        }
+      }
+    }
+    catch (const system_error& e)
+    {
+      fail << "unable to iterate over " << d << ": " << e << endf;
+    }
+
+    return false;
+  }
+
+  prerequisites dir::
+  collect_implied (const scope& bs)
+  {
+    prerequisites_type r;
+    const dir_path& d (bs.src_path ());
+
+    try
+    {
+      for (const dir_entry& e: dir_iterator (d, true /* ignore_dangling */))
+      {
+        if (e.type () == entry_type::directory)
+          r.push_back (
+            prerequisite (nullopt,
+                          dir::static_type,
+                          dir_path (e.path ().representation ()), // Relative.
+                          dir_path (), // In the out tree.
+                          string (),
+                          nullopt,
+                          bs));
+      }
+    }
+    catch (const system_error& e)
+    {
+      fail << "unable to iterate over " << d << ": " << e;
+    }
+
+    return r;
+  }
+
   static const target*
   dir_search (const target&, const prerequisite_key& pk)
   {
@@ -869,7 +936,7 @@ namespace build2
 
             const dir_path& src_base (base.src_path ());
 
-            path bf (src_base / "buildfile");
+            path bf (src_base / buildfile_file);
 
             if (exists (bf))
             {
diff --git a/build2/target.hxx b/build2/target.hxx
index cc9c9bd..ca93b24 100644
--- a/build2/target.hxx
+++ b/build2/target.hxx
@@ -1668,6 +1668,17 @@ namespace build2
     template <typename K>
     static const target*
     search_implied (const scope&, const K&, tracer&);
+
+    // Return true if the implied buildfile is plausible for the specified
+    // directory, that is, there is a buildfile in at least one of its
+    // subdirectories. Note that the directory must exist.
+    //
+    static bool
+    check_implied (const dir_path&);
+
+  private:
+    static prerequisites_type
+    collect_implied (const scope&);
   };
 
   // While a filesystem directory is mtime-based, the semantics is not very
diff --git a/build2/target.txx b/build2/target.txx
index a29a289..921d585 100644
--- a/build2/target.txx
+++ b/build2/target.txx
@@ -156,34 +156,13 @@ namespace build2
   //
   template <typename K>
   const target* dir::
-  search_implied (const scope& base, const K& k, tracer& trace)
+  search_implied (const scope& bs, const K& k, tracer& trace)
   {
     using namespace butl;
 
-    // See if we have any subdirectories.
+    // See if we have any prerequisites.
     //
-    prerequisites_type ps;
-
-    try
-    {
-      for (const dir_entry& e: dir_iterator (base.src_path (),
-                                             true /* ignore_dangling */))
-      {
-        if (e.type () == entry_type::directory)
-          ps.push_back (
-            prerequisite (nullopt,
-                          dir::static_type,
-                          dir_path (e.path ().representation ()),
-                          dir_path (), // In the out tree.
-                          string (),
-                          nullopt,
-                          base));
-      }
-    }
-    catch (const system_error& e)
-    {
-      fail << "unable to iterate over " << base.src_path () << ": " << e;
-    }
+    prerequisites_type ps (collect_implied (bs));
 
     if (ps.empty ())
       return nullptr;
@@ -194,7 +173,7 @@ namespace build2
     // buildfile. Thus not implied.
     //
     target& t (targets.insert (dir::static_type,
-                               base.out_path (),
+                               bs.out_path (),
                                dir_path (),
                                string (),
                                nullopt,
-- 
cgit v1.1