From 4d24834a0a066a7af4a9c935e857a5972acd19c2 Mon Sep 17 00:00:00 2001 From: Esteban Zimanyi Date: Mon, 25 May 2026 21:38:46 +0200 Subject: [PATCH] icu: graceful fallback when unavailable + version-agnostic staging Make the single-timezone model resilient and multi-version-ready, consolidating the ICU test-staging work of #136 and #141. LoadInternal auto-loads ICU and sets the session TimeZone to Europe/Brussels. When ICU is unavailable (no on-disk copy AND no network egress: CI docker images, edge/musl deployments, offline installs) the TimeZone set threw out of LoadInternal and failed the whole extension load. Wrap the ICU autoload + TimeZone set so it degrades gracefully to the session default instead; the extension now always loads and only bare TIMESTAMPTZ display falls back to UTC when ICU is absent. Make the test-time ICU staging version- and platform-agnostic: derive the DuckDB version tag and platform from the freshly-built duckdb binary instead of hardcoding v1.4.4 + a uname map, so staging follows the v1.4.x LTS line and the v1.5.x multi-version matrix (the analog of MobilityDB's PostgreSQL 13-18 support). --- Makefile | 40 ++++++++++++++++++++++++++++++++++ src/mobilityduck_extension.cpp | 25 ++++++++++++++++----- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index bc17d050..5ddf037c 100644 --- a/Makefile +++ b/Makefile @@ -11,9 +11,49 @@ include extension-ci-tools/makefiles/duckdb_extension.Makefile # both MEOS (meos_initialize_timezone) and DuckDB (DBConfig::SetOptionByName # "TimeZone") to Europe/Brussels. Tests pass on any OS timezone — the # extension is the single source of truth, no TZ env var needed. +# +# LoadInternal also auto-loads ICU so the named "Europe/Brussels" zone +# resolves. DuckDB autoloads ICU from +# $HOME/.duckdb/extensions///icu.duckdb_extension +# and otherwise falls back to a hub download — which fails inside the CI test +# docker (empty path, no network egress) and on the macOS osx_arm64 runner +# (hub ICU not reliably resolvable). So we stage the locally-built ICU into the +# expected path before the unittester runs. +# +# Target DuckDB is the v1.4.x LTS line, with later versions (v1.5.x) supported +# in a multi-version matrix the same way MobilityDB supports PostgreSQL 13-18 — +# so the staging path must NOT hardcode the version or platform. We derive both +# from the freshly-built duckdb binary (authoritative for whatever is being +# tested); DUCKDB_VERSION_TAG and the uname map are fallbacks only. +DUCKDB_VERSION_TAG := v1.4.4 + +define stage_icu + @if [ -f ./build/$(1)/extension/icu/icu.duckdb_extension ]; then \ + duckdb_bin=./build/$(1)/duckdb; \ + version_tag=$$( [ -x "$$duckdb_bin" ] && "$$duckdb_bin" --version 2>/dev/null | grep -oE 'v[0-9]+\.[0-9]+\.[0-9]+' | head -1 ); \ + platform=$$( [ -x "$$duckdb_bin" ] && echo 'PRAGMA platform;' | "$$duckdb_bin" -noheader -list 2>/dev/null | tr -d '[:space:]' ); \ + [ -n "$$version_tag" ] || version_tag=$(DUCKDB_VERSION_TAG); \ + if [ -z "$$platform" ]; then \ + case "$$(uname -s)-$$(uname -m)" in \ + Linux-x86_64) platform=linux_amd64 ;; \ + Linux-aarch64) platform=linux_arm64 ;; \ + Darwin-arm64) platform=osx_arm64 ;; \ + Darwin-x86_64) platform=osx_amd64 ;; \ + *) platform=$$(uname -m) ;; \ + esac; \ + fi; \ + target=$$HOME/.duckdb/extensions/$$version_tag/$$platform; \ + mkdir -p "$$target" && cp -f ./build/$(1)/extension/icu/icu.duckdb_extension "$$target/" && \ + echo "Staged icu.duckdb_extension at $$target/ (duckdb $$version_tag / $$platform)"; \ + fi +endef + test_release_internal: + $(call stage_icu,release) ./build/release/$(TEST_PATH) "$(PROJ_DIR)test/*" test_debug_internal: + $(call stage_icu,debug) ./build/debug/$(TEST_PATH) "$(PROJ_DIR)test/*" test_reldebug_internal: + $(call stage_icu,reldebug) ./build/reldebug/$(TEST_PATH) "$(PROJ_DIR)test/*" \ No newline at end of file diff --git a/src/mobilityduck_extension.cpp b/src/mobilityduck_extension.cpp index 0eb7ebd4..c44ddc18 100644 --- a/src/mobilityduck_extension.cpp +++ b/src/mobilityduck_extension.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #if defined(_WIN32) @@ -227,12 +228,26 @@ static void LoadInternal(ExtensionLoader &loader) { // Single-timezone model: ensure DuckDB's session timezone matches the // MEOS timezone so bare TIMESTAMPTZ display agrees with MEOS composite - // type strings. Auto-load ICU (without it, the test framework keeps - // session timezone at UTC) and set the TimeZone option to Brussels. + // type strings. This needs ICU for the named "Europe/Brussels" zone. + // + // If ICU cannot be auto-loaded (no on-disk copy AND no network egress: + // CI docker images, edge/musl deployments, offline installs), degrade + // gracefully to the session default (UTC) instead of failing the whole + // extension load. Tests that assert Brussels display stage ICU locally + // via the Makefile's stage_icu. auto &db = loader.GetDatabaseInstance(); - ExtensionHelper::AutoLoadExtension(db, "icu"); - auto &config = DBConfig::GetConfig(db); - config.SetOptionByName("TimeZone", Value("Europe/Brussels")); + try { + ExtensionHelper::AutoLoadExtension(db, "icu"); + auto &config = DBConfig::GetConfig(db); + config.SetOptionByName("TimeZone", Value("Europe/Brussels")); + } catch (const std::exception &e) { + // ICU unavailable: leave the session timezone at its default. + // Temporal-type text I/O is unaffected; only bare TIMESTAMPTZ display + // falls back to UTC. + fprintf(stderr, + "mobilityduck: ICU not available (%s); session timezone left " + "at default instead of Europe/Brussels.\n", e.what()); + } // Register scalar function: mobilityduck_openssl_version