From 522254dfc12485d1dedbea7e38d12ea93ba2a42b Mon Sep 17 00:00:00 2001 From: khodowany Date: Thu, 24 Sep 2026 19:43:50 -0500 Subject: [PATCH] Cleanup --- .gitea/workflows/ci.yml | 4 +- conanfile.py | 5 +- include/luma/overlays.hpp | 2 +- modulus.py | 110 +++++++++++++------------------------- src/assets.cpp | 52 ++++-------------- src/overlays.cpp | 2 +- src/theme.cpp | 63 ++++++++-------------- tests/test.cpp | 61 ++++++++++++++++++++- 8 files changed, 136 insertions(+), 163 deletions(-) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index ebe6f23..eb389aa 100644 --- a/.gitea/workflows/ci.yml +++ b/.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 diff --git a/conanfile.py b/conanfile.py index dbd16d0..2a3845e 100644 --- a/conanfile.py +++ b/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", diff --git a/include/luma/overlays.hpp b/include/luma/overlays.hpp index bd7bdda..fad915d 100644 --- a/include/luma/overlays.hpp +++ b/include/luma/overlays.hpp @@ -20,7 +20,7 @@ namespace luma { enum class dialog_result { NONE, ACCEPTED, - CANCELLED, + CANCELED, }; struct dialog_options { diff --git a/modulus.py b/modulus.py index 1b69392..856cec7 100755 --- a/modulus.py +++ b/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", - "", - ], - ) + 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 diff --git a/src/assets.cpp b/src/assets.cpp index 50aaabf..edc9d2f 100644 --- a/src/assets.cpp +++ b/src/assets.cpp @@ -55,11 +55,7 @@ auto luma::asset_cache::texture(std::string const &path) -> std::expectedsecond != nullptr); return modulus::success(cached->second); } - if (!loading->result) { - return modulus::fail>(loading->result.error()); - } - assert(*loading->result != nullptr); - return modulus::success(*loading->result); + return loading->result; } loading = std::make_shared(); loading->owner = std::this_thread::get_id(); @@ -75,49 +71,23 @@ auto luma::asset_cache::texture(std::string const &path) -> std::expected>(luma::detail::error_internal, "texture load failed with an unknown exception"); } + if (loaded && !*loaded) { + loaded = luma::detail::fail>(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>(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>(luma::detail::error_not_found, "texture load returned no texture: " + path); - auto lock = std::scoped_lock{mutex_}; - loading->result = modulus::fail>(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 { diff --git a/src/overlays.cpp b/src/overlays.cpp index 06a2a76..07d6c84 100644 --- a/src/overlays.cpp +++ b/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()))) { diff --git a/src/theme.cpp b/src/theme.cpp index 7e02118..319d046 100644 --- a/src/theme.cpp +++ b/src/theme.cpp @@ -4,7 +4,7 @@ #include #include #include -#include +#include #include #include #include @@ -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; +auto apply_number(boost::json::object const &object, std::char_t const *name, std::float32_t &target) -> std::expected { + auto value = modulus::json_config::optional_float64_field(object, name, target, 0.0, std::numeric_limits::max(), json_options); + if (!value) { + return modulus::fail(value.error()); + } + target = static_cast(*value); + return {}; +} auto apply_text_style(boost::json::object const &object, std::char_t const *name, luma::text_style &target) -> std::expected { 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(unknown.error()); } - if (auto const *font = (*style)->if_contains("font")) { - auto const *text = font->if_string(); - if (text == nullptr) { - return luma::detail::fail(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(font.error()); } - if (auto const *size = (*style)->if_contains("size")) { - auto number = number_value(*size); - if (!number || *number <= 0.0) { - return luma::detail::fail(luma::detail::error_invalid_argument, std::string{name} + ".size must be positive"); - } - target.size = static_cast(*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(luma::detail::error_invalid_argument, std::string{name} + ".line_height must be positive"); - } - target.line_height = static_cast(*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 { - if (auto const *number = value.if_double()) { - return *number; - } - if (auto const *integer = value.if_int64()) { - return static_cast(*integer); - } - return std::nullopt; -} - } auto luma::default_theme() -> luma::theme { @@ -247,19 +234,11 @@ auto luma::parse_theme(boost::json::value const &value) -> std::expectedif_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(*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(*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()}); diff --git a/tests/test.cpp b/tests/test.cpp index 82615b6..85dc524 100644 --- a/tests/test.cpp +++ b/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, 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, modulus::error> override { + if (++calls == 1U) { + if (path == "null") { + return std::shared_ptr{}; + } + if (path == "throw") { + throw std::runtime_error{"loader failed"}; + } + return std::unexpected(modulus::make_error("loader", 5, "failed")); + } + return std::make_shared(); + } + }; + + 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::infinity(), std::numeric_limits::quiet_NaN(), std::numeric_limits::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{}; 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(result) == static_cast(luma::dialog_result::CANCELLED)); + BOOST_TEST(static_cast(result) == static_cast(luma::dialog_result::CANCELED)); } BOOST_AUTO_TEST_CASE(dialog_uses_matching_touch_when_not_first_point) {