From f8c0bc7bc345395d6c47ec8a88bb145d50813978 Mon Sep 17 00:00:00 2001 From: bcumming Date: Fri, 4 Oct 2024 09:15:57 +0200 Subject: [PATCH 01/11] version 1 works --- meson.build | 4 +- src/uenv/env.cpp | 28 ++++++++++++++ src/util/fs.cpp | 98 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/fs.h | 31 +++++++++++++++ test/unit/fs.cpp | 40 ++++++++++++++++++++ 5 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 src/util/fs.cpp create mode 100644 src/util/fs.h create mode 100644 test/unit/fs.cpp diff --git a/meson.build b/meson.build index 7b5aac9..72b2910 100644 --- a/meson.build +++ b/meson.build @@ -37,10 +37,11 @@ lib_src = [ 'src/uenv/meta.cpp', 'src/uenv/parse.cpp', 'src/uenv/repository.cpp', + 'src/uenv/uenv.cpp', + 'src/util/fs.cpp', 'src/util/shell.cpp', 'src/util/strings.cpp', 'src/util/subprocess.cpp', - 'src/uenv/uenv.cpp', ] lib_inc = include_directories('src') @@ -76,6 +77,7 @@ endif unit_src = [ 'test/unit/env.cpp', 'test/unit/envvars.cpp', + 'test/unit/fs.cpp', 'test/unit/lex.cpp', 'test/unit/main.cpp', 'test/unit/parse.cpp', diff --git a/src/uenv/env.cpp b/src/uenv/env.cpp index 44d46b6..36229a4 100644 --- a/src/uenv/env.cpp +++ b/src/uenv/env.cpp @@ -13,11 +13,39 @@ #include #include #include +#include namespace uenv { using util::unexpected; +std::filesystem::path meta_path(const std::filesystem::path& sqfs_path) { + namespace fs = std::filesystem; + + // set the meta data path and env.json path if they exist + auto meta_p = sqfs_path.parent_path() / "meta"; + auto env_meta_p = meta_p / "env.json"; + + // the meta path exists - use it + if (fs::is_directory(meta_p)) { + return meta_p; + } + + // open the image + + auto p = util::run({ + "unsquashfs", + sqfs_path.string(), + }); + + /* + std::optional meta_path = + fs::is_directory(meta_p) ? meta_p : std::optional{}; + */ + + return {}; +} + util::expected concretise_env(const std::string& uenv_args, std::optional view_args, diff --git a/src/util/fs.cpp b/src/util/fs.cpp new file mode 100644 index 0000000..835af45 --- /dev/null +++ b/src/util/fs.cpp @@ -0,0 +1,98 @@ +#include +#include +#include + +#include +#include +#include + +#include "expected.h" +#include "fs.h" +#include "subprocess.h" + +namespace util { + +temp_dir::~temp_dir() { + std::error_code ec; + + if (std::filesystem::is_directory(path_)) { + // ignore the error code - being unable to delete a temp path is not the + // end of the world. + auto n = std::filesystem::remove_all(path_, ec); + spdlog::debug("temp_dir: deleted {} files in {}", n, path_.string()); + } + + // path_ is either set, or default constructed as + // std::filesystem::path::empty + // https://en.cppreference.com/w/cpp/filesystem/path/empty deleting an + // empty + // path is safe - it is a noop. +} + +temp_dir::temp_dir(temp_dir&& p) : path_(std::move(p.path_)) { +} + +temp_dir::temp_dir(std::filesystem::path p) : path_(std::move(p)) { +} + +const std::filesystem::path& temp_dir::path() const { + return path_; +} + +temp_dir make_temp_dir() { + namespace fs = std::filesystem; + auto tmp_template = + fs::temp_directory_path().string() + "/uenv-XXXXXXXXXXXX"; + std::vector base(tmp_template.data(), + tmp_template.data() + tmp_template.size() + 1); + + fs::path tmp_path = mkdtemp(base.data()); + + spdlog::debug("make_temp_dir: created {}", tmp_path.string(), + fs::is_directory(tmp_path)); + + return tmp_path; +} + +util::expected +unsquashfs_tmp(const std::filesystem::path& sqfs, + const std::filesystem::path& contents) { + + namespace fs = std::filesystem; + + if (!fs::is_regular_file(sqfs)) { + return unexpected(fmt::format("unsquashfs_tmp: {} file does not exist", + sqfs.string())); + } + + auto base = make_temp_dir(); + std::vector command{"unsquashfs", "-no-exit", + "-d", base.path().string(), + sqfs.string(), contents.string()}; + spdlog::debug("unsquashfs_tmp: attempting to unpack {} from {}", + contents.string(), sqfs.string()); + + auto proc = run(command); + + if (!proc) { + return unexpected(fmt::format( + "unsquashfs_tmp: unable to run unsquashfs: {}", proc.error())); + } + + auto status = proc->wait(); + + spdlog::debug("unsquashfs_tmp: command '{}' retured status {}", + fmt::join(command, " "), status); + + if (status != 0) { + spdlog::warn("unsquashfs_tmp: unable to extract {} from {}", + contents.string(), sqfs.string()); + return unexpected( + fmt::format("unsquashfs_tmp: unable to extract {} from {}", + contents.string(), sqfs.string())); + } + + spdlog::info("unsquashfs_tmp: data unpacked to {}", base.path().string()); + return base; +} +} // namespace util diff --git a/src/util/fs.h b/src/util/fs.h new file mode 100644 index 0000000..ac11eae --- /dev/null +++ b/src/util/fs.h @@ -0,0 +1,31 @@ +#pragma once + +#include +#include + +#include + +namespace util { + +class temp_dir { + std::filesystem::path path_{}; + + public: + temp_dir() = delete; + temp_dir(const temp_dir&) = delete; + + temp_dir(temp_dir&& p); + temp_dir(std::filesystem::path p); + + const std::filesystem::path& path() const; + + ~temp_dir(); +}; + +temp_dir make_temp_dir(); + +util::expected +unsquashfs_tmp(const std::filesystem::path& sqfs, + const std::filesystem::path& contents); + +} // namespace util diff --git a/test/unit/fs.cpp b/test/unit/fs.cpp new file mode 100644 index 0000000..64f6a0c --- /dev/null +++ b/test/unit/fs.cpp @@ -0,0 +1,40 @@ +#include + +#include +#include + +#include +#include + +namespace fs = std::filesystem; + +TEST_CASE("make_temp_dir", "[fs]") { + auto dir1 = util::make_temp_dir(); + REQUIRE(fs::is_directory(dir1.path())); + auto dir2 = util::make_temp_dir(); + REQUIRE(dir1.path() != dir2.path()); + fmt::println("end-of-funtion"); +} + +TEST_CASE("unsquashfs", "[fs]") { + std::string sqfs = + "/home/bcumming/software/uenv2/test/scratch/repos/apptool/images/" + "0343a5636e6d59ed8e5f7fcc35c34719597ff024be0ce796cc0532804b3aeefa/" + "store.squashfs"; + { + auto meta = util::unsquashfs_tmp(sqfs, "meta"); + fmt::println("returned from unsquashfs"); + REQUIRE(meta); + REQUIRE(fs::is_directory(meta->path())); + REQUIRE(fs::is_directory(meta->path() / "meta")); + } + { + std::vector buffer; + const int nbuf = 5; + { + for (int i = 0; i < nbuf; ++i) { + buffer.push_back(*util::unsquashfs_tmp(sqfs, "meta/env.json")); + } + } + } +} From f9c6a9a8515325fbdabd1f6bddace3ec33f8c6eb Mon Sep 17 00:00:00 2001 From: bcumming Date: Fri, 4 Oct 2024 09:31:19 +0200 Subject: [PATCH 02/11] simplify and improve cleanup of temporary paths --- src/util/fs.cpp | 47 ++++++++++++++++++++--------------------------- src/util/fs.h | 19 ++----------------- test/unit/fs.cpp | 11 +++++------ 3 files changed, 27 insertions(+), 50 deletions(-) diff --git a/src/util/fs.cpp b/src/util/fs.cpp index 835af45..f7511c3 100644 --- a/src/util/fs.cpp +++ b/src/util/fs.cpp @@ -12,34 +12,25 @@ namespace util { -temp_dir::~temp_dir() { +struct temp_dir_wrap { + std::filesystem::path path; std::error_code ec; - - if (std::filesystem::is_directory(path_)) { - // ignore the error code - being unable to delete a temp path is not the - // end of the world. - auto n = std::filesystem::remove_all(path_, ec); - spdlog::debug("temp_dir: deleted {} files in {}", n, path_.string()); + ~temp_dir_wrap() { + if (std::filesystem::is_directory(path)) { + // ignore the error code - being unable to delete a temp path is not + // the end of the world. + auto n = std::filesystem::remove_all(path, ec); + spdlog::debug("temp_dir: deleted {} files in {}", n, path.string()); + } } +}; - // path_ is either set, or default constructed as - // std::filesystem::path::empty - // https://en.cppreference.com/w/cpp/filesystem/path/empty deleting an - // empty - // path is safe - it is a noop. -} +std::filesystem::path make_temp_dir() { + // persistant storage for the temporary paths that will delete the paths on + // exit. This makes temporary paths persistent for the duration of the + // application's execution. + static std::vector cache; -temp_dir::temp_dir(temp_dir&& p) : path_(std::move(p.path_)) { -} - -temp_dir::temp_dir(std::filesystem::path p) : path_(std::move(p)) { -} - -const std::filesystem::path& temp_dir::path() const { - return path_; -} - -temp_dir make_temp_dir() { namespace fs = std::filesystem; auto tmp_template = fs::temp_directory_path().string() + "/uenv-XXXXXXXXXXXX"; @@ -51,10 +42,12 @@ temp_dir make_temp_dir() { spdlog::debug("make_temp_dir: created {}", tmp_path.string(), fs::is_directory(tmp_path)); + cache.emplace_back(tmp_path); + return tmp_path; } -util::expected +util::expected unsquashfs_tmp(const std::filesystem::path& sqfs, const std::filesystem::path& contents) { @@ -67,7 +60,7 @@ unsquashfs_tmp(const std::filesystem::path& sqfs, auto base = make_temp_dir(); std::vector command{"unsquashfs", "-no-exit", - "-d", base.path().string(), + "-d", base.string(), sqfs.string(), contents.string()}; spdlog::debug("unsquashfs_tmp: attempting to unpack {} from {}", contents.string(), sqfs.string()); @@ -92,7 +85,7 @@ unsquashfs_tmp(const std::filesystem::path& sqfs, contents.string(), sqfs.string())); } - spdlog::info("unsquashfs_tmp: data unpacked to {}", base.path().string()); + spdlog::info("unsquashfs_tmp: data unpacked to {}", base.string()); return base; } } // namespace util diff --git a/src/util/fs.h b/src/util/fs.h index ac11eae..e2a2411 100644 --- a/src/util/fs.h +++ b/src/util/fs.h @@ -7,24 +7,9 @@ namespace util { -class temp_dir { - std::filesystem::path path_{}; +std::filesystem::path make_temp_dir(); - public: - temp_dir() = delete; - temp_dir(const temp_dir&) = delete; - - temp_dir(temp_dir&& p); - temp_dir(std::filesystem::path p); - - const std::filesystem::path& path() const; - - ~temp_dir(); -}; - -temp_dir make_temp_dir(); - -util::expected +util::expected unsquashfs_tmp(const std::filesystem::path& sqfs, const std::filesystem::path& contents); diff --git a/test/unit/fs.cpp b/test/unit/fs.cpp index 64f6a0c..b917886 100644 --- a/test/unit/fs.cpp +++ b/test/unit/fs.cpp @@ -10,9 +10,9 @@ namespace fs = std::filesystem; TEST_CASE("make_temp_dir", "[fs]") { auto dir1 = util::make_temp_dir(); - REQUIRE(fs::is_directory(dir1.path())); + REQUIRE(fs::is_directory(dir1)); auto dir2 = util::make_temp_dir(); - REQUIRE(dir1.path() != dir2.path()); + REQUIRE(dir1 != dir2); fmt::println("end-of-funtion"); } @@ -25,15 +25,14 @@ TEST_CASE("unsquashfs", "[fs]") { auto meta = util::unsquashfs_tmp(sqfs, "meta"); fmt::println("returned from unsquashfs"); REQUIRE(meta); - REQUIRE(fs::is_directory(meta->path())); - REQUIRE(fs::is_directory(meta->path() / "meta")); + REQUIRE(fs::is_directory(*meta)); + REQUIRE(fs::is_directory(*meta / "meta")); } { - std::vector buffer; const int nbuf = 5; { for (int i = 0; i < nbuf; ++i) { - buffer.push_back(*util::unsquashfs_tmp(sqfs, "meta/env.json")); + auto x = util::unsquashfs_tmp(sqfs, "meta/env.json"); } } } From 96dd75844666aea706d9fc6da58d735af2250e92 Mon Sep 17 00:00:00 2001 From: bcumming Date: Mon, 14 Oct 2024 21:32:43 +0200 Subject: [PATCH 03/11] use the unsquash feature to use in-image meta data --- src/uenv/env.cpp | 125 ++++++++++++++++++++++------------------------- 1 file changed, 59 insertions(+), 66 deletions(-) diff --git a/src/uenv/env.cpp b/src/uenv/env.cpp index 36229a4..4ee7a04 100644 --- a/src/uenv/env.cpp +++ b/src/uenv/env.cpp @@ -13,37 +13,37 @@ #include #include #include +#include #include namespace uenv { using util::unexpected; -std::filesystem::path meta_path(const std::filesystem::path& sqfs_path) { - namespace fs = std::filesystem; +struct meta_info { + std::optional path; + std::optional env; +}; - // set the meta data path and env.json path if they exist - auto meta_p = sqfs_path.parent_path() / "meta"; - auto env_meta_p = meta_p / "env.json"; +meta_info find_meta_path(const std::filesystem::path& sqfs_path) { + namespace fs = std::filesystem; - // the meta path exists - use it - if (fs::is_directory(meta_p)) { - return meta_p; + meta_info meta; + if (fs::is_directory(sqfs_path.parent_path() / "meta")) { + meta.path = sqfs_path.parent_path() / "meta"; + } else if (auto p = util::unsquashfs_tmp(sqfs_path, "meta")) { + meta.path = *p; } - // open the image - - auto p = util::run({ - "unsquashfs", - sqfs_path.string(), - }); + if (meta.path) { + auto env_meta = meta.path.value() / "env.json"; - /* - std::optional meta_path = - fs::is_directory(meta_p) ? meta_p : std::optional{}; - */ + if (fs::is_regular_file(env_meta)) { + meta.env = env_meta; + } + } - return {}; + return meta; } util::expected @@ -52,12 +52,12 @@ concretise_env(const std::string& uenv_args, std::optional repo_arg) { namespace fs = std::filesystem; - // parse the uenv description that was provided as a command line argument. - // the command line argument is a comma-separated list of uenvs, where each - // uenv is either + // parse the uenv description that was provided as a command line + // argument. the command line argument is a comma-separated list of + // uenvs, where each uenv is either // - the path of a squashfs image; or - // - a uenv description of the form name[/version][:tag][@system][%uarch] - // with an optional mount point. + // - a uenv description of the form + // name[/version][:tag][@system][%uarch] with an optional mount point. const auto uenv_descriptions = uenv::parse_uenv_args(uenv_args); if (!uenv_descriptions) { return unexpected(fmt::format("invalid uenv description: {}", @@ -66,8 +66,8 @@ concretise_env(const std::string& uenv_args, // concretise the uenv descriptions by looking for the squashfs file, or // looking up the uenv descrition in a registry. - // after this loop, we have fully validated list of uenvs, mount points and - // meta data (if they have meta data). + // after this loop, we have fully validated list of uenvs, mount points + // and meta data (if they have meta data). std::unordered_map uenvs; std::set used_mounts; @@ -80,9 +80,10 @@ concretise_env(const std::string& uenv_args, // it has to be looked up in a repo. if (auto label = desc.label()) { if (!repo_arg) { - return unexpected( - "a repo needs to be provided either using the --repo flag " - "or by setting the UENV_REPO_PATH environment variable"); + return unexpected("a repo needs to be provided either " + "using the --repo flag " + "or by setting the UENV_REPO_PATH " + "environment variable"); } auto store = uenv::open_repository(*repo_arg); if (!store) { @@ -147,22 +148,22 @@ concretise_env(const std::string& uenv_args, } spdlog::info("{} squashfs image {}", desc, sqfs_path.string()); - // set the meta data path and env.json path if they exist - const auto meta_p = sqfs_path.parent_path() / "meta"; - const auto env_meta_p = meta_p / "env.json"; - const std::optional meta_path = - fs::is_directory(meta_p) ? meta_p : std::optional{}; - const std::optional env_meta_path = - fs::is_regular_file(env_meta_p) ? env_meta_p - : std::optional{}; - - // if meta/env.json exists, parse the json therein - std::string name; - std::string description; + // create a default "anonymous" name for the uenv that will be + // overwritten if meta data is provided. + std::string name = "anonymous"; + unsigned name_idx = 0; + while (uenvs.count(name)) { + name = fmt::format("anonymous{}", name_idx); + ++name_idx; + } + std::string description = ""; std::optional mount_meta; std::unordered_map views; - if (env_meta_path) { - if (const auto result = uenv::load_meta(*env_meta_path)) { + + // if meta/env.json exists, parse the json therein + auto meta = find_meta_path(sqfs_path); + if (meta.env) { + if (const auto result = uenv::load_meta(*(meta.env))) { name = std::move(result->name); description = result->description.value_or(""); mount_meta = result->mount; @@ -171,27 +172,19 @@ concretise_env(const std::string& uenv_args, mount_meta); } else { spdlog::warn("{} opening the uenv meta data {}: {}", desc, - *env_meta_path, result.error()); + meta.env->string(), result.error()); } } else { - spdlog::debug("{} no meta file found at expected location {}", desc, - meta_path); - description = ""; - // generate a unique name for the uenv - name = "anonymous"; - unsigned i = 1; - while (uenvs.count(name)) { - name = fmt::format("anonymous{}", i); - ++i; - } + spdlog::warn("{} no meta file available for {}", desc, + sqfs_path.string()); } // if an explicit mount point was provided, use that // otherwise use the mount point provided in the meta data auto mount_string = desc.mount() ? desc.mount() : mount_meta; - // handle the case where no mount point was provided by the CLI or meta - // data + // handle the case where no mount point was provided by the CLI or + // meta data if (!mount_string) { return unexpected( fmt::format("no mount point provided for {}", desc)); @@ -240,7 +233,7 @@ concretise_env(const std::string& uenv_args, } uenvs[name] = concrete_uenv{name, mount, sqfs_path, - meta_path, description, std::move(views)}; + meta.path, description, std::move(views)}; } // A dictionary with view name as a key, and a list of uenv that provide @@ -277,7 +270,8 @@ concretise_env(const std::string& uenv_args, // a list of uenv that have a view with name v.name const auto& matching_uenvs = view2uenv[view.name]; - // handle the case where no uenv name was provided, e.g. develop + // handle the case where no uenv name was provided, e.g. + // develop if (!view.uenv) { // it is ambiguous if more than one option is available if (matching_uenvs.size() > 1) { @@ -291,8 +285,8 @@ concretise_env(const std::string& uenv_args, } views.push_back({matching_uenvs[0], view.name}); } - // handle the case where both uenv and view name are provided, - // e.g. prgenv-gnu:develop + // handle the case where both uenv and view name are + // provided, e.g. prgenv-gnu:develop else { auto it = std::find_if( matching_uenvs.begin(), matching_uenvs.end(), @@ -326,8 +320,8 @@ std::unordered_map getenv(const env& environment) { // returns the value of an environment variable. // if the variable has been recorded in env_vars, that value is returned - // else the cstdlib getenv function is called to get the currently set value - // returns nullptr if the variable is not set anywhere + // else the cstdlib getenv function is called to get the currently set + // value returns nullptr if the variable is not set anywhere auto ge = [&env_vars](const std::string& name) -> const char* { if (env_vars.count(name)) { return env_vars[name].c_str(); @@ -335,10 +329,9 @@ std::unordered_map getenv(const env& environment) { return ::getenv(name.c_str()); }; - // iterate over each view in order, and set the environment variables that - // each view configures. - // the variables are not set directly, instead they are accumulated in - // env_vars. + // iterate over each view in order, and set the environment variables + // that each view configures. the variables are not set directly, + // instead they are accumulated in env_vars. for (auto& view : environment.views) { auto result = environment.uenvs.at(view.uenv) .views.at(view.name) From 6ad89ea89ce49efd46239d2a86059ed92d0f5239 Mon Sep 17 00:00:00 2001 From: bcumming Date: Tue, 15 Oct 2024 20:45:45 +0200 Subject: [PATCH 04/11] fix meta path finding --- src/uenv/env.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/uenv/env.cpp b/src/uenv/env.cpp index 4ee7a04..d383b8d 100644 --- a/src/uenv/env.cpp +++ b/src/uenv/env.cpp @@ -31,8 +31,15 @@ meta_info find_meta_path(const std::filesystem::path& sqfs_path) { meta_info meta; if (fs::is_directory(sqfs_path.parent_path() / "meta")) { meta.path = sqfs_path.parent_path() / "meta"; + spdlog::debug("find_meta_path: {} found external meta path {}", + sqfs_path.string(), meta.path->string()); } else if (auto p = util::unsquashfs_tmp(sqfs_path, "meta")) { - meta.path = *p; + meta.path = *p / "meta"; + spdlog::debug("find_meta_path: {} found internal meta path {}", + sqfs_path.string(), meta.path->string()); + } else { + spdlog::debug("find_meta_path: {} no meta path found", + sqfs_path.string()); } if (meta.path) { @@ -40,6 +47,10 @@ meta_info find_meta_path(const std::filesystem::path& sqfs_path) { if (fs::is_regular_file(env_meta)) { meta.env = env_meta; + spdlog::debug("find_meta_path: {} found env meta {}", + sqfs_path.string(), meta.env->string()); + } else { + spdlog::debug("find_meta_path: {} no env meta", sqfs_path.string()); } } From 66400867d35452d73e94ff974b159d8ef52d38b4 Mon Sep 17 00:00:00 2001 From: bcumming Date: Tue, 15 Oct 2024 21:18:41 +0200 Subject: [PATCH 05/11] delete temporary paths before execve to ensure they are always deleted --- src/util/fs.cpp | 17 +++++++++++------ src/util/fs.h | 2 ++ src/util/shell.cpp | 6 ++++++ 3 files changed, 19 insertions(+), 6 deletions(-) diff --git a/src/util/fs.cpp b/src/util/fs.cpp index f7511c3..1dd50af 100644 --- a/src/util/fs.cpp +++ b/src/util/fs.cpp @@ -25,12 +25,17 @@ struct temp_dir_wrap { } }; -std::filesystem::path make_temp_dir() { - // persistant storage for the temporary paths that will delete the paths on - // exit. This makes temporary paths persistent for the duration of the - // application's execution. - static std::vector cache; +// persistant storage for the temporary paths that will delete the paths on +// exit. This makes temporary paths persistent for the duration of the +// application's execution. +static std::vector tmp_dir_cache; + +void clear_temp_dirs() { + tmp_dir_cache.clear(); + spdlog::debug("clear_temp_dir: deleted all temp files"); +} +std::filesystem::path make_temp_dir() { namespace fs = std::filesystem; auto tmp_template = fs::temp_directory_path().string() + "/uenv-XXXXXXXXXXXX"; @@ -42,7 +47,7 @@ std::filesystem::path make_temp_dir() { spdlog::debug("make_temp_dir: created {}", tmp_path.string(), fs::is_directory(tmp_path)); - cache.emplace_back(tmp_path); + tmp_dir_cache.emplace_back(tmp_path); return tmp_path; } diff --git a/src/util/fs.h b/src/util/fs.h index e2a2411..752382e 100644 --- a/src/util/fs.h +++ b/src/util/fs.h @@ -13,4 +13,6 @@ util::expected unsquashfs_tmp(const std::filesystem::path& sqfs, const std::filesystem::path& contents); +void clear_temp_dirs(); + } // namespace util diff --git a/src/util/shell.cpp b/src/util/shell.cpp index a7ddf51..8dde7d3 100644 --- a/src/util/shell.cpp +++ b/src/util/shell.cpp @@ -10,6 +10,7 @@ #include #include +#include namespace util { @@ -37,6 +38,11 @@ util::expected current_shell() { int exec(const std::vector& args) { std::vector argv; + // clean up temporary files before calling execve, because the descructor + // that manages their deletion will not be deleted after the current process + // has been replaced. + util::clear_temp_dirs(); + // unsafe { for (auto& arg : args) { argv.push_back(const_cast(arg.c_str())); From e5e23911dd8811054dbb2fbb47961aaad733c2d0 Mon Sep 17 00:00:00 2001 From: bcumming Date: Wed, 16 Oct 2024 09:12:20 +0200 Subject: [PATCH 06/11] tunable logging in unit tests - disable verbose logging by default --- test/unit/main.cpp | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/test/unit/main.cpp b/test/unit/main.cpp index 9f94c54..5a64ffb 100644 --- a/test/unit/main.cpp +++ b/test/unit/main.cpp @@ -5,6 +5,34 @@ static struct unit_init_log { unit_init_log() { - uenv::init_log(spdlog::level::trace, spdlog::level::off); + // use warn as the default log level + auto log_level = spdlog::level::warn; + bool invalid_env = false; + + // check the environment variable UENV_LOG_LEVEL + auto log_level_str = std::getenv("UENV_LOG_LEVEL"); + if (log_level_str != nullptr) { + int lvl; + auto [ptr, ec] = std::from_chars( + log_level_str, log_level_str + std::strlen(log_level_str), lvl); + + if (ec == std::errc()) { + if (lvl == 1) { + log_level = spdlog::level::info; + } else if (lvl == 2) { + log_level = spdlog::level::debug; + } else if (lvl > 2) { + log_level = spdlog::level::trace; + } + } else { + invalid_env = true; + } + } + uenv::init_log(log_level, spdlog::level::info); + if (invalid_env) { + spdlog::warn(fmt::format("UENV_LOG_LEVEL invalid value '{}' -- " + "expected a value between 0 and 3", + log_level_str)); + } } } uil{}; From 8b8919a614915aaaa897e265ff72ed79c9f8cb2d Mon Sep 17 00:00:00 2001 From: bcumming Date: Wed, 16 Oct 2024 09:13:17 +0200 Subject: [PATCH 07/11] generate sqfs images for testing that have no external meta data --- test/setup/setup_repos.bash | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/setup/setup_repos.bash b/test/setup/setup_repos.bash index 08f6091..206846c 100755 --- a/test/setup/setup_repos.bash +++ b/test/setup/setup_repos.bash @@ -39,6 +39,11 @@ function setup_repo_apptool() { cp ${sqfs} $img_path/store.squashfs cp -R ${sources}/${name}/meta $img_path done + + img_path="$sqfs_path/standalone" + mkdir -p $img_path + cp ${sqfs} $img_path/${name}.squashfs + rm ${sqfs} sed -i "s|{${name}-sha}|${sha}|g" schema.sql From da9547614c469d74c20d37297ca678fac91486ed Mon Sep 17 00:00:00 2001 From: bcumming Date: Wed, 16 Oct 2024 10:11:11 +0200 Subject: [PATCH 08/11] use deque instead of vector for persistance of temp paths --- src/util/fs.cpp | 13 ++++++++----- test/unit/fs.cpp | 30 ++++++++++++++++++++++-------- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/util/fs.cpp b/src/util/fs.cpp index 1dd50af..ac64b52 100644 --- a/src/util/fs.cpp +++ b/src/util/fs.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -14,13 +15,14 @@ namespace util { struct temp_dir_wrap { std::filesystem::path path; - std::error_code ec; ~temp_dir_wrap() { if (std::filesystem::is_directory(path)) { // ignore the error code - being unable to delete a temp path is not // the end of the world. + std::error_code ec; auto n = std::filesystem::remove_all(path, ec); - spdlog::debug("temp_dir: deleted {} files in {}", n, path.string()); + spdlog::debug("temp_dir_wrap: deleted {} files in {}", n, + path.string()); } } }; @@ -28,11 +30,11 @@ struct temp_dir_wrap { // persistant storage for the temporary paths that will delete the paths on // exit. This makes temporary paths persistent for the duration of the // application's execution. -static std::vector tmp_dir_cache; +// Use a dequeu because it will not copy/move/delete its contents as it grows. +static std::deque tmp_dir_cache; void clear_temp_dirs() { tmp_dir_cache.clear(); - spdlog::debug("clear_temp_dir: deleted all temp files"); } std::filesystem::path make_temp_dir() { @@ -90,7 +92,8 @@ unsquashfs_tmp(const std::filesystem::path& sqfs, contents.string(), sqfs.string())); } - spdlog::info("unsquashfs_tmp: data unpacked to {}", base.string()); + spdlog::info("unsquashfs_tmp: unpacked {} from {} to {}", contents.string(), + sqfs.string(), base.string()); return base; } } // namespace util diff --git a/test/unit/fs.cpp b/test/unit/fs.cpp index b917886..0f02d09 100644 --- a/test/unit/fs.cpp +++ b/test/unit/fs.cpp @@ -13,26 +13,40 @@ TEST_CASE("make_temp_dir", "[fs]") { REQUIRE(fs::is_directory(dir1)); auto dir2 = util::make_temp_dir(); REQUIRE(dir1 != dir2); - fmt::println("end-of-funtion"); } TEST_CASE("unsquashfs", "[fs]") { - std::string sqfs = - "/home/bcumming/software/uenv2/test/scratch/repos/apptool/images/" - "0343a5636e6d59ed8e5f7fcc35c34719597ff024be0ce796cc0532804b3aeefa/" - "store.squashfs"; + std::string sqfs = "../test/scratch/sqfs/apptool/standalone/app43.squashfs"; { + REQUIRE(fs::is_regular_file(sqfs)); auto meta = util::unsquashfs_tmp(sqfs, "meta"); - fmt::println("returned from unsquashfs"); REQUIRE(meta); REQUIRE(fs::is_directory(*meta)); REQUIRE(fs::is_directory(*meta / "meta")); + REQUIRE(fs::is_regular_file(*meta / "meta" / "env.json")); } + // unpack from a squashfs image many times to validate that the unpacked + // data is persistant. { - const int nbuf = 5; + const int nbuf = 128; { + std::vector paths; for (int i = 0; i < nbuf; ++i) { - auto x = util::unsquashfs_tmp(sqfs, "meta/env.json"); + auto meta = util::unsquashfs_tmp(sqfs, "meta/env.json"); + REQUIRE(meta); + auto file = *meta / "meta/env.json"; + REQUIRE(fs::is_regular_file(file)); + paths.push_back(file); + } + + // the generated paths should be unique + std::sort(paths.begin(), paths.end()); + auto e = std::unique(paths.begin(), paths.end()); + REQUIRE(e == paths.end()); + + // the generated paths should still exist + for (auto& file : paths) { + REQUIRE(fs::is_regular_file(file)); } } } From 03effa7b979d3d85dc95164ff3c235e606780001 Mon Sep 17 00:00:00 2001 From: bcumming Date: Wed, 16 Oct 2024 11:42:11 +0200 Subject: [PATCH 09/11] only use SQFSMNT_FWD_ prefix for setuid vars - so that PATH etc are set when squashfs-mount is called --- src/uenv/env.cpp | 45 ++++++++++++++++++++++++++++++++++++++- test/integration/cli.bats | 34 +++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/uenv/env.cpp b/src/uenv/env.cpp index d383b8d..3570116 100644 --- a/src/uenv/env.cpp +++ b/src/uenv/env.cpp @@ -355,11 +355,53 @@ std::unordered_map getenv(const env& environment) { return env_vars; } +// list of environment variables that are ignored in setuid applications +// the full list is defined here: +// https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/generic/unsecvars.h +std::set unsecure_envvars__{ + "GCONV_PATH", + "GETCONF_DIR", + "GLIBC_TUNABLES", + "HOSTALIASES", + "LD_AUDIT", + "LD_BIND_NOT", + "LD_BIND_NOW", + "LD_DEBUG", + "LD_DEBUG_OUTPUT", + "LD_DYNAMIC_WEAK", + "LD_LIBRARY_PATH", + "LD_ORIGIN_PATH", + "LD_PRELOAD", + "LD_PROFILE", + "LD_SHOW_AUXV", + "LD_VERBOSE", + "LD_WARN", + "LOCALDOMAIN", + "LOCPATH", + "MALLOC_ARENA_MAX", + "MALLOC_ARENA_TEST", + "MALLOC_MMAP_MAX_", + "MALLOC_MMAP_THRESHOLD_", + "MALLOC_PERTURB_", + "MALLOC_TOP_PAD_", + "MALLOC_TRACE", + "MALLOC_TRIM_THRESHOLD_", + "NIS_PATH", + "NLSPATH", + "RESOLV_HOST_CONF", + "RES_OPTIONS", + "TMPDIR", +}; + util::expected setenv(const std::unordered_map& variables, const std::string& prefix) { for (auto var : variables) { - std::string fwd_name = prefix + var.first; + // prepend prefix to unsecure environment variables + std::string fwd_name = unsecure_envvars__.contains(var.first) + ? prefix + var.first + : var.first; + fmt::println("setting {} to {}", fwd_name, var.second); if (auto rcode = ::setenv(fwd_name.c_str(), var.second.c_str(), true)) { switch (rcode) { case EINVAL: @@ -372,6 +414,7 @@ setenv(const std::unordered_map& variables, fmt::format("unknown error setting {}", fwd_name)); } } + fmt::println("set!"); } return 0; } diff --git a/test/integration/cli.bats b/test/integration/cli.bats index f550717..1efcb7d 100644 --- a/test/integration/cli.bats +++ b/test/integration/cli.bats @@ -154,3 +154,37 @@ function teardown() { assert_failure assert_line --partial "Permission denied" } + +@test "run" { + export UENV_REPO_PATH=$REPOS/apptool + + # + # check that run looks up images in the repo and mounts at the correct location + # + run uenv run tool -- ls /user-tools + assert_success + assert_output --regexp "meta" + + run uenv run app/42.0:v1 -- ls /user-environment + assert_success + assert_output --regexp "meta" + + # + # check --view + # + run uenv run --view=tool tool -- tool + assert_success + assert_output "hello tool" + + run uenv run --view=app app/42.0:v1 -- app + assert_success + assert_output "hello app" + + # + # check --view works when reading meta data from inside a standalone sqfs file + # + + run uenv run --view=tool $SQFS_LIB/apptool/standalone/tool.squashfs -- tool + assert_success + assert_output "hello tool" +} From c5538c418810c827f962fb702326602937ca7f80 Mon Sep 17 00:00:00 2001 From: bcumming Date: Wed, 16 Oct 2024 12:00:43 +0200 Subject: [PATCH 10/11] clean up comments and formatting --- src/util/fs.cpp | 8 ++++++-- test/integration/cli.bats | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/util/fs.cpp b/src/util/fs.cpp index ac64b52..ee9f4fc 100644 --- a/src/util/fs.cpp +++ b/src/util/fs.cpp @@ -30,9 +30,14 @@ struct temp_dir_wrap { // persistant storage for the temporary paths that will delete the paths on // exit. This makes temporary paths persistent for the duration of the // application's execution. -// Use a dequeu because it will not copy/move/delete its contents as it grows. +// Use a deque because it will not copy/move/delete its contents as it grows. static std::deque tmp_dir_cache; +// the temp paths are deleted automatically when tmp_dir_cache is cleaned up +// at the end of execution, except when execvp is used to replace the current +// process. +// use this function to force early clean up in such situations, so that no +// tmp paths remain after execution. void clear_temp_dirs() { tmp_dir_cache.clear(); } @@ -57,7 +62,6 @@ std::filesystem::path make_temp_dir() { util::expected unsquashfs_tmp(const std::filesystem::path& sqfs, const std::filesystem::path& contents) { - namespace fs = std::filesystem; if (!fs::is_regular_file(sqfs)) { diff --git a/test/integration/cli.bats b/test/integration/cli.bats index 1efcb7d..ef64d13 100644 --- a/test/integration/cli.bats +++ b/test/integration/cli.bats @@ -11,7 +11,7 @@ function setup() { export SRC_PATH=$(realpath ../../) - export PATH="$(realpath ../../install/bin):$PATH" + export PATH="$(realpath ../../build):$PATH" unset UENV_MOUNT_LIST From 0d674be9101bb2eb81d9aff3cb9448fa1b267694 Mon Sep 17 00:00:00 2001 From: bcumming Date: Wed, 16 Oct 2024 12:19:50 +0200 Subject: [PATCH 11/11] fix missing header in tests; clean up output --- test/unit/main.cpp | 2 ++ test/unit/repository.cpp | 5 ----- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/test/unit/main.cpp b/test/unit/main.cpp index 5a64ffb..a086582 100644 --- a/test/unit/main.cpp +++ b/test/unit/main.cpp @@ -1,3 +1,5 @@ +#include + #define CATCH_CONFIG_MAIN #include diff --git a/test/unit/repository.cpp b/test/unit/repository.cpp index b6e3824..1645bc4 100644 --- a/test/unit/repository.cpp +++ b/test/unit/repository.cpp @@ -16,7 +16,6 @@ TEST_CASE("read-only", "[repository]") { SKIP(fmt::format("{}", store.error())); } - fmt::println("db path: {}", store->db_path().string()); { auto results = store->query({{}, {}, {}, {}, {}}); if (!results) { @@ -29,7 +28,6 @@ TEST_CASE("read-only", "[repository]") { //} } - fmt::println(""); { auto results = store->query({"mch", {}, {}, {}, {}}); if (!results) { @@ -42,7 +40,6 @@ TEST_CASE("read-only", "[repository]") { //} } - fmt::println(""); { auto results = store->query({{}, "v7", {}, {}, {}}); if (!results) { @@ -55,7 +52,6 @@ TEST_CASE("read-only", "[repository]") { //} } - fmt::println(""); { auto results = store->query({{}, "24.7", "v1-rc1", {}, "a100"}); if (!results) { @@ -68,7 +64,6 @@ TEST_CASE("read-only", "[repository]") { //} } - fmt::println(""); { auto results = store->query({"wombat", {}, {}, {}, {}}); if (!results) {