Add depdb-dyndep --drop-cycles option

7 files changed, 107 insertions, 60 deletions

diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx
index 0648c3d..ea4ab60 100644
--- a/libbuild2/adhoc-rule-buildscript.cxx
+++ b/libbuild2/adhoc-rule-buildscript.cxx
@@ -582,28 +582,6 @@ namespace build2
                 fp, true /* cache */, true /* normalized */,
                 map_ext, *byp.default_type).first)
           {
-            // Skip if this is one of the static prerequisites.
-            //
-            for (size_t i (0); i != pts_n; ++i)
-            {
-              const prerequisite_target& p (pts[i]);
-
-              if (const target* pt =
-                  (p.target != nullptr ? p.target :
-                   p.data   != 0       ? reinterpret_cast<target*> (p.data) :
-                   nullptr))
-              {
-                if (pt == ft)
-                {
-                  // Note that we have to increment the skip count since we
-                  // skip before performing this test.
-                  //
-                  skip_count++;
-                  return false;
-                }
-              }
-            }
-
             if (optional<bool> u = dyndep::inject_existing_file (
                   trace, what,
                   a, t,
@@ -826,9 +804,11 @@ namespace build2
       //
       // Note that fp is expected to be absolute.
       //
+      size_t skip (md.skip_count);
+
       auto add = [&trace, what,
                   a, &bs, &t, &pts, pts_n = md.pts_n,
-                  &byp, &map_ext, &dd] (path fp)
+                  &byp, &map_ext, &dd, &skip] (path fp)
       {
         normalize_external (fp, what);
 
@@ -849,11 +829,30 @@ namespace build2
                  p.data   != 0       ? reinterpret_cast<target*> (p.data) :
                  nullptr))
             {
-              if (pt == ft)
+              if (ft == pt)
+                return;
+            }
+          }
+
+          // Skip if this is one of the targets.
+          //
+          if (byp.drop_cycles)
+          {
+            for (const target* m (&t); m != nullptr; m = m->adhoc_member)
+            {
+              if (ft == m)
                 return;
             }
           }
 
+          // Skip until where we left off.
+          //
+          if (skip != 0)
+          {
+            --skip;
+            return;
+          }
+
           // Verify it has noop recipe.
           //
           dyndep::verify_existing_file (trace, what, a, t, *ft);
@@ -881,7 +880,6 @@ namespace build2
       }
 
       location il (file, 1);
-      size_t skip (md.skip_count);
 
       // The way we parse things is format-specific.
       //
@@ -924,14 +922,6 @@ namespace build2
               if (r.first == make_type::target)
                 continue;
 
-              // Skip until where we left off.
-              //
-              if (skip != 0)
-              {
-                skip--;
-                continue;
-              }
-
               path f (move (r.second));
 
               if (f.relative ())
diff --git a/libbuild2/build/script/builtin-options.cxx b/libbuild2/build/script/builtin-options.cxx
index f66fe47..dc54194 100644
--- a/libbuild2/build/script/builtin-options.cxx
+++ b/libbuild2/build/script/builtin-options.cxx
@@ -413,7 +413,8 @@ namespace build2
         default_type_ (),
         default_type_specified_ (false),
         cwd_ (),
-        cwd_specified_ (false)
+        cwd_specified_ (false),
+        drop_cycles_ ()
       {
       }
 
@@ -511,6 +512,8 @@ namespace build2
           _cli_depdb_dyndep_options_map_["--cwd"] =
           &::build2::build::script::cli::thunk< depdb_dyndep_options, dir_path, &depdb_dyndep_options::cwd_,
             &depdb_dyndep_options::cwd_specified_ >;
+          _cli_depdb_dyndep_options_map_["--drop-cycles"] =
+          &::build2::build::script::cli::thunk< depdb_dyndep_options, bool, &depdb_dyndep_options::drop_cycles_ >;
         }
       };
 
diff --git a/libbuild2/build/script/builtin-options.hxx b/libbuild2/build/script/builtin-options.hxx
index 15119f4..39b8667 100644
--- a/libbuild2/build/script/builtin-options.hxx
+++ b/libbuild2/build/script/builtin-options.hxx
@@ -433,6 +433,15 @@ namespace build2
         void
         cwd_specified (bool);
 
+        const bool&
+        drop_cycles () const;
+
+        bool&
+        drop_cycles ();
+
+        void
+        drop_cycles (const bool&);
+
         // Implementation details.
         //
         protected:
@@ -458,6 +467,7 @@ namespace build2
         bool default_type_specified_;
         dir_path cwd_;
         bool cwd_specified_;
+        bool drop_cycles_;
       };
     }
   }
diff --git a/libbuild2/build/script/builtin-options.ixx b/libbuild2/build/script/builtin-options.ixx
index c6266d0..075bad8 100644
--- a/libbuild2/build/script/builtin-options.ixx
+++ b/libbuild2/build/script/builtin-options.ixx
@@ -358,6 +358,24 @@ namespace build2
       {
         this->cwd_specified_ = x;
       }
+
+      inline const bool& depdb_dyndep_options::
+      drop_cycles () const
+      {
+        return this->drop_cycles_;
+      }
+
+      inline bool& depdb_dyndep_options::
+      drop_cycles ()
+      {
+        return this->drop_cycles_;
+      }
+
+      inline void depdb_dyndep_options::
+      drop_cycles (const bool& x)
+      {
+        this->drop_cycles_ = x;
+      }
     }
   }
 }
diff --git a/libbuild2/build/script/builtin.cli b/libbuild2/build/script/builtin.cli
index fafb330..938c554 100644
--- a/libbuild2/build/script/builtin.cli
+++ b/libbuild2/build/script/builtin.cli
@@ -42,6 +42,11 @@ namespace build2
         dir_path --cwd;               // Builtin's working directory used
                                       // to complete relative paths (only
                                       // in --byproduct mode).
+        bool --drop-cycles;           // Drop prerequisites that are also
+                                      // targets. Only use if you are sure
+                                      // such cycles are harmless, that is,
+                                      // the output is not affected by such
+                                      // prerequisites' content.
       };
     }
   }
diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx
index 15545cf..2210cc8 100644
--- a/libbuild2/build/script/parser.cxx
+++ b/libbuild2/build/script/parser.cxx
@@ -1442,7 +1442,8 @@ namespace build2
             move (cwd),
             move (*file),
             ops.what_specified () ? move (ops.what ()) : string (what),
-            def_pt};
+            def_pt,
+            ops.drop_cycles ()};
 
           return;
         }
@@ -1600,13 +1601,15 @@ namespace build2
 
         auto add = [this, &trace, what,
                     a, &bs, &t, &pts, pts_n = pts.size (),
-                    &map_ext, def_pt, &pfx_map, &so_map,
+                    &ops, &map_ext, def_pt, &pfx_map, &so_map,
                     &dd, &skip_count] (path fp,
-                                       bool cache,
+                                       size_t* skip,
                                        timestamp mt) -> optional<bool>
         {
           context& ctx (t.ctx);
 
+          bool cache (skip == nullptr);
+
           // We can only defer the failure if we will be running the recipe
           // body.
           //
@@ -1636,26 +1639,51 @@ namespace build2
                 fp, cache, cache /* normalized */,
                 map_ext, *def_pt, pfx_map, so_map).first)
           {
-            // Skip if this is one of the static prerequisites.
+            // We don't need to do these tests for the cached case since such
+            // prerequisites would have been skipped (and we won't get here if
+            // the target/prerequisite set changes since we hash them).
             //
-            for (size_t i (0); i != pts_n; ++i)
+            if (!cache)
             {
-              const prerequisite_target& p (pts[i]);
+              // Skip if this is one of the static prerequisites.
+              //
+              for (size_t i (0); i != pts_n; ++i)
+              {
+                const prerequisite_target& p (pts[i]);
 
-              if (const target* pt =
-                  (p.target != nullptr ? p.target :
-                   p.data   != 0       ? reinterpret_cast<target*> (p.data) :
-                   nullptr))
+                if (const target* pt =
+                    (p.target != nullptr ? p.target :
+                     p.data   != 0       ? reinterpret_cast<target*> (p.data) :
+                     nullptr))
+                {
+                  if (ft == pt)
+                    return false;
+                }
+              }
+
+              // Skip if this is one of the targets.
+              //
+              if (ops.drop_cycles ())
               {
-                if (pt == ft)
+                for (const target* m (&t); m != nullptr; m = m->adhoc_member)
                 {
-                  // Note that we have to increment the skip count since we
-                  // skip before performing this test.
-                  //
-                  skip_count++;
-                  return false;
+                  if (ft == m)
+                    return false;
                 }
               }
+
+              // Skip until where we left off.
+              //
+              // Note that we used to do this outside of this lambda and
+              // before calling enter_file() but due to the above skips we can
+              // only do it here if we want to have a consistent view of the
+              // prerequisite lists between the cached and non-cached cases.
+              //
+              if (*skip != 0)
+              {
+                --(*skip);
+                return false;
+              }
             }
 
             if (optional<bool> u = dyndep::inject_file (
@@ -1724,7 +1752,7 @@ namespace build2
               if (l->empty ()) // Done, nothing changed.
                 return;
 
-              if (optional<bool> r = add (path (move (*l)), true /*cache*/, mt))
+              if (optional<bool> r = add (path (move (*l)), nullptr, mt))
               {
                 restart = *r;
 
@@ -1880,16 +1908,8 @@ namespace build2
                     if (r.first == make_type::target)
                       continue;
 
-                    // Skip until where we left off.
-                    //
-                    if (skip != 0)
-                    {
-                      skip--;
-                      continue;
-                    }
-
                     if (optional<bool> u = add (path (move (r.second)),
-                                                false /* cache */,
+                                                &skip,
                                                 rmt))
                     {
                       restart = *u;
diff --git a/libbuild2/build/script/parser.hxx b/libbuild2/build/script/parser.hxx
index bbdb052..d6f88f4 100644
--- a/libbuild2/build/script/parser.hxx
+++ b/libbuild2/build/script/parser.hxx
@@ -143,6 +143,7 @@ namespace build2
           path               file;
           string             what;
           const target_type* default_type;
+          bool               drop_cycles;
         };
 
         dyndep_byproduct