Store dynamic group member types in depdb, use in clean

5 files changed, 203 insertions, 102 deletions

diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx
index 06c8867..4277573 100644
--- a/libbuild2/adhoc-rule-buildscript.cxx
+++ b/libbuild2/adhoc-rule-buildscript.cxx
@@ -220,6 +220,9 @@ namespace build2
                   perform_update_id) != actions.end ());
   }
 
+  using dynamic_target = build::script::parser::dynamic_target;
+  using dynamic_targets = build::script::parser::dynamic_targets;
+
   struct adhoc_buildscript_rule::match_data
   {
     match_data (action a, const target& t, const scope& bs, bool temp_dir)
@@ -229,7 +232,7 @@ namespace build2
     build::script::default_runner run;
 
     path dd;
-    paths dyn_targets;
+    dynamic_targets dyn_targets;
 
     const scope* bs;
     timestamp mt;
@@ -551,11 +554,11 @@ namespace build2
     // Read the list of dynamic targets from depdb, if exists (used in a few
     // depdb-dyndep --dyn-target handling places below).
     //
-    auto read_dyn_targets = [] (path ddp) -> paths
+    auto read_dyn_targets = [] (path ddp) -> dynamic_targets
     {
       depdb dd (move (ddp), true /* read_only */);
 
-      paths r;
+      dynamic_targets r;
       while (dd.reading ()) // Breakout loop.
       {
         string* l;
@@ -597,7 +600,15 @@ namespace build2
           if (!read () || l->empty ())
             break;
 
-          r.push_back (path (*l));
+          // Split into type and path.
+          //
+          size_t p (l->find (' '));
+          if (p == string::npos || // Invalid format.
+              p == 0            || // Empty type.
+              p + 1 == l->size ()) // Empty path.
+            break;
+
+          r.emplace_back (string (*l, 0, p), path (*l, p + 1, string::npos));
         }
 
         break;
@@ -640,12 +651,6 @@ namespace build2
         //
         using dyndep = dyndep_rule;
 
-        function<dyndep::map_extension_func> map_ext (
-          [] (const scope& bs, const string& n, const string& e)
-          {
-            return dyndep::map_extension (bs, n, e, nullptr);
-          });
-
         function<dyndep::group_filter_func> filter;
         if (g != nullptr)
         {
@@ -656,33 +661,20 @@ namespace build2
           };
         }
 
-        // @@ We don't have --target-what, --target-default-type here. Could
-        //    we do the same thing as byproduct to get them? That would
-        //    require us running the first half of the depdb preamble but
-        //    ignoring all the depdb builtins (we still want all the variable
-        //    assignments -- maybe we could automatically skip them if we see
-        //    depdb is not open). Wonder if there would be any other
-        //    complications...
-        //
-        //    BTW, this sort of works for --target-default-type since we just
-        //    clean them as file{} targets (but diagnostics is off). It does
-        //    break, however, if there is s batch, since then we end up
-        //    detecting different targets sharing a path. This will also not
-        //    work at all if/when we support specifying custom extension to
-        //    type mapping in order to resolve ambiguities.
-        //
-        const char* what ("file");
-        const target_type& def_tt (file::static_type);
-
-        for (path& f: read_dyn_targets (target_path () + ".d"))
+        for (dynamic_target& dt: read_dyn_targets (target_path () + ".d"))
         {
+          path& f (dt.path);
+
+          // Resolve target type. Clean it as file if unable to.
+          //
+          const target_type* tt (bs.find_target_type (dt.type));
+          if (tt == nullptr)
+            tt = &file::static_type;
+
           if (g != nullptr)
           {
             pair<const build2::file&, bool> r (
-              dyndep::inject_group_member (what,
-                                           a, bs, *g,
-                                           move (f),
-                                           map_ext, def_tt, filter));
+              dyndep::inject_group_member (a, bs, *g, move (f), *tt, filter));
 
             if (r.second)
               g->members.push_back (&r.first);
@@ -692,10 +684,7 @@ namespace build2
             // Note that here we don't bother cleaning any old dynamic targets
             // -- the more we can clean, the merrier.
             //
-            dyndep::inject_adhoc_group_member (what,
-                                               a, bs, t,
-                                               move (f),
-                                               map_ext, def_tt);
+            dyndep::inject_adhoc_group_member (a, bs, t, move (f), *tt);
           }
         }
       }
@@ -824,7 +813,7 @@ namespace build2
     unique_ptr<match_data> md;
     unique_ptr<match_data_byproduct> mdb;
 
-    paths old_dyn_targets;
+    dynamic_targets old_dyn_targets;
 
     if (script.depdb_dyndep_byproduct)
     {
@@ -976,7 +965,9 @@ namespace build2
         const path* p (
           g->members_static != 0
           ? &tp /* first static member path */
-          : (!old_dyn_targets.empty () ? &old_dyn_targets.front () : nullptr));
+          : (!old_dyn_targets.empty ()
+             ? &old_dyn_targets.front ().path
+             : nullptr));
 
         if (p != nullptr)
           mt = g->load_mtime (*p);
@@ -1181,10 +1172,15 @@ namespace build2
       // unconditionally, update or not, since if everything is up to date,
       // then old and new sets should be the same.
       //
-      for (const path& f: old_dyn_targets)
+      for (const dynamic_target& dt: old_dyn_targets)
       {
-        if (find (md->dyn_targets.begin (), md->dyn_targets.end (), f) ==
-            md->dyn_targets.end ())
+        const path& f (dt.path);
+
+        if (find_if (md->dyn_targets.begin (), md->dyn_targets.end (),
+                     [&f] (const dynamic_target& dt)
+                     {
+                       return dt.path == f;
+                     }) == md->dyn_targets.end ())
         {
           // This is an optimization so best effort.
           //
diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx
index c6eb9cd..3814305 100644
--- a/libbuild2/build/script/parser.cxx
+++ b/libbuild2/build/script/parser.cxx
@@ -1291,7 +1291,7 @@ namespace build2
                            environment& e, const script& s, runner& r,
                            lines_iterator begin, lines_iterator end,
                            depdb& dd,
-                           paths* dyn_targets,
+                           dynamic_targets* dyn_targets,
                            bool* update,
                            optional<timestamp> mt,
                            bool* deferred_failure,
@@ -1320,7 +1320,7 @@ namespace build2
           const script& scr;
 
           depdb& dd;
-          paths* dyn_targets;
+          dynamic_targets* dyn_targets;
           bool* update;
           bool* deferred_failure;
           optional<timestamp> mt;
@@ -1647,7 +1647,7 @@ namespace build2
                          size_t li, const location& ll,
                          action a, const scope& bs, target& t,
                          depdb& dd,
-                         paths& dyn_targets,
+                         dynamic_targets& dyn_targets,
                          bool& update,
                          timestamp mt,
                          bool& deferred_failure,
@@ -2647,10 +2647,25 @@ namespace build2
                     break;
                   }
 
-                  if (l->empty ()) // Done with target.
+                  if (l->empty ()) // Done with targets.
                     break;
 
-                  dyn_targets.push_back (path (move (*l)));
+                  // Split into type and path (see below for background).
+                  //
+                  size_t p (l->find (' '));
+                  if (p == string::npos || // Invalid format.
+                      p == 0            || // Empty type.
+                      p + 1 == l->size ()) // Empty path.
+                  {
+                    dd.write (); // Invalidate this line.
+                    restart = true;
+                    break;
+                  }
+
+                  string t (*l, 0, p);
+                  l->erase (0, p + 1);
+
+                  dyn_targets.emplace_back (move (t), path (move (*l)));
                 }
               }
 
@@ -2861,7 +2876,9 @@ namespace build2
                                     << "directory " << rs.out_path ();
                         }
 
-                        dyn_targets.push_back (move (f));
+                        // Note: type is resolved later.
+                        //
+                        dyn_targets.emplace_back (string (), move (f));
                       }
 
                       continue;
@@ -2969,7 +2986,9 @@ namespace build2
                                 << rs.out_path ();
                     }
 
-                    dyn_targets.push_back (move (f));
+                    // Note: type is resolved later.
+                    //
+                    dyn_targets.emplace_back (string (), move (f));
                   }
                   else
                   {
@@ -3099,11 +3118,23 @@ namespace build2
             };
           }
 
-          for (const path& f: dyn_targets)
+          // Unlike for prerequisites, for targets we store in depdb both the
+          // resolved target type and path. The target type is used in clean
+          // (see adhoc_rule_buildscript::apply()) where we cannot easily get
+          // hold of all the dyndep options to map the path to target type.
+          // So the format of the target line is:
+          //
+          // <type> <path>
+          //
+          string l; // Reuse the buffer.
+          for (dynamic_target& dt: dyn_targets)
           {
+            const path& f (dt.path);
+
             // Note that this logic should be consistent with what we have in
             // adhoc_buildscript_rule::apply() for perform_clean.
             //
+            const build2::file* ft (nullptr);
             if (g != nullptr)
             {
               pair<const build2::file&, bool> r (
@@ -3117,12 +3148,17 @@ namespace build2
               // each update.
               //
               if (!r.second)
+              {
+                dt.type.clear (); // Static indicator.
                 continue;
+              }
+
+              ft = &r.first;
 
               // Note: we only currently support dynamic file members so it
               // will be file if first.
               //
-              g->members.push_back (&r.first);
+              g->members.push_back (ft);
             }
             else
             {
@@ -3137,14 +3173,41 @@ namespace build2
               // already a member (think `b update && b clean update`).
               //
               if (!r.second && r.first.decl == target_decl::real)
+              {
+                dt.type.clear (); // Static indicator.
                 continue;
+              }
+
+              ft = &r.first;
 
               if (dts)
-                dts->push_back (&r.first);
+                dts->push_back (ft);
+            }
+
+            const char* tn (ft->type ().name);
+
+            if (dt.type.empty ())
+              dt.type = tn;
+            else if (dt.type != tn)
+            {
+              // This can, for example, happen if the user changed the
+              // extension to target type mapping. Say swapped extension
+              // variable values of two target types.
+              //
+              fail << "mapping of " << what_tgt << " target path " << f
+                   << " to target type has changed" <<
+                info << "previously mapped to " << dt.type << "{}" <<
+                info << "now mapped to " << tn << "{}" <<
+                info << "perform from scratch rebuild of " << t;
             }
 
             if (!cache)
-              dd.expect (f);
+            {
+              l = dt.type;
+              l += ' ';
+              l += f.string ();
+              dd.expect (l);
+            }
           }
 
           // Add the dynamic targets terminating blank line.
diff --git a/libbuild2/build/script/parser.hxx b/libbuild2/build/script/parser.hxx
index f975194..ce550fc 100644
--- a/libbuild2/build/script/parser.hxx
+++ b/libbuild2/build/script/parser.hxx
@@ -121,12 +121,20 @@ namespace build2
             dd);
         }
 
+        struct dynamic_target
+        {
+          string       type; // Target type name (absent if static member).
+          build2::path path;
+        };
+
+        using dynamic_targets = vector<dynamic_target>;
+
         void
         execute_depdb_preamble_dyndep (
           action a, const scope& base, target& t,
           environment& e, const script& s, runner& r,
           depdb& dd,
-          paths& dyn_targets,
+          dynamic_targets& dyn_targets,
           bool& update, timestamp mt, bool& deferred_failure)
         {
           exec_depdb_preamble (
@@ -162,11 +170,13 @@ namespace build2
           environment& e, const script& s, runner& r,
           depdb& dd, bool& update, timestamp mt)
         {
+          // Dummies.
+          //
           // This is getting a bit ugly (we also don't really need to pass
           // depdb here). One day we will find a better way...
           //
-          paths dyn_targets;
-          bool deferred_failure; // Dymmy.
+          dynamic_targets dyn_targets;
+          bool deferred_failure;
 
           dyndep_byproduct v;
           exec_depdb_preamble (
@@ -228,7 +238,7 @@ namespace build2
                              environment&, const script&, runner&,
                              lines_iterator begin, lines_iterator end,
                              depdb&,
-                             paths* dyn_targets = nullptr,
+                             dynamic_targets* dyn_targets = nullptr,
                              bool* update = nullptr,
                              optional<timestamp> mt = nullopt,
                              bool* deferred_failure = nullptr,
@@ -241,7 +251,7 @@ namespace build2
                            size_t line_index, const location&,
                            action, const scope& base, target&,
                            depdb&,
-                           paths& dyn_targets,
+                           dynamic_targets& dyn_targets,
                            bool& update,
                            timestamp,
                            bool& deferred_failure,
diff --git a/libbuild2/dyndep.cxx b/libbuild2/dyndep.cxx
index 9c0f8a8..6d1a32c 100644
--- a/libbuild2/dyndep.cxx
+++ b/libbuild2/dyndep.cxx
@@ -910,9 +910,11 @@ namespace build2
     return pair<const file&, bool> (t, true);
   }
 
-  const file& dyndep_rule::
+  pair<const file&, bool> dyndep_rule::
   inject_group_member (action a, const scope& bs, mtime_target& g,
-                       path f, const target_type& tt)
+                       path f,
+                       const target_type& tt,
+                       const function<group_filter_func>& filter)
   {
     path n (f.leaf ());
     string e (n.extension ());
@@ -921,7 +923,7 @@ namespace build2
     return inject_group_member_impl (a, bs, g,
                                      move (f), move (n).string (), move (e),
                                      tt,
-                                     nullptr /* filter */).first;
+                                     filter);
   }
 
   static const target_type&
@@ -943,7 +945,7 @@ namespace build2
     {
       diag_record dr (fail);
 
-      dr << "mapping of " << what << " target file " << f
+      dr << "mapping of " << what << " target path " << f
          << " to target type is ambiguous";
 
       for (const target_type* tt: tts)
@@ -954,7 +956,7 @@ namespace build2
 
     if (!tt.is_a<file> ())
     {
-      fail << what << " target file " << f << " mapped to non-file-based "
+      fail << what << " target path " << f << " mapped to non-file-based "
            << "target type " << tt.name << "{}";
     }
 
@@ -984,22 +986,11 @@ namespace build2
                                      filter);
   }
 
-  pair<const file&, bool> dyndep_rule::
-  inject_adhoc_group_member (const char* what,
-                             action, const scope& bs, target& t,
-                             path f,
-                             const function<map_extension_func>& map_ext,
-                             const target_type& fallback)
+  pair<const file&, bool>
+  inject_adhoc_group_member_impl (action, const scope& bs, target& t,
+                                  path f, string n, string e,
+                                  const target_type& tt)
   {
-    path n (f.leaf ());
-    string e (n.extension ());
-    n.make_base ();
-
-    // Map extension to the target type, falling back to the fallback type.
-    //
-    const target_type& tt (
-      map_target_type (what, bs, f, n.string (), e, map_ext, fallback));
-
     // Assume nobody else can insert these members (seems reasonable seeing
     // that their names are dynamically discovered).
     //
@@ -1008,7 +999,7 @@ namespace build2
               tt,
               f.directory (),
               dir_path (), // Always in out.
-              move (n).string (),
+              move (n),
               &e,
               &bs));
 
@@ -1040,8 +1031,8 @@ namespace build2
       return pair<const file&, bool> (*ft, false);
 
     if (!l.second)
-      fail << "dynamic " << what << " target " << *ft << " already exists "
-           << "and cannot be made ad hoc member of group " << t;
+      fail << "dynamic target " << *ft << " already exists and cannot be "
+           << "made ad hoc member of group " << t;
 
     ft->group = &t;
     l.second.unlock ();
@@ -1056,4 +1047,38 @@ namespace build2
 
     return pair<const file&, bool> (*ft, true);
   }
+
+  pair<const file&, bool> dyndep_rule::
+  inject_adhoc_group_member (action a, const scope& bs, target& t,
+                             path f,
+                             const target_type& tt)
+  {
+    path n (f.leaf ());
+    string e (n.extension ());
+    n.make_base ();
+
+    return inject_adhoc_group_member_impl (
+      a, bs, t, move (f), move (n).string (), move (e), tt);
+  }
+
+  pair<const file&, bool> dyndep_rule::
+  inject_adhoc_group_member (const char* what,
+                             action a, const scope& bs, target& t,
+                             path f,
+                             const function<map_extension_func>& map_ext,
+                             const target_type& fallback)
+  {
+    path n (f.leaf ());
+    string e (n.extension ());
+    n.make_base ();
+
+    // Map extension to the target type, falling back to the fallback type.
+    //
+    const target_type& tt (
+      map_target_type (what, bs, f, n.string (), e, map_ext, fallback));
+
+
+    return inject_adhoc_group_member_impl (
+      a, bs, t, move (f), move (n).string (), move (e), tt);
+  }
 }
diff --git a/libbuild2/dyndep.hxx b/libbuild2/dyndep.hxx
index ee9c1dd..1de2858 100644
--- a/libbuild2/dyndep.hxx
+++ b/libbuild2/dyndep.hxx
@@ -228,40 +228,41 @@ namespace build2
                const srcout_map& = {});
 
     // Find or insert a target file path as a target of the specified type,
-    // make it a member of the specified (non-ad hoc) mtime target group,
-    // set its path, and match it with group_recipe.
+    // make it a member of the specified (non-ad hoc) mtime target group and
+    // set its path. Return the target and an indication of whether it was
+    // made a member (can only be false if a filter is provided; see below).
     //
     // The file path must be absolute and normalized. Note that this function
     // assumes that this member can only be matched via this group. The group
     // type must have the target_type::flag::dyn_members flag.
     //
-    // Note: we can split this function into {enter,match}_group_member()
-    //       if necessary.
+    // If specified, the group_filter function is called on the target before
+    // making it a group member, skipping it if this function returns false.
+    // Note that the filter is skipped if the target is newly inserted (the
+    // filter is meant to be used to skip duplicates).
     //
-    static const file&
+    using group_filter_func = bool (mtime_target& g, const file&);
+
+    static pair<const file&, bool>
     inject_group_member (action, const scope& base, mtime_target&,
-                         path, const target_type&);
+                         path,
+                         const target_type&,
+                         const function<group_filter_func>& = nullptr);
 
     template <typename T>
-    static const T&
-    inject_group_member (action a, const scope& bs, mtime_target& g, path f)
+    static pair<const T&, bool>
+    inject_group_member (action a, const scope& bs, mtime_target& g,
+                         path f,
+                         const function<group_filter_func>& filter = nullptr)
     {
       return inject_group_member (
-        a, bs, g, move (f), T::static_type).template as<T> ();
+        a, bs, g, move (f), T::static_type, filter).template as<T> ();
     }
 
     // As above but the target type is determined using the map_extension
     // function if specified, falling back to the fallback type if unable to
-    // (the what argument is used for diagnostics during this process). Return
-    // the target and an indication of whether it was made a member.
+    // (the what argument is used for diagnostics during this process).
     //
-    // If specified, the group_filter function is called on the target before
-    // making it a group member, skipping it if this function returns false.
-    // Note that the filter is skipped if the target is newly inserted (the
-    // filter is meant to be used to skip duplicates).
-    //
-    using group_filter_func = bool (mtime_target& g, const file&);
-
     static pair<const file&, bool>
     inject_group_member (const char* what,
                          action, const scope& base, mtime_target& g,
@@ -275,13 +276,19 @@ namespace build2
     // specified ad hoc group unless it already is, and set its path. Return
     // the target and an indication of whether it was added as a member.
     //
-    // The target type is determined using the map_extension function if
-    // specified, falling back to the fallback type if unable to.
-    //
     // The file path must be absolute and normalized. Note that this function
     // assumes that this target can only be known as a member of this group.
     //
     static pair<const file&, bool>
+    inject_adhoc_group_member (action, const scope& base, target& g,
+                               path,
+                               const target_type&);
+
+    // As above but the target type is determined using the map_extension
+    // function if specified, falling back to the fallback type if unable to
+    // (the what argument is used for diagnostics during this process).
+    //
+    static pair<const file&, bool>
     inject_adhoc_group_member (const char* what,
                                action, const scope& base, target& g,
                                path,