Fix dist bug where missing source file would be silently ignored

8 files changed, 101 insertions, 48 deletions

diff --git a/build2/algorithm.hxx b/build2/algorithm.hxx
index dbc7d9b..ff70b21 100644
--- a/build2/algorithm.hxx
+++ b/build2/algorithm.hxx
@@ -29,6 +29,11 @@ namespace build2
   const target*
   search_existing (const prerequisite&);
 
+  // As above but cache a target searched in a custom way.
+  //
+  const target&
+  search_custom (const prerequisite&, const target&);
+
   // As above but specify the prerequisite to search as a key.
   //
   const target&
diff --git a/build2/algorithm.ixx b/build2/algorithm.ixx
index 928386d..565414a 100644
--- a/build2/algorithm.ixx
+++ b/build2/algorithm.ixx
@@ -23,16 +23,7 @@ namespace build2
     const target* r (p.target.load (memory_order_consume));
 
     if (r == nullptr)
-    {
-      r = &search (t, p.key ());
-
-      const target* e (nullptr);
-      if (!p.target.compare_exchange_strong (
-            e, r,
-            memory_order_release,
-            memory_order_consume))
-        assert (e == r);
-    }
+      r = &search_custom (p, search (t, p.key ()));
 
     return *r;
   }
@@ -49,20 +40,28 @@ namespace build2
       r = search_existing (p.key ());
 
       if (r != nullptr)
-      {
-        const target* e (nullptr);
-        if (!p.target.compare_exchange_strong (
-              e, r,
-              memory_order_release,
-              memory_order_consume))
-          assert (e == r);
-      }
+        search_custom (p, *r);
     }
 
     return r;
   }
 
   inline const target&
+  search_custom (const prerequisite& p, const target& t)
+  {
+    assert (phase == run_phase::match || phase == run_phase::execute);
+
+    const target* e (nullptr);
+    if (!p.target.compare_exchange_strong (
+          e, &t,
+          memory_order_release,
+          memory_order_consume))
+      assert (e == &t);
+
+    return t;
+  }
+
+  inline const target&
   search (const target& t, const target_type& tt, const prerequisite_key& k)
   {
     return search (
diff --git a/build2/dist/operation.cxx b/build2/dist/operation.cxx
index f42217e..6692156 100644
--- a/build2/dist/operation.cxx
+++ b/build2/dist/operation.cxx
@@ -79,8 +79,8 @@ namespace build2
         fail << "in-tree distribution of target " << t <<
           info << "distribution requires out-of-tree build";
 
-      // Make sure we have the necessary configuration before
-      // we get down to business.
+      // Make sure we have the necessary configuration before we get down to
+      // business.
       //
       auto l (rs->vars["dist.root"]);
 
@@ -155,10 +155,9 @@ namespace build2
         }
       }
 
-      // Add buildfiles that are not normally loaded as part of the
-      // project, for example, the export stub. They will still be
-      // ignored on the next step if the user explicitly marked them
-      // nodist.
+      // Add buildfiles that are not normally loaded as part of the project,
+      // for example, the export stub. They will still be ignored on the next
+      // step if the user explicitly marked them dist=false.
       //
       auto add_adhoc = [&trace] (const scope& rs, const path& f)
       {
@@ -204,9 +203,8 @@ namespace build2
         }
       }
 
-      // Collect the files. We want to take the snapshot of targets
-      // since updating some of them may result in more targets being
-      // entered.
+      // Collect the files. We want to take the snapshot of targets since
+      // updating some of them may result in more targets being entered.
       //
       action_targets files;
       const variable& dist_var (var_pool["dist"]);
diff --git a/build2/dist/rule.cxx b/build2/dist/rule.cxx
index ab00d41..b288a66 100644
--- a/build2/dist/rule.cxx
+++ b/build2/dist/rule.cxx
@@ -36,12 +36,45 @@ namespace build2
         if (p.proj ())
           continue;
 
-        const target& pt (p.search (t));
+        // We used to always search and match but that resulted in the
+        // undesirable behavior in case one of the "source" files is
+        // missing. In this case we would enter a target as "output", this
+        // rule would match it, and then dist_execute() would ignore it by
+        // default.
+        //
+        // So now if this is a file target (we still want to always "see
+        // through" other targets like aliases), we will only match it if (1)
+        // it exists in src or (2) it exists as a target. It feels like we
+        // don't need the stronger "... and not implied" condition since if it
+        // is mentioned as a target, then it is in out (we don't do the same
+        // target in both src/out).
+        //
+        const target* pt (nullptr);
+        if (p.is_a<file> ())
+        {
+          pt = p.load ();
+
+          if (pt == nullptr)
+          {
+            // Search for an existing target or existing file in src.
+            //
+            const prerequisite_key& k (p.key ());
+            pt = k.tk.type->search (t, k);
+
+            if (pt == nullptr)
+              fail << "prerequisite " << k << " is not existing source file "
+                   << "nor known output target" << endf;
+
+            search_custom (p.prerequisite, *pt); // Cache.
+          }
+        }
+        else
+          pt = &p.search (t);
 
         // Don't match targets that are outside of our project.
         //
-        if (pt.dir.sub (out_root))
-          build2::match (a, pt);
+        if (pt->dir.sub (out_root))
+          build2::match (a, *pt);
       }
 
       return noop_recipe; // We will never be executed.
diff --git a/build2/search.cxx b/build2/search.cxx
index 892141a..a7d40c6 100644
--- a/build2/search.cxx
+++ b/build2/search.cxx
@@ -86,7 +86,15 @@ namespace build2
     tracer trace ("search_existing_file");
 
     const target_key& ctk (cpk.tk);
-    assert (ctk.dir->relative ());
+    const scope* s (cpk.scope);
+
+    if (ctk.dir->absolute ())
+    {
+      // Bail out if not inside project's src_root.
+      //
+      if (s == nullptr || !ctk.dir->sub (s->root_scope ()->src_path ()))
+        return nullptr;
+    }
 
     // Figure out the extension. Pretty similar logic to file::derive_path().
     //
@@ -95,7 +103,7 @@ namespace build2
     if (!ext)
     {
       if (auto f = ctk.type->extension)
-        ext = f (ctk, *cpk.scope, true);
+        ext = f (ctk, *s, true);
 
       if (!ext)
       {
@@ -118,14 +126,21 @@ namespace build2
 
     // Check if there is a file.
     //
-    const dir_path& s (pk.scope->src_path ());
+    path f;
 
-    path f (s);
-    if (!tk.dir->empty ())
+    if (tk.dir->absolute ())
+      f = *tk.dir; // Already normalized.
+    else
     {
-      f /= *tk.dir;
-      f.normalize ();
+      f = s->src_path ();
+
+      if (!tk.dir->empty ())
+      {
+        f /= *tk.dir;
+        f.normalize ();
+      }
     }
+
     f /= *tk.name;
 
     if (!ext->empty ())
@@ -163,8 +178,8 @@ namespace build2
 
     if (tk.out->empty ())
     {
-      if (pk.scope->out_path () != s)
-        out = out_src (d, *pk.scope->root_scope ());
+      if (s->out_path () != s->src_path ())
+        out = out_src (d, *s->root_scope ());
     }
     else
       out = *tk.out;
diff --git a/build2/search.hxx b/build2/search.hxx
index 043a216..79c7853 100644
--- a/build2/search.hxx
+++ b/build2/search.hxx
@@ -18,8 +18,11 @@ namespace build2
   const target*
   search_existing_target (const prerequisite_key&);
 
-  // Search for an existing file in the scope's src directory. Prerequisite
-  // directory should be relative.
+  // Search for an existing file. If the prerequisite directory is relative,
+  // then look in the scope's src directory. Otherwise, if the absolute
+  // directory is inside the project's root scope, look there. In case of
+  // the absolute directory, if the scope is NULL, assume the file is not
+  // in src.
   //
   // Originally the plan was to have a target-type specific variable that
   // contains the search paths. But there wasn't any need for this yet.
diff --git a/build2/target.cxx b/build2/target.cxx
index d81ccd5..64ab412 100644
--- a/build2/target.cxx
+++ b/build2/target.cxx
@@ -640,7 +640,7 @@ namespace build2
 
     // Then look for an existing file in the src tree.
     //
-    return pk.tk.dir->relative () ? search_existing_file (pk) : nullptr;
+    return search_existing_file (pk);
   }
 
   optional<string>
diff --git a/build2/target.hxx b/build2/target.hxx
index 0ed1aac..e541667 100644
--- a/build2/target.hxx
+++ b/build2/target.hxx
@@ -935,12 +935,6 @@ namespace build2
     }
 
     const target_type*
-    load (memory_order mo = memory_order_consume)
-    {
-      return target != nullptr ? target : prerequisite.target.load (mo);
-    }
-
-    const target_type*
     search_existing () const
     {
       return target != nullptr
@@ -948,6 +942,12 @@ namespace build2
         : build2::search_existing (prerequisite);
     }
 
+    const target_type*
+    load (memory_order mo = memory_order_consume)
+    {
+      return target != nullptr ? target : prerequisite.target.load (mo);
+    }
+
     // Return as a new prerequisite instance.
     //
     prerequisite_type