From 75163dafca6d171e18f228a60cb067c5ce8fb726 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Fri, 9 Aug 2024 10:23:08 +0100 Subject: [PATCH 01/21] ReloadingConfigFile --- examples/example-server-lib/CMakeLists.txt | 1 + .../reloading_config_file.cpp | 146 ++++++++++++++++++ .../reloading_config_file.h | 58 +++++++ 3 files changed, 205 insertions(+) create mode 100644 examples/example-server-lib/reloading_config_file.cpp create mode 100644 examples/example-server-lib/reloading_config_file.h diff --git a/examples/example-server-lib/CMakeLists.txt b/examples/example-server-lib/CMakeLists.txt index 2552723a1e3..082b6148f8c 100644 --- a/examples/example-server-lib/CMakeLists.txt +++ b/examples/example-server-lib/CMakeLists.txt @@ -11,6 +11,7 @@ add_library(example-shell-lib STATIC wayland_app.cpp wayland_app.h wayland_surface.cpp wayland_surface.h wayland_shm.cpp wayland_shm.h + reloading_config_file.cpp reloading_config_file.h ) target_include_directories(example-shell-lib diff --git a/examples/example-server-lib/reloading_config_file.cpp b/examples/example-server-lib/reloading_config_file.cpp new file mode 100644 index 00000000000..aeb62acfba0 --- /dev/null +++ b/examples/example-server-lib/reloading_config_file.cpp @@ -0,0 +1,146 @@ +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 or 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "reloading_config_file.h" + +#include + +#include + +#include + +#include +#include + +#include +#include + +using namespace std::filesystem; + +namespace +{ +auto config_directory(path const& file) -> std::optional +{ + if (file.has_parent_path()) + { + return file.parent_path(); + } + else if (auto config_home = getenv("XDG_CONFIG_HOME")) + { + return config_home; + } + else if (auto home = getenv("HOME")) + { + return path(home) / ".config"; + } + else + { + return std::nullopt; + } +} + +auto watch_fd(mir::Fd const& inotify_fd, std::optional const& path) -> std::optional +{ + if (!path.has_value()) + return std::nullopt; + + if (inotify_fd < 0) + BOOST_THROW_EXCEPTION((std::system_error{errno, std::system_category(), "Failed to initialize inotify_fd"})); + + return mir::Fd{inotify_add_watch(inotify_fd, path.value().c_str(), IN_CLOSE_WRITE | IN_CREATE | IN_MOVED_TO)}; +} +} + +miral::ReloadingConfigFile::ReloadingConfigFile(MirRunner& runner, path file, Loader load_config) : + inotify_fd{inotify_init()}, + load_config{load_config}, + filename{file.filename()}, + directory{config_directory(file)}, + directory_watch_fd{watch_fd(inotify_fd, directory)} +{ + register_handler(runner); + + // With C++26 we should be able to use the optional directory as a range to + // initialize config_roots. Until then, we'll just do it the long way... + std::vector config_roots; + + if (directory) + { + config_roots.push_back(directory.value()); + } + + if (auto config_dirs = getenv("XDG_CONFIG_DIRS")) + { + std::istringstream config_stream{config_dirs}; + for (std::string config_root; getline(config_stream, config_root, ':');) + { + config_roots.push_back(config_root); + } + } + else + { + config_roots.push_back("/etc/xdg"); + } + + /* Read config file */ + for (auto const& config_root : config_roots) + { + auto filepath = config_root / filename; + if (std::ifstream config_file{filepath}) + { + load_config(config_file, filepath); + break; + } + } +} + +miral::ReloadingConfigFile::~ReloadingConfigFile() = default; + +void miral::ReloadingConfigFile::register_handler(MirRunner& runner) +{ + if (directory_watch_fd) + { + fd_handle = runner.register_fd_handler(inotify_fd, [icf=inotify_fd, this] (int) + { + union + { + inotify_event event; + char buffer[sizeof(inotify_event) + NAME_MAX + 1]; + } inotify_buffer; + + if (read(icf, &inotify_buffer, sizeof(inotify_buffer)) < static_cast(sizeof(inotify_event))) + return; + + if (inotify_buffer.event.mask & (IN_CLOSE_WRITE | IN_MOVED_TO)) + try + { + if (inotify_buffer.event.name == filename) + { + auto const& file = directory.value() / filename; + + if (std::ifstream config_file{file}) + { + load_config(config_file, file); + } + } + } + catch (...) + { + mir::log(mir::logging::Severity::warning, "ReloadingConfigFile", std::current_exception(), "Failed to reload configuration"); + } + }); + } +} diff --git a/examples/example-server-lib/reloading_config_file.h b/examples/example-server-lib/reloading_config_file.h new file mode 100644 index 00000000000..b151e052a1c --- /dev/null +++ b/examples/example-server-lib/reloading_config_file.h @@ -0,0 +1,58 @@ +/* + * Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 or 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef MIRAL_RELOADING_CONFIG_FILE_H +#define MIRAL_RELOADING_CONFIG_FILE_H + +#include + +#include +#include +#include +#include + +namespace miral { class MirRunner; class FdHandle; } + +namespace miral +{ +/** + * Utility to locate and monitor a config file via + * the XDG Base Directory Specification. Vis: + * ($XDG_CONFIG_HOME or $HOME/.config followed by $XDG_CONFIG_DIRS) + * If, instead of a filename, a path is given, then that is used instead + */ +class ReloadingConfigFile +{ +public: + /// Loader functor is passed both the open stream and the actual path (for use in reporting problems) + using Loader = std::function; + + ReloadingConfigFile(MirRunner& runner, std::filesystem::path file, Loader load_config); + ~ReloadingConfigFile(); + +private: + mir::Fd const inotify_fd; + Loader const load_config; + std::filesystem::path const filename; + std::optional const directory; + std::optional const directory_watch_fd; + + void register_handler(MirRunner& runner); + std::unique_ptr fd_handle; +}; +} + +#endif //MIRAL_RELOADING_CONFIG_FILE_H From dfb08aad7a5c8d8bb00f5179dfdea3766e4823e1 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Fri, 9 Aug 2024 14:31:43 +0100 Subject: [PATCH 02/21] miral style implementation hiding --- examples/example-server-lib/CMakeLists.txt | 1 - .../miral/miral}/reloading_config_file.h | 17 ++++------- src/miral/CMakeLists.txt | 1 + .../miral}/reloading_config_file.cpp | 29 +++++++++++++++++-- 4 files changed, 33 insertions(+), 15 deletions(-) rename {examples/example-server-lib => include/miral/miral}/reloading_config_file.h (75%) rename {examples/example-server-lib => src/miral}/reloading_config_file.cpp (84%) diff --git a/examples/example-server-lib/CMakeLists.txt b/examples/example-server-lib/CMakeLists.txt index 082b6148f8c..2552723a1e3 100644 --- a/examples/example-server-lib/CMakeLists.txt +++ b/examples/example-server-lib/CMakeLists.txt @@ -11,7 +11,6 @@ add_library(example-shell-lib STATIC wayland_app.cpp wayland_app.h wayland_surface.cpp wayland_surface.h wayland_shm.cpp wayland_shm.h - reloading_config_file.cpp reloading_config_file.h ) target_include_directories(example-shell-lib diff --git a/examples/example-server-lib/reloading_config_file.h b/include/miral/miral/reloading_config_file.h similarity index 75% rename from examples/example-server-lib/reloading_config_file.h rename to include/miral/miral/reloading_config_file.h index b151e052a1c..d8e523885a2 100644 --- a/examples/example-server-lib/reloading_config_file.h +++ b/include/miral/miral/reloading_config_file.h @@ -20,9 +20,8 @@ #include #include -#include +#include #include -#include namespace miral { class MirRunner; class FdHandle; } @@ -33,25 +32,21 @@ namespace miral * the XDG Base Directory Specification. Vis: * ($XDG_CONFIG_HOME or $HOME/.config followed by $XDG_CONFIG_DIRS) * If, instead of a filename, a path is given, then that is used instead + * \remark MirAL 5.1 */ class ReloadingConfigFile { public: /// Loader functor is passed both the open stream and the actual path (for use in reporting problems) - using Loader = std::function; + using Loader = std::function; ReloadingConfigFile(MirRunner& runner, std::filesystem::path file, Loader load_config); ~ReloadingConfigFile(); private: - mir::Fd const inotify_fd; - Loader const load_config; - std::filesystem::path const filename; - std::optional const directory; - std::optional const directory_watch_fd; - - void register_handler(MirRunner& runner); - std::unique_ptr fd_handle; + + class Self; + std::shared_ptr self; }; } diff --git a/src/miral/CMakeLists.txt b/src/miral/CMakeLists.txt index 2a268148e0c..07172c39e6d 100644 --- a/src/miral/CMakeLists.txt +++ b/src/miral/CMakeLists.txt @@ -74,6 +74,7 @@ add_library(miral-external OBJECT window_specification.cpp ${miral_include}/miral/window_specification.h internal_client.cpp ${miral_include}/miral/internal_client.h prepend_event_filter.cpp ${miral_include}/miral/prepend_event_filter.h + reloading_config_file.cpp ${miral_include}/miral/reloading_config_file.h runner.cpp ${miral_include}/miral/runner.h session_lock_listener.cpp ${miral_include}/miral/session_lock_listener.h set_command_line_handler.cpp ${miral_include}/miral/set_command_line_handler.h diff --git a/examples/example-server-lib/reloading_config_file.cpp b/src/miral/reloading_config_file.cpp similarity index 84% rename from examples/example-server-lib/reloading_config_file.cpp rename to src/miral/reloading_config_file.cpp index aeb62acfba0..724e0c2c7b6 100644 --- a/examples/example-server-lib/reloading_config_file.cpp +++ b/src/miral/reloading_config_file.cpp @@ -14,7 +14,7 @@ * along with this program. If not, see . */ -#include "reloading_config_file.h" +#include #include @@ -25,6 +25,8 @@ #include #include +#include +#include #include #include @@ -64,7 +66,23 @@ auto watch_fd(mir::Fd const& inotify_fd, std::optional const& path) -> std } } -miral::ReloadingConfigFile::ReloadingConfigFile(MirRunner& runner, path file, Loader load_config) : +class miral::ReloadingConfigFile::Self +{ +public: + Self(MirRunner& runner, path file, Loader load_config); + +private: + mir::Fd const inotify_fd; + Loader const load_config; + std::filesystem::path const filename; + std::optional const directory; + std::optional const directory_watch_fd; + + void register_handler(MirRunner& runner); + std::unique_ptr fd_handle; +}; + +miral::ReloadingConfigFile::Self::Self(MirRunner& runner, path file, Loader load_config) : inotify_fd{inotify_init()}, load_config{load_config}, filename{file.filename()}, @@ -107,9 +125,14 @@ miral::ReloadingConfigFile::ReloadingConfigFile(MirRunner& runner, path file, Lo } } +miral::ReloadingConfigFile::ReloadingConfigFile(MirRunner& runner, path file, Loader load_config) : + self{std::make_shared(runner, file, load_config)} +{ +} + miral::ReloadingConfigFile::~ReloadingConfigFile() = default; -void miral::ReloadingConfigFile::register_handler(MirRunner& runner) +void miral::ReloadingConfigFile::Self::register_handler(MirRunner& runner) { if (directory_watch_fd) { From e55a453546e66e0d9f49b852152f3cc03ec5da7c Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Fri, 9 Aug 2024 14:51:56 +0100 Subject: [PATCH 03/21] Symbols! --- debian/libmiral7.symbols | 2 ++ src/miral/symbols.map | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/debian/libmiral7.symbols b/debian/libmiral7.symbols index 192c2df8320..7065eea1cba 100644 --- a/debian/libmiral7.symbols +++ b/debian/libmiral7.symbols @@ -445,4 +445,6 @@ libmiral.so.7 libmiral7 #MINVER# (c++)"miral::IdleListener::on_off(std::function const&)@MIRAL_5.1" 5.1.0 (c++)"miral::IdleListener::on_wake(std::function const&)@MIRAL_5.1" 5.1.0 (c++)"miral::IdleListener::operator()(mir::Server&) const@MIRAL_5.1" 5.1.0 + (c++)"miral::ReloadingConfigFile::ReloadingConfigFile(miral::MirRunner&, std::filesystem::__cxx11::path, std::function >&, std::filesystem::__cxx11::path const&)>)@MIRAL_5.1" 5.1.0 + (c++)"miral::ReloadingConfigFile::~ReloadingConfigFile()@MIRAL_5.1" 5.1.0 (c++)"miral::WindowManagerTools::move_cursor_to(mir::geometry::generic::Point)@MIRAL_5.1" 5.1.0 diff --git a/src/miral/symbols.map b/src/miral/symbols.map index 79b4961cfd4..542f24cfb72 100644 --- a/src/miral/symbols.map +++ b/src/miral/symbols.map @@ -468,8 +468,12 @@ global: miral::IdleListener::on_off*; miral::IdleListener::on_wake*; miral::IdleListener::operator*; + miral::ReloadingConfigFile::?ReloadingConfigFile*; + miral::ReloadingConfigFile::ReloadingConfigFile*; miral::WindowManagerTools::move_cursor_to*; - typeinfo?for?miral::IdleListener; typeinfo?for?miral::Decorations; + typeinfo?for?miral::IdleListener; + typeinfo?for?miral::ReloadingConfigFile; }; } MIRAL_5.0; + From ccf4b40a4d23de28c64a40913b5dbbde196beb00 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Wed, 14 Aug 2024 11:23:07 +0100 Subject: [PATCH 04/21] Watch descriptors are not FDs --- src/miral/reloading_config_file.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/miral/reloading_config_file.cpp b/src/miral/reloading_config_file.cpp index 724e0c2c7b6..6891da3d154 100644 --- a/src/miral/reloading_config_file.cpp +++ b/src/miral/reloading_config_file.cpp @@ -54,7 +54,7 @@ auto config_directory(path const& file) -> std::optional } } -auto watch_fd(mir::Fd const& inotify_fd, std::optional const& path) -> std::optional +auto watch_descriptor(mir::Fd const& inotify_fd, std::optional const& path) -> std::optional { if (!path.has_value()) return std::nullopt; @@ -62,7 +62,7 @@ auto watch_fd(mir::Fd const& inotify_fd, std::optional const& path) -> std if (inotify_fd < 0) BOOST_THROW_EXCEPTION((std::system_error{errno, std::system_category(), "Failed to initialize inotify_fd"})); - return mir::Fd{inotify_add_watch(inotify_fd, path.value().c_str(), IN_CLOSE_WRITE | IN_CREATE | IN_MOVED_TO)}; + return inotify_add_watch(inotify_fd, path.value().c_str(), IN_CLOSE_WRITE | IN_CREATE | IN_MOVED_TO); } } @@ -76,7 +76,7 @@ class miral::ReloadingConfigFile::Self Loader const load_config; std::filesystem::path const filename; std::optional const directory; - std::optional const directory_watch_fd; + std::optional const directory_watch_descriptor; void register_handler(MirRunner& runner); std::unique_ptr fd_handle; @@ -87,7 +87,7 @@ miral::ReloadingConfigFile::Self::Self(MirRunner& runner, path file, Loader load load_config{load_config}, filename{file.filename()}, directory{config_directory(file)}, - directory_watch_fd{watch_fd(inotify_fd, directory)} + directory_watch_descriptor{watch_descriptor(inotify_fd, directory)} { register_handler(runner); @@ -134,7 +134,7 @@ miral::ReloadingConfigFile::~ReloadingConfigFile() = default; void miral::ReloadingConfigFile::Self::register_handler(MirRunner& runner) { - if (directory_watch_fd) + if (directory_watch_descriptor) { fd_handle = runner.register_fd_handler(inotify_fd, [icf=inotify_fd, this] (int) { From 354dcc669f600c3366f2086dbbf02428ee8df736 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Wed, 14 Aug 2024 13:00:06 +0100 Subject: [PATCH 05/21] inotify_init1(IN_CLOEXEC) --- src/miral/reloading_config_file.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/miral/reloading_config_file.cpp b/src/miral/reloading_config_file.cpp index 6891da3d154..0f97db7cb9a 100644 --- a/src/miral/reloading_config_file.cpp +++ b/src/miral/reloading_config_file.cpp @@ -83,7 +83,7 @@ class miral::ReloadingConfigFile::Self }; miral::ReloadingConfigFile::Self::Self(MirRunner& runner, path file, Loader load_config) : - inotify_fd{inotify_init()}, + inotify_fd{inotify_init1(IN_CLOEXEC)}, load_config{load_config}, filename{file.filename()}, directory{config_directory(file)}, From 336c8066983aa5ebda95555f8f18714aec7c58f7 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Wed, 14 Aug 2024 11:42:39 +0100 Subject: [PATCH 06/21] A TestReloadingConfigFile test fixture --- tests/miral/CMakeLists.txt | 1 + tests/miral/reloading_config_file.cpp | 116 ++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 tests/miral/reloading_config_file.cpp diff --git a/tests/miral/CMakeLists.txt b/tests/miral/CMakeLists.txt index e7d52891c62..24db9802976 100644 --- a/tests/miral/CMakeLists.txt +++ b/tests/miral/CMakeLists.txt @@ -69,6 +69,7 @@ target_link_libraries(miral-test-internal mir_add_wrapped_executable(miral-test NOINSTALL external_client.cpp + reloading_config_file.cpp runner.cpp wayland_extensions.cpp zone.cpp diff --git a/tests/miral/reloading_config_file.cpp b/tests/miral/reloading_config_file.cpp new file mode 100644 index 00000000000..553ff7a6e45 --- /dev/null +++ b/tests/miral/reloading_config_file.cpp @@ -0,0 +1,116 @@ +/* +* Copyright © Canonical Ltd. + * + * This program is free software: you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 or 3 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#include "miral/test_server.h" +#include "miral/reloading_config_file.h" + +#include +#include + +#include + +using miral::ReloadingConfigFile; + +namespace +{ +struct TestReloadingConfigFile : miral::TestServer +{ + TestReloadingConfigFile(); + MOCK_METHOD(void, load, (std::istream& in, std::filesystem::path path), ()); + + std::optional reloading_config_file; +}; + +char const* const no_such_file = "no/such/file"; +char const* const a_file = "/tmp/a_file"; + +TestReloadingConfigFile::TestReloadingConfigFile() +{ + unlink(a_file); +} + +void write_a_file() +{ + std::ofstream file(a_file); + file << "some content"; +} +} + +TEST_F(TestReloadingConfigFile, with_no_file_nothing_is_loaded) +{ + EXPECT_CALL(*this, load).Times(0); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ReloadingConfigFile{ + runner, + no_such_file, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); +} + +TEST_F(TestReloadingConfigFile, with_a_file_something_is_loaded) +{ + EXPECT_CALL(*this, load).Times(1); + + write_a_file(); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ReloadingConfigFile{ + runner, + a_file, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); +} + +TEST_F(TestReloadingConfigFile, when_a_file_is_written_something_is_loaded) +{ + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ReloadingConfigFile{ + runner, + a_file, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + EXPECT_CALL(*this, load).Times(1); + + write_a_file(); +} + + +TEST_F(TestReloadingConfigFile, each_time_a_file_is_rewritten_something_is_loaded) +{ + auto const times = 42; + + EXPECT_CALL(*this, load).Times(times+1); + + write_a_file(); // Initial write + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ReloadingConfigFile{ + runner, + a_file, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + for (auto i = 0; i < times; ++i) + { + write_a_file(); + } +} \ No newline at end of file From 8d7ee4343cd716d6183674c356d4af3bfb3cfd09 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Wed, 14 Aug 2024 16:59:43 +0100 Subject: [PATCH 07/21] Synchronisation so that loads happen before test exits --- tests/miral/reloading_config_file.cpp | 45 +++++++++++++++++++++------ 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/tests/miral/reloading_config_file.cpp b/tests/miral/reloading_config_file.cpp index 553ff7a6e45..4c1252ce16d 100644 --- a/tests/miral/reloading_config_file.cpp +++ b/tests/miral/reloading_config_file.cpp @@ -26,27 +26,49 @@ using miral::ReloadingConfigFile; namespace { +char const* const no_such_file = "no/such/file"; +char const* const a_file = "/tmp/a_file"; + struct TestReloadingConfigFile : miral::TestServer { TestReloadingConfigFile(); MOCK_METHOD(void, load, (std::istream& in, std::filesystem::path path), ()); std::optional reloading_config_file; -}; -char const* const no_such_file = "no/such/file"; -char const* const a_file = "/tmp/a_file"; + std::atomic pending_load = false; + void write_a_file() + { + std::ofstream file(a_file); + file << "some content"; + pending_load = true; + } + + void wait_for_load() + { + while (bool const current = pending_load) + { + pending_load.wait(current); + } + } + + void notify_load() + { + pending_load = false; pending_load.notify_one(); + } + + void SetUp() override + { + miral::TestServer::SetUp(); + ON_CALL(*this, load(testing::_, testing::_)).WillByDefault([this]{ notify_load(); }); + } +}; TestReloadingConfigFile::TestReloadingConfigFile() { unlink(a_file); } -void write_a_file() -{ - std::ofstream file(a_file); - file << "some content"; -} } TEST_F(TestReloadingConfigFile, with_no_file_nothing_is_loaded) @@ -90,9 +112,9 @@ TEST_F(TestReloadingConfigFile, when_a_file_is_written_something_is_loaded) EXPECT_CALL(*this, load).Times(1); write_a_file(); + wait_for_load(); } - TEST_F(TestReloadingConfigFile, each_time_a_file_is_rewritten_something_is_loaded) { auto const times = 42; @@ -109,8 +131,11 @@ TEST_F(TestReloadingConfigFile, each_time_a_file_is_rewritten_something_is_loade [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); - for (auto i = 0; i < times; ++i) + for (auto i = 0; i != times; ++i) { + wait_for_load(); write_a_file(); } + + wait_for_load(); } \ No newline at end of file From 77440a5b706105606352698cc376efef97a01b5b Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Wed, 14 Aug 2024 17:34:04 +0100 Subject: [PATCH 08/21] TODO --- tests/miral/reloading_config_file.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/miral/reloading_config_file.cpp b/tests/miral/reloading_config_file.cpp index 4c1252ce16d..72eddc21733 100644 --- a/tests/miral/reloading_config_file.cpp +++ b/tests/miral/reloading_config_file.cpp @@ -138,4 +138,6 @@ TEST_F(TestReloadingConfigFile, each_time_a_file_is_rewritten_something_is_loade } wait_for_load(); -} \ No newline at end of file +} + +// TODO - tests around the XDG Base Directory paths \ No newline at end of file From 5c67e21b450c1515fb5454f68f70aab2b0d66b5f Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Thu, 15 Aug 2024 12:19:45 +0100 Subject: [PATCH 09/21] Improve synchronization (and make it more explicit) --- tests/miral/reloading_config_file.cpp | 50 +++++++++++++++++++-------- 1 file changed, 36 insertions(+), 14 deletions(-) diff --git a/tests/miral/reloading_config_file.cpp b/tests/miral/reloading_config_file.cpp index 72eddc21733..914c9dfe1ef 100644 --- a/tests/miral/reloading_config_file.cpp +++ b/tests/miral/reloading_config_file.cpp @@ -29,32 +29,53 @@ namespace char const* const no_such_file = "no/such/file"; char const* const a_file = "/tmp/a_file"; -struct TestReloadingConfigFile : miral::TestServer +class PendingLoad { - TestReloadingConfigFile(); - MOCK_METHOD(void, load, (std::istream& in, std::filesystem::path path), ()); - - std::optional reloading_config_file; - - std::atomic pending_load = false; - void write_a_file() +public: + void mark_pending() { - std::ofstream file(a_file); - file << "some content"; - pending_load = true; + std::lock_guard lock{mutex}; + pending_loads = true; } void wait_for_load() { - while (bool const current = pending_load) + std::unique_lock lock{mutex}; + + if (!cv.wait_for(lock, std::chrono::milliseconds{1000}, [this] { return !pending_loads; })) { - pending_load.wait(current); + std::cerr << "wait_for_load() failed" << std::endl; } } void notify_load() { - pending_load = false; pending_load.notify_one(); + { + std::lock_guard lock{mutex}; + pending_loads = false; + } + + cv.notify_one(); + } + +private: + std::mutex mutex; + std::condition_variable cv; + bool pending_loads = false; +}; + +struct TestReloadingConfigFile : miral::TestServer, PendingLoad +{ + TestReloadingConfigFile(); + MOCK_METHOD(void, load, (std::istream& in, std::filesystem::path path), ()); + + std::optional reloading_config_file; + + void write_a_file() + { + mark_pending(); + std::ofstream file(a_file); + file << "some content"; } void SetUp() override @@ -97,6 +118,7 @@ TEST_F(TestReloadingConfigFile, with_a_file_something_is_loaded) a_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); + wait_for_load(); } TEST_F(TestReloadingConfigFile, when_a_file_is_written_something_is_loaded) From 4616aa79a4c5e58ab14494667222ab70fa81b194 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Thu, 15 Aug 2024 12:59:49 +0100 Subject: [PATCH 10/21] DISABLED_when_a_file_is_written_something_is_loaded (for now) --- tests/miral/reloading_config_file.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/miral/reloading_config_file.cpp b/tests/miral/reloading_config_file.cpp index 914c9dfe1ef..3250dc74883 100644 --- a/tests/miral/reloading_config_file.cpp +++ b/tests/miral/reloading_config_file.cpp @@ -121,7 +121,7 @@ TEST_F(TestReloadingConfigFile, with_a_file_something_is_loaded) wait_for_load(); } -TEST_F(TestReloadingConfigFile, when_a_file_is_written_something_is_loaded) +TEST_F(TestReloadingConfigFile, DISABLED_when_a_file_is_written_something_is_loaded) { invoke_runner([this](miral::MirRunner& runner) { From ad258290bab827cabbda5b36a0f5401859bf1975 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Thu, 15 Aug 2024 18:06:34 +0100 Subject: [PATCH 11/21] More than one event may be read from the inotify fd --- src/miral/reloading_config_file.cpp | 34 +++++++++++++++++++---------- 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/miral/reloading_config_file.cpp b/src/miral/reloading_config_file.cpp index 0f97db7cb9a..d50cc0376fe 100644 --- a/src/miral/reloading_config_file.cpp +++ b/src/miral/reloading_config_file.cpp @@ -144,25 +144,35 @@ void miral::ReloadingConfigFile::Self::register_handler(MirRunner& runner) char buffer[sizeof(inotify_event) + NAME_MAX + 1]; } inotify_buffer; - if (read(icf, &inotify_buffer, sizeof(inotify_buffer)) < static_cast(sizeof(inotify_event))) + ssize_t readsize = read(icf, &inotify_buffer, sizeof(inotify_buffer)); + if (readsize < static_cast(sizeof(inotify_event))) + { return; + } - if (inotify_buffer.event.mask & (IN_CLOSE_WRITE | IN_MOVED_TO)) - try + auto raw_buffer = inotify_buffer.buffer; + while (raw_buffer != inotify_buffer.buffer + readsize) { - if (inotify_buffer.event.name == filename) + auto& event = reinterpret_cast(*raw_buffer); + if (event.mask & (IN_CLOSE_WRITE | IN_MOVED_TO)) + try { - auto const& file = directory.value() / filename; - - if (std::ifstream config_file{file}) + if (event.name == filename) { - load_config(config_file, file); + auto const& file = directory.value() / filename; + + if (std::ifstream config_file{file}) + { + load_config(config_file, file); + } } } - } - catch (...) - { - mir::log(mir::logging::Severity::warning, "ReloadingConfigFile", std::current_exception(), "Failed to reload configuration"); + catch (...) + { + mir::log(mir::logging::Severity::warning, "ReloadingConfigFile", std::current_exception(), "Failed to reload configuration"); + } + + raw_buffer += sizeof(struct inotify_event)+event.len; } }); } From d90d37a8ced6b79f2fd9fc2e86002119db156929 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Thu, 15 Aug 2024 18:07:14 +0100 Subject: [PATCH 12/21] Revert "DISABLED_when_a_file_is_written_something_is_loaded (for now)" This reverts commit efbc8a6ffadfe1ed8f62e99e8430dbe8d0c921f8. --- tests/miral/reloading_config_file.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/miral/reloading_config_file.cpp b/tests/miral/reloading_config_file.cpp index 3250dc74883..914c9dfe1ef 100644 --- a/tests/miral/reloading_config_file.cpp +++ b/tests/miral/reloading_config_file.cpp @@ -121,7 +121,7 @@ TEST_F(TestReloadingConfigFile, with_a_file_something_is_loaded) wait_for_load(); } -TEST_F(TestReloadingConfigFile, DISABLED_when_a_file_is_written_something_is_loaded) +TEST_F(TestReloadingConfigFile, when_a_file_is_written_something_is_loaded) { invoke_runner([this](miral::MirRunner& runner) { From e02d6bd780fb4f82895f16fc5d3515d2a38dc177 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Fri, 16 Aug 2024 11:23:38 +0100 Subject: [PATCH 13/21] Refactor --- src/miral/reloading_config_file.cpp | 38 +++++++++++++++-------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/src/miral/reloading_config_file.cpp b/src/miral/reloading_config_file.cpp index d50cc0376fe..6e3680087c5 100644 --- a/src/miral/reloading_config_file.cpp +++ b/src/miral/reloading_config_file.cpp @@ -136,43 +136,45 @@ void miral::ReloadingConfigFile::Self::register_handler(MirRunner& runner) { if (directory_watch_descriptor) { - fd_handle = runner.register_fd_handler(inotify_fd, [icf=inotify_fd, this] (int) + fd_handle = runner.register_fd_handler(inotify_fd, [this] (int) { + static size_t const sizeof_inotify_event = sizeof(inotify_event); + + // A union ensures buffer is aligned for inotify_event union { - inotify_event event; - char buffer[sizeof(inotify_event) + NAME_MAX + 1]; - } inotify_buffer; + char buffer[sizeof_inotify_event + NAME_MAX + 1]; + inotify_event unused [[maybe_unused]]; + } inotify_magic; - ssize_t readsize = read(icf, &inotify_buffer, sizeof(inotify_buffer)); - if (readsize < static_cast(sizeof(inotify_event))) + auto const readsize = read(inotify_fd, &inotify_magic, sizeof(inotify_magic)); + if (readsize < static_cast(sizeof_inotify_event)) { return; } - auto raw_buffer = inotify_buffer.buffer; - while (raw_buffer != inotify_buffer.buffer + readsize) + auto raw_buffer = inotify_magic.buffer; + while (raw_buffer != inotify_magic.buffer + readsize) { + // This is safe because inotify_magic.buffer is aligned and event.len includes padding for alignment auto& event = reinterpret_cast(*raw_buffer); - if (event.mask & (IN_CLOSE_WRITE | IN_MOVED_TO)) + if (event.mask & (IN_CLOSE_WRITE | IN_MOVED_TO) && event.wd == directory_watch_descriptor.value()) try { - if (event.name == filename) - { - auto const& file = directory.value() / filename; + auto const& file = directory.value() / filename; - if (std::ifstream config_file{file}) - { - load_config(config_file, file); - } + if (std::ifstream config_file{file}) + { + load_config(config_file, file); } } catch (...) { - mir::log(mir::logging::Severity::warning, "ReloadingConfigFile", std::current_exception(), "Failed to reload configuration"); + mir::log(mir::logging::Severity::warning, "ReloadingConfigFile", std::current_exception(), + "Failed to reload configuration"); } - raw_buffer += sizeof(struct inotify_event)+event.len; + raw_buffer += sizeof_inotify_event+event.len; } }); } From bd3af19d9daa1df3c5663274e03185e01e6644a6 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Fri, 16 Aug 2024 12:08:42 +0100 Subject: [PATCH 14/21] Additional check needed on Alpine --- src/miral/reloading_config_file.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/miral/reloading_config_file.cpp b/src/miral/reloading_config_file.cpp index 6e3680087c5..c9e96199664 100644 --- a/src/miral/reloading_config_file.cpp +++ b/src/miral/reloading_config_file.cpp @@ -161,11 +161,14 @@ void miral::ReloadingConfigFile::Self::register_handler(MirRunner& runner) if (event.mask & (IN_CLOSE_WRITE | IN_MOVED_TO) && event.wd == directory_watch_descriptor.value()) try { - auto const& file = directory.value() / filename; - - if (std::ifstream config_file{file}) + if (event.name == filename) { - load_config(config_file, file); + auto const& file = directory.value() / filename; + + if (std::ifstream config_file{file}) + { + load_config(config_file, file); + } } } catch (...) From ec826f28022eb0cd02d85000e9bf6ba8e64a52ed Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Fri, 16 Aug 2024 15:09:39 +0100 Subject: [PATCH 15/21] tests around the XDG Base Directory paths --- tests/miral/reloading_config_file.cpp | 198 +++++++++++++++++++++++++- 1 file changed, 195 insertions(+), 3 deletions(-) diff --git a/tests/miral/reloading_config_file.cpp b/tests/miral/reloading_config_file.cpp index 914c9dfe1ef..2ab01b11ee2 100644 --- a/tests/miral/reloading_config_file.cpp +++ b/tests/miral/reloading_config_file.cpp @@ -20,6 +20,7 @@ #include #include +#include #include using miral::ReloadingConfigFile; @@ -27,7 +28,8 @@ using miral::ReloadingConfigFile; namespace { char const* const no_such_file = "no/such/file"; -char const* const a_file = "/tmp/a_file"; +char const* const a_file = "/tmp/test_reloading_config_file/a_file"; +std::filesystem::path const config_file = "test_reloading_config_file.config"; class PendingLoad { @@ -78,6 +80,13 @@ struct TestReloadingConfigFile : miral::TestServer, PendingLoad file << "some content"; } + void write_config_in(std::filesystem::path path) + { + mark_pending(); + std::ofstream file(path/config_file); + file << "some content"; + } + void SetUp() override { miral::TestServer::SetUp(); @@ -85,9 +94,25 @@ struct TestReloadingConfigFile : miral::TestServer, PendingLoad } }; +char const* const home = "/tmp/test_reloading_config_file/home"; +char const* const home_config = "/tmp/test_reloading_config_file/home/.config"; +char const* const xdg_conf_home = "/tmp/test_reloading_config_file/xdg_conf_dir_home"; +char const* const xdg_conf_dir0 = "/tmp/test_reloading_config_file/xdg_conf_dir_zero"; +char const* const xdg_conf_dir1 = "/tmp/test_reloading_config_file/xdg_conf_dir_one"; +char const* const xdg_conf_dir2 = "/tmp/test_reloading_config_file/xdg_conf_dir_two"; + TestReloadingConfigFile::TestReloadingConfigFile() { - unlink(a_file); + std::filesystem::remove_all("/tmp/test_reloading_config_file/"); + + for (auto dir : {home_config, xdg_conf_home, xdg_conf_dir0, xdg_conf_dir1, xdg_conf_dir2}) + { + std::filesystem::create_directories(dir); + } + + add_to_environment("HOME", home); + add_to_environment("XDG_CONFIG_HOME", xdg_conf_home); + add_to_environment("XDG_CONFIG_DIRS", std::format("{}:{}:{}", xdg_conf_dir0, xdg_conf_dir1, xdg_conf_dir2).c_str()); } } @@ -162,4 +187,171 @@ TEST_F(TestReloadingConfigFile, each_time_a_file_is_rewritten_something_is_loade wait_for_load(); } -// TODO - tests around the XDG Base Directory paths \ No newline at end of file +TEST_F(TestReloadingConfigFile, when_config_home_unset_a_file_in_home_config_is_loaded) +{ + add_to_environment("XDG_CONFIG_HOME", nullptr); + using testing::_; + EXPECT_CALL(*this, load(_, home_config/config_file)).Times(1); + + write_config_in(xdg_conf_home); + write_config_in(home_config); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ReloadingConfigFile{ + runner, + config_file, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); +} + +TEST_F(TestReloadingConfigFile, a_file_in_xdg_config_home_is_loaded) +{ + using testing::_; + EXPECT_CALL(*this, load(_, xdg_conf_home/config_file)).Times(1); + + write_config_in(xdg_conf_dir0); + write_config_in(xdg_conf_home); + write_config_in(home_config); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ReloadingConfigFile{ + runner, + config_file, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); +} + +TEST_F(TestReloadingConfigFile, a_file_in_xdg_config_home_is_reloaded) +{ + using testing::_; + EXPECT_CALL(*this, load(_, xdg_conf_home/config_file)).Times(2); + + write_config_in(xdg_conf_dir0); + write_config_in(xdg_conf_home); + write_config_in(home_config); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ReloadingConfigFile{ + runner, + config_file, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); + + write_config_in(xdg_conf_dir0); + write_config_in(xdg_conf_home); + write_config_in(home_config); + wait_for_load(); +} + +TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir0_is_loaded) +{ + using testing::_; + EXPECT_CALL(*this, load(_, xdg_conf_dir0/config_file)).Times(1); + + write_config_in(xdg_conf_dir0); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ReloadingConfigFile{ + runner, + config_file, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); +} + +TEST_F(TestReloadingConfigFile, after_a_config_in_xdg_conf_dir0_is_loaded_a_new_config_in_xdg_conf_home_is_loaded) +{ + using testing::_; + + testing::InSequence sequence; + EXPECT_CALL(*this, load(_, xdg_conf_dir0/config_file)).Times(1); + EXPECT_CALL(*this, load(_, xdg_conf_home/config_file)).Times(1); + + write_config_in(xdg_conf_dir2); + write_config_in(xdg_conf_dir1); + write_config_in(xdg_conf_dir0); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ReloadingConfigFile{ + runner, + config_file, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); + + write_config_in(xdg_conf_home); + wait_for_load(); +} + +TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir0_is_loaded_in_preference_to_dir1_or_2) +{ + using testing::_; + + EXPECT_CALL(*this, load(_, xdg_conf_dir0/config_file)).Times(1); + + write_config_in(xdg_conf_dir2); + write_config_in(xdg_conf_dir1); + write_config_in(xdg_conf_dir0); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ReloadingConfigFile{ + runner, + config_file, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); +} + +TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir1_is_loaded_in_preference_to_dir2) +{ + using testing::_; + + EXPECT_CALL(*this, load(_, xdg_conf_dir1/config_file)).Times(1); + + write_config_in(xdg_conf_dir2); + write_config_in(xdg_conf_dir1); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ReloadingConfigFile{ + runner, + config_file, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); +} + +TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir2_is_loaded) +{ + using testing::_; + + EXPECT_CALL(*this, load(_, xdg_conf_dir2/config_file)).Times(1); + + write_config_in(xdg_conf_dir2); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ReloadingConfigFile{ + runner, + config_file, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); +} From d7710173d738e96ae3dae4db8224fa9731674b5f Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Mon, 19 Aug 2024 14:35:29 +0100 Subject: [PATCH 16/21] Reword documentation --- include/miral/miral/reloading_config_file.h | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/include/miral/miral/reloading_config_file.h b/include/miral/miral/reloading_config_file.h index d8e523885a2..999ae3275cc 100644 --- a/include/miral/miral/reloading_config_file.h +++ b/include/miral/miral/reloading_config_file.h @@ -28,10 +28,14 @@ namespace miral { class MirRunner; class FdHandle; } namespace miral { /** - * Utility to locate and monitor a config file via - * the XDG Base Directory Specification. Vis: - * ($XDG_CONFIG_HOME or $HOME/.config followed by $XDG_CONFIG_DIRS) - * If, instead of a filename, a path is given, then that is used instead + * Utility to locate and monitor a configuration file via the XDG Base Directory + * Specification. Vis: ($XDG_CONFIG_HOME or $HOME/.config followed by + * $XDG_CONFIG_DIRS). + * The user-specific configuration file base ($XDG_CONFIG_HOME or $HOME/.config) + * is monitored for subsequent changes (but not directories in $XDG_CONFIG_DIRS) + * + * If, instead of a filename, a path is given, then the base directories are not + * applied and the supplied path monitored for changes. * \remark MirAL 5.1 */ class ReloadingConfigFile From 2f3b4edf77fbe41e2581000b00ae7ed0f116a727 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Tue, 20 Aug 2024 10:53:48 +0100 Subject: [PATCH 17/21] More logging --- src/miral/reloading_config_file.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/miral/reloading_config_file.cpp b/src/miral/reloading_config_file.cpp index c9e96199664..b5c0e544855 100644 --- a/src/miral/reloading_config_file.cpp +++ b/src/miral/reloading_config_file.cpp @@ -18,6 +18,7 @@ #include +#define MIR_LOG_COMPONENT "ReloadingConfigFile" #include #include @@ -91,6 +92,11 @@ miral::ReloadingConfigFile::Self::Self(MirRunner& runner, path file, Loader load { register_handler(runner); + if (directory_watch_descriptor.has_value()) + { + mir::log_debug("Monitoring %s for configuration changes", (directory.value()/filename).c_str()); + } + // With C++26 we should be able to use the optional directory as a range to // initialize config_roots. Until then, we'll just do it the long way... std::vector config_roots; @@ -120,6 +126,7 @@ miral::ReloadingConfigFile::Self::Self(MirRunner& runner, path file, Loader load if (std::ifstream config_file{filepath}) { load_config(config_file, filepath); + mir::log_debug("Loaded %s", filepath.c_str()); break; } } @@ -168,13 +175,18 @@ void miral::ReloadingConfigFile::Self::register_handler(MirRunner& runner) if (std::ifstream config_file{file}) { load_config(config_file, file); + mir::log_debug("(Re)loaded %s", file.c_str()); + } + else + { + mir::log_debug("Failed to open %s", file.c_str()); } } } catch (...) { - mir::log(mir::logging::Severity::warning, "ReloadingConfigFile", std::current_exception(), - "Failed to reload configuration"); + mir::log(mir::logging::Severity::warning, MIR_LOG_COMPONENT, std::current_exception(), + "Failed to reload configuration"); } raw_buffer += sizeof_inotify_event+event.len; From 64546a4518a2cebf0ce5ec6db2c2f8b1959bff18 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Tue, 20 Aug 2024 12:19:50 +0100 Subject: [PATCH 18/21] Rename => ConfigFile --- debian/libmiral7.symbols | 4 +-- ...{reloading_config_file.h => config_file.h} | 12 ++++---- src/miral/CMakeLists.txt | 2 +- ...oading_config_file.cpp => config_file.cpp} | 12 ++++---- src/miral/symbols.map | 6 ++-- tests/miral/CMakeLists.txt | 2 +- ...oading_config_file.cpp => config_file.cpp} | 30 +++++++++---------- 7 files changed, 34 insertions(+), 34 deletions(-) rename include/miral/miral/{reloading_config_file.h => config_file.h} (85%) rename src/miral/{reloading_config_file.cpp => config_file.cpp} (93%) rename tests/miral/{reloading_config_file.cpp => config_file.cpp} (92%) diff --git a/debian/libmiral7.symbols b/debian/libmiral7.symbols index 7065eea1cba..87cb25f47e9 100644 --- a/debian/libmiral7.symbols +++ b/debian/libmiral7.symbols @@ -434,6 +434,8 @@ libmiral.so.7 libmiral7 #MINVER# (c++)"vtable for miral::MinimalWindowManager@MIRAL_5.0" 5.0.0 (c++)"vtable for miral::WindowManagementPolicy@MIRAL_5.0" 5.0.0 MIRAL_5.1@MIRAL_5.1 5.1.0 + (c++)"miral::ConfigFile::ConfigFile(miral::MirRunner&, std::filesystem::__cxx11::path, std::function >&, std::filesystem::__cxx11::path const&)>)@MIRAL_5.1" 5.1.0 + (c++)"miral::ConfigFile::~ConfigFile()@MIRAL_5.1" 5.1.0 (c++)"miral::Decorations::Decorations(std::shared_ptr)@MIRAL_5.1" 5.1.0 (c++)"miral::Decorations::always_csd()@MIRAL_5.1" 5.1.0 (c++)"miral::Decorations::always_ssd()@MIRAL_5.1" 5.1.0 @@ -445,6 +447,4 @@ libmiral.so.7 libmiral7 #MINVER# (c++)"miral::IdleListener::on_off(std::function const&)@MIRAL_5.1" 5.1.0 (c++)"miral::IdleListener::on_wake(std::function const&)@MIRAL_5.1" 5.1.0 (c++)"miral::IdleListener::operator()(mir::Server&) const@MIRAL_5.1" 5.1.0 - (c++)"miral::ReloadingConfigFile::ReloadingConfigFile(miral::MirRunner&, std::filesystem::__cxx11::path, std::function >&, std::filesystem::__cxx11::path const&)>)@MIRAL_5.1" 5.1.0 - (c++)"miral::ReloadingConfigFile::~ReloadingConfigFile()@MIRAL_5.1" 5.1.0 (c++)"miral::WindowManagerTools::move_cursor_to(mir::geometry::generic::Point)@MIRAL_5.1" 5.1.0 diff --git a/include/miral/miral/reloading_config_file.h b/include/miral/miral/config_file.h similarity index 85% rename from include/miral/miral/reloading_config_file.h rename to include/miral/miral/config_file.h index 999ae3275cc..409a4b9a1f6 100644 --- a/include/miral/miral/reloading_config_file.h +++ b/include/miral/miral/config_file.h @@ -14,8 +14,8 @@ * along with this program. If not, see . */ -#ifndef MIRAL_RELOADING_CONFIG_FILE_H -#define MIRAL_RELOADING_CONFIG_FILE_H +#ifndef MIRAL_CONFIG_FILE_H +#define MIRAL_CONFIG_FILE_H #include @@ -38,14 +38,14 @@ namespace miral * applied and the supplied path monitored for changes. * \remark MirAL 5.1 */ -class ReloadingConfigFile +class ConfigFile { public: /// Loader functor is passed both the open stream and the actual path (for use in reporting problems) using Loader = std::function; - ReloadingConfigFile(MirRunner& runner, std::filesystem::path file, Loader load_config); - ~ReloadingConfigFile(); + ConfigFile(MirRunner& runner, std::filesystem::path file, Loader load_config); + ~ConfigFile(); private: @@ -54,4 +54,4 @@ class ReloadingConfigFile }; } -#endif //MIRAL_RELOADING_CONFIG_FILE_H +#endif //MIRAL_CONFIG_FILE_H diff --git a/src/miral/CMakeLists.txt b/src/miral/CMakeLists.txt index 07172c39e6d..84351a2e81c 100644 --- a/src/miral/CMakeLists.txt +++ b/src/miral/CMakeLists.txt @@ -55,6 +55,7 @@ add_library(miral-external OBJECT application_authorizer.cpp ${miral_include}/miral/application_authorizer.h application_info.cpp ${miral_include}/miral/application_info.h canonical_window_manager.cpp ${miral_include}/miral/canonical_window_manager.h + config_file.cpp ${miral_include}/miral/config_file.h configuration_option.cpp ${miral_include}/miral/configuration_option.h ${miral_include}/miral/command_line_option.h cursor_theme.cpp ${miral_include}/miral/cursor_theme.h @@ -74,7 +75,6 @@ add_library(miral-external OBJECT window_specification.cpp ${miral_include}/miral/window_specification.h internal_client.cpp ${miral_include}/miral/internal_client.h prepend_event_filter.cpp ${miral_include}/miral/prepend_event_filter.h - reloading_config_file.cpp ${miral_include}/miral/reloading_config_file.h runner.cpp ${miral_include}/miral/runner.h session_lock_listener.cpp ${miral_include}/miral/session_lock_listener.h set_command_line_handler.cpp ${miral_include}/miral/set_command_line_handler.h diff --git a/src/miral/reloading_config_file.cpp b/src/miral/config_file.cpp similarity index 93% rename from src/miral/reloading_config_file.cpp rename to src/miral/config_file.cpp index b5c0e544855..1ae807de8df 100644 --- a/src/miral/reloading_config_file.cpp +++ b/src/miral/config_file.cpp @@ -14,7 +14,7 @@ * along with this program. If not, see . */ -#include +#include #include @@ -67,7 +67,7 @@ auto watch_descriptor(mir::Fd const& inotify_fd, std::optional const& path } } -class miral::ReloadingConfigFile::Self +class miral::ConfigFile::Self { public: Self(MirRunner& runner, path file, Loader load_config); @@ -83,7 +83,7 @@ class miral::ReloadingConfigFile::Self std::unique_ptr fd_handle; }; -miral::ReloadingConfigFile::Self::Self(MirRunner& runner, path file, Loader load_config) : +miral::ConfigFile::Self::Self(MirRunner& runner, path file, Loader load_config) : inotify_fd{inotify_init1(IN_CLOEXEC)}, load_config{load_config}, filename{file.filename()}, @@ -132,14 +132,14 @@ miral::ReloadingConfigFile::Self::Self(MirRunner& runner, path file, Loader load } } -miral::ReloadingConfigFile::ReloadingConfigFile(MirRunner& runner, path file, Loader load_config) : +miral::ConfigFile::ConfigFile(MirRunner& runner, path file, Loader load_config) : self{std::make_shared(runner, file, load_config)} { } -miral::ReloadingConfigFile::~ReloadingConfigFile() = default; +miral::ConfigFile::~ConfigFile() = default; -void miral::ReloadingConfigFile::Self::register_handler(MirRunner& runner) +void miral::ConfigFile::Self::register_handler(MirRunner& runner) { if (directory_watch_descriptor) { diff --git a/src/miral/symbols.map b/src/miral/symbols.map index 542f24cfb72..a2d1fd49702 100644 --- a/src/miral/symbols.map +++ b/src/miral/symbols.map @@ -456,6 +456,8 @@ local: *; MIRAL_5.1 { global: extern "C++" { + miral::ConfigFile::?ConfigFile*; + miral::ConfigFile::ConfigFile*; miral::Decorations::Decorations*; miral::Decorations::always_csd*; miral::Decorations::always_ssd*; @@ -468,12 +470,10 @@ global: miral::IdleListener::on_off*; miral::IdleListener::on_wake*; miral::IdleListener::operator*; - miral::ReloadingConfigFile::?ReloadingConfigFile*; - miral::ReloadingConfigFile::ReloadingConfigFile*; miral::WindowManagerTools::move_cursor_to*; + typeinfo?for?miral::ConfigFile; typeinfo?for?miral::Decorations; typeinfo?for?miral::IdleListener; - typeinfo?for?miral::ReloadingConfigFile; }; } MIRAL_5.0; diff --git a/tests/miral/CMakeLists.txt b/tests/miral/CMakeLists.txt index 24db9802976..f01d0bb0e33 100644 --- a/tests/miral/CMakeLists.txt +++ b/tests/miral/CMakeLists.txt @@ -69,7 +69,7 @@ target_link_libraries(miral-test-internal mir_add_wrapped_executable(miral-test NOINSTALL external_client.cpp - reloading_config_file.cpp + config_file.cpp runner.cpp wayland_extensions.cpp zone.cpp diff --git a/tests/miral/reloading_config_file.cpp b/tests/miral/config_file.cpp similarity index 92% rename from tests/miral/reloading_config_file.cpp rename to tests/miral/config_file.cpp index 2ab01b11ee2..02a1b0f741c 100644 --- a/tests/miral/reloading_config_file.cpp +++ b/tests/miral/config_file.cpp @@ -15,7 +15,7 @@ */ #include "miral/test_server.h" -#include "miral/reloading_config_file.h" +#include "miral/config_file.h" #include #include @@ -23,7 +23,7 @@ #include #include -using miral::ReloadingConfigFile; +using miral::ConfigFile; namespace { @@ -71,7 +71,7 @@ struct TestReloadingConfigFile : miral::TestServer, PendingLoad TestReloadingConfigFile(); MOCK_METHOD(void, load, (std::istream& in, std::filesystem::path path), ()); - std::optional reloading_config_file; + std::optional reloading_config_file; void write_a_file() { @@ -123,7 +123,7 @@ TEST_F(TestReloadingConfigFile, with_no_file_nothing_is_loaded) invoke_runner([this](miral::MirRunner& runner) { - reloading_config_file = ReloadingConfigFile{ + reloading_config_file = ConfigFile{ runner, no_such_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; @@ -138,7 +138,7 @@ TEST_F(TestReloadingConfigFile, with_a_file_something_is_loaded) invoke_runner([this](miral::MirRunner& runner) { - reloading_config_file = ReloadingConfigFile{ + reloading_config_file = ConfigFile{ runner, a_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; @@ -150,7 +150,7 @@ TEST_F(TestReloadingConfigFile, when_a_file_is_written_something_is_loaded) { invoke_runner([this](miral::MirRunner& runner) { - reloading_config_file = ReloadingConfigFile{ + reloading_config_file = ConfigFile{ runner, a_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; @@ -172,7 +172,7 @@ TEST_F(TestReloadingConfigFile, each_time_a_file_is_rewritten_something_is_loade invoke_runner([this](miral::MirRunner& runner) { - reloading_config_file = ReloadingConfigFile{ + reloading_config_file = ConfigFile{ runner, a_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; @@ -198,7 +198,7 @@ TEST_F(TestReloadingConfigFile, when_config_home_unset_a_file_in_home_config_is_ invoke_runner([this](miral::MirRunner& runner) { - reloading_config_file = ReloadingConfigFile{ + reloading_config_file = ConfigFile{ runner, config_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; @@ -218,7 +218,7 @@ TEST_F(TestReloadingConfigFile, a_file_in_xdg_config_home_is_loaded) invoke_runner([this](miral::MirRunner& runner) { - reloading_config_file = ReloadingConfigFile{ + reloading_config_file = ConfigFile{ runner, config_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; @@ -238,7 +238,7 @@ TEST_F(TestReloadingConfigFile, a_file_in_xdg_config_home_is_reloaded) invoke_runner([this](miral::MirRunner& runner) { - reloading_config_file = ReloadingConfigFile{ + reloading_config_file = ConfigFile{ runner, config_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; @@ -261,7 +261,7 @@ TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir0_is_loaded) invoke_runner([this](miral::MirRunner& runner) { - reloading_config_file = ReloadingConfigFile{ + reloading_config_file = ConfigFile{ runner, config_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; @@ -284,7 +284,7 @@ TEST_F(TestReloadingConfigFile, after_a_config_in_xdg_conf_dir0_is_loaded_a_new_ invoke_runner([this](miral::MirRunner& runner) { - reloading_config_file = ReloadingConfigFile{ + reloading_config_file = ConfigFile{ runner, config_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; @@ -308,7 +308,7 @@ TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir0_is_loaded_in_preferenc invoke_runner([this](miral::MirRunner& runner) { - reloading_config_file = ReloadingConfigFile{ + reloading_config_file = ConfigFile{ runner, config_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; @@ -328,7 +328,7 @@ TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir1_is_loaded_in_preferenc invoke_runner([this](miral::MirRunner& runner) { - reloading_config_file = ReloadingConfigFile{ + reloading_config_file = ConfigFile{ runner, config_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; @@ -347,7 +347,7 @@ TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir2_is_loaded) invoke_runner([this](miral::MirRunner& runner) { - reloading_config_file = ReloadingConfigFile{ + reloading_config_file = ConfigFile{ runner, config_file, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; From 618d5fc92cce8f91a38195e997aafbc0a6482c47 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Tue, 20 Aug 2024 14:05:50 +0100 Subject: [PATCH 19/21] Add non-reloading option --- include/miral/miral/config_file.h | 21 ++- src/miral/config_file.cpp | 47 +++-- tests/miral/config_file.cpp | 291 ++++++++++++++++++++++++++++-- 3 files changed, 325 insertions(+), 34 deletions(-) diff --git a/include/miral/miral/config_file.h b/include/miral/miral/config_file.h index 409a4b9a1f6..5edf291247c 100644 --- a/include/miral/miral/config_file.h +++ b/include/miral/miral/config_file.h @@ -30,12 +30,14 @@ namespace miral /** * Utility to locate and monitor a configuration file via the XDG Base Directory * Specification. Vis: ($XDG_CONFIG_HOME or $HOME/.config followed by - * $XDG_CONFIG_DIRS). - * The user-specific configuration file base ($XDG_CONFIG_HOME or $HOME/.config) - * is monitored for subsequent changes (but not directories in $XDG_CONFIG_DIRS) + * $XDG_CONFIG_DIRS). If, instead of a filename, a path is given, then the base + * directories are not applied. * - * If, instead of a filename, a path is given, then the base directories are not - * applied and the supplied path monitored for changes. + * If mode is `no_reloading`, then the file is loaded on startup and not reloaded + * + * If mode is `reload_on_change`, then the file is loaded on startup and either + * the user-specific configuration file base ($XDG_CONFIG_HOME or $HOME/.config), + * or the supplied path is monitored for changes. * \remark MirAL 5.1 */ class ConfigFile @@ -44,7 +46,14 @@ class ConfigFile /// Loader functor is passed both the open stream and the actual path (for use in reporting problems) using Loader = std::function; - ConfigFile(MirRunner& runner, std::filesystem::path file, Loader load_config); + /// Mode of reloading + enum class Mode + { + no_reloading, + reload_on_change + }; + + ConfigFile(MirRunner& runner, std::filesystem::path file, Mode mode, Loader load_config); ~ConfigFile(); private: diff --git a/src/miral/config_file.cpp b/src/miral/config_file.cpp index 1ae807de8df..de9bef0d8c2 100644 --- a/src/miral/config_file.cpp +++ b/src/miral/config_file.cpp @@ -65,25 +65,34 @@ auto watch_descriptor(mir::Fd const& inotify_fd, std::optional const& path return inotify_add_watch(inotify_fd, path.value().c_str(), IN_CLOSE_WRITE | IN_CREATE | IN_MOVED_TO); } -} -class miral::ConfigFile::Self +class Watcher { public: - Self(MirRunner& runner, path file, Loader load_config); + using Loader = miral::ConfigFile::Loader; + Watcher(miral::MirRunner& runner, path file, miral::ConfigFile::Loader load_config); -private: mir::Fd const inotify_fd; Loader const load_config; - std::filesystem::path const filename; - std::optional const directory; + path const filename; + std::optional const directory; std::optional const directory_watch_descriptor; - void register_handler(MirRunner& runner); + void register_handler(miral::MirRunner& runner); std::unique_ptr fd_handle; }; +} + +class miral::ConfigFile::Self +{ +public: + Self(MirRunner& runner, path file, Mode mode, Loader load_config); + +private: + std::unique_ptr watcher; +}; -miral::ConfigFile::Self::Self(MirRunner& runner, path file, Loader load_config) : +Watcher::Watcher(miral::MirRunner& runner, path file, miral::ConfigFile::Loader load_config) : inotify_fd{inotify_init1(IN_CLOEXEC)}, load_config{load_config}, filename{file.filename()}, @@ -96,6 +105,12 @@ miral::ConfigFile::Self::Self(MirRunner& runner, path file, Loader load_config) { mir::log_debug("Monitoring %s for configuration changes", (directory.value()/filename).c_str()); } +} + +miral::ConfigFile::Self::Self(MirRunner& runner, path file, Mode mode, Loader load_config) +{ + auto const filename{file.filename()}; + auto const directory{config_directory(file)}; // With C++26 we should be able to use the optional directory as a range to // initialize config_roots. Until then, we'll just do it the long way... @@ -130,16 +145,26 @@ miral::ConfigFile::Self::Self(MirRunner& runner, path file, Loader load_config) break; } } + + switch (mode) + { + case Mode::no_reloading: + break; + + case Mode::reload_on_change: + watcher = std::make_unique(runner, file, std::move(load_config)); + break; + } } -miral::ConfigFile::ConfigFile(MirRunner& runner, path file, Loader load_config) : - self{std::make_shared(runner, file, load_config)} +miral::ConfigFile::ConfigFile(MirRunner& runner, path file, Mode mode, Loader load_config) : + self{std::make_shared(runner, file, mode, load_config)} { } miral::ConfigFile::~ConfigFile() = default; -void miral::ConfigFile::Self::register_handler(MirRunner& runner) +void Watcher::register_handler(miral::MirRunner& runner) { if (directory_watch_descriptor) { diff --git a/tests/miral/config_file.cpp b/tests/miral/config_file.cpp index 02a1b0f741c..f56bd6ad474 100644 --- a/tests/miral/config_file.cpp +++ b/tests/miral/config_file.cpp @@ -44,9 +44,9 @@ class PendingLoad { std::unique_lock lock{mutex}; - if (!cv.wait_for(lock, std::chrono::milliseconds{1000}, [this] { return !pending_loads; })) + if (!cv.wait_for(lock, std::chrono::milliseconds{10}, [this] { return !pending_loads; })) { - std::cerr << "wait_for_load() failed" << std::endl; + std::cerr << "wait_for_load() timed out" << std::endl; } } @@ -66,9 +66,9 @@ class PendingLoad bool pending_loads = false; }; -struct TestReloadingConfigFile : miral::TestServer, PendingLoad +struct TestConfigFile : miral::TestServer, PendingLoad { - TestReloadingConfigFile(); + TestConfigFile(); MOCK_METHOD(void, load, (std::istream& in, std::filesystem::path path), ()); std::optional reloading_config_file; @@ -101,7 +101,7 @@ char const* const xdg_conf_dir0 = "/tmp/test_reloading_config_file/xdg_conf_dir_ char const* const xdg_conf_dir1 = "/tmp/test_reloading_config_file/xdg_conf_dir_one"; char const* const xdg_conf_dir2 = "/tmp/test_reloading_config_file/xdg_conf_dir_two"; -TestReloadingConfigFile::TestReloadingConfigFile() +TestConfigFile::TestConfigFile() { std::filesystem::remove_all("/tmp/test_reloading_config_file/"); @@ -117,7 +117,7 @@ TestReloadingConfigFile::TestReloadingConfigFile() } -TEST_F(TestReloadingConfigFile, with_no_file_nothing_is_loaded) +TEST_F(TestConfigFile, with_reload_on_change_and_no_file_nothing_is_loaded) { EXPECT_CALL(*this, load).Times(0); @@ -126,11 +126,12 @@ TEST_F(TestReloadingConfigFile, with_no_file_nothing_is_loaded) reloading_config_file = ConfigFile{ runner, no_such_file, + ConfigFile::Mode::reload_on_change, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); } -TEST_F(TestReloadingConfigFile, with_a_file_something_is_loaded) +TEST_F(TestConfigFile, with_reload_on_change_and_a_file_something_is_loaded) { EXPECT_CALL(*this, load).Times(1); @@ -141,18 +142,20 @@ TEST_F(TestReloadingConfigFile, with_a_file_something_is_loaded) reloading_config_file = ConfigFile{ runner, a_file, + ConfigFile::Mode::reload_on_change, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); wait_for_load(); } -TEST_F(TestReloadingConfigFile, when_a_file_is_written_something_is_loaded) +TEST_F(TestConfigFile, with_reload_on_change_when_a_file_is_written_something_is_loaded) { invoke_runner([this](miral::MirRunner& runner) { reloading_config_file = ConfigFile{ runner, a_file, + ConfigFile::Mode::reload_on_change, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); @@ -162,7 +165,7 @@ TEST_F(TestReloadingConfigFile, when_a_file_is_written_something_is_loaded) wait_for_load(); } -TEST_F(TestReloadingConfigFile, each_time_a_file_is_rewritten_something_is_loaded) +TEST_F(TestConfigFile, with_reload_on_change_each_time_a_file_is_rewritten_something_is_loaded) { auto const times = 42; @@ -175,6 +178,7 @@ TEST_F(TestReloadingConfigFile, each_time_a_file_is_rewritten_something_is_loade reloading_config_file = ConfigFile{ runner, a_file, + ConfigFile::Mode::reload_on_change, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); @@ -187,7 +191,7 @@ TEST_F(TestReloadingConfigFile, each_time_a_file_is_rewritten_something_is_loade wait_for_load(); } -TEST_F(TestReloadingConfigFile, when_config_home_unset_a_file_in_home_config_is_loaded) +TEST_F(TestConfigFile, with_reload_on_change_when_config_home_unset_a_file_in_home_config_is_loaded) { add_to_environment("XDG_CONFIG_HOME", nullptr); using testing::_; @@ -201,13 +205,14 @@ TEST_F(TestReloadingConfigFile, when_config_home_unset_a_file_in_home_config_is_ reloading_config_file = ConfigFile{ runner, config_file, + ConfigFile::Mode::reload_on_change, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); wait_for_load(); } -TEST_F(TestReloadingConfigFile, a_file_in_xdg_config_home_is_loaded) +TEST_F(TestConfigFile, with_reload_on_change_a_file_in_xdg_config_home_is_loaded) { using testing::_; EXPECT_CALL(*this, load(_, xdg_conf_home/config_file)).Times(1); @@ -221,13 +226,14 @@ TEST_F(TestReloadingConfigFile, a_file_in_xdg_config_home_is_loaded) reloading_config_file = ConfigFile{ runner, config_file, + ConfigFile::Mode::reload_on_change, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); wait_for_load(); } -TEST_F(TestReloadingConfigFile, a_file_in_xdg_config_home_is_reloaded) +TEST_F(TestConfigFile, with_reload_on_change_a_file_in_xdg_config_home_is_reloaded) { using testing::_; EXPECT_CALL(*this, load(_, xdg_conf_home/config_file)).Times(2); @@ -241,6 +247,7 @@ TEST_F(TestReloadingConfigFile, a_file_in_xdg_config_home_is_reloaded) reloading_config_file = ConfigFile{ runner, config_file, + ConfigFile::Mode::reload_on_change, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); @@ -252,7 +259,7 @@ TEST_F(TestReloadingConfigFile, a_file_in_xdg_config_home_is_reloaded) wait_for_load(); } -TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir0_is_loaded) +TEST_F(TestConfigFile, with_reload_on_change_a_config_in_xdg_conf_dir0_is_loaded) { using testing::_; EXPECT_CALL(*this, load(_, xdg_conf_dir0/config_file)).Times(1); @@ -264,13 +271,14 @@ TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir0_is_loaded) reloading_config_file = ConfigFile{ runner, config_file, + ConfigFile::Mode::reload_on_change, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); wait_for_load(); } -TEST_F(TestReloadingConfigFile, after_a_config_in_xdg_conf_dir0_is_loaded_a_new_config_in_xdg_conf_home_is_loaded) +TEST_F(TestConfigFile, with_reload_on_change_after_a_config_in_xdg_conf_dir0_is_loaded_a_new_config_in_xdg_conf_home_is_loaded) { using testing::_; @@ -287,6 +295,7 @@ TEST_F(TestReloadingConfigFile, after_a_config_in_xdg_conf_dir0_is_loaded_a_new_ reloading_config_file = ConfigFile{ runner, config_file, + ConfigFile::Mode::reload_on_change, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); @@ -296,7 +305,7 @@ TEST_F(TestReloadingConfigFile, after_a_config_in_xdg_conf_dir0_is_loaded_a_new_ wait_for_load(); } -TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir0_is_loaded_in_preference_to_dir1_or_2) +TEST_F(TestConfigFile, with_reload_on_change_a_config_in_xdg_conf_dir0_is_loaded_in_preference_to_dir1_or_2) { using testing::_; @@ -311,13 +320,14 @@ TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir0_is_loaded_in_preferenc reloading_config_file = ConfigFile{ runner, config_file, + ConfigFile::Mode::reload_on_change, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); wait_for_load(); } -TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir1_is_loaded_in_preference_to_dir2) +TEST_F(TestConfigFile, with_reload_on_change_a_config_in_xdg_conf_dir1_is_loaded_in_preference_to_dir2) { using testing::_; @@ -331,13 +341,14 @@ TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir1_is_loaded_in_preferenc reloading_config_file = ConfigFile{ runner, config_file, + ConfigFile::Mode::reload_on_change, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); wait_for_load(); } -TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir2_is_loaded) +TEST_F(TestConfigFile, with_reload_on_change_a_config_in_xdg_conf_dir2_is_loaded) { using testing::_; @@ -350,6 +361,252 @@ TEST_F(TestReloadingConfigFile, a_config_in_xdg_conf_dir2_is_loaded) reloading_config_file = ConfigFile{ runner, config_file, + ConfigFile::Mode::reload_on_change, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); +} + +TEST_F(TestConfigFile, with_no_reloading_and_no_file_nothing_is_loaded) +{ + EXPECT_CALL(*this, load).Times(0); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ConfigFile{ + runner, + no_such_file, + ConfigFile::Mode::no_reloading, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); +} + +TEST_F(TestConfigFile, with_no_reloading_and_a_file_something_is_loaded) +{ + EXPECT_CALL(*this, load).Times(1); + + write_a_file(); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ConfigFile{ + runner, + a_file, + ConfigFile::Mode::no_reloading, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + wait_for_load(); +} + +TEST_F(TestConfigFile, with_no_reloading_when_a_file_is_written_nothing_is_loaded) +{ + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ConfigFile{ + runner, + a_file, + ConfigFile::Mode::no_reloading, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + EXPECT_CALL(*this, load).Times(0); + + write_a_file(); + wait_for_load(); +} + +TEST_F(TestConfigFile, with_no_reloading_when_a_file_is_rewritten_nothing_is_reloaded) +{ + EXPECT_CALL(*this, load).Times(1); + + write_a_file(); // Initial write + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ConfigFile{ + runner, + a_file, + ConfigFile::Mode::no_reloading, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); + write_a_file(); + wait_for_load(); +} + +TEST_F(TestConfigFile, with_no_reloading_when_config_home_unset_a_file_in_home_config_is_loaded) +{ + add_to_environment("XDG_CONFIG_HOME", nullptr); + using testing::_; + EXPECT_CALL(*this, load(_, home_config/config_file)).Times(1); + + write_config_in(xdg_conf_home); + write_config_in(home_config); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ConfigFile{ + runner, + config_file, + ConfigFile::Mode::no_reloading, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); +} + +TEST_F(TestConfigFile, with_no_reloading_a_file_in_xdg_config_home_is_loaded) +{ + using testing::_; + EXPECT_CALL(*this, load(_, xdg_conf_home/config_file)).Times(1); + + write_config_in(xdg_conf_dir0); + write_config_in(xdg_conf_home); + write_config_in(home_config); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ConfigFile{ + runner, + config_file, + ConfigFile::Mode::no_reloading, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); +} + +TEST_F(TestConfigFile, with_no_reloading_a_file_in_xdg_config_home_is_not_reloaded) +{ + using testing::_; + EXPECT_CALL(*this, load(_, xdg_conf_home/config_file)).Times(1); + + write_config_in(xdg_conf_dir0); + write_config_in(xdg_conf_home); + write_config_in(home_config); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ConfigFile{ + runner, + config_file, + ConfigFile::Mode::no_reloading, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); + + write_config_in(xdg_conf_dir0); + write_config_in(xdg_conf_home); + write_config_in(home_config); + wait_for_load(); +} + +TEST_F(TestConfigFile, with_no_reloading_a_config_in_xdg_conf_dir0_is_loaded) +{ + using testing::_; + EXPECT_CALL(*this, load(_, xdg_conf_dir0/config_file)).Times(1); + + write_config_in(xdg_conf_dir0); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ConfigFile{ + runner, + config_file, + ConfigFile::Mode::no_reloading, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); +} + +TEST_F(TestConfigFile, with_no_reloading_after_a_config_in_xdg_conf_dir0_is_loaded_a_new_config_in_xdg_conf_home_is_not_loaded) +{ + using testing::_; + + testing::InSequence sequence; + EXPECT_CALL(*this, load(_, xdg_conf_dir0/config_file)).Times(1); + EXPECT_CALL(*this, load(_, xdg_conf_home/config_file)).Times(0); + + write_config_in(xdg_conf_dir2); + write_config_in(xdg_conf_dir1); + write_config_in(xdg_conf_dir0); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ConfigFile{ + runner, + config_file, + ConfigFile::Mode::no_reloading, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); + + write_config_in(xdg_conf_home); + wait_for_load(); +} + +TEST_F(TestConfigFile, with_no_reloading_a_config_in_xdg_conf_dir0_is_loaded_in_preference_to_dir1_or_2) +{ + using testing::_; + + EXPECT_CALL(*this, load(_, xdg_conf_dir0/config_file)).Times(1); + + write_config_in(xdg_conf_dir2); + write_config_in(xdg_conf_dir1); + write_config_in(xdg_conf_dir0); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ConfigFile{ + runner, + config_file, + ConfigFile::Mode::no_reloading, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); +} + +TEST_F(TestConfigFile, with_no_reloading_a_config_in_xdg_conf_dir1_is_loaded_in_preference_to_dir2) +{ + using testing::_; + + EXPECT_CALL(*this, load(_, xdg_conf_dir1/config_file)).Times(1); + + write_config_in(xdg_conf_dir2); + write_config_in(xdg_conf_dir1); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ConfigFile{ + runner, + config_file, + ConfigFile::Mode::no_reloading, + [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; + }); + + wait_for_load(); +} + +TEST_F(TestConfigFile, with_no_reloading_a_config_in_xdg_conf_dir2_is_loaded) +{ + using testing::_; + + EXPECT_CALL(*this, load(_, xdg_conf_dir2/config_file)).Times(1); + + write_config_in(xdg_conf_dir2); + + invoke_runner([this](miral::MirRunner& runner) + { + reloading_config_file = ConfigFile{ + runner, + config_file, + ConfigFile::Mode::no_reloading, [this](std::istream& in, std::filesystem::path path) { load(in, path); }}; }); From 1cc1e6ad847fbf284e8500d63a2e6b72699ae6e8 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Tue, 20 Aug 2024 14:42:50 +0100 Subject: [PATCH 20/21] Fix whitespace --- tests/miral/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/miral/CMakeLists.txt b/tests/miral/CMakeLists.txt index f01d0bb0e33..86865a67d0f 100644 --- a/tests/miral/CMakeLists.txt +++ b/tests/miral/CMakeLists.txt @@ -69,7 +69,7 @@ target_link_libraries(miral-test-internal mir_add_wrapped_executable(miral-test NOINSTALL external_client.cpp - config_file.cpp + config_file.cpp runner.cpp wayland_extensions.cpp zone.cpp From 4dfe34d0cc238163eed8fd4b516fe47ab8e54028 Mon Sep 17 00:00:00 2001 From: Alan Griffiths Date: Tue, 20 Aug 2024 16:52:26 +0100 Subject: [PATCH 21/21] Fix symbols --- debian/libmiral7.symbols | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/debian/libmiral7.symbols b/debian/libmiral7.symbols index 87cb25f47e9..46560b2caba 100644 --- a/debian/libmiral7.symbols +++ b/debian/libmiral7.symbols @@ -434,7 +434,7 @@ libmiral.so.7 libmiral7 #MINVER# (c++)"vtable for miral::MinimalWindowManager@MIRAL_5.0" 5.0.0 (c++)"vtable for miral::WindowManagementPolicy@MIRAL_5.0" 5.0.0 MIRAL_5.1@MIRAL_5.1 5.1.0 - (c++)"miral::ConfigFile::ConfigFile(miral::MirRunner&, std::filesystem::__cxx11::path, std::function >&, std::filesystem::__cxx11::path const&)>)@MIRAL_5.1" 5.1.0 + (c++)"miral::ConfigFile::ConfigFile(miral::MirRunner&, std::filesystem::__cxx11::path, miral::ConfigFile::Mode, std::function >&, std::filesystem::__cxx11::path const&)>)@MIRAL_5.1" 5.1.0 (c++)"miral::ConfigFile::~ConfigFile()@MIRAL_5.1" 5.1.0 (c++)"miral::Decorations::Decorations(std::shared_ptr)@MIRAL_5.1" 5.1.0 (c++)"miral::Decorations::always_csd()@MIRAL_5.1" 5.1.0