From ea20413107744161f716e181c5a9d3a7c676455f Mon Sep 17 00:00:00 2001 From: Aaron Fiore Date: Wed, 26 Nov 2025 17:34:25 -0800 Subject: [PATCH 1/3] container: root: common: utility: refactor/deprecate, add/enhance free functions - Factors out (and deprecates) `Command` - Deprecates certain free functions - Adds better error handling - Adds more free functions - Refactors file handling - Improves documentation Notes on file loading (and general file handlers): The interpreter command `.L` will load files but appears to not fail if the file isn't source code or a library. Calling `gSystem->Load()` will load a given library but not source file, and appears to not accept paths (which will be needed for pluggables). Because `gInterpreter->LoadFile("path")` will load either a source file or library by given "path", and is consistent with `dfi`'s approach to path handling, this function appears to be the winner (for now). --- container/src/root/common/utility.hh | 178 +++++++++++++++++++++------ 1 file changed, 140 insertions(+), 38 deletions(-) diff --git a/container/src/root/common/utility.hh b/container/src/root/common/utility.hh index f3a385c..1d50aa4 100644 --- a/container/src/root/common/utility.hh +++ b/container/src/root/common/utility.hh @@ -27,6 +27,7 @@ #include #include #include +#include #include #include @@ -86,8 +87,48 @@ inline void throw_ex_if( } } -//! \brief Wrapper to ROOT Cling commands +// TODO(afiore): add logger with loglevels + +//! \brief Process a line of code or interpreter command +//! \since docker-finance 1.1.0 +inline void line(const std::string& line) +{ + using EErrorCode = ::TInterpreter::EErrorCode; + auto ecode = std::make_unique(); + + // TODO(afiore): log levels + std::cout << "Interpreting: '" << line << "'" << std::endl; + gInterpreter->ProcessLine(line.c_str(), ecode.get()); + throw_ex_if(*ecode != EErrorCode::kNoError, line); +} + +//! \brief Load file with given path into interpreter +//! \details Can load a source file or library file +//! \param path Operating system path (absolute or relative) +//! \since docker-finance 1.1.0 +inline void load(const std::string& path) +{ + throw_ex_if( + !std::filesystem::is_regular_file(path), "invalid file: '" + path + "'"); + + // TODO(afiore): log levels + std::cout << "Loading: '" << path << "'" << std::endl; + gInterpreter->LoadFile(path.c_str()); +} + +//! \brief Load files with given paths into interpreter +//! \details Can load source files and/or library files +//! \param path Operating system paths (absolute or relative) +//! \since docker-finance 1.1.0 +inline void load(const std::initializer_list& paths) +{ + for (const auto& path : paths) + load(path); +} + +//! \brief Wrappers to commonly used interpreter abstractions //! \ingroup cpp_common_impl +//! \deprecated This will be removed in the v2 API; use the `dfi::common` free functions instead //! \since docker-finance 1.0.0 class Command { @@ -101,45 +142,106 @@ class Command Command(Command&&) = default; Command& operator=(Command&&) = default; - private: - static void cmd_handler(const std::initializer_list& command) - { - for (const auto& cmd : command) - { - std::cout << "Interpreting: '" << cmd << "'" << std::endl; - gInterpreter->ProcessLine(cmd.c_str()); - } - } - public: - //! \brief Load given file path - static void load(const std::string& path) + //! \deprecated This will be removed in the v2 API; use the `dfi::common` free function instead + //! \since docker-finance 1.0.0 + static void load(const std::string& path) { ::dfi::common::load(path); } + + //! \deprecated This will be removed in the v2 API; use the `dfi::common` free function instead + //! \since docker-finance 1.0.0 + static void load(const std::initializer_list& paths) { - std::filesystem::path p(path); - - throw_ex_if( - !std::filesystem::exists(p), "'" + path + "' does not exist!"); - - throw_ex_if( - !std::filesystem::is_regular_file(p), - "'" + path + "' is not a regular file!"); - - std::string cmd{".L " + path}; - Command::cmd_handler({cmd}); + ::dfi::common::load(paths); } - - //! \brief Load given file paths - static void load(const std::initializer_list& commands) - { - for (const auto& cmd : commands) - Command::load(cmd); - } - - // TODO(afiore): unload }; -//! \brief Get ROOT environment variable -//! \param var ROOT variable +//! \brief Unload file with given path out of interpreter +//! \details Can unload a source file or library file +//! \param path Operating system path (absolute or relative) +//! \since docker-finance 1.1.0 +inline void unload(const std::string& path) +{ + throw_ex_if( + !std::filesystem::is_regular_file(path), "invalid file: '" + path + "'"); + + // TODO(afiore): log levels + std::cout << "Unloading: '" << path << "'" << std::endl; + gInterpreter->UnloadFile(path.c_str()); +} + +//! \brief Unload files with given paths out of interpreter +//! \details Can unload source files and/or library files +//! \param path Operating system paths (absolute or relative) +//! \since docker-finance 1.1.0 +inline void unload(const std::initializer_list& paths) +{ + for (const auto& path : paths) + unload(path); +} + +//! \brief Reload file with given path out of/into interpreter +//! \details Can reload source files and/or library files +//! \param path Operating system paths (absolute or relative) +//! \since docker-finance 1.1.0 +inline void reload(const std::string& path) +{ + unload(path); + load(path); +} + +//! \brief Reload files with given paths out of/into interpreter +//! \details Can reload source files and/or library files +//! \param path Operating system paths (absolute or relative) +//! \since docker-finance 1.1.0 +inline void reload(const std::initializer_list& paths) +{ + unload(paths); + load(paths); +} + +//! \brief Add an include directory to interpreter +//! \param path Path to include (absolute or relative) +//! \warning Only pass the complete path to directory and not "-I" +//! \warning To pass multiple paths, call this function multiple times +//! \exception type::RuntimeError if path doesn't exist or cannot be included +//! \since docker-finance 1.1.0 +inline void add_include_dir(const std::string& path) +{ + throw_ex_if( + !std::filesystem::is_directory(path), + "invalid directory: '" + path + "'"); + + // TODO(afiore): log levels + std::cout << "Adding: '" << path << "'" << std::endl; + gInterpreter->AddIncludePath(path.c_str()); +} + +//! \brief Add a linked library into interpreter +//! \param path Path of library to add (absolute or relative) +//! \warning Only pass the complete path to library and not "-l" +//! \warning To pass multiple libraries, call this function multiple times +//! \exception type::RuntimeError if library doesn't exist or cannot be linked +//! \since docker-finance 1.1.0 +inline void add_linked_lib(const std::string& path) +{ + load(path); +} + +//! \brief Remove a linked library from interpreter +//! \param path Path of library to remove (absolute or relative) +//! \warning Only pass the complete path to library and not "-l" +//! \warning To pass multiple libraries, call this function multiple times +//! \exception type::RuntimeError if library doesn't exist or cannot be linked +//! \since docker-finance 1.1.0 +inline void remove_linked_lib(const std::string& path) +{ + unload(path); +} + +//! \brief Get underlying environment variable +//! \param var Environment variable +//! \return Value of given environment variable +//! \exception type::RuntimeError Throws if env var is not set or unavailable //! \note ROOT environment variables include shell (and shell caller) environment //! \since docker-finance 1.0.0 std::string get_env(const std::string& var) @@ -152,9 +254,9 @@ std::string get_env(const std::string& var) return std::string{env}; } -//! \brief Execute command in shell -//! \param cmd Shell command [args] -//! \returns Return value of command +//! \brief Execute a command in operating system shell +//! \param cmd Operating system shell command [args] +//! \return Return value of command //! \since docker-finance 1.0.0 int exec(const std::string& cmd) { From d156f62fa79b5467d1b393c2a5ee8a87cf6ae96f Mon Sep 17 00:00:00 2001 From: Aaron Fiore Date: Tue, 2 Dec 2025 15:11:04 -0800 Subject: [PATCH 2/3] container: root: common: utility: refactor/add tests --- container/src/root/test/common/utility.hh | 3 - container/src/root/test/unit/utility.hh | 206 ++++++++++++++-------- 2 files changed, 133 insertions(+), 76 deletions(-) diff --git a/container/src/root/test/common/utility.hh b/container/src/root/test/common/utility.hh index e845446..6e02a43 100644 --- a/container/src/root/test/common/utility.hh +++ b/container/src/root/test/common/utility.hh @@ -102,9 +102,6 @@ struct ByteFixture struct CommonFixture { protected: - using EErrorCode = ::TInterpreter::EErrorCode; - std::unique_ptr ecode; - CommonFixture() { ecode = std::make_unique(); } }; } // namespace tests } // namespace dfi diff --git a/container/src/root/test/unit/utility.hh b/container/src/root/test/unit/utility.hh index 3767a51..91e9b17 100644 --- a/container/src/root/test/unit/utility.hh +++ b/container/src/root/test/unit/utility.hh @@ -282,11 +282,64 @@ TEST_F(ByteTransform, unordered_set) ASSERT_EQ(byte.decode(two), one); } +//! \brief Common fixture (free functions) +//! \note Not a 'common' fixture but rather a fixture for 'common' +//! \since docker-finance 1.1.0 +struct CommonFree : public ::testing::Test, public ::dfi::tests::CommonFixture +{ +}; + +TEST_F(CommonFree, Line) +{ + ASSERT_NO_THROW(common::line("")); + ASSERT_THROW(common::line("a bar without a foo"), type::RuntimeError); + ASSERT_NO_THROW(common::line("#define FOO 1")); +} + +TEST_F(CommonFree, add_include_dir) +{ + ASSERT_THROW( + common::add_include_dir("/should-not-exist"), type::RuntimeError); + ASSERT_NO_THROW(common::add_include_dir("/usr/include")); +} + +TEST_F(CommonFree, add_linked_lib) +{ + ASSERT_THROW( + common::add_linked_lib("/should-not-exist.so"), type::RuntimeError); + ASSERT_NO_THROW(common::add_linked_lib("/usr/lib/LLVMgold.so")); +} + +TEST_F(CommonFree, remove_linked_lib) +{ + ASSERT_THROW( + common::remove_linked_lib("/should-not-exist.so"), type::RuntimeError); + ASSERT_NO_THROW(common::add_linked_lib("/usr/lib/LLVMgold.so")); + ASSERT_NO_THROW(common::remove_linked_lib("/usr/lib/LLVMgold.so")); +} + +TEST_F(CommonFree, get_env) +{ + ASSERT_THROW(common::get_env("should-not-exist"), type::RuntimeError); + ASSERT_NO_THROW(common::get_env("DOCKER_FINANCE_VERSION")); +} + +TEST_F(CommonFree, exec) +{ + ASSERT_NE(common::exec("should-not-exist"), 0); + ASSERT_EQ(common::exec("pwd"), 0); +} + +TEST_F(CommonFree, make_timestamp) +{ + ASSERT_EQ(common::make_timestamp().size(), 20); +} + //! \brief Common fixture (raw files) //! \note Not a 'common' fixture but rather a fixture for 'common' //! \since docker-finance 1.1.0 -struct CommonRawFiles : public ::testing::Test, - public ::dfi::tests::CommonFixture +struct CommonFreeFiles : public ::testing::Test, + public ::dfi::tests::CommonFixture { std::string path_1, path_2; @@ -325,107 +378,114 @@ struct CommonRawFiles : public ::testing::Test, } }; -TEST_F(CommonRawFiles, RawLoadSingle) +TEST_F(CommonFreeFiles, LoadSingle) { - ASSERT_THROW( - common::Command::load({path_1 + "should-not-exist"}), type::RuntimeError); - gInterpreter->ProcessLine("my_foo foo;", ecode.get()); - ASSERT_NE(ecode, nullptr); - EXPECT_NE(*ecode, EErrorCode::kNoError); + ASSERT_THROW(common::load({path_1 + "should-not-exist"}), type::RuntimeError); + ASSERT_THROW(common::line("my_foo foo;"), type::RuntimeError); - ASSERT_NO_THROW(common::Command::load(path_1)); - gInterpreter->ProcessLine("my_foo foo;", ecode.get()); - ASSERT_NE(ecode, nullptr); - EXPECT_EQ(*ecode, EErrorCode::kNoError); + ASSERT_NO_THROW(common::load(path_1)); + ASSERT_NO_THROW(common::line("my_foo foo;")); } -TEST_F(CommonRawFiles, RawLoadMultiple) +TEST_F(CommonFreeFiles, LoadMultiple) { ASSERT_THROW( - common::Command::load( + common::load( {{path_1 + "should-not-exist"}, {path_2 + "nor-should-this"}}), type::RuntimeError); - gInterpreter->ProcessLine("my_bar bar;", ecode.get()); - ASSERT_NE(ecode, nullptr); - EXPECT_NE(*ecode, EErrorCode::kNoError); + ASSERT_THROW(common::line("my_bar bar;"), type::RuntimeError); - ASSERT_NO_THROW(common::Command::load({path_1, path_2})); - gInterpreter->ProcessLine("my_bar bar;", ecode.get()); - ASSERT_NE(ecode, nullptr); - EXPECT_EQ(*ecode, EErrorCode::kNoError); + ASSERT_NO_THROW(common::load({path_1, path_2})); + ASSERT_NO_THROW(common::line("my_bar bar;")); } -// TODO(afiore): RawUnload -//! \brief Common fixture (repo files) -//! \note Not a 'common' fixture but rather a fixture for 'common' +TEST_F(CommonFreeFiles, UnloadSingle) +{ + ASSERT_NO_THROW(common::load(path_1)); + ASSERT_NO_THROW(common::line("my_foo foo;")); + + ASSERT_NO_THROW(common::unload(path_1)); + ASSERT_THROW(common::line("my_foo foo;"), type::RuntimeError); +} + +TEST_F(CommonFreeFiles, UnloadMultiple) +{ + ASSERT_NO_THROW(common::load({path_1, path_2})); + ASSERT_NO_THROW(common::line("my_foo foo;")); + ASSERT_NO_THROW(common::line("my_bar bar;")); + + ASSERT_NO_THROW(common::unload({path_1, path_2})); + ASSERT_THROW(common::line("my_foo foo;"), type::RuntimeError); + ASSERT_THROW(common::line("my_bar bar;"), type::RuntimeError); +} + +TEST_F(CommonFreeFiles, ReloadSingle) +{ + ASSERT_NO_THROW(common::load(path_1)); + ASSERT_NO_THROW(common::line("my_foo foo;")); + + ASSERT_NO_THROW(common::reload(path_1)); + ASSERT_NO_THROW(common::line("my_foo foo;")); +} + +TEST_F(CommonFreeFiles, ReloadMultiple) +{ + ASSERT_NO_THROW(common::load({path_1, path_2})); + ASSERT_NO_THROW(common::line("my_foo foo;")); + ASSERT_NO_THROW(common::line("my_bar bar;")); + + ASSERT_NO_THROW(common::reload({path_1, path_2})); + ASSERT_NO_THROW(common::line("my_foo foo;")); + ASSERT_NO_THROW(common::line("my_bar bar;")); +} + +//! \brief MacroFreeFiles fixture //! \since docker-finance 1.1.0 -struct CommonRepoFiles : public ::testing::Test, - public ::dfi::tests::CommonFixture +//! \todo Move to Macro tests +struct MacroFreeFiles : public ::testing::Test, + public ::dfi::tests::CommonFixture { }; -TEST_F(CommonRepoFiles, MacroLoad) +TEST_F(MacroFreeFiles, LoadSingle) { ASSERT_THROW( ::dfi::macro::load("macro/should-not/exist.C"), type::RuntimeError); - gInterpreter->ProcessLine( - "dfi::macro::common::crypto::botan::Hash h;", ecode.get()); - ASSERT_NE(ecode, nullptr); - EXPECT_NE(*ecode, EErrorCode::kNoError); + ASSERT_THROW( + common::line("dfi::macro::common::crypto::botan::Hash h;"), + type::RuntimeError); // TODO(afiore): macro loading should not need to be prepended with "macro/" // (see TODO in common impl regarding plugins-like functionality) ASSERT_NO_THROW(::dfi::macro::load("macro/crypto/hash.C")); - gInterpreter->ProcessLine( - "dfi::macro::common::crypto::botan::Hash h;", ecode.get()); - ASSERT_NE(ecode, nullptr); - EXPECT_EQ(*ecode, EErrorCode::kNoError); - // TODO(afiore): multiple load + ASSERT_NO_THROW(common::line("dfi::macro::common::crypto::botan::Hash h;")); } -// TODO(afiore): MacroUnload +// TODO(afiore): multiple load +// TODO(afiore): unload, reload -TEST_F(CommonRepoFiles, PluginLoad) +//! \brief PluginFreeFiles fixture +//! \since docker-finance 1.1.0 +//! \todo Move to Plugin tests +struct PluginFreeFiles : public ::testing::Test, + public ::dfi::tests::CommonFixture +{ + // NOTE: custom plugin bind-mount is read-only +}; + +TEST_F(PluginFreeFiles, LoadSingle) { ASSERT_THROW( ::dfi::plugin::load("repo/should-not/exist.cc"), type::RuntimeError); - gInterpreter->ProcessLine( - "dfi::plugin::my_plugin_namespace::example2();", ecode.get()); - ASSERT_NE(ecode, nullptr); - EXPECT_NE(*ecode, EErrorCode::kNoError); + ASSERT_THROW( + common::line("dfi::plugin::my_plugin_namespace::example2();"), + type::RuntimeError); ASSERT_NO_THROW(::dfi::plugin::load("repo/example.cc")); - gInterpreter->ProcessLine( - "dfi::plugin::my_plugin_namespace::example2();", ecode.get()); - ASSERT_NE(ecode, nullptr); - EXPECT_EQ(*ecode, EErrorCode::kNoError); - - // TODO(afiore): multiple load -} -// TODO(afiore): PluginUnload - -//! \brief Common fixture (free functions) -//! \note Not a 'common' fixture but rather a fixture for 'common' -//! \since docker-finance 1.1.0 -struct CommonFree : public ::testing::Test, public ::dfi::tests::CommonFixture -{ -}; - -TEST_F(CommonFree, env) -{ - ASSERT_THROW(common::get_env("should-not-exist"), type::RuntimeError); - ASSERT_NO_THROW(common::get_env("DOCKER_FINANCE_VERSION")); -} - -TEST_F(CommonFree, exec) -{ - ASSERT_NE(common::exec("should-not-exist"), 0); - ASSERT_EQ(common::exec("pwd"), 0); -} - -TEST_F(CommonFree, make_timestamp) -{ - ASSERT_EQ(common::make_timestamp().size(), 20); + ASSERT_NO_THROW( + common::line("dfi::plugin::my_plugin_namespace::example2();")); } +// TODO(afiore): multiple load +// TODO(afiore): unload, reload } // namespace unit } // namespace tests From 7ca01776fce4e0d150490d3a3ba9a3ccd58ed767 Mon Sep 17 00:00:00 2001 From: Aaron Fiore Date: Tue, 2 Dec 2025 17:14:10 -0800 Subject: [PATCH 3/3] container: root: refactor using common utility --- container/plugins/root/example.cc | 2 +- container/src/root/macro/rootlogon.C | 30 +++++++++++++++++----------- 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/container/plugins/root/example.cc b/container/plugins/root/example.cc index 6d6be48..63c3fd4 100644 --- a/container/plugins/root/example.cc +++ b/container/plugins/root/example.cc @@ -59,7 +59,7 @@ void example1() //! \ingroup cpp_plugin_impl void example2() { - namespace common = ::dfi::macro::common; + namespace common = ::dfi::common; auto print_env = [](const std::string& env) { std::cout << env << "=" << common::get_env(env) << "\n"; diff --git a/container/src/root/macro/rootlogon.C b/container/src/root/macro/rootlogon.C index 321db19..2e186cb 100644 --- a/container/src/root/macro/rootlogon.C +++ b/container/src/root/macro/rootlogon.C @@ -26,6 +26,8 @@ #include #include +#include "./common/utility.hh" // The one-and-only `dfi` header required by rootlogon + //! \namespace dfi //! \since docker-finance 1.0.0 namespace dfi @@ -192,24 +194,28 @@ void help() //! \since docker-finance 1.0.0 void rootlogon() { + namespace common = dfi::common; + // Add nested directory headers - gInterpreter->AddIncludePath("/usr/include/botan-3"); + common::add_include_dir("/usr/include/botan-3"); // Link default packaged libraries - gSystem->AddLinkedLibs("-lgtest"); // gtest/gmock - gSystem->AddLinkedLibs("-lbenchmark"); // gbenchmark - gSystem->AddLinkedLibs("-pthread"); // gtest/gmock/gbenchmark + // TODO(afiore): linking here appears to no longer be needed? + // Is ROOT loading all libraries on-the-fly (in default paths)? + common::add_linked_lib("/usr/lib/libgtest.so"); // gtest/gmock + common::add_linked_lib("/usr/lib/libbenchmark.so"); // gbenchmark + common::add_linked_lib("/usr/lib/libpthread.so.0"); // gtest/gmock/gbenchmark - gSystem->AddLinkedLibs("-lbotan-3"); // Botan - gSystem->AddLinkedLibs("-lcryptopp"); // Crypto++ - gSystem->AddLinkedLibs("-lsodium"); // libsodium + common::add_linked_lib("/usr/lib/libbotan-3.so"); // Botan + common::add_linked_lib("/usr/lib/libcryptopp.so"); // Crypto++ + common::add_linked_lib("/usr/lib/libsodium.so"); // libsodium // Load default `dfi` public consumables - gInterpreter->ProcessLine(".L plugin/common/utility.hh"); - gInterpreter->ProcessLine(".L macro/common/utility.hh"); - gInterpreter->ProcessLine(".L src/hash.hh"); - gInterpreter->ProcessLine(".L src/random.hh"); - gInterpreter->ProcessLine(".L src/utility.hh"); + common::load("plugin/common/utility.hh"); + common::load("macro/common/utility.hh"); + common::load("src/hash.hh"); + common::load("src/random.hh"); + common::load("src/utility.hh"); } #endif // CONTAINER_SRC_ROOT_MACRO_ROOTLOGON_C_