From 87a7253a3bd82b59063172f3799b0a5587e7b2a5 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Tue, 2 Apr 2024 08:03:55 +0200
Subject: Detect and diagnose attempt to create new target in src directory

GitHub issue #277.
---
 libbuild2/dyndep.cxx | 10 +++++++---
 libbuild2/scope.cxx  |  8 ++++----
 libbuild2/scope.hxx  |  8 +++++++-
 libbuild2/search.cxx | 27 +++++++++++++++++++++++++++
 libbuild2/search.hxx |  2 ++
 5 files changed, 47 insertions(+), 8 deletions(-)

(limited to 'libbuild2')

diff --git a/libbuild2/dyndep.cxx b/libbuild2/dyndep.cxx
index 68260fb..dbeb47e 100644
--- a/libbuild2/dyndep.cxx
+++ b/libbuild2/dyndep.cxx
@@ -442,7 +442,7 @@ namespace build2
     // which case return the target that would have been inserted.
     //
     // The directory is only moved from if insert is true. Note that it must
-    // be normalized.
+    // be absolute and normalized.
     //
     auto find = [&trace, what, &bs, &t,
                  &map_extension,
@@ -632,7 +632,7 @@ namespace build2
         //
         // While it may seem like there is not much difference, the caller may
         // actually do more than just issue more specific diagnostics. For
-        // example, if may defer the failure to the tool diagnostics.
+        // example, it may defer the failure to the tool diagnostics.
         //
 #if 0
         r = &search (t, *tts[0], d, out, n, &e, s);
@@ -642,7 +642,11 @@ namespace build2
         r = pk.tk.type->search (ctx, &t, pk);
 
         if (r == nullptr && pk.tk.out->empty ())
-          r = &create_new_target (ctx, pk);
+        {
+          auto p (ctx.scopes.find (d, false));
+          if (*p.first != nullptr || ++p.first == p.second)
+            r = &create_new_target (ctx, pk);
+        }
 #endif
       }
 
diff --git a/libbuild2/scope.cxx b/libbuild2/scope.cxx
index 6ed7bab..23781a8 100644
--- a/libbuild2/scope.cxx
+++ b/libbuild2/scope.cxx
@@ -1205,8 +1205,8 @@ namespace build2
   }
 
   auto scope_map::
-  find (const dir_path& k) const -> pair<scopes::const_iterator,
-                                         scopes::const_iterator>
+  find (const dir_path& k, bool sno) const -> pair<scopes::const_iterator,
+                                                   scopes::const_iterator>
   {
     assert (k.normalized (false));
     auto i (map_.find_sup (k));
@@ -1215,9 +1215,9 @@ namespace build2
     auto b (i->second.begin ());
     auto e (i->second.end ());
 
-    // Skip NULL first element.
+    // Skip NULL first element if requested.
     //
-    if (*b == nullptr)
+    if (sno && *b == nullptr)
       ++b;
 
     assert (b != e);
diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx
index 968727b..09d61e9 100644
--- a/libbuild2/scope.hxx
+++ b/libbuild2/scope.hxx
@@ -793,6 +793,8 @@ namespace build2
     // The first element, if not NULL, is for the "owning" out path. The rest
     // of the elements are for the src path shallow references.
     //
+    // Note that the global scope is in the first element.
+    //
     struct scopes: small_vector<scope*, 3>
     {
       scopes () = default;
@@ -832,6 +834,10 @@ namespace build2
 
     // Find all the scopes that encompass this path (out or src).
     //
+    // If skip_null_out is false, then the first element always corresponds to
+    // the out scope and is NULL if there is none (see struct scopes above for
+    // details).
+    //
     // Note that the returned range will never be empty (there is always the
     // global scope).
     //
@@ -864,7 +870,7 @@ namespace build2
     // "island append" restriction we have on loading additional buildfile.
     //
     LIBBUILD2_SYMEXPORT pair<scopes::const_iterator, scopes::const_iterator>
-    find (const dir_path&) const;
+    find (const dir_path&, bool skip_null_out = true) const;
 
     const_iterator begin () const {return map_.begin ();}
     const_iterator end () const {return map_.end ();}
diff --git a/libbuild2/search.cxx b/libbuild2/search.cxx
index dee2ae8..4e855e3 100644
--- a/libbuild2/search.cxx
+++ b/libbuild2/search.cxx
@@ -265,7 +265,24 @@ namespace build2
     //
     dir_path d;
     if (tk.dir->absolute ())
+    {
       d = *tk.dir; // Already normalized.
+
+      // Even if out is empty, it may still be (only) in src.
+      //
+      // Note: issue diagnostics consistent with search() after skipping this
+      // function due to non-empty out.
+      //
+      // @@ PERF: we could first check if it's in pk.scope, which feels like
+      //          the common case. Though this doesn't seem to affect
+      //          performance in any noticeable way.
+      //
+      auto p (ctx.scopes.find (d, false)); // Note: never empty.
+      if (*p.first == nullptr && ++p.first != p.second)
+      {
+        fail << "no existing source file for prerequisite " << pk << endf;
+      }
+    }
     else
     {
       d = pk.scope->out_path ();
@@ -313,7 +330,17 @@ namespace build2
     //
     dir_path d;
     if (tk.dir->absolute ())
+    {
       d = *tk.dir; // Already normalized.
+
+      // As above.
+      //
+      auto p (ctx.scopes.find (d, false));
+      if (*p.first == nullptr && ++p.first != p.second)
+      {
+        fail << "no existing source file for prerequisite " << pk << endf;
+      }
+    }
     else
     {
       d = pk.scope->out_path ();
diff --git a/libbuild2/search.hxx b/libbuild2/search.hxx
index e3b1442..198c65f 100644
--- a/libbuild2/search.hxx
+++ b/libbuild2/search.hxx
@@ -37,6 +37,8 @@ namespace build2
 
   // Create a new target in this prerequisite's scope.
   //
+  // Fail if the target is in src directory.
+  //
   LIBBUILD2_SYMEXPORT const target&
   create_new_target (context&, const prerequisite_key&);
 
-- 
cgit v1.1