handle null description in uenv meta data (#7)

Fixes a bug in when reading `env.json`, whereby a `null` description created an uncaught `nlohmann::json::exception`.

* handle the `null` case as an empty description
* catch all json exceptions and return an error message from `uenv::load_meta`
* print a warning, not an error, when there is a problem loading meta data

.github/workflows/build_and_test.yml

@@ -33,5 +33,7 @@ jobs:
               build .
       - name: Build
         run: ninja -C build
+      - name: Setup Test
+        run: cd test/setup && ./setup
       - name: Test
-        run: ./build/unit
+        run: cd build && ./unit

src/uenv/env.cpp

@@ -142,8 +142,8 @@ concretise_env(const std::string& uenv_args,
                 spdlog::info("{}: loaded meta (name {}, mount {})", desc, name,
                              mount_meta);
             } else {
-                spdlog::error("{} opening the uenv meta data {}: {}", desc,
-                              *env_meta_path, result.error());
+                spdlog::warn("{} opening the uenv meta data {}: {}", desc,
+                             *env_meta_path, result.error());
             }
         } else {
             spdlog::debug("{} no meta file found at expected location {}", desc,

src/uenv/meta.cpp

@@ -6,6 +6,7 @@
 #include <fmt/core.h>
 #include <fmt/std.h>
 #include <nlohmann/json.hpp>
+#include <spdlog/spdlog.h>
 
 #include <util/expected.h>
 
@@ -18,59 +19,77 @@ namespace uenv {
 util::expected<meta, std::string> load_meta(const std::filesystem::path& file) {
     using json = nlohmann::json;
 
-    if (!std::filesystem::is_regular_file(file)) {
-        return util::unexpected(fmt::format(
-            "the uenv meta data file {} does not exist", file.string()));
-    }
-
-    auto fid = std::ifstream(file);
+    spdlog::debug("uenv::load_meta attempting to open uenv meta data file {}",
+                  file.string());
 
-    nlohmann::json raw;
     try {
-        raw = json::parse(fid);
-    } catch (std::exception& e) {
-        return util::unexpected(
-            fmt::format("error parsing meta data file for uenv {}: {}",
-                        file.string(), e.what()));
-    }
+        if (!std::filesystem::is_regular_file(file)) {
+            return util::unexpected(fmt::format(
+                "the uenv meta data file {} does not exist", file.string()));
+        }
+        spdlog::debug("uenv::load_meta file opened");
+
+        auto fid = std::ifstream(file);
+
+        nlohmann::json raw;
+        try {
+            raw = json::parse(fid);
+        } catch (std::exception& e) {
+            return util::unexpected(
+                fmt::format("error parsing meta data file for uenv {}: {}",
+                            file.string(), e.what()));
+        }
+        spdlog::debug("uenv::load_meta raw json read");
 
-    const std::string name = raw.contains("name") ? raw["name"] : "unnamed";
-    using ostring = std::optional<std::string>;
-    const ostring description =
-        raw.contains("description") ? ostring(raw["description"]) : ostring{};
-    const ostring mount =
-        raw.contains("mount") ? ostring(raw["mount"]) : ostring{};
+        const std::string name = raw.contains("name") ? raw["name"] : "unnamed";
+        using ostring = std::optional<std::string>;
+        const ostring description =
+            (raw.contains("description") && !raw["description"].is_null())
+                ? ostring(raw["description"])
+                : ostring{};
+        const ostring mount =
+            raw.contains("mount") ? ostring(raw["mount"]) : ostring{};
 
-    std::unordered_map<std::string, concrete_view> views;
-    if (auto& jviews = raw["views"]; jviews.is_object()) {
-        for (auto& [name, desc] : jviews.items()) {
-            uenv::envvarset envvars;
-            if (auto& list = desc["env"]["values"]["list"]; list.is_object()) {
-                for (auto& [var_name, updates] : list.items()) {
-                    for (auto& u : updates) {
-                        const uenv::update_kind op =
-                            u["op"] == "append"    ? uenv::update_kind::append
-                            : u["op"] == "prepend" ? uenv::update_kind::prepend
-                                                   : uenv::update_kind::set;
-                        std::vector<std::string> paths = u["value"];
-                        envvars.update_prefix_path(var_name, {op, paths});
+        spdlog::debug("uenv::load_meta name '{}' mount {} description '{}'",
+                      name, mount, description);
+
+        std::unordered_map<std::string, concrete_view> views;
+        if (auto& jviews = raw["views"]; jviews.is_object()) {
+            for (auto& [name, desc] : jviews.items()) {
+                uenv::envvarset envvars;
+                if (auto& list = desc["env"]["values"]["list"];
+                    list.is_object()) {
+                    for (auto& [var_name, updates] : list.items()) {
+                        for (auto& u : updates) {
+                            const uenv::update_kind op =
+                                u["op"] == "append" ? uenv::update_kind::append
+                                : u["op"] == "prepend"
+                                    ? uenv::update_kind::prepend
+                                    : uenv::update_kind::set;
+                            std::vector<std::string> paths = u["value"];
+                            envvars.update_prefix_path(var_name, {op, paths});
+                        }
                     }
                 }
-            }
-            if (auto& scalar = desc["env"]["values"]["scalar"];
-                scalar.is_object()) {
-                for (auto& [var_name, val] : scalar.items()) {
-                    envvars.update_scalar(var_name, val);
+                if (auto& scalar = desc["env"]["values"]["scalar"];
+                    scalar.is_object()) {
+                    for (auto& [var_name, val] : scalar.items()) {
+                        envvars.update_scalar(var_name, val);
+                    }
                 }
-            }
 
-            const std::string description =
-                desc.contains("description") ? desc["description"] : "";
-            views[name] = concrete_view{name, description, envvars};
+                const std::string description =
+                    desc.contains("description") ? desc["description"] : "";
+                views[name] = concrete_view{name, description, envvars};
+            }
         }
-    }
 
-    return meta{name, description, mount, views};
+        return meta{name, description, mount, views};
+    } catch (json::exception& e) {
+        return util::unexpected(
+            fmt::format("internal error parsing uenv meta data in {}: {}",
+                        file.string(), e.what()));
+    }
 }
 
 } // namespace uenv

test/data/env-files/cp2k-2024.2-v1.json

@@ -0,0 +1 @@
+{"description": null, "modules": {"root": "/user-environment/modules"}, "mount": "/user-environment", "name": "cp2k", "views": {"cp2k": {"activate": "/user-environment/env/cp2k/activate.sh", "description": "", "root": "/user-environment/env/cp2k", "env": {"version": 1, "values": {"list": {"ACLOCAL_PATH": [{"op": "prepend", "value": ["/user-environment/env/cp2k/share/aclocal", "/usr/share/aclocal"]}], "CMAKE_PREFIX_PATH": [{"op": "prepend", "value": ["/user-environment/env/cp2k", "/opt/cray/xpmem/default", "/opt/cray/libfabric/1.15.2.0"]}], "LD_LIBRARY_PATH": [{"op": "prepend", "value": ["/opt/cray/libfabric/1.15.2.0/lib64", "/opt/cray/libfabric/1.15.2.0/lib", "/user-environment/env/cp2k/lib"]}], "MANPATH": [{"op": "prepend", "value": ["/user-environment/env/cp2k/share/man", "/user-environment/env/cp2k/man", "/opt/cray/libfabric/1.15.2.0/share/man", "/usr/share/man", "/usr/man"]}], "PATH": [{"op": "prepend", "value": ["/user-environment/env/cp2k/bin", "/opt/cray/libfabric/1.15.2.0/bin", "/usr/bin", "/bin"]}, {"op": "prepend", "value": ["/user-environment/linux-sles15-neoverse_v2/gcc-12.3.0/gcc-12.3.0-piwjdjrnxpppwsbyvcqeg2wmsf4izw5y/bin"]}], "PKG_CONFIG_PATH": [{"op": "prepend", "value": ["/user-environment/env/cp2k/lib/pkgconfig", "/user-environment/env/cp2k/lib64/pkgconfig", "/opt/cray/xpmem/default/lib64/pkgconfig", "/opt/cray/libfabric/1.15.2.0/lib64/pkgconfig", "/usr/share/pkgconfig", "/usr/lib64/pkgconfig", "/usr/lib/pkgconfig", "/user-environment/env/cp2k/share/pkgconfig"]}], "PYTHONPATH": [{"op": "prepend", "value": ["/user-environment/env/cp2k/misc", "/user-environment/env/cp2k/lib/python3.11/site-packages"]}]}, "scalar": {"BOOST_ROOT": "/user-environment/env/cp2k", "CUDA_HOME": "/user-environment/env/cp2k", "GSL_ROOT_DIR": "/user-environment/env/cp2k", "MPICC": "/user-environment/env/cp2k/bin/mpicc", "MPICXX": "/user-environment/env/cp2k/bin/mpic++", "MPIF77": "/user-environment/env/cp2k/bin/mpif77", "MPIF90": "/user-environment/env/cp2k/bin/mpif90", "NVHPC_CUDA_HOME": "/user-environment/env/cp2k"}}}, "type": "spack-view"}, "develop": {"activate": "/user-environment/env/develop/activate.sh", "description": "", "root": "/user-environment/env/develop", "env": {"version": 1, "values": {"list": {"ACLOCAL_PATH": [{"op": "prepend", "value": ["/user-environment/env/develop/share/aclocal", "/usr/share/aclocal"]}], "CMAKE_PREFIX_PATH": [{"op": "prepend", "value": ["/user-environment/env/develop", "/opt/cray/xpmem/default", "/opt/cray/libfabric/1.15.2.0"]}], "LD_LIBRARY_PATH": [{"op": "prepend", "value": ["/opt/cray/libfabric/1.15.2.0/lib64", "/opt/cray/libfabric/1.15.2.0/lib", "/user-environment/env/develop/lib"]}], "MANPATH": [{"op": "prepend", "value": ["/user-environment/env/develop/share/man", "/user-environment/env/develop/man", "/opt/cray/libfabric/1.15.2.0/share/man", "/usr/share/man", "/usr/man"]}], "PATH": [{"op": "prepend", "value": ["/user-environment/env/develop/bin", "/opt/cray/libfabric/1.15.2.0/bin", "/usr/bin", "/bin"]}, {"op": "prepend", "value": ["/user-environment/linux-sles15-neoverse_v2/gcc-12.3.0/gcc-12.3.0-piwjdjrnxpppwsbyvcqeg2wmsf4izw5y/bin"]}], "PKG_CONFIG_PATH": [{"op": "prepend", "value": ["/user-environment/env/develop/lib/pkgconfig", "/user-environment/env/develop/lib64/pkgconfig", "/opt/cray/xpmem/default/lib64/pkgconfig", "/opt/cray/libfabric/1.15.2.0/lib64/pkgconfig", "/usr/share/pkgconfig", "/usr/lib64/pkgconfig", "/usr/lib/pkgconfig", "/user-environment/env/develop/share/pkgconfig"]}], "PYTHONPATH": [{"op": "prepend", "value": ["/user-environment/env/develop/misc", "/user-environment/env/develop/lib/python3.11/site-packages"]}]}, "scalar": {"BOOST_ROOT": "/user-environment/env/develop", "CUDA_HOME": "/user-environment/env/develop", "GSL_ROOT_DIR": "/user-environment/env/develop", "MPICC": "/user-environment/env/develop/bin/mpicc", "MPICXX": "/user-environment/env/develop/bin/mpic++", "MPIF77": "/user-environment/env/develop/bin/mpif77", "MPIF90": "/user-environment/env/develop/bin/mpif90", "NVHPC_CUDA_HOME": "/user-environment/env/develop"}}}, "type": "spack-view"}, "modules": {"activate": "/dev/null", "description": "activate modules", "root": "/user-environment/modules", "env": {"version": 1, "type": "augment", "values": {"list": {"MODULEPATH": [{"op": "prepend", "value": ["/user-environment/modules"]}]}, "scalar": {}}}}, "spack": {"activate": "/dev/null", "description": "configure spack upstream", "root": "/user-environment/config", "env": {"version": 1, "type": "augment", "values": {"list": {}, "scalar": {"UENV_SPACK_CONFIG_PATH": "/user-environment/config", "UENV_SPACK_COMMIT": "bd66dce2d668a6234504594661506cdd1eaca4adc", "UENV_SPACK_URL": "https://github.com/spack/spack.git"}}}}}}

test/data/env-files/readme.md

@@ -0,0 +1 @@
+`env.json` files for testing

test/unit/env.cpp

@@ -2,3 +2,9 @@
 #include <fmt/core.h>
 
 #include <uenv/env.h>
+#include <uenv/log.h>
+#include <uenv/meta.h>
+
+TEST_CASE("load_meta", "[env]") {
+    REQUIRE(uenv::load_meta("../test/data/env-files/cp2k-2024.2-v1.json"));
+}

test/unit/repository.cpp

@@ -9,7 +9,7 @@
 namespace fs = std::filesystem;
 
 TEST_CASE("read-only", "[repository]") {
-    fs::path repo_path{"../test/scratch/repo"};
+    fs::path repo_path{"../test/scratch/repos/apptool"};
     auto store = uenv::open_repository(repo_path);
 
     if (!store) {
@@ -24,9 +24,9 @@ TEST_CASE("read-only", "[repository]") {
         }
         REQUIRE(results);
 
-        for (auto& r : *results) {
-            fmt::println("{}", r);
-        }
+        // for (auto& r : *results) {
+        // fmt::println("{}", r);
+        //}
     }
 
     fmt::println("");
@@ -37,9 +37,9 @@ TEST_CASE("read-only", "[repository]") {
         }
         REQUIRE(results);
 
-        for (auto& r : *results) {
-            fmt::println("{}", r);
-        }
+        // for (auto& r : *results) {
+        // fmt::println("{}", r);
+        //}
     }
 
     fmt::println("");
@@ -50,9 +50,9 @@ TEST_CASE("read-only", "[repository]") {
         }
         REQUIRE(results);
 
-        for (auto& r : *results) {
-            fmt::println("{}", r);
-        }
+        // for (auto& r : *results) {
+        // fmt::println("{}", r);
+        //}
     }
 
     fmt::println("");
@@ -63,9 +63,9 @@ TEST_CASE("read-only", "[repository]") {
         }
         REQUIRE(results);
 
-        for (auto& r : *results) {
-            fmt::println("{}", r);
-        }
+        // for (auto& r : *results) {
+        // fmt::println("{}", r);
+        //}
     }
 
     fmt::println("");
@@ -77,8 +77,8 @@ TEST_CASE("read-only", "[repository]") {
         REQUIRE(results);
         REQUIRE(results->empty());
 
-        for (auto& r : *results) {
-            fmt::println("{}", r);
-        }
+        // for (auto& r : *results) {
+        // fmt::println("{}", r);
+        //}
     }
 }