'pkg repo remove' subcommand, and tweak did-you-mean error handling

src/dds/cli/cmd/pkg_repo_err_handle.cpp

@@ -1,5 +1,10 @@
 #include "./pkg_repo_err_handle.hpp"
 
+#include "../options.hpp"
+
+#include <dds/dym.hpp>
+#include <dds/error/errors.hpp>
+#include <dds/pkg/remote.hpp>
 #include <dds/util/http/pool.hpp>
 #include <dds/util/log.hpp>
 #include <dds/util/result.hpp>
@@ -53,5 +58,17 @@ int dds::cli::cmd::handle_pkg_repo_remote_errors(std::function<int()> fn) {
                     conn.port,
                     e.message);
             return 1;
+        },
+        [](matchv<pkg_repo_subcommand::remove>,
+           user_error<errc::no_catalog_remote_info>,
+           e_remote_name       reponame,
+           dds::e_did_you_mean dym) {
+            dds_log(error,
+                    "Cannot delete remote '{}', as no such remote repository is locally "
+                    "registered by that name.",
+                    reponame.value);
+            dym.log_as_error();
+            write_error_marker("repo-rm-no-such-repo");
+            return 1;
         });
 }

src/dds/cli/cmd/pkg_repo_remove.cpp

@@ -0,0 +1,26 @@
+#include "../options.hpp"
+
+#include "./pkg_repo_err_handle.hpp"
+
+#include <dds/pkg/db.hpp>
+#include <dds/pkg/remote.hpp>
+#include <dds/util/result.hpp>
+
+namespace dds::cli::cmd {
+
+static int _pkg_repo_remove(const options& opts) {
+    auto cat = opts.open_pkg_db();
+    for (auto&& rm_name : opts.pkg.repo.remove.names) {
+        dds::remove_remote(cat, rm_name);
+    }
+    return 0;
+}
+
+int pkg_repo_remove(const options& opts) {
+    return handle_pkg_repo_remote_errors([&] {
+        DDS_E_SCOPE(opts.pkg.repo.subcommand);
+        return _pkg_repo_remove(opts);
+    });
+}
+
+}  // namespace dds::cli::cmd

src/dds/cli/dispatch_main.cpp

@@ -22,6 +22,7 @@ command pkg_ls;
 command pkg_repo_add;
 command pkg_repo_update;
 command pkg_repo_ls;
+command pkg_repo_remove;
 command repoman_add;
 command repoman_import;
 command repoman_init;
@@ -54,13 +55,15 @@ int dispatch_main(const options& opts) noexcept {
                 return cmd::pkg_import(opts);
             case pkg_subcommand::repo:
                 switch (opts.pkg.repo.subcommand) {
-                case cli_pkg_repo_subcommand::add:
+                case pkg_repo_subcommand::add:
                     return cmd::pkg_repo_add(opts);
-                case cli_pkg_repo_subcommand::update:
+                case pkg_repo_subcommand::update:
                     return cmd::pkg_repo_update(opts);
-                case cli_pkg_repo_subcommand::ls:
+                case pkg_repo_subcommand::ls:
                     return cmd::pkg_repo_ls(opts);
-                case cli_pkg_repo_subcommand::_none_:;
+                case pkg_repo_subcommand::remove:
+                    return cmd::pkg_repo_remove(opts);
+                case pkg_repo_subcommand::_none_:;
                 }
                 neo::unreachable();
             case pkg_subcommand::_none_:;

src/dds/cli/options.cpp

@@ -26,6 +26,13 @@ struct setup {
         .action         = put_into(opts.if_exists),
     };
 
+    argument if_missing_arg{
+        .long_spellings = {"if-missing"},
+        .help           = "What to do if the resource does not exist",
+        .valname        = "{fail,ignore}",
+        .action         = put_into(opts.if_missing),
+    };
+
     argument toolchain_arg{
         .long_spellings  = {"toolchain"},
         .short_spellings = {"t"},
@@ -291,6 +298,10 @@ struct setup {
             .name = "add",
             .help = "Add a package repository",
         }));
+        setup_pkg_repo_remove_cmd(pkg_repo_grp.add_parser({
+            .name = "remove",
+            .help = "Remove one or more package repositories",
+        }));
 
         pkg_repo_grp.add_parser({
             .name = "update",
@@ -317,6 +328,17 @@ struct setup {
         });
     }
 
+    void setup_pkg_repo_remove_cmd(argument_parser& pkg_repo_remove_cmd) noexcept {
+        pkg_repo_remove_cmd.add_argument({
+            .help       = "Name of one or more repositories to remove",
+            .valname    = "<repo-name>",
+            .can_repeat = true,
+            .action     = push_back_onto(opts.pkg.repo.remove.names),
+        });
+        pkg_repo_remove_cmd.add_argument(if_missing_arg.dup()).help
+            = "What to do if any of the named repositories do not exist";
+    }
+
     void setup_sdist_cmd(argument_parser& sdist_cmd) noexcept {
         auto& sdist_grp = sdist_cmd.add_subparsers({
             .valname = "<sdist-subcommand>",

src/dds/cli/options.hpp

@@ -51,9 +51,10 @@ enum class pkg_subcommand {
 /**
  * @brief 'dds pkg repo' subcommands
  */
-enum class cli_pkg_repo_subcommand {
+enum class pkg_repo_subcommand {
     _none_,
     add,
+    remove,
     update,
     ls,
 };
@@ -80,6 +81,11 @@ enum class if_exists {
     ignore,
 };
 
+enum class if_missing {
+    fail,
+    ignore,
+};
+
 /**
  * @brief Complete aggregate of all dds command-line options, and some utilities
  */
@@ -114,6 +120,8 @@ struct options {
 
     // Shared `--if-exists` argument:
     cli::if_exists if_exists = cli::if_exists::fail;
+    // Shared '--if-missing' argument:
+    cli::if_missing if_missing = cli::if_missing::fail;
 
     /**
      * @brief Open the package pkg_db based on the user-specified options.
@@ -178,7 +186,7 @@ struct options {
          */
         struct {
             /// The 'pkg repo' subcommand
-            cli_pkg_repo_subcommand subcommand;
+            pkg_repo_subcommand subcommand;
 
             /**
              * @brief Parameters of 'dds pkg repo add'
@@ -189,6 +197,14 @@ struct options {
                 /// Whether we should update repo data after adding the repository
                 bool update = true;
             } add;
+
+            /**
+             * @brief Parameters of 'dds pkg repo remove'
+             */
+            struct {
+                /// Repositories to remove (by name)
+                std::vector<string> names;
+            } remove;
         } repo;
 
         /**

src/dds/dym.cpp

@@ -1,6 +1,7 @@
 #include <dds/dym.hpp>
 
 #include <dds/error/errors.hpp>
+#include <dds/util/log.hpp>
 
 #include <range/v3/algorithm/min_element.hpp>
 #include <range/v3/view/cartesian_product.hpp>
@@ -45,3 +46,9 @@ std::size_t dds::lev_edit_distance(std::string_view a, std::string_view b) noexc
 
     return matrix.back().back();
 }
+
+void dds::e_did_you_mean::log_as_error() const noexcept {
+    if (value) {
+        dds_log(error, "  (Did you mean \"{}\"?)", *value);
+    }
+}

src/dds/dym.hpp

@@ -11,6 +11,12 @@ namespace dds {
 
 std::size_t lev_edit_distance(std::string_view a, std::string_view b) noexcept;
 
+struct e_did_you_mean {
+    std::optional<std::string> value;
+
+    void log_as_error() const noexcept;
+};
+
 class dym_target {
     std::optional<std::string>      _candidate;
     dym_target*                     _tls_prev = nullptr;
@@ -33,6 +39,8 @@ public:
 
     auto& candidate() const noexcept { return _candidate; }
 
+    auto e_value() const noexcept { return e_did_you_mean{_candidate}; }
+
     std::string sentence_suffix() const noexcept {
         if (_candidate) {
             return " (Did you mean '" + *_candidate + "'?)";
@@ -58,4 +66,14 @@ did_you_mean(std::string_view given, std::initializer_list<std::string_view> str
     return did_you_mean(given, ranges::views::all(strings));
 }
 
+template <typename Range>
+e_did_you_mean calc_e_did_you_mean(std::string_view given, Range&& strings) noexcept {
+    return {did_you_mean(given, strings)};
+}
+
+inline e_did_you_mean calc_e_did_you_mean(std::string_view                        given,
+                                          std::initializer_list<std::string_view> il) noexcept {
+    return calc_e_did_you_mean(given, ranges::views::all(il));
+}
+
 }  // namespace dds

src/dds/error/errors.cpp

@@ -34,7 +34,7 @@ std::string error_url_suffix(dds::errc ec) noexcept {
     case errc::invalid_catalog_json:
         return "invalid-catalog-json.html";
     case errc::no_catalog_remote_info:
-        return "no-catalog-remote-info.html";
+        return "no-pkg-remote.html";
     case errc::git_clone_failure:
         return "git-clone-failure.html";
     case errc::invalid_remote_url:
@@ -167,8 +167,7 @@ Check the JSON schema and try your submission again.
 )";
     case errc::no_catalog_remote_info:
         return R"(
-The catalog entry requires information regarding the remote acquisition method.
-Refer to the documentation for details.
+There is no package remote with the given name
 )";
     case errc::git_clone_failure:
         return R"(
@@ -292,8 +291,7 @@ std::string_view dds::default_error_string(dds::errc ec) noexcept {
     case errc::invalid_catalog_json:
         return "The given catalog JSON data is not valid";
     case errc::no_catalog_remote_info:
-        return "The catalog JSON is missing remote acquisition information for one or more\n"
-               "packages";
+        return "Tne named remote does not exist." BUG_STRING_SUFFIX;
     case errc::git_clone_failure:
         return "A git-clone operation failed.";
     case errc::invalid_remote_url:

src/dds/pkg/remote.cpp

@@ -1,6 +1,8 @@
 #include "./remote.hpp"
 
+#include <dds/dym.hpp>
 #include <dds/error/errors.hpp>
+#include <dds/pkg/db.hpp>
 #include <dds/temp.hpp>
 #include <dds/util/http/pool.hpp>
 #include <dds/util/log.hpp>
@@ -17,6 +19,7 @@
 #include <neo/url.hpp>
 #include <neo/utility.hpp>
 #include <range/v3/range/conversion.hpp>
+#include <range/v3/view/transform.hpp>
 
 using namespace dds;
 namespace nsql = neo::sqlite3;
@@ -213,3 +216,29 @@ void dds::update_all_remotes(nsql::database_ref db) {
     dds_log(info, "Recompacting database...");
     db.exec("VACUUM");
 }
+
+void dds::remove_remote(pkg_db& pkdb, std::string_view name) {
+    auto&                           db = pkdb.database();
+    neo::sqlite3::transaction_guard tr{db};
+    auto get_rowid_st          = db.prepare("SELECT remote_id FROM dds_pkg_remotes WHERE name = ?");
+    get_rowid_st.bindings()[1] = name;
+    auto row                   = neo::sqlite3::unpack_single_opt<std::int64_t>(get_rowid_st);
+    if (!row) {
+        auto calc_dym = [&] {
+            auto all_st = db.prepare("SELECT name FROM dds_pkg_remotes");
+            auto tups   = neo::sqlite3::iter_tuples<std::string>(all_st);
+            auto names  = tups | ranges::views::transform([](auto&& tup) {
+                             auto&& [n] = tup;
+                             return n;
+                         })
+                | ranges::to_vector;
+            return calc_e_did_you_mean(name, names);
+        };
+        BOOST_LEAF_THROW_EXCEPTION(make_user_error<errc::no_catalog_remote_info>(
+                                       "There is no remote with name '{}'", name),
+                                   DDS_E_ARG(e_remote_name{std::string(name)}),
+                                   calc_dym);
+    }
+    auto [rowid] = *row;
+    neo::sqlite3::exec(db.prepare("DELETE FROM dds_pkg_remotes WHERE remote_id = ?"), rowid);
+}

src/dds/pkg/remote.hpp

@@ -7,6 +7,12 @@
 
 namespace dds {
 
+class pkg_db;
+
+struct e_remote_name {
+    std::string value;
+};
+
 class pkg_remote {
     std::string _name;
     neo::url    _base_url;
@@ -26,5 +32,6 @@ public:
 };
 
 void update_all_remotes(neo::sqlite3::database_ref);
+void remove_remote(pkg_db& db, std::string_view name);
 
 }  // namespace dds

src/dds/util/result.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <boost/leaf/handle_error.hpp>
 #include <boost/leaf/on_error.hpp>
 #include <boost/leaf/result.hpp>
 #include <neo/concepts.hpp>
@@ -23,6 +24,9 @@ constexpr T value_or(const result<T>& res, U&& arg) {
     return res ? res.value() : static_cast<T>(arg);
 }
 
+template <auto Val>
+using matchv = boost::leaf::match<decltype(Val), Val>;
+
 /**
  * @brief Error object representing a captured system_error exception
  */

tests/test_pkg_db.py

@@ -1,22 +1,25 @@
 from dds_ci.dds import DDSWrapper
-from dds_ci.testing import Project, RepoFixture
+from dds_ci.testing import Project, RepoFixture, PackageJSON
+from dds_ci.testing.error import expect_error_marker
 
-
-def test_pkg_get(http_repo: RepoFixture, tmp_project: Project) -> None:
-    http_repo.import_json_data({
-        'packages': {
-            'neo-sqlite3': {
-                '0.3.0': {
-                    'remote': {
-                        'git': {
-                            'url': 'https://github.com/vector-of-bool/neo-sqlite3.git',
-                            'ref': '0.3.0',
-                        }
+NEO_SQLITE_PKG_JSON = {
+    'packages': {
+        'neo-sqlite3': {
+            '0.3.0': {
+                'remote': {
+                    'git': {
+                        'url': 'https://github.com/vector-of-bool/neo-sqlite3.git',
+                        'ref': '0.3.0',
                     }
                 }
             }
         }
-    })
+    }
+}
+
+
+def test_pkg_get(http_repo: RepoFixture, tmp_project: Project) -> None:
+    http_repo.import_json_data(NEO_SQLITE_PKG_JSON)
     tmp_project.dds.repo_add(http_repo.url)
     tmp_project.dds.pkg_get('neo-sqlite3@0.3.0')
     assert tmp_project.root.joinpath('neo-sqlite3@0.3.0').is_dir()
@@ -26,4 +29,21 @@ def test_pkg_get(http_repo: RepoFixture, tmp_project: Project) -> None:
 def test_pkg_repo(http_repo: RepoFixture, tmp_project: Project) -> None:
     dds = tmp_project.dds
     dds.repo_add(http_repo.url)
-    dds.run(['pkg', 'repo', 'ls'])
+    dds.run(['pkg', 'repo', dds.catalog_path_arg, 'ls'])
+
+
+def test_pkg_repo_rm(http_repo: RepoFixture, tmp_project: Project) -> None:
+    http_repo.import_json_data(NEO_SQLITE_PKG_JSON)
+    dds = tmp_project.dds
+    dds.repo_add(http_repo.url)
+    # Okay:
+    tmp_project.dds.pkg_get('neo-sqlite3@0.3.0')
+    # Remove the repo:
+    dds.run(['pkg', dds.catalog_path_arg, 'repo', 'ls'])
+    dds.repo_remove(http_repo.repo_name)
+    # Cannot double-remove a repo:
+    with expect_error_marker('repo-rm-no-such-repo'):
+        dds.repo_remove(http_repo.repo_name)
+    # Now, fails:
+    with expect_error_marker('pkg-get-no-pkg-id-listing'):
+        tmp_project.dds.pkg_get('neo-sqlite3@0.3.0')

tools/dds_ci/dds.py

@@ -76,6 +76,9 @@ class DDSWrapper:
     def repo_add(self, url: str) -> None:
         self.run(['pkg', 'repo', 'add', self.catalog_path_arg, url])
 
+    def repo_remove(self, name: str) -> None:
+        self.run(['pkg', 'repo', 'remove', self.catalog_path_arg, name])
+
     def repo_import(self, sdist: Path) -> None:
         self.run(['repo', self.repo_dir_arg, 'import', sdist])
 

tools/dds_ci/testing/http.py

@@ -10,6 +10,7 @@ import sys
 import subprocess
 
 import pytest
+from _pytest.fixtures import FixtureRequest
 
 
 class DirectoryServingHTTPRequestHandler(SimpleHTTPRequestHandler):
@@ -67,7 +68,8 @@ class RepoFixture:
     """
     A fixture handle to a dds HTTP repository, including a path and URL.
     """
-    def __init__(self, dds_exe: Path, info: ServerInfo) -> None:
+    def __init__(self, dds_exe: Path, info: ServerInfo, repo_name: str) -> None:
+        self.repo_name = repo_name
         self.server = info
         self.url = info.base_url
         self.dds_exe = dds_exe
@@ -97,10 +99,11 @@ class RepoFixture:
 
 
 @pytest.fixture()
-def http_repo(dds_exe: Path, http_tmp_dir_server: ServerInfo) -> Iterator[RepoFixture]:
+def http_repo(dds_exe: Path, http_tmp_dir_server: ServerInfo, request: FixtureRequest) -> Iterator[RepoFixture]:
     """
     Fixture that creates a new empty dds repository and an HTTP server to serve
     it.
     """
-    subprocess.check_call([str(dds_exe), 'repoman', 'init', str(http_tmp_dir_server.root)])
-    yield RepoFixture(dds_exe, http_tmp_dir_server)
+    name = f'test-repo-{request.function.__name__}'
+    subprocess.check_call([str(dds_exe), 'repoman', 'init', str(http_tmp_dir_server.root), f'--name={name}'])
+    yield RepoFixture(dds_exe, http_tmp_dir_server, repo_name=name)