From 8336fe72ab8b4a93c54ca775ca88c567cc91c82c Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Mon, 12 Jun 2023 15:06:00 +0200 Subject: Check for configuration name/id/path clash before creating/wiping it (GH issue #241) --- bdep/config.cxx | 120 ++++++++++++++++++++++++++++++++++++++------------------ bdep/config.hxx | 10 +++-- 2 files changed, 88 insertions(+), 42 deletions(-) diff --git a/bdep/config.cxx b/bdep/config.cxx index 58b588c..3c54fa3 100644 --- a/bdep/config.cxx +++ b/bdep/config.cxx @@ -151,8 +151,37 @@ namespace bdep bool default_, bool forward, bool auto_sync, - optional id) + optional id, + bool dry_run) { + database& db (t.database ()); + + //@@ TODO: Maybe redo by querying the conflicting configuration and then + // printing its path, like in rename? + + using count = configuration_count; + using query = bdep::query; + + // See if there is an id, name, or path conflict. + // + if (id && db.query_value (query::id == *id) != 0) + fail << "configuration with id " << *id << " already exists " + << "in project " << prj << + info << "use 'bdep config remove --config-id " << *id << "' to remove"; + + if (name && db.query_value (query::name == *name) != 0) + fail << "configuration with name '" << *name << "' already exists " + << "in project " << prj << + info << "use 'bdep config remove @" << *name << "' to remove"; + + if (db.query_value (query::path == path.string ()) != 0) + fail << "configuration with directory " << path << " already exists " + << "in project " << prj << + info << "use 'bdep config remove --config " << path << "' to remove"; + + if (dry_run) + return nullptr; + shared_ptr r ( new configuration { id, @@ -165,38 +194,7 @@ namespace bdep 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; - } + db.persist (r); // Shouldn't fail due to a conflict. return r; } @@ -246,14 +244,17 @@ namespace bdep optional name, optional type, optional id, - const char* what) + const char* what, + bool dry_run) { translate_path_name (prj, path, name); if (name && name->empty ()) fail << "empty configuration name specified"; - if (!exists (path)) + // Note: if dry_run, then configuration may not yet exist. + // + if (!dry_run && !exists (path)) fail << "configuration directory " << path << " does not exist"; // Make sure the configuration path is absolute and normalized. Also @@ -269,7 +270,11 @@ namespace bdep // vector> host_configs; - if (!type) + if (type) + ; + else if (dry_run) + type = string (); // Not used. + else { fdpipe pipe (open_pipe ()); // Text mode seems appropriate. @@ -416,7 +421,14 @@ namespace bdep *def, *fwd, !ao.no_auto_sync (), - id)); + id, + dry_run)); + + if (dry_run) + { + t.commit (); + return r; + } // Let's issue a single warning about non-associated host configurations // (rather than for each of them) and do it after the configuration is @@ -532,6 +544,20 @@ namespace bdep const strings& args, optional id) { + // Check for any path/name/id conflicts before actually creating the + // configuration. This is especially helpful with --wipe. + // + cmd_config_add (prj, + t, + path, + name, + type, + default_, + forward, + auto_sync, + id, + true /* dry_run */); + create_config (co, path, name, type, existing, wipe, args); return cmd_config_add (prj, @@ -565,18 +591,34 @@ namespace bdep verify_configuration_path (path, prj, pkgs); + // Check for any path/name/id conflicts before actually creating the + // configuration. This is especially helpful with --wipe. + // + const char* what (ao.existing () ? "initialized" : "created"); + cmd_config_add (co, + ao, + prj, + package_locations {}, // Already verified. + db, + path, + name, + type, + id, + what, + true /* dry_run */); + create_config (co, path, name, type, ao.existing (), ao.wipe (), args); return cmd_config_add (co, ao, prj, - package_locations {}, // Already verified. + package_locations {}, db, move (path), move (name), move (type), id, - ao.existing () ? "initialized" : "created"); + what); } void diff --git a/bdep/config.hxx b/bdep/config.hxx index 3c66b72..2b4788a 100644 --- a/bdep/config.hxx +++ b/bdep/config.hxx @@ -15,7 +15,9 @@ namespace bdep // If type is nullopt, then assume that an existing configuration is being // added. If that's the case, query the bpkg configuration type and links // and warn the user if any host or build2 configurations are linked, unless - // they are already associated with the project. + // they are already associated with the project. If dry_run is true, then + // make sure the configuration can be added without path/name/id clash but + // don't actually add anything, returning NULL. // shared_ptr cmd_config_add (const common_options&, @@ -27,7 +29,8 @@ namespace bdep optional name, optional type, optional id = nullopt, - const char* what = "added"); + const char* what = "added", + bool dry_run = false); // Configuration directory should exist and its path should be absolute and // normalized. @@ -41,7 +44,8 @@ namespace bdep bool default_ = true, bool forward = true, bool auto_sync = true, - optional id = nullopt); + optional id = nullopt, + bool dry_run = false); void cmd_config_add_print (diag_record&, -- cgit v1.1