From 9664d2849ee30d63cd10ce5b258f1433a650b488 Mon Sep 17 00:00:00 2001 From: Karen Arutyunov Date: Wed, 21 Jul 2021 21:31:18 +0300 Subject: Add support for automatic creation of configurations for build-time dependencies --- bdep/config.cxx | 251 ++++++++++++----- bdep/config.hxx | 49 ++++ bdep/init.cli | 12 + bdep/init.cxx | 119 +++++--- bdep/init.hxx | 4 +- bdep/sync.cli | 18 ++ bdep/sync.cxx | 731 +++++++++++++++++++++++++++++++++++++++++++++++--- bdep/sync.hxx | 26 +- bdep/types.hxx | 4 +- tests/sync.testscript | 113 +++++++- 10 files changed, 1177 insertions(+), 150 deletions(-) diff --git a/bdep/config.cxx b/bdep/config.cxx index c8954b3..4347b1d 100644 --- a/bdep/config.cxx +++ b/bdep/config.cxx @@ -151,6 +151,88 @@ namespace bdep } shared_ptr + cmd_config_add (const dir_path& prj, + transaction& t, + const dir_path& path, + const optional& name, + string type, + bool default_, + bool forward, + bool auto_sync, + optional id) + { + shared_ptr r ( + new configuration { + id, + name, + move (type), + path, + path.try_relative (prj), + default_, + forward, + auto_sync, + {} /* packages */}); + + database& db (t.database ()); + + try + { + db.persist (r); + } + catch (const odb::exception&) + { + //@@ TODO: Maybe redo by querying the conflicting configuration and then + // printing its path, line in rename? Also do it before persist. + + using count = configuration_count; + using query = bdep::query; + + // See if this is id, name, or path conflict. + // + if (id && db.query_value (query::id == *id) != 0) + fail << "configuration with id " << *id << " already exists " + << "in project " << prj; + + if (name && db.query_value (query::name == *name) != 0) + fail << "configuration with name '" << *name << "' already exists " + << "in project " << prj; + + if (db.query_value (query::path == path.string ()) != 0) + fail << "configuration with directory " << path << " already exists " + << "in project " << prj; + + // Hm, what could that be? + // + throw; + } + + return r; + } + + // Quote the directory if it contains spaces. + // + static string + quote (const dir_path& d) + { + const string& s (d.string ()); + return s.find (' ') == string::npos ? s : '"' + s + '"'; + } + + void + cmd_config_add_print (diag_record& dr, + const dir_path& prj, + const dir_path& path, + const optional& name) + { + dr << "bdep config add -d " << quote (prj); + + if (name) + dr << " @" << *name; + + dr << ' ' << quote (path); + } + + shared_ptr cmd_config_add (const common_options& co, const configuration_add_options& ao, const dir_path& prj, @@ -264,46 +346,15 @@ namespace bdep } shared_ptr r ( - new configuration { - id, - name, - move (*type), - path, - path.try_relative (prj), - *def, - *fwd, - !ao.no_auto_sync (), - {} /* packages */}); - - try - { - db.persist (r); - } - catch (const odb::exception&) - { - //@@ TODO: Maybe redo by querying the conflicting configuration and then - // printing its path, line in rename? Also do it before persist. - - using query = bdep::query; - - // See if this is id, name, or path conflict. - // - if (id && db.query_value (query::id == *id) != 0) - fail << "configuration with id " << *id << " already exists " - << "in project " << prj; - - if (name && db.query_value (query::name == *name) != 0) - fail << "configuration with name '" << *name << "' already exists " - << "in project " << prj; - - if (db.query_value (query::path == path.string ()) != 0) - fail << "configuration with directory " << path << " already exists " - << "in project " << prj; - - // Hm, what could that be? - // - throw; - } + cmd_config_add (prj, + t, + path, + name, + move (*type), + *def, + *fwd, + !ao.no_auto_sync (), + id)); t.commit (); @@ -317,6 +368,76 @@ namespace bdep return r; } + // Call bpkg to create the configuration. + // + static void + create_config (const common_options& co, + const dir_path& path, + const optional& name, + const string& type, + bool existing, + bool wipe, + const strings& args) + { + run_bpkg (2, + co, + "create", + "-d", path, + (name + ? strings ({"--name", *name}) + : strings ()), + "--type", type, + (existing ? "--existing" : nullptr), + (wipe ? "--wipe" : nullptr), + "--no-host-config", + "--no-build2-config", + args); + } + + void + cmd_config_create_print (diag_record& dr, + const dir_path& prj, + const dir_path& path, + const optional& name, + const string& type) + { + dr << "bdep config create -d " << quote (prj) + << " --config-type " << type; + + if (name) + dr << " @" << *name; + + dr << ' ' << quote (path); + } + + shared_ptr + cmd_config_create (const common_options& co, + const dir_path& prj, + transaction& t, + const dir_path& path, + const optional& name, + string type, + bool default_, + bool forward, + bool auto_sync, + bool existing, + bool wipe, + const strings& args, + optional id) + { + create_config (co, path, name, type, existing, wipe, args); + + return cmd_config_add (prj, + t, + path, + name, + move (type), + default_, + forward, + auto_sync, + id); + } + shared_ptr cmd_config_create (const common_options& co, const configuration_add_options& ao, @@ -337,19 +458,7 @@ namespace bdep verify_configuration_path (path, prj, pkgs); - // Call bpkg to create the configuration. - // - run_bpkg (2, - co, - "create", - "-d", path, - (name - ? strings ({"--name", *name}) - : strings ()), - "--type", type, - (ao.existing () ? "--existing" : nullptr), - (ao.wipe () ? "--wipe" : nullptr), - args); + create_config (co, path, name, type, ao.existing (), ao.wipe (), args); return cmd_config_add (co, ao, @@ -363,6 +472,27 @@ namespace bdep ao.existing () ? "initialized" : "created"); } + void + cmd_config_link (const common_options& o, + const shared_ptr& cc, + const shared_ptr& lc) + { + const dir_path& cd (cc->path); + const dir_path& ld (lc->path); + + // Call bpkg to link the configurations. + // + // If possible, rebase the linked configuration directory path relative to + // the other configuration path. + // + run_bpkg (2, + o, + "cfg-link", + ld.try_relative (cd) ? "--relative" : nullptr, + "-d", cd, + ld); + } + static int cmd_config_add (const cmd_config_options& o, cli::scanner& args) { @@ -490,20 +620,7 @@ namespace bdep if (cfgs.size () != 2) fail << "two configurations must be specified for config link"; - const dir_path& cd (cfgs[0]->path); - const dir_path& ld (cfgs[1]->path); - - // Call bpkg to link the configurations. - // - // If possible, rebase the linked configuration directory path relative to - // the other configuration path. - // - run_bpkg (2, - o, - "cfg-link", - ld.try_relative (cd) ? "--relative" : nullptr, - "-d", cd, - ld); + cmd_config_link (o, cfgs[0], cfgs[1]); if (verb) text << "linked configuration " << *cfgs[0] << " (" << cfgs[0]->type diff --git a/bdep/config.hxx b/bdep/config.hxx index 111ce15..b8ad1cc 100644 --- a/bdep/config.hxx +++ b/bdep/config.hxx @@ -26,6 +26,26 @@ namespace bdep optional id = nullopt, const char* what = "added"); + // Configuration directory should exist and its path should be absolute and + // normalized. + // + shared_ptr + cmd_config_add (const dir_path& prj, + transaction&, + const dir_path& path, + const optional& name, + string type, + bool default_ = true, + bool forward = true, + bool auto_sync = true, + optional id = nullopt); + + void + cmd_config_add_print (diag_record&, + const dir_path& prj, + const dir_path&, + const optional& name); + shared_ptr cmd_config_create (const common_options&, const configuration_add_options&, @@ -38,6 +58,35 @@ namespace bdep string type, optional id = nullopt); + // Configuration directory path should be absolute and normalized. + // + shared_ptr + cmd_config_create (const common_options&, + const dir_path& prj, + transaction&, + const dir_path& path, + const optional& name, + string type, + bool default_ = true, + bool forward = true, + bool auto_sync = true, + bool existing = false, + bool wipe = false, + const strings& args = {}, + optional id = nullopt); + + void + cmd_config_create_print (diag_record&, + const dir_path& prj, + const dir_path&, + const optional& name, + const string& type); + + void + cmd_config_link (const common_options&, + const shared_ptr&, + const shared_ptr&); + int cmd_config (cmd_config_options&&, cli::scanner& args); diff --git a/bdep/init.cli b/bdep/init.cli index 658c81e..10e6ab2 100644 --- a/bdep/init.cli +++ b/bdep/init.cli @@ -173,6 +173,18 @@ namespace bdep build configurations. The initialization can be finished later with an explicit \l{bdep-sync(1)} command." } + + bool --create-host-config + { + "Create a configuration for build-time dependencies without prompt (see + \l{bdep-sync(1)} for details)." + } + + bool --create-build2-config + { + "Create a configuration for build system module dependencies without + prompt (see \l{bdep-sync(1)} for details)." + } }; " diff --git a/bdep/init.cxx b/bdep/init.cxx index 2dbc1bb..a8d0658 100644 --- a/bdep/init.cxx +++ b/bdep/init.cxx @@ -94,7 +94,9 @@ namespace bdep const configurations& cfgs, const package_locations& pkgs, const strings& pkg_args, - bool sync) + bool sync, + bool create_host_config, + bool create_build2_config) { // Return true if a package is initialized in the specified configuration. // @@ -208,55 +210,90 @@ namespace bdep "--type", "dir", prj); - transaction t (db.begin ()); - - // Reload the configuration object that could have been changed during - // verification. This is required for skipping the already initialized - // packages. - // - db.reload (*c); + vector> created_cfgs; - for (const package_location& p: pkgs) + try { - if (initialized (p, c)) + transaction t (db.begin ()); + + // Reload the configuration object that could have been changed during + // verification. This is required for skipping the already initialized + // packages. + // + db.reload (*c); + + for (const package_location& p: pkgs) { - if (verb) - info << "package " << p.name << " is already initialized"; + if (initialized (p, c)) + { + if (verb) + info << "package " << p.name << " is already initialized"; + + continue; + } + + // If we are initializing multiple packages or there will be no sync, + // print their names. + // + if (verb && (pkgs.size () > 1 || !sync)) + text << "initializing package " << p.name; - continue; + c->packages.push_back (package_state {p.name}); } - // If we are initializing multiple packages or there will be no sync, - // print their names. + // Should we sync then commit the database or commit and then sync? + // Either way we can end up with an inconsistent state. Note, however, + // that the state in the build configuration can in most cases be + // corrected with a retry (e.g., "upgrade" the package to the fixed + // version, etc) while if we think (from the database state) that the + // package has already been initialized, then there will be no way to + // retry anything (though it could probably be corrected with a sync + // or, failed that, deinit/init). + // + // However, there is a drawback to doing it this way: if we trigger an + // implicit sync (e.g., via a hook) of something that uses the same + // database, we will get the "database is used by another process" + // error. This can be worked around by disabling the implicit sync + // (BDEP_SYNC=0). + // + // Note: semantically equivalent to the first form of the sync + // command. // - if (verb && (pkgs.size () > 1 || !sync)) - text << "initializing package " << p.name; + if (sync) + cmd_sync (o, + prj, + c, + pkg_args, + false /* implicit */, + true /* fetch */, + true /* yes */, + false /* name_cfg */, + create_host_config, + create_build2_config, + &t, + &created_cfgs); - c->packages.push_back (package_state {p.name}); + db.update (c); + t.commit (); } + catch (const failed&) + { + if (!created_cfgs.empty ()) + { + transaction t (db.begin ()); - // Should we sync then commit the database or commit and then sync? - // Either way we can end up with an inconsistent state. Note, however, - // that the state in the build configuration can in most cases be - // corrected with a retry (e.g., "upgrade" the package to the fixed - // version, etc) while if we think (from the database state) that the - // package has already been initialized, then there will be no way to - // retry anything (though it could probably be corrected with a sync or, - // failed that, deinit/init). - // - // However, there is a drawback to doing it this way: if we trigger an - // implicit sync (e.g., via a hook) of something that uses the same - // database, we will get the "database is used by another process" - // error. This can be worked around by disabling the implicit sync - // (BDEP_SYNC=0). - // - // Note: semantically equivalent to the first form of the sync command. - // - if (sync) - cmd_sync (o, prj, c, pkg_args, false /* implicit */); + for (const auto& c: created_cfgs) + cmd_config_add (prj, + t, + c.first /* path */, + c.second /* name */, + c.second /* type */); - db.update (c); - t.commit (); + t.commit (); + } + + throw; + } } } @@ -372,7 +409,9 @@ namespace bdep cfgs, pp.packages, scan_arguments (args) /* pkg_args */, - !o.no_sync ()); + !o.no_sync (), + o.create_host_config (), + o.create_build2_config ()); return 0; } diff --git a/bdep/init.hxx b/bdep/init.hxx index fff5f84..e72abb2 100644 --- a/bdep/init.hxx +++ b/bdep/init.hxx @@ -36,7 +36,9 @@ namespace bdep const configurations&, const package_locations&, const strings& pkg_args, - bool sync = true); + bool sync = true, + bool create_host_config = false, + bool create_build2_config = false); int cmd_init (const cmd_init_options&, cli::group_scanner& args); diff --git a/bdep/sync.cli b/bdep/sync.cli index 07c46a2..86c6095 100644 --- a/bdep/sync.cli +++ b/bdep/sync.cli @@ -67,6 +67,13 @@ namespace bdep Note also that \c{\b{--immediate}|\b{-i}} or \c{\b{--recursive}|\b{-r}} can only be specified with an explicit \cb{--upgrade} or \cb{--patch}. + If during synchronization a build-time dependency is encountered and + there is no build configuration of a suitable type associated with the + project, then the user is prompted (unless the respective + \cb{--create-*-config} option is specified) to automatically create such + a configuration and associate it with the project. See + \l{bpkg-cfg-create(1)} for background on configuration types. + \h|EXAMPLES| As an example, consider project \cb{prj} with two packages, \cb{foo} @@ -205,6 +212,17 @@ namespace bdep "Don't prompt for confirmation when up/down-grading dependencies." } + bool --create-host-config + { + "Create a configuration for build-time dependencies without prompt." + } + + bool --create-build2-config + { + "Create a configuration for build system module dependencies without + prompt." + } + bool --implicit { "Perform implicit synchronization. This mode is normally used by other diff --git a/bdep/sync.cxx b/bdep/sync.cxx index c8a7e4b..cc0894a 100644 --- a/bdep/sync.cxx +++ b/bdep/sync.cxx @@ -12,6 +12,7 @@ #include #include +#include using namespace std; @@ -185,6 +186,477 @@ namespace bdep } } + // Find/create and link a configuration suitable for build-time dependency. + // + static void + link_dependency_config (const common_options& co, + const dir_path& cfg, + const projects& prjs, + const dir_path& origin_prj, + const shared_ptr& origin_config, + const strings& dep_chain, + bool create_host_config, + bool create_build2_config, + transaction* tr, + vector>* created_cfgs, + tracer& trace) + { + // If the configuration is required, then the bpkg output contains the + // build-time dependency line followed by the dependent lines (starting + // from the immediate dependent; see bpkg-pkg-build(1) for details). + // + if (dep_chain.size () < 2) + fail << "invalid bpkg-pkg-build output: invalid dependency chain"; + + // Extract the build-time dependency package name. + // + package_name dep; + + try + { + const string& l (dep_chain[0]); + dep = package_name (string (l, 0, l.find (' '))); + } + catch (const invalid_argument& e) + { + fail << "invalid bpkg-pkg-build output line '" << dep_chain[0] + << "': invalid package name: " << e; + } + + // Determine the required configuration type. + // + string dep_type (dep.string ().compare (0, 10, "libbuild2-") == 0 + ? "build2" + : "host"); + + // Extract the dependent package names, versions, and configuration + // directories. Note that these configurations can differ from the + // configuration which we are syncing. + // + struct dependent + { + package_name name; + bpkg::version version; + dir_path config; + + // Parse the bpkg's output's dependent line. + // + explicit + dependent (const string& line) + { + const char* ep ("invalid bpkg-pkg-build output line "); + + size_t n (line.find (' ')); + + try + { + if (n == string::npos) + throw invalid_path (""); + + config = dir_path (string (line, n + 1)); + } + catch (const invalid_path&) + { + fail << ep << "'" << line << "': invalid dependent package " + << "configuration directory"; + } + + string s (line, 0, n); + + try + { + name = bpkg::extract_package_name (s); + } + catch (const invalid_argument& e) + { + fail << ep << "'" << line << "': invalid dependent package " + << "name: " << e; + } + + try + { + version = bpkg::extract_package_version (s); + + if (version.empty ()) + fail << ep << "'" << line << "': dependent package version is " + << "not specified"; + } + catch (const invalid_argument& e) + { + fail << ep << "'" << line << "': invalid dependent package " + << "version: " << e; + } + } + }; + vector dependents; + + for (size_t i (1); i != dep_chain.size (); ++i) + dependents.emplace_back (dep_chain[i]); + + // Check if the specified configuration is associated with the specified + // project returning its configuration object if found and NULL + // otherwise. + // + auto find_config = [&origin_config, &origin_prj] (database& db, + const dir_path& prj, + const dir_path& cfg) + { + // Note that this is not merely an optimization since the origin + // configuration can be changed but not updated in the database yet (see + // cmd_init() for a use case). + // + if (origin_config != nullptr && + origin_config->path == cfg && + origin_prj == prj) + return origin_config; + + using query = bdep::query; + return db.query_one (query::path == cfg.string ()); + }; + + // Given that we can potentially be inside a transaction started on the + // origin project's database, the transaction handling is a bit + // complicated. + // + // If the transaction is specified, then assumed it is started on the + // specified project's database and so just wrap it. + // + // Otherwise, open the project database and start the transaction on it, + // stashing the current transaction, if present. In destructor restore the + // current transaction, if stashed. + // + class database_transaction + { + public: + database_transaction (transaction* t, const dir_path& prj, tracer& tr) + : ct_ (nullptr) + { + if (t == nullptr) + { + if (transaction::has_current ()) + { + ct_ = &transaction::current (); + transaction::reset_current (); + } + + db_.reset (new database_type (open (prj, tr))); + t_.reset (db_->begin ()); + } + else + ct_ = t; + } + + ~database_transaction () + { + if (ct_ != nullptr && db_ != nullptr) + transaction::current (*ct_); + } + + void + commit () + { + if (!t_.finalized ()) + t_.commit (); + } + + using database_type = bdep::database; + + database_type& + database () + { + assert (db_ != nullptr || ct_ != nullptr); + return db_ != nullptr ? *db_ : ct_->database (); + } + + static transaction& + current () + { + return transaction::current (); + } + + private: + transaction t_; + transaction* ct_; // Current transaction. + unique_ptr db_; + }; + + // Show how we got here (used for both info and text). + // + auto add_info = [&dep, &dependents, &cfg] (const basic_mark& bm) + { + const dependent& dpt (dependents[0]); + bm << "while searching for configuration for build-time dependency " + << dep << " of package " << dpt.name << "/" << dpt.version + << " [" << dpt.config << "]"; + bm << "while synchronizing configuration " << cfg; + }; + + // Show how we got here if things go wrong. + // + // To suppress printing this information clear the dependency package name + // before throwing an exception. + // + auto g (make_exception_guard ([&dep, &add_info] () + { + if (!dep.empty ()) + add_info (info); + })); + + // The immediate dependent configuration directory (the one which needs to + // be linked with the build-time dependency configuration). + // + const dir_path& dpt_dir (dependents[0].config); + + // Now find the "source" project along with the associated immediate + // dependent configuration and the associated build-time dependency + // configuration, if present. + // + // Note that if we end up creating the dependency configuration, then we + // also need to associate it with all other projects. Such an association + // can potentially be redundant for some of them. However, it's hard to + // detect if a project other than the "source" also depends on a package + // which will be built in this newly created configuration. Generally, it + // feels worse not to associate when required than create a redundant + // association. + // + // The "source" project comes first. + // + vector> dpt_prjs; + + // Immediate dependent configuration, which is associated with the + // "source" project and needs to be linked with the build-time dependency + // configuration. + // + shared_ptr dpt_cfg; + + // Configuration associated with the "source" project and suitable for + // building the build-time dependency. NULL if such a configuration + // doesn't exist and needs to be created. + // + shared_ptr dep_cfg; + + for (const dependent& dpt: dependents) + { + for (const project& prj: prjs) + { + database_transaction t (prj.path == origin_prj ? tr : nullptr, + prj.path, + trace); + + database& db (t.database ()); + + // Check if the project is associated with the immediate dependent's + // configuration and also the source of the configured (not + // necessarily immediate) dependent package belongs to it. If that's + // the case, we will use this project as a "source" of the build-time + // dependency configuration. + // + if (shared_ptr dtc = + find_config (db, prj.path, dpt_dir)) + { + shared_ptr dc (find_config (db, + prj.path, + dpt.config)); + + if (dc != nullptr && + find_if (dc->packages.begin (), + dc->packages.end (), + [&dpt] (const package_state& s) + { + return dpt.name == s.name; + }) != dc->packages.end ()) + dpt_cfg = move (dtc); + } + + if (dpt_cfg != nullptr) + { + // Try to find an associated configuration of the suitable type. + // + using query = bdep::query; + + dep_cfg = db.query_one (query::default_ && + query::type == dep_type); + + if (dep_cfg == nullptr) + { + for (shared_ptr c: + pointer_result ( + db.query (query::type == dep_type))) + { + if (dep_cfg == nullptr) + { + dep_cfg = move (c); + } + else + fail << "multiple configurations of " << dep_type + << " type associated with project " << prj.path << + info << dep_cfg->path << + info << c->path << + info << "consider making one of them the default"; + } + } + + // Collect the projects that need to be associated with the + // dependency configuration (the "source" project comes first). + // + dpt_prjs.push_back (prj.path); + + if (dep_cfg == nullptr) // Need to create the config? + { + for (const project& p: prjs) + { + if (p.path != prj.path) + { + for (shared_ptr c: + pointer_result ( + db.query (query::type == dep_type))) + fail << "project " << p.path << " is already associated " + << "with configuration of " << dep_type << " type" << + info << "configuration of " << dep_type << " type: " + << c->path << + info << "consider associating it with project " + << prj.path; + + dpt_prjs.push_back (p.path); + } + } + } + } + + t.commit (); + + if (!dpt_prjs.empty ()) + break; // Bail out from the loop over the projects. + } + + if (!dpt_prjs.empty ()) + break; // Bail out from the loop over the dependents. + } + + // Fail if no "source" project is found. + // + if (dpt_prjs.empty ()) + fail << "build-time dependency " << dep << " cannot be attributed to " + << "any project"; + + // If no configuration of the suitable type is found, then create it and + // associate with all the projects involved. + // + if (dep_cfg == nullptr) + { + const dir_path& src_prj (dpt_prjs[0]); + + // Path to the build-time dependency configuration which needs to be + // created. + // + dir_path dep_dir (dpt_dir.directory () / src_prj.leaf ()); + dep_dir += "-"; + dep_dir += dep_type; + + // Unless explicitly allowed via the respective create_*_config + // argument, prompt the user before performing any action. But fail if + // stderr is redirected. + // + if (!((dep_type == "host" && create_host_config) || + (dep_type == "build2" && create_build2_config))) + { + if (!stderr_term) + fail << "unable to find configuration of " << dep_type + << " type for build-time dependency" << + info << "run sync command explicitly"; + + { + diag_record dr (text); + + // Separate prompt from some potential bpkg output (repository + // fetch, etc). + // + dr << '\n' + << "creating configuration of " << dep_type << " type in " + << dep_dir << '\n' + << "and associate it with projects:" << '\n'; + + for (const dir_path& d: dpt_prjs) + dr << " " << d << '\n'; + + dr << "as if by executing commands:" << '\n'; + + dr << " "; + cmd_config_create_print (dr, src_prj, dep_dir, dep_type, dep_type); + + for (size_t i (1); i != dpt_prjs.size (); ++i) + { + dr << "\n "; + cmd_config_add_print (dr, dpt_prjs[i], dep_dir, dep_type); + } + } + + add_info (text); + + if (!yn_prompt ("continue? [Y/n]", 'y')) + { + // The dependency information have already been printed, so + // suppress printing it repeatedly by the above exception guard. + // + dep = package_name (); + + throw failed (); + } + } + + // Verify that the configuration directory doesn't exist yet (we do it + // after the prompt to give the user some context). + // + if (exists (dep_dir)) + fail << "configuration directory " << dep_dir << " already exists"; + + bool create (true); + for (const dir_path& prj: dpt_prjs) + { + database_transaction t (prj == origin_prj ? tr : nullptr, + prj, + trace); + + if (create) + { + // Before we committed the newly created dependency configuration + // association to the project database or linked the dependent + // configuration to it, we can safely remove it on error. + // + auto_rmdir rmd (dep_dir); + + dep_cfg = cmd_config_create (co, + prj, + transaction::current (), + dep_dir, + dep_type /* name */, + dep_type); + + cmd_config_link (co, dpt_cfg, dep_cfg); + + rmd.cancel (); + + if (created_cfgs != nullptr) + created_cfgs->emplace_back (dep_dir, dep_type); + + create = false; + } + else + { + cmd_config_add (prj, + transaction::current (), + dep_dir, + dep_type /* name */, + dep_type /* type */); + } + + t.commit (); + } + } + else + cmd_config_link (co, dpt_cfg, dep_cfg); + } + // Sync with optional upgrade. // // If upgrade is not nullopt, then: If there are dep_pkgs, then we are @@ -203,11 +675,28 @@ namespace bdep optional upgrade, // true - upgrade, false - patch optional recursive, // true - recursive, false - immediate const package_locations& prj_pkgs, - const strings& dep_pkgs) + const strings& dep_pkgs, + bool create_host_config, + bool create_build2_config, + transaction* tr = nullptr, + vector>* created_cfgs = nullptr) { + tracer trace ("cmd_sync"); + assert (origin_config == nullptr || !origin_config->packages.empty ()); assert (prj_pkgs.empty () || dep_pkgs.empty ()); // Can't have both. + // If a transaction is specified, then it must be started on the origin + // project's database (which therefore must be specified) and it must be + // the current. + // + if (tr != nullptr) + assert (!origin_prj.empty () && tr == &transaction::current ()); + + // Must both be either specified or not. + // + assert ((tr == nullptr) == (created_cfgs == nullptr)); + projects prjs; if (origin_config != nullptr) @@ -375,17 +864,153 @@ namespace bdep } plan += ':'; - run_bpkg (2, - co, - "build", - "-d", cfg, - "--no-fetch", - "--no-refinement", - "--configure-only", - "--keep-out", - "--plan", plan, - (yes ? "--yes" : nullptr), - args); + // Now configure the requested packages, preventing bpkg-pkg-build from + // creating private configurations for build-time dependencies and + // providing them ourselves on demand. + // + // Specifically, if the build-time dependency configuration needs to be + // provided, then the plan is as follows: + // + // 1. Select a project which we can use as a "source" of the build-time + // dependency configuration: use its associated configuration of a + // suitable type, if exist, or create a new one using this project's + // name as a stem. + // + // Such a project needs to have the immediate dependent configuration + // associated and contain a source directory of some of the dependents, + // not necessary immediate but the closer to the dependency the better. + // Not being able to find such a project is an error. + // + // 2. If the "source" project already has a default configuration of the + // suitable type associated, then use that. Otherwise, if the project + // has a single configuration of the suitable type associated, then use + // that. Otherwise, if the project has no suitable configuration + // associated, then create it. Fail if multiple such configurations are + // associated. + // + // 3. If the dependency configuration needs to be created then perform the + // following steps: + // + // - Confirm with the user the list of commands as-if to be executed, + // unless the respective create_*_config argument is true. + // + // - Create the configuration. + // + // - Associate the created configuration with the "source" project and + // all other projects, which should have no configurations of this + // type associated (fail if that's not the case before creating + // anything). + // + // 4. Link the immediate dependent configuration with the dependency + // configuration (either found via the "source" project or newly + // created). + // + // 5. Re-run bpkg-pkg-build and if it turns out that (another) build-time + // dependency configuration is required, then go to p.1. + // + for (;;) + { + // Run bpkg with the --no-private-config option, so that it reports the + // need for the build-time dependency configuration via the specified + // exit code. + // + bool need_config (false); + strings dep_chain; + { + fdpipe pipe (open_pipe ()); // Text mode seems appropriate. + + process pr (start_bpkg (2, + co, + pipe /* stdout */, + 2 /* stderr */, + "build", + "-d", cfg, + "--no-fetch", + "--no-refinement", + "--no-private-config", 125, + "--configure-only", + "--keep-out", + "--plan", plan, + (yes ? "--yes" : nullptr), + args)); + + // Shouldn't throw, unless something is severely damaged. + // + pipe.out.close (); + + // Note that we need to try reading the dependency chain before we + // even know that the build-time dependency configuration will be + // required (not to block the process). + // + bool io (false); + try + { + ifdstream is (move (pipe.in), + fdstream_mode::skip, + ifdstream::badbit); + + for (string l; !eof (getline (is, l)); ) + dep_chain.push_back (move (l)); + + is.close (); // Detect errors. + } + catch (const io_error&) + { + // Presumably the child process failed and issued diagnostics. + // + io = true; + } + + // Handle the process exit, detecting if the build-time dependency + // configuration is required. + // + if (!pr.wait ()) + { + const process_exit& e (*pr.exit); + + if (e.normal ()) + { + need_config = (e.code () == 125); + + if (!need_config) + throw failed (); // Assume the child issued diagnostics. + } + else + fail << "process " << name_bpkg (co) << " " << e; + } + + if (io) + fail << "error reading " << name_bpkg (co) << " output"; + } + + // Bail out if no build-time dependency configuration is required, but + // make sure there is no unexpected bpkg output first (think some + // buildfile printing junk to stdout). + // + if (!need_config) + { + if (!dep_chain.empty ()) + { + diag_record dr (fail); + dr << "unexpected " << name_bpkg (co) << " output:"; + for (const string& s: dep_chain) + dr << '\n' << s ; + } + + break; + } + + link_dependency_config (co, + cfg, + prjs, + origin_prj, origin_config, + dep_chain, + create_host_config, + create_build2_config, + tr, + created_cfgs, + trace); + } // Handle configuration forwarding. // @@ -610,7 +1235,11 @@ namespace bdep bool implicit, bool fetch, bool yes, - bool name_cfg) + bool name_cfg, + bool create_host_config, + bool create_build2_config, + transaction* t, + vector>* created_cfgs) { if (!synced (c->path, implicit)) cmd_sync (co, @@ -625,7 +1254,11 @@ namespace bdep nullopt /* upgrade */, nullopt /* recursive */, package_locations () /* prj_pkgs */, - strings () /* dep_pkgs */); + strings () /* dep_pkgs */, + create_host_config, + create_build2_config, + t, + created_cfgs); } void @@ -633,7 +1266,9 @@ namespace bdep const dir_path& cfg, bool fetch, bool yes, - bool name_cfg) + bool name_cfg, + bool create_host_config, + bool create_build2_config) { if (!synced (cfg, true /* implicit */)) cmd_sync (co, @@ -641,14 +1276,16 @@ namespace bdep dir_path (), nullptr, strings (), - true /* implicit */, + true /* implicit */, fetch, yes, name_cfg, - nullopt /* upgrade */, - nullopt /* recursive */, - package_locations () /* prj_pkgs */, - strings () /* dep_pkgs */); + nullopt /* upgrade */, + nullopt /* recursive */, + package_locations () /* prj_pkgs */, + strings () /* dep_pkgs */, + create_host_config, + create_build2_config); } int @@ -736,6 +1373,7 @@ namespace bdep package_locations prj_pkgs; configurations cfgs; dir_paths cfg_dirs; + bool default_fallback (false); // In the implicit mode we don't search the current working directory // for a project. @@ -781,6 +1419,7 @@ namespace bdep prj = move (pp.project); prj_pkgs = move (pp.packages); cfgs = move (cs.first); + default_fallback = cs.second; } else { @@ -830,6 +1469,7 @@ namespace bdep // Synchronize each configuration. // + bool empty (true); // All configurations are empty. for (size_t i (0), n (cfgs.size ()); i != n; ++i) { const shared_ptr& c (cfgs[i]); // Can be NULL. @@ -838,19 +1478,30 @@ namespace bdep // Check if this configuration is already (being) synchronized. // if (synced (cd, o.implicit ())) + { + empty = false; continue; + } // Skipping empty ones. // + // Note that we would normally be printing that for build-time + // dependency configurations (which normally will not have any + // initialized packages) and that would be annying. So we suppress it in + // case of the default configuration fallback (but also check and warn + // if all of them were empty below). + // if (c != nullptr && c->packages.empty ()) { - if (verb) + if (verb && !default_fallback) info << "skipping configuration " << *c << info << "configuration is empty"; continue; } + empty = false; + // If we are synchronizing multiple configurations, separate them with a // blank line and print the configuration name/directory. // @@ -879,15 +1530,17 @@ namespace bdep prj, c, pkg_args, - false /* implicit */, + false /* implicit */, !fetch, o.recursive () || o.immediate () ? o.yes () : true, - false /* name_cfg */, + false /* name_cfg */, !o.patch (), // Upgrade by default unless patch requested. (o.recursive () ? optional (true) : o.immediate () ? optional (false) : nullopt), - package_locations () /* prj_pkgs */, - dep_pkgs); + package_locations () /* prj_pkgs */, + dep_pkgs, + o.create_host_config (), + o.create_build2_config ()); } else if (o.upgrade () || o.patch ()) { @@ -899,14 +1552,16 @@ namespace bdep prj, c, pkg_args, - false /* implicit */, + false /* implicit */, !fetch, o.yes (), - false /* name_cfg */, + false /* name_cfg */, o.upgrade (), o.recursive (), prj_pkgs, - strings () /* dep_pkgs */); + strings () /* dep_pkgs */, + o.create_host_config (), + o.create_build2_config ()); } else { @@ -922,15 +1577,23 @@ namespace bdep pkg_args, o.implicit (), !fetch, - true /* yes */, - o.implicit () /* name_cfg */, - nullopt /* upgrade */, - nullopt /* recursive */, - package_locations () /* prj_pkgs */, - strings () /* dep_pkgs */); + true /* yes */, + o.implicit () /* name_cfg */, + nullopt /* upgrade */, + nullopt /* recursive */, + package_locations () /* prj_pkgs */, + strings () /* dep_pkgs */, + o.create_host_config (), + o.create_build2_config ()); } } + if (empty && default_fallback) + { + if (verb) + info << "no packages initialized in default configuration(s)"; + } + return 0; } diff --git a/bdep/sync.hxx b/bdep/sync.hxx index 725377e..d3f5429 100644 --- a/bdep/sync.hxx +++ b/bdep/sync.hxx @@ -20,6 +20,21 @@ namespace bdep // name_cfg is true then include the configuration name/directory into // progress. // + // Before automatically creating a configuration for a build-time dependency + // and associating it with the project(s), the user is prompted unless the + // respective create_*_config argument is true. + // + // If the transaction is already started on the project's database, then it + // must also be passed to the function along with a pointer to the + // configuration paths/types list. If the function will be associating + // build-time dependency configurations with the project, it will do it as + // part of this transaction and will also save the created dependency + // configurations into this list. If this transaction is rolled back for any + // reason (for example, because the failed exception is thrown by cmd_sync() + // call), then the caller must start the new transaction and re-associate + // the created configurations with the project using the configuration types + // also as their names. + // void cmd_sync (const common_options&, const dir_path& prj, @@ -28,7 +43,11 @@ namespace bdep bool implicit, bool fetch = true, bool yes = true, - bool name_cfg = false); + bool name_cfg = false, + bool create_host_config = false, + bool create_build2_config = false, + transaction* = nullptr, + vector>* created_cfgs = nullptr); // As above but perform an implicit sync without a configuration object // (i.e., as if from the hook). @@ -38,12 +57,13 @@ namespace bdep const dir_path& cfg, bool fetch = true, bool yes = true, - bool name_cfg = 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); - // Return the list of additional (to prj, if not empty) projects that are // using this configuration. // diff --git a/bdep/types.hxx b/bdep/types.hxx index 2564851..23ea3f2 100644 --- a/bdep/types.hxx +++ b/bdep/types.hxx @@ -93,8 +93,8 @@ namespace bdep using butl::path_cast; - using paths = std::vector; - using dir_paths = std::vector; + using paths = vector; + using dir_paths = vector; // // diff --git a/tests/sync.testscript b/tests/sync.testscript index c03abbb..7005dae 100644 --- a/tests/sync.testscript +++ b/tests/sync.testscript @@ -6,7 +6,8 @@ config_cxx = cc config.cxx=$quote($recall($cxx.path) $cxx.config.mode, true) new += 2>! -init += $config_cxx -d prj 2>! +init += 2>! +config += 2>! status += --all --recursive -d prj deinit += -d prj @@ -86,8 +87,10 @@ deinit += -d prj $new --package pkg1 -d prj; $new --package pkg2 -d prj; - $init -C @cfg1 &prj-cfg1/***; - $init -C @cfg2 &prj-cfg2/***; + init += -d prj; + + $init -C @cfg1 $config_cxx &prj-cfg1/***; + $init -C @cfg2 $config_cxx &prj-cfg2/***; $new -t lib libprj &libprj/***; @@ -201,3 +204,107 @@ deinit += -d prj drop pkg2 EOE } + +: dependency-config +: +{ + +$new -t lib libfoo &libfoo/*** + +$new -t lib libfix &libfix/*** + +$new bar &bar/*** + + +cat <+libfoo/repositories.manifest + : + role: prerequisite + location: ../bar + type: dir + EOI + + +cat <+libfoo/manifest + depends: * bar + EOI + + +cat <+libfix/repositories.manifest + : + role: prerequisite + location: ../bar + type: dir + EOI + + +cat <+libfix/manifest + depends: * bar + EOI + + : create + { + cp -rp ../libfoo ./; + cp -rp ../libfix ./; + cp -rp ../bar ./; + + $init -d libfoo -C @cfg --no-sync $config_cxx &libfoo-cfg/***; + $init -d libfix -A libfoo-cfg @cfg --no-sync; + + $* -d libfoo 2>>~%EOE% != 0; + %fetching dir:.+bar \(prerequisite of dir:.+libfoo\)% + error: unable to find configuration of host type for build-time dependency + info: run sync command explicitly + %info: while searching for configuration for build-time dependency bar of package libfoo/.+ \[.+libfoo-cfg.\]% + %info: while synchronizing configuration .+libfoo-cfg.% + EOE + + $* -d libfoo --create-host-config &libfoo-host/*** &libf??/**/bootstrap/*** 2>>~%EOE% + synchronizing: + % new bar/.+ \[.+libfoo-host.\] \(required by libfix, libfoo\)% + % new libfoo/.+% + % new libfix/.+% + EOE + } + + : find + : + { + cp -rp ../libfoo ./; + cp -rp ../libfix ./; + cp -rp ../bar ./; + + $config create -d libfoo @cfg libfoo-cfg &libfoo-cfg/***; + $config create -d libfoo --config-type host @host host &host/***; + + $config link -d libfoo @cfg @host; + + $init -d libfix -A libfoo-cfg @cfg --no-sync; + + # While at it, test synchronizing via the init command. + # + $init -d libfoo @cfg &libf??/**/bootstrap/*** 2>>~%EOE% + %initializing in project .+libfoo.% + %fetching dir:.+bar \(prerequisite of dir:.+libfoo\)% + synchronizing: + % new bar/.+ \[.+host.\] \(required by libfix, libfoo\)% + % new libfoo/.+% + % new libfix/.+% + EOE + } + + : re-associate + : + : Test that while failing, bdep-init re-associates the automatically created + : build-time dependency configurations. + : + { + cp -rp ../libfoo ./; + cp -rp ../libfix ./; + cp -rp ../bar ./; + + cat <+bar/manifest; + depends: * libbuild2-baz + EOI + + $config create -d libfoo @cfg libfoo-cfg &libfoo-cfg/***; + + $init -d libfoo --create-host-config --create-build2-config @cfg &libfoo-host/*** &libfoo-build2/*** != 0; + + $config list -d libfoo >>~%EOO% + %(@cfg|@host|@build2).+%{3} + EOO + } +} -- cgit v1.1