Cleanup

.gitea/workflows/ci.yml

@@ -90,7 +90,7 @@ jobs:
           key: conan-${{ runner.os }}-${{ hashFiles('conanfile.py', 'modulus.json') }}
           restore-keys: conan-${{ runner.os }}-
       - name: Configure native release
-        run: python3 modulus.py configure --target x86_64_gcc12_release
+        run: python3 modulus.py configure --target x86_64_gcc12_release --update
       - name: Check code style
         run: |
           python3 -m pip install --no-cache-dir -r build/build_tools/docker/format-requirements.txt
@@ -106,4 +106,4 @@ jobs:
           CONAN_TOKEN: ${{ secrets.CONAN_TOKEN }}
           LUMA_CONAN_UPLOAD: "1"
           MODULUS_ALLOW_PACKAGE_REPLACE: "1"
-        run: python3 modulus.py package --all --replace
+        run: python3 modulus.py package --all --replace --update

conanfile.py

@@ -28,8 +28,11 @@ class luma_conan(conan.ConanFile):
     "fPIC": True,
     "with_tests": False,
   }
-  exports = "modulus.json"
+  exports = "modulus.json", "!build", "!.git", "!test_package/build"
   exports_sources = (
+    "!build",
+    "!.git",
+    "!test_package/build",
     "CMakeLists.txt",
     "cmake/*.in",
     "include/*.hpp",

include/luma/overlays.hpp

@@ -20,7 +20,7 @@ namespace luma {
 enum class dialog_result {
   NONE,
   ACCEPTED,
-  CANCELLED,
+  CANCELED,
 };
 
 struct dialog_options {

modulus.py

@@ -1,4 +1,5 @@
 #!/usr/bin/env python3
+# Canonical consumer bootstrap. Copy this file unchanged to a project's modulus.py.
 import json
 import os
 import pathlib
@@ -17,6 +18,7 @@ COMMANDS = (
   "lsp",
   "clangd",
   "format",
+  "run",
 )
 COMMON_DIR = pathlib.Path("build") / "build_common"
 TOOLS_DIR = pathlib.Path("build") / "build_tools"
@@ -66,13 +68,6 @@ def runtime_ready(root):
     and (root / RUNTIME_DIR / "modulus" / "format_layout.py").is_file()
   )
 
-def optional_credentials():
-  user = os.environ.get("CONAN_USER")
-  token = os.environ.get("CONAN_TOKEN")
-  if bool(user) != bool(token):
-    raise SystemExit("CONAN_USER and CONAN_TOKEN must either both be set or both be unset")
-  return user, token
-
 def env_flag(name):
   return os.environ.get(name, "").lower() in {"1", "true", "yes", "on"}
 
@@ -128,7 +123,7 @@ RUN python -m pip install --no-cache-dir conan=={CONAN_VERSION}
 WORKDIR /workspace
 """
 
-def ensure_fetch_image():
+def ensure_fetch_image(offline=False):
   inspected = subprocess.run(
     [
       "docker",
@@ -140,12 +135,15 @@ def ensure_fetch_image():
     stderr=subprocess.DEVNULL,
     check=False,
   )
-  if inspected.returncode == 0:
+  if inspected.returncode == 0 and (offline or not docker_platform_args()):
     return
+  if offline:
+    raise SystemExit('offline bootstrap requires the existing Modulus fetch image or MODULUS_DIRECT=1 with Conan installed')
   run(
     [
       "docker",
       "build",
+      *docker_platform_args(),
       "--tag",
       FETCH_IMAGE,
       "-",
@@ -156,6 +154,10 @@ def ensure_fetch_image():
 def in_container():
   return pathlib.Path("/.dockerenv").exists()
 
+def docker_platform_args():
+  platform = os.environ.get("MODULUS_DOCKER_PLATFORM", "").strip()
+  return ["--platform", platform] if platform else []
+
 def docker_workspace_args(root):
   if in_container():
     container = os.environ.get("HOSTNAME", "").strip()
@@ -172,18 +174,24 @@ def docker_workspace_args(root):
 
 def fetch_env(root):
   env = os.environ.copy()
+  for name in list(env):
+    if name in {"CONAN_USER", "CONAN_TOKEN"} or name.startswith(("CONAN_LOGIN_USERNAME", "CONAN_PASSWORD")):
+      del env[name]
   env["CONAN_HOME"] = str(root / CONAN_HOME_DIR)
+  env["CONAN_NON_INTERACTIVE"] = "1"
   return env
 
-def run_fetch(root, command, capture=False, display_command=None, check=True):
+def run_fetch(root, command, capture=False, display_command=None, check=True, offline=False):
   if direct_mode(root):
     return run(command, capture=capture, display_command=display_command, env=fetch_env(root), check=check)
-  ensure_fetch_image()
+  ensure_fetch_image(offline=offline)
   docker_command = [
     "docker",
     "run",
     "--rm",
     "--interactive",
+    *(["--pull=never", "--network=none"] if offline else []),
+    *docker_platform_args(),
     "--user",
     f"{os.getuid()}:{os.getgid()}",
     "--env",
@@ -191,9 +199,7 @@ def run_fetch(root, command, capture=False, display_command=None, check=True):
     "--env",
     "CONAN_HOME",
     "--env",
-    "CONAN_USER",
-    "--env",
-    "CONAN_TOKEN",
+    "CONAN_NON_INTERACTIVE",
     *docker_workspace_args(root),
     "--workdir",
     str(root),
@@ -203,7 +209,6 @@ def run_fetch(root, command, capture=False, display_command=None, check=True):
   return run(docker_command, capture=capture, display_command=display_command, env=fetch_env(root), check=check)
 
 def configure_remote(root, remote_name, remote_url):
-  user, token = optional_credentials()
   run_fetch(
     root,
     [
@@ -215,29 +220,7 @@ def configure_remote(root, remote_name, remote_url):
       remote_url,
     ],
   )
-  if not user and not token:
-    return
-  run_fetch(
-    root,
-    [
-      "conan",
-      "remote",
-      "login",
-      remote_name,
-      user,
-      "-p",
-      token,
-    ],
-    display_command=[
-      "conan",
-      "remote",
-      "login",
-      remote_name,
-      user,
-      "-p",
-      "<hidden>",
-    ],
-  )
+  run_fetch(root, ["conan", "remote", "logout", remote_name])
 
 def package_refs(data):
   refs = []
@@ -274,46 +257,26 @@ def package_refs(data):
             refs.append(f"{PACKAGE_REF}#{recipe_revision}:{package_id}")
   return sorted(refs)
 
-def remove_cached_modulus(root):
-  run_fetch(
-    root,
-    [
-      "conan",
-      "remove",
-      PACKAGE_REF + ":*",
-      "-c",
-    ],
-    check=False,
-  )
-
-def download_modulus(root, remote_name):
-  remove_cached_modulus(root)
-  run_fetch(
-    root,
-    [
-      "conan",
-      "download",
-      PACKAGE_REF + ":*",
-      "-r",
-      remote_name,
-    ],
-  )
+def find_modulus(root, remote_name=None):
   completed = run_fetch(
     root,
     [
       "conan",
       "list",
-      PACKAGE_REF + ":*",
-      "-c",
+      PACKAGE_REF + "#latest:*#latest",
+      *(["-r", remote_name] if remote_name else ["-c"]),
       "-f",
       "json",
     ],
     capture=True,
+    offline=remote_name is None,
   )
   refs = package_refs(json.loads(completed.stdout))
   if not refs:
-    raise SystemExit(f"no downloaded {PACKAGE_REF} package found in the Conan cache")
+    raise SystemExit(f"no {PACKAGE_REF} package available in {remote_name or 'the local Conan cache'}")
   package_ref = refs[-1]
+  if remote_name:
+    run_fetch(root, ["conan", "download", package_ref, "-r", remote_name])
   completed = run_fetch(
     root,
     [
@@ -323,24 +286,25 @@ def download_modulus(root, remote_name):
       package_ref,
     ],
     capture=True,
+    offline=remote_name is None,
   )
   package_root = pathlib.Path(completed.stdout.strip())
   if not (package_root / "modulus" / "export.py").is_file() or not (package_root / "tools").is_dir():
     raise SystemExit(f"downloaded {PACKAGE_REF} package does not contain Modulus export files: {package_root}")
   return package_root
 
-def materialize_runtime(root):
+def materialize_runtime(root, no_remote=False):
   remote_name, remote_url = remote_config(root)
-  configure_remote(root, remote_name, remote_url)
-  package_root = download_modulus(root, remote_name)
+  if not no_remote:
+    configure_remote(root, remote_name, remote_url)
+  package_root = find_modulus(root, None if no_remote else remote_name)
   sys.path.insert(0, str(package_root))
   import modulus.export
   modulus.export.sync_project(root, package_root)
 
-def ensure_runtime(root):
-  if runtime_ready(root):
-    return
-  materialize_runtime(root)
+def ensure_runtime(root, update=False, no_remote=False):
+  if update or not runtime_ready(root):
+    materialize_runtime(root, no_remote=no_remote)
 
 def delegate(command, argv):
   root = repo_root()
@@ -349,7 +313,7 @@ def delegate(command, argv):
       print(f"Modulus {command} tooling is missing; run ./modulus.py configure first", file=sys.stderr)
       return 1
   else:
-    ensure_runtime(root)
+    ensure_runtime(root, update=command == "configure" and "--update" in argv, no_remote="--no-remote" in argv)
   sys.path.insert(0, str(root / RUNTIME_DIR))
   import modulus.cli
   previous_argv = sys.argv

src/assets.cpp

@@ -55,11 +55,7 @@ auto luma::asset_cache::texture(std::string const &path) -> std::expected<std::s
         assert(cached->second != nullptr);
         return modulus::success(cached->second);
       }
-      if (!loading->result) {
-        return modulus::fail<std::shared_ptr<luma::texture>>(loading->result.error());
-      }
-      assert(*loading->result != nullptr);
-      return modulus::success(*loading->result);
+      return loading->result;
     }
     loading = std::make_shared<luma::asset_cache::in_flight_load>();
     loading->owner = std::this_thread::get_id();
@@ -75,49 +71,23 @@ auto luma::asset_cache::texture(std::string const &path) -> std::expected<std::s
   } catch (...) {
     loaded = luma::detail::fail<std::shared_ptr<luma::texture>>(luma::detail::error_internal, "texture load failed with an unknown exception");
   }
+  if (loaded && !*loaded) {
+    loaded = luma::detail::fail<std::shared_ptr<luma::texture>>(luma::detail::error_not_found, "texture load returned no texture: " + path);
+  }
   if (!loaded) {
     modulus::log_debug("luma texture load failed: " + path);
+  }
+  {
     auto lock = std::scoped_lock{mutex_};
-    loading->result = modulus::fail<std::shared_ptr<luma::texture>>(loaded.error());
+    if (loaded && generation == generation_) {
+      loaded = textures_.emplace(path, *loaded).first->second;
+    }
+    loading->result = loaded;
     loading->done = true;
     loading_.erase(path);
-    loading->ready.notify_all();
-    return loaded;
   }
-  if (*loaded == nullptr) {
-    modulus::log_debug("luma texture load returned null: " + path);
-    auto result = luma::detail::fail<std::shared_ptr<luma::texture>>(luma::detail::error_not_found, "texture load returned no texture: " + path);
-    auto lock = std::scoped_lock{mutex_};
-    loading->result = modulus::fail<std::shared_ptr<luma::texture>>(result.error());
-    loading->done = true;
-    loading_.erase(path);
-    loading->ready.notify_all();
-    return result;
-  }
-  auto texture = *loaded;
-  auto lock = std::scoped_lock{mutex_};
-  if (auto found = textures_.find(path); found != textures_.end()) {
-    assert(found->second != nullptr);
-    loading->result = modulus::success(found->second);
-    loading->done = true;
-    loading_.erase(path);
-    loading->ready.notify_all();
-    return modulus::success(found->second);
-  }
-  if (generation != generation_) {
-    loading->result = modulus::success(texture);
-    loading->done = true;
-    loading_.erase(path);
-    loading->ready.notify_all();
-    return modulus::success(std::move(texture));
-  }
-  auto inserted = textures_.emplace(path, std::move(texture));
-  assert(inserted.first->second != nullptr);
-  loading->result = modulus::success(inserted.first->second);
-  loading->done = true;
-  loading_.erase(path);
   loading->ready.notify_all();
-  return modulus::success(inserted.first->second);
+  return loaded;
 }
 
 auto luma::asset_cache::clear() -> void {

src/overlays.cpp

@@ -323,7 +323,7 @@ auto luma::overlay_host::handle_touch(luma::touch_event const &event) -> std::bo
       return true;
     }
     if (has_cancel && first_point_inside(event, cancel_bounds(bounds()), luma::touch_phase::UP) != nullptr) {
-      throw_if_failed(close_dialog(luma::dialog_result::CANCELLED));
+      throw_if_failed(close_dialog(luma::dialog_result::CANCELED));
       return true;
     }
     if (dialog_->body && any_point_inside(event, dialog_body_bounds(bounds()))) {

src/theme.cpp

@@ -4,7 +4,7 @@
 #include <cmath>
 #include <cstdint>
 #include <expected>
-#include <optional>
+#include <limits>
 #include <string>
 #include <string_view>
 #include <utility>
@@ -60,7 +60,14 @@ auto apply_color(boost::json::object const &object, std::char_t const *name, lum
   return {};
 }
 
-auto number_value(boost::json::value const &value) -> std::optional<std::float64_t>;
+auto apply_number(boost::json::object const &object, std::char_t const *name, std::float32_t &target) -> std::expected<void, modulus::error> {
+  auto value = modulus::json_config::optional_float64_field(object, name, target, 0.0, std::numeric_limits<std::float32_t>::max(), json_options);
+  if (!value) {
+    return modulus::fail<void>(value.error());
+  }
+  target = static_cast<std::float32_t>(*value);
+  return {};
+}
 
 auto apply_text_style(boost::json::object const &object, std::char_t const *name, luma::text_style &target) -> std::expected<void, modulus::error> {
   assert(name != nullptr);
@@ -78,41 +85,21 @@ auto apply_text_style(boost::json::object const &object, std::char_t const *name
     if (!unknown) {
       return modulus::fail<void>(unknown.error());
     }
-    if (auto const *font = (*style)->if_contains("font")) {
-      auto const *text = font->if_string();
-      if (text == nullptr) {
-        return luma::detail::fail<void>(luma::detail::error_invalid_argument, std::string{name} + ".font must be a string");
-      }
-      target.font = std::string{text->c_str()};
+    auto font = modulus::json_config::optional_string_field(**style, "font", target.font, json_options);
+    if (!font) {
+      return modulus::fail<void>(font.error());
     }
-    if (auto const *size = (*style)->if_contains("size")) {
-      auto number = number_value(*size);
-      if (!number || *number <= 0.0) {
-        return luma::detail::fail<void>(luma::detail::error_invalid_argument, std::string{name} + ".size must be positive");
-      }
-      target.size = static_cast<std::float32_t>(*number);
+    target.font = std::move(*font);
+    if (auto status = apply_number(**style, "size", target.size); !status) {
+      return status;
     }
-    if (auto const *line_height = (*style)->if_contains("line_height")) {
-      auto number = number_value(*line_height);
-      if (!number || *number <= 0.0) {
-        return luma::detail::fail<void>(luma::detail::error_invalid_argument, std::string{name} + ".line_height must be positive");
-      }
-      target.line_height = static_cast<std::float32_t>(*number);
+    if (auto status = apply_number(**style, "line_height", target.line_height); !status) {
+      return status;
     }
   }
   return {};
 }
 
-auto number_value(boost::json::value const &value) -> std::optional<std::float64_t> {
-  if (auto const *number = value.if_double()) {
-    return *number;
-  }
-  if (auto const *integer = value.if_int64()) {
-    return static_cast<std::float64_t>(*integer);
-  }
-  return std::nullopt;
-}
-
 }
 
 auto luma::default_theme() -> luma::theme {
@@ -247,19 +234,11 @@ auto luma::parse_theme(boost::json::value const &value) -> std::expected<luma::t
   if (auto status = apply_text_style(**object, "title", parsed.title); !status) {
     return invalid_theme(std::string{status.error().message()});
   }
-  if (auto const *radius = (*object)->if_contains("radius")) {
-    auto number = number_value(*radius);
-    if (!number || *number < 0.0) {
-      return invalid_theme("theme.radius must be non-negative");
-    }
-    parsed.radius = static_cast<std::float32_t>(*number);
+  if (auto status = apply_number(**object, "radius", parsed.radius); !status) {
+    return std::unexpected(status.error());
   }
-  if (auto const *spacing = (*object)->if_contains("spacing")) {
-    auto number = number_value(*spacing);
-    if (!number || *number < 0.0) {
-      return invalid_theme("theme.spacing must be non-negative");
-    }
-    parsed.spacing = static_cast<std::float32_t>(*number);
+  if (auto status = apply_number(**object, "spacing", parsed.spacing); !status) {
+    return std::unexpected(status.error());
   }
   if (auto status = luma::validate(parsed); !status) {
     return invalid_theme(std::string{status.error().message()});

tests/test.cpp

@@ -564,7 +564,7 @@ auto closing_dialog_control::close_if(close_phase phase) const -> void {
     return;
   }
   state_->in_handle = true;
-  auto status = host_.close_dialog(luma::dialog_result::CANCELLED);
+  auto status = host_.close_dialog(luma::dialog_result::CANCELED);
   state_->status_ok = status.has_value();
   state_->in_handle = false;
 }
@@ -1085,6 +1085,63 @@ BOOST_AUTO_TEST_CASE(asset_cache_rejects_recursive_same_path_load) {
   BOOST_TEST(resources.recursive_code == 3);
 }
 
+BOOST_AUTO_TEST_CASE(asset_cache_clear_does_not_retain_an_in_flight_texture) {
+  auto resources = blocking_resources{};
+  auto assets = luma::asset_cache{resources};
+  auto first = std::expected<std::shared_ptr<luma::texture>, modulus::error>{};
+  auto thread = std::thread{[&] { first = assets.texture("page.png"); }};
+  auto entered = resources.wait_for_loads(1);
+  assets.clear();
+  resources.release();
+  thread.join();
+  BOOST_REQUIRE(entered);
+  BOOST_REQUIRE(first);
+  auto second = assets.texture("page.png");
+  BOOST_REQUIRE(second);
+  BOOST_TEST(resources.load_count() == 2);
+  BOOST_TEST(*first != *second);
+  BOOST_REQUIRE(assets.texture("page.png"));
+  BOOST_TEST(resources.load_count() == 2);
+}
+
+BOOST_AUTO_TEST_CASE(asset_cache_failed_loads_complete_and_can_be_retried) {
+  struct resources final : luma::renderer_resources {
+    std::uint32_t calls{};
+    auto load_texture(std::string const &path) -> std::expected<std::shared_ptr<luma::texture>, modulus::error> override {
+      if (++calls == 1U) {
+        if (path == "null") {
+          return std::shared_ptr<luma::texture>{};
+        }
+        if (path == "throw") {
+          throw std::runtime_error{"loader failed"};
+        }
+        return std::unexpected(modulus::make_error("loader", 5, "failed"));
+      }
+      return std::make_shared<fake_texture>();
+    }
+  };
+
+  for (auto const *path : {"null", "throw", "error"}) {
+    auto loader = resources{};
+    auto assets = luma::asset_cache{loader};
+    BOOST_CHECK(!assets.texture(path));
+    BOOST_REQUIRE(assets.texture(path));
+    BOOST_REQUIRE(assets.texture(path));
+    BOOST_TEST(loader.calls == 2U);
+  }
+}
+
+BOOST_AUTO_TEST_CASE(theme_numbers_reject_nonfinite_and_unrepresentable_values) {
+  for (auto number : {-1.0, std::numeric_limits<double>::infinity(), std::numeric_limits<double>::quiet_NaN(), std::numeric_limits<double>::max()}) {
+    auto parsed = luma::parse_theme(boost::json::object{{"spacing", number}});
+    BOOST_REQUIRE(!parsed);
+    BOOST_TEST(std::string{parsed.error().category()} == "luma");
+  }
+  auto parsed = luma::parse_theme(boost::json::object{{"spacing", std::uint64_t{8}}});
+  BOOST_REQUIRE(parsed);
+  BOOST_TEST(parsed->spacing == 8.0F);
+}
+
 BOOST_AUTO_TEST_CASE(text_input_state_filters_modes_and_keyboard_updates_bound_field) {
   auto changes = std::vector<std::string>{};
   auto input = luma::make_text_input({
@@ -1270,7 +1327,7 @@ BOOST_AUTO_TEST_CASE(dialog_accept_and_cancel_close_with_callbacks) {
       },
     },
   });
-  BOOST_TEST(static_cast<std::int32_t>(result) == static_cast<std::int32_t>(luma::dialog_result::CANCELLED));
+  BOOST_TEST(static_cast<std::int32_t>(result) == static_cast<std::int32_t>(luma::dialog_result::CANCELED));
 }
 
 BOOST_AUTO_TEST_CASE(dialog_uses_matching_touch_when_not_first_point) {