Fix pre-sync'inc of configurations belonging to same cluster

4 files changed, 231 insertions, 18 deletions

diff --git a/bdep/build.txx b/bdep/build.txx
index 0c128b4..cdf7237 100644
--- a/bdep/build.txx
+++ b/bdep/build.txx
@@ -125,7 +125,7 @@ namespace bdep
       // Pre-sync the configuration to avoid triggering the build system hook
       // (see sync for details).
       //
-      cmd_sync (o, prj, c, true /* implicit */);
+      synced_configs_guard g (cmd_sync (o, prj, c, true /* implicit */));
 
       build (o, c, ps, cfg_vars);
     }
diff --git a/bdep/publish.cxx b/bdep/publish.cxx
index 3eb2d04..3034055 100644
--- a/bdep/publish.cxx
+++ b/bdep/publish.cxx
@@ -1003,8 +1003,7 @@ namespace bdep
       // Pre-sync the configurations to avoid triggering the build system hook
       // (see sync for details).
       //
-      for (const dir_path& cfg: cfgs)
-        cmd_sync_implicit (o, cfg);
+      cmd_sync_implicit (o, cfgs);
     }
     else
     {
@@ -1064,8 +1063,7 @@ namespace bdep
           scs.push_back (move (pc));
       }
 
-      for (const shared_ptr<configuration>& c: scs)
-        cmd_sync (o, prj, c, true /* implicit */);
+      cmd_sync (o, prj, scs, true /* implicit */);
     }
 
     return cmd_publish (o, prj, move (pkgs), move (dist_dirs));
diff --git a/bdep/sync.cxx b/bdep/sync.cxx
index 15222be..b446990 100644
--- a/bdep/sync.cxx
+++ b/bdep/sync.cxx
@@ -1792,18 +1792,31 @@ namespace bdep
   // The BDEP_SYNCED_CONFIGS environment variable.
   //
   // Note that it covers both depth and breadth (i.e., we don't restore the
-  // previous value before returning). The idea here is for commands like
-  // update or test would perform an implicit sync which will then be
-  // "noticed" by the build system hook. This should be both faster (no need
-  // to spawn multiple bdep processes) and simpler (no need to worry about
-  // who has the database open, etc).
+  // previous value before returning, instead returning the "restorer"
+  // guard). The idea here is for commands like update or test would perform
+  // an implicit sync which will then be "noticed" by the build system
+  // hook. This should be both faster (no need to spawn multiple bdep
+  // processes) and simpler (no need to worry about who has the database open,
+  // etc).
   //
   // We also used to do this only in the first form of sync but it turns out
   // we may end up invoking a hook during upgrade (e.g., to prepare a
   // distribution of a package as part of pkg-checkout which happens in the
   // configuration we have "hooked" -- yeah, this rabbit hole is deep).
   //
-  static const char synced_name[] = "BDEP_SYNCED_CONFIGS";
+  static const char synced_configs[] = "BDEP_SYNCED_CONFIGS";
+
+  synced_configs_guard::
+  ~synced_configs_guard ()
+  {
+    if (original)
+    {
+      if (*original)
+        setenv (synced_configs, **original);
+      else
+        unsetenv (synced_configs);
+    }
+  }
 
   // Check if the specified configuration directory is already (being)
   // synchronized. If it is not and add is true, then add it to the
@@ -1813,7 +1826,7 @@ namespace bdep
   synced (const dir_path& d, bool implicit, bool add = true)
   {
     string v;
-    if (optional<string> e = getenv (synced_name))
+    if (optional<string> e = getenv (synced_configs))
       v = move (*e);
 
     if (contains (v, d))
@@ -1827,7 +1840,7 @@ namespace bdep
     if (add)
     {
       v += (v.empty () ? "\"" : " \"") + d.string () + '"';
-      setenv (synced_name, v);
+      setenv (synced_configs, v);
     }
 
     return false;
@@ -1935,7 +1948,7 @@ namespace bdep
     return r;
   }
 
-  void
+  synced_configs_guard
   cmd_sync (const common_options& co,
             const dir_path& prj,
             const shared_ptr<configuration>& c,
@@ -1951,8 +1964,13 @@ namespace bdep
   {
     assert (!c->packages.empty ());
 
+    synced_configs_guard r (getenv (synced_configs));
+
     if (synced (c->path, implicit))
-      return;
+    {
+      r.original = nullopt; // Nothing changed.
+      return r;
+    }
 
     linked_configs lcfgs (find_config_cluster (co, c->path));
     for (auto j (lcfgs.begin () + 1); j != lcfgs.end (); ++j)
@@ -1978,9 +1996,84 @@ namespace bdep
               create_build2_config,
               t,
               created_cfgs);
+
+    return r;
   }
 
   void
+  cmd_sync (const common_options& co,
+            const dir_path& prj,
+            const configurations& xcfgs,
+            bool implicit,
+            const strings& pkg_args,
+            bool fetch,
+            bool yes,
+            bool name_cfg,
+            bool create_host_config,
+            bool create_build2_config)
+  {
+    // Similar approach to the args overload below.
+    //
+    list<sync_config> cfgs (xcfgs.begin (), xcfgs.end ());
+
+    while (!cfgs.empty ())
+    {
+      sync_configs ocfgs; // Originating configurations for this sync.
+
+      ocfgs.push_back (move (cfgs.front ()));
+      cfgs.pop_front ();
+
+      assert (!ocfgs.back ()->packages.empty ());
+
+      const dir_path& cd (ocfgs.back ().path ());
+
+      if (synced (cd, implicit))
+        continue;
+
+      linked_configs lcfgs (find_config_cluster (co, cd));
+
+      for (auto j (lcfgs.begin () + 1); j != lcfgs.end (); ++j)
+      {
+        const linked_config& cfg (*j);
+
+        bool r (synced (cfg.path, true /* implicit */));
+        assert (!r);
+
+        for (auto i (cfgs.begin ()); i != cfgs.end (); )
+        {
+          if (cfg.path == i->path ())
+          {
+            ocfgs.push_back (move (*i));
+            i = cfgs.erase (i);
+
+            assert (!ocfgs.back ()->packages.empty ());
+          }
+          else
+            ++i;
+        }
+      }
+
+      cmd_sync (co,
+                prj,
+                move (ocfgs),
+                move (lcfgs),
+                pkg_args,
+                implicit,
+                fetch,
+                yes,
+                name_cfg,
+                nullopt              /* upgrade   */,
+                nullopt              /* recursive */,
+                package_locations () /* prj_pkgs  */,
+                strings ()           /* dep_pkgs  */,
+                create_host_config,
+                create_build2_config,
+                nullptr,
+                nullptr);
+    }
+  }
+
+  synced_configs_guard
   cmd_sync_implicit (const common_options& co,
                      const dir_path& cfg,
                      bool fetch,
@@ -1989,8 +2082,13 @@ namespace bdep
                      bool create_host_config,
                      bool create_build2_config)
   {
+    synced_configs_guard r (getenv (synced_configs));
+
     if (synced (cfg, true /* implicit */))
-      return;
+    {
+      r.original = nullopt; // Nothing changed.
+      return r;
+    }
 
     linked_configs lcfgs (find_config_cluster (co, cfg));
     for (auto j (lcfgs.begin () + 1); j != lcfgs.end (); ++j)
@@ -2014,6 +2112,72 @@ namespace bdep
               strings ()            /* dep_pkgs  */,
               create_host_config,
               create_build2_config);
+
+    return r;
+  }
+
+  void
+  cmd_sync_implicit (const common_options& co,
+                     const dir_paths& xcfgs,
+                     bool fetch,
+                     bool yes,
+                     bool name_cfg,
+                     bool create_host_config,
+                     bool create_build2_config)
+  {
+    // Similar approach to the args overload below.
+    //
+    list<sync_config> cfgs (xcfgs.begin (), xcfgs.end ());
+
+    while (!cfgs.empty ())
+    {
+      sync_configs ocfgs; // Originating configurations for this sync.
+
+      ocfgs.push_back (move (cfgs.front ()));
+      cfgs.pop_front ();
+
+      const dir_path& cd (ocfgs.back ().path ());
+
+      if (synced (cd, true /* implicit */))
+        continue;
+
+      linked_configs lcfgs (find_config_cluster (co, cd));
+
+      for (auto j (lcfgs.begin () + 1); j != lcfgs.end (); ++j)
+      {
+        const linked_config& cfg (*j);
+
+        bool r (synced (cfg.path, true /* implicit */));
+        assert (!r);
+
+        for (auto i (cfgs.begin ()); i != cfgs.end (); )
+        {
+          if (cfg.path == i->path ())
+          {
+            ocfgs.push_back (move (*i));
+            i = cfgs.erase (i);
+          }
+          else
+            ++i;
+        }
+      }
+
+      cmd_sync (co,
+                dir_path () /* prj */,
+                move (ocfgs),
+                move (lcfgs),
+                strings ()            /* pkg_args */,
+                true                  /* implicit */,
+                fetch,
+                yes,
+                name_cfg,
+                nullopt               /* upgrade   */,
+                nullopt               /* recursive */,
+                package_locations ()  /* prj_pkgs  */,
+                strings ()            /* dep_pkgs  */,
+                create_host_config,
+                create_build2_config);
+    }
   }
 
   int
diff --git a/bdep/sync.hxx b/bdep/sync.hxx
index 4e092dc..4d2ddbd 100644
--- a/bdep/sync.hxx
+++ b/bdep/sync.hxx
@@ -12,6 +12,30 @@
 
 namespace bdep
 {
+  // A RAII class that restores the original value of the BDEP_SYNCED_CONFIGS
+  // environment variable on destruction.
+  //
+  class synced_configs_guard
+  {
+  public:
+    optional<optional<string>> original; // Set to absent to disarm.
+
+    synced_configs_guard () = default;
+    synced_configs_guard (optional<string> o): original (move (o)) {}
+    ~synced_configs_guard ();
+
+    synced_configs_guard (synced_configs_guard&& r)
+        : original (move (r.original))
+    {
+      r.original = nullopt;
+    }
+
+    synced_configs_guard (const synced_configs_guard&) = delete;
+
+    synced_configs_guard& operator= (synced_configs_guard&&) = delete;
+    synced_configs_guard& operator= (const synced_configs_guard&) = delete;
+  };
+
   // The optional pkg_args are the additional dependency packages and/or
   // configuration variables to pass to bpkg-pkg-build (see bdep-init).
   //
@@ -35,7 +59,7 @@ namespace bdep
   // the created configurations with the project using the configuration types
   // also as their names.
   //
-  void
+  synced_configs_guard
   cmd_sync (const common_options&,
             const dir_path& prj,
             const shared_ptr<configuration>&,
@@ -49,10 +73,25 @@ namespace bdep
             transaction* = nullptr,
             vector<pair<dir_path, string>>* created_cfgs = nullptr);
 
+  // As above but sync multiple configurations. If some configurations belong
+  // to the same cluster, then they are synced at once.
+  //
+  void
+  cmd_sync (const common_options&,
+            const dir_path& prj,
+            const configurations&,
+            bool implicit,
+            const strings& pkg_args = strings (),
+            bool fetch = true,
+            bool yes = true,
+            bool name_cfg = false,
+            bool create_host_config = false,
+            bool create_build2_config = false);
+
   // As above but perform an implicit sync without a configuration object
   // (i.e., as if from the hook).
   //
-  void
+  synced_configs_guard
   cmd_sync_implicit (const common_options&,
                      const dir_path& cfg,
                      bool fetch = true,
@@ -61,6 +100,18 @@ namespace bdep
                      bool create_host_config = false,
                      bool create_build2_config = false);
 
+  // As above but sync multiple configurations. If some configurations belong
+  // to the same cluster, then they are synced at once.
+  //
+  void
+  cmd_sync_implicit (const common_options&,
+                     const dir_paths& cfgs,
+                     bool fetch = true,
+                     bool yes = true,
+                     bool name_cfg = true,
+                     bool create_host_config = false,
+                     bool create_build2_config = false);
+
   int
   cmd_sync (cmd_sync_options&&, cli::group_scanner& args);