From 928718963f32e73b9b6787cc2768437e1c1914ce Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 15 Sep 2021 11:48:23 +0200 Subject: Fix pre-sync'inc of configurations belonging to same cluster --- bdep/build.txx | 2 +- bdep/publish.cxx | 6 +- bdep/sync.cxx | 186 +++++++++++++++++++++++++++++++++++++++++++++++++++---- bdep/sync.hxx | 55 +++++++++++++++- 4 files changed, 231 insertions(+), 18 deletions(-) (limited to 'bdep') 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& 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 e = getenv (synced_name)) + if (optional 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& 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 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 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> original; // Set to absent to disarm. + + synced_configs_guard () = default; + synced_configs_guard (optional 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&, @@ -49,10 +73,25 @@ namespace bdep transaction* = nullptr, vector>* 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); -- cgit v1.1