From b8166bd3b81988a142360c4c58677d3999847e19 Mon Sep 17 00:00:00 2001 From: David de Hilster Date: Tue, 14 Jul 2026 10:01:55 -0400 Subject: [PATCH] feat: load pre-compiled NLP++ analyzers (engine 3.7.11 + ICU preload) Bump the nlp-engine submodule to v3.7.11 (82ff50e), which loads a compiled analyzer/KB as a single combined DLL and lazy-opens *-full.dict/*-full.kbb at runtime in compiled mode. Fix the real reason analyze(..., compiled=True) failed with "[Couldn't load library: bin/kb.dll]": a compiled analyzer DLL imports icuin.dll + icuuc.dll by their real names and is dlopened by the engine via a plain LoadLibrary(fullpath), whose dependency resolution only consults already-loaded modules (by base name), the DLL's own dir, or PATH -- not os.add_dll_directory. delvewheel's default repair mangled the vendored ICU names and never vendored icuin (our extension only imports icuuc/icudt), so the analyzer's ICU imports went unresolved. - NLPPlus/__init__.py: _preload_icu() ctypes-loads every vendored icu*/msvcp* DLL by real name at import (dep order icudt -> icuuc -> icuin) so a compiled analyzer binds them from memory. - publish.yml: repair wheels with `delvewheel --no-mangle-all --add-dll icuin78.dll` so released wheels keep real ICU names and vendor icuin. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/publish.yml | 11 ++++++- NLPPlus/__init__.py | 58 +++++++++++++++++++++++++++++++++-- nlp-engine | 2 +- 3 files changed, 67 insertions(+), 4 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index e4ed450..1e5c80b 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -104,7 +104,16 @@ jobs: env: CIBW_BUILD_VERBOSITY: 3 CIBW_BEFORE_BUILD_WINDOWS: "pip install delvewheel" - CIBW_REPAIR_WHEEL_COMMAND_WINDOWS: "delvewheel repair --add-path ${{ github.workspace }}/vcpkg/installed/x64-windows-release/bin -w {dest_dir} {wheel}" + # --no-mangle-all + --add-dll icuin.dll are required so a *compiled* + # NLP++ analyzer (bin/run.dll / bin/kb.dll, dlopened at analyze time) + # can resolve its ICU imports. It imports icuin.dll + icuuc.dll + # by their real names; the default delvewheel run mangles those names and + # only vendors icuuc/icudt (what our extension itself imports), so the + # analyzer's icuin import goes unresolved and kb.dll fails to load. Keep + # real names (--no-mangle-all) and force-vendor icuin (--add-dll); the + # package then preloads them by name at import (NLPPlus._preload_icu). + # NOTE: bump "78" if the vcpkg ICU major version changes. + CIBW_REPAIR_WHEEL_COMMAND_WINDOWS: "delvewheel repair --no-mangle-all --add-dll icuin78.dll --add-path ${{ github.workspace }}/vcpkg/installed/x64-windows-release/bin -w {dest_dir} {wheel}" CIBW_CONFIG_SETTINGS: "cmake.args='-A x64;-DCMAKE_BUILD_TYPE=Release;-DCMAKE_TOOLCHAIN_FILE=${{ github.workspace }}/vcpkg/scripts/buildsystems/vcpkg.cmake'" - uses: actions/upload-artifact@v4 diff --git a/NLPPlus/__init__.py b/NLPPlus/__init__.py index 4fdaa00..d871cdf 100644 --- a/NLPPlus/__init__.py +++ b/NLPPlus/__init__.py @@ -18,11 +18,65 @@ import os import glob -from .bindings import NLP_ENGINE # type: ignore - LOGGER = logging.getLogger("NLPPlus") +def _preload_icu() -> None: + """Load the vendored ICU (and MSVC runtime) DLLs by their real names. + + A *compiled* NLP++ analyzer ships as a self-contained shared library + (``bin/run.`` / ``bin/kb.``) that imports ``icuin.dll`` + and ``icuuc.dll`` **by name**. The engine loads that library at + analyze time with a plain ``LoadLibrary(full_path)``, whose dependency + resolution only consults modules already loaded in the process (matched + by base name), the library's own directory, and the system path — it + does *not* consult ``os.add_dll_directory`` entries. + + Our own extension only pulls in ``icuuc`` (+ its ``icudt`` data), so + without help ``icuin`` is never loaded and the compiled KB library fails + with ``[Couldn't load library: ...\\bin\\kb.dll]``. Pre-loading every + vendored ``icu*``/``msvcp*`` DLL here, in dependency order and under + their real (un-mangled) names, makes them memory-resident so a compiled + analyzer binds them straight from memory. Requires the wheel to have + been repaired with ``--no-mangle-all`` (real names) and ``icuin`` force + added via ``--add-dll`` since it is not a dependency of our extension. + + No-op off Windows (Linux/macOS resolve ``.so``/``.dylib`` deps via + ``RPATH``/``@loader_path`` set by ``auditwheel``/``delocate``). + """ + if os.name != "nt": + return + import ctypes + + here = Path(__file__).resolve().parent + # delvewheel vendors DLLs into a sibling ``.libs`` folder. + libdirs = [ + here.parent / "nlpplus.libs", + here.parent / "NLPPlus.libs", + ] + for libs in libdirs: + if not libs.is_dir(): + continue + try: + os.add_dll_directory(str(libs)) + except (OSError, AttributeError): + pass + # Dependency order: data (icudt) -> common (icuuc) -> i18n (icuin). + # msvcp is pulled first as the C++ runtime the analyzer also imports. + for stem in ("msvcp", "vcruntime", "icudt", "icuuc", "icuin"): + for dll in sorted(libs.glob(stem + "*.dll")): + try: + ctypes.WinDLL(str(dll)) + except OSError: + pass + break + + +_preload_icu() + +from .bindings import NLP_ENGINE # type: ignore # noqa: E402 + + def maybe_readfile(path: Path) -> Optional[str]: """Bogus utility function to maybe read a file.""" if not path.exists(): diff --git a/nlp-engine b/nlp-engine index 7b2768d..82ff50e 160000 --- a/nlp-engine +++ b/nlp-engine @@ -1 +1 @@ -Subproject commit 7b2768dd1861601b3777e99e0d1f8cf991406c76 +Subproject commit 82ff50ec8764c24891a091f70a2ab161beb1edb6