diff --git a/README.md b/README.md index 3f7b764..8c54d9d 100644 --- a/README.md +++ b/README.md @@ -33,10 +33,11 @@ under CI). Windows CI has **failed on every completed run so far**. The `-A x64` argument fix is now confirmed working — the run carrying it got as far as building libobs — but it exposed a deeper blocker: OBS 30.0.2's opt-in modern CMake path never defines the `OBS::w32-pthreads` target its own -Windows libobs links against. The PowerShell rewrite of the Windows steps is -still queued and unproven. See "Where the Windows bootstrap got to" under CI -below for the trace and the options, and check current CI status rather than -trusting this paragraph's age. +Windows libobs links against. A targeted bootstrap patch for that gap has been +pushed but not yet confirmed by a completed green run. The PowerShell rewrite +of the Windows steps is also still unproven. See "Where the Windows bootstrap +got to" under CI below for the trace and the options, and check current CI +status rather than trusting this paragraph's age. See "What is verified, and how" below for exactly what has and has not been checked, and "Testing this by hand" for what a human still needs to do. @@ -229,7 +230,7 @@ runners available to this repo under the `CyberCoveLLC` org. |---|---|---|---| | `linux` | `ubuntu-24.04` | `localhost.localdomain` | **Green.** Builds the real adapter against Ubuntu's libobs-dev 30.0.2, runs all six test suites, uploads `build/package` as an artifact | | `macos` | `macos-latest` | `home-mac` (Global) | **Green.** Builds libobs 30.0.2 from source, then the real adapter; 6/6 tests; artifact uploaded. But see the macOS packaging gap below | -| `windows` | `windows-latest` | `winvm-builder` (org-scoped) | **Failing** — every completed run on this branch has failed. The latest gets as far as building libobs and stops on an OBS-side `OBS::w32-pthreads` target that its own modern CMake path never defines; see below | +| `windows` | `windows-latest` | `winvm-builder` (org-scoped) | **Failing, fix pushed and awaiting a completed run.** Every completed run so far has failed; the latest got as far as building libobs and stopped on an OBS-side `OBS::w32-pthreads` target that its own modern CMake path never defines. A bootstrap patch for that gap has been pushed but not yet confirmed by a green run; see below | The Linux job is pinned to `ubuntu-24.04` rather than `ubuntu-latest`: this instance's two Linux runners answer `ubuntu-latest` with different releases, @@ -350,16 +351,53 @@ Deliberately **not** "fixed" by bumping the pin: 30.0.2, 30.1.2, 30.2.3, `OBS::w32-pthreads` from `libobs/cmake/os-windows.cmake` while none of them add `deps/w32-pthreads` from `libobs/CMakeLists.txt`. A version bump is therefore not obviously the answer and needs checking rather than assuming. -Options, roughly in order of preference: -1. Work out how obs-plugintemplate's own Windows CI satisfies this target at - its 31.1.1 pin — it builds `obs-frontend-api` rather than `libobs`, which - may pull in a different subdirectory set. If so, building that target (and - accepting the Qt dependency on Windows only) is the smallest change. -2. Have the bootstrap add `add_subdirectory(deps/w32-pthreads)` to the - extracted OBS tree before configuring. Effective, but a patch against a - third-party tree that must be carried across pin bumps. -3. Drop the from-source libobs on Windows and find a prebuilt OBS SDK. +Option 1 (build `obs-frontend-api`, matching obs-plugintemplate's own CI, and +accept the Qt dependency on Windows only) was investigated and **rejected**: +`UI/obs-frontend-api/CMakeLists.txt` only links `OBS::libobs`, nothing else — +it does not itself pull in `deps/w32-pthreads`. What actually satisfies the +target upstream is that `UI/CMakeLists.txt` returns early when +`ENABLE_UI=OFF`, *before* reaching `include(cmake/os-windows.cmake)` — the +file that (via its own `if(NOT TARGET OBS::w32-pthreads)` guard) adds +`deps/w32-pthreads`. Upstream's CI never sets `ENABLE_UI=OFF`, so that +add-as-a-side-effect-of-Qt always happens for them. Building +`obs-frontend-api` instead of `libobs` would not change any of that; the only +way to get the same side effect is to stop passing `-DENABLE_UI:BOOL=OFF`, +which is exactly the ~100 MB Qt6 download this bootstrap was trimmed to avoid +(see the Windows `buildspec.cmake` comment) and buys this plugin nothing, +since its properties UI is plain `obs_properties_*`. + +Went with Option 2 instead: `cmake/common/buildspec_common.cmake` now carries +`_patch_obs_studio_w32_pthreads()`, called for `OS_WINDOWS` right before +`_setup_obs_studio()`. It patches the freshly-extracted +`libobs/CMakeLists.txt` to add the one missing subdirectory itself, using the +exact same `if(NOT TARGET OBS::w32-pthreads)` guard +`UI/cmake/os-windows.cmake` already relies on upstream: + +```cmake +if(OS_WINDOWS) + if(NOT TARGET OBS::w32-pthreads) + add_subdirectory("${CMAKE_SOURCE_DIR}/deps/w32-pthreads" "${CMAKE_BINARY_DIR}/deps/w32-pthreads") + endif() + include(cmake/os-windows.cmake) +... +``` + +It is idempotent (checks for `deps/w32-pthreads` already present in the file +before patching, so re-running against a previously-patched extraction is a +no-op) and fails loudly with `FATAL_ERROR` if the anchor text it expects to +find is not there, rather than silently doing nothing on a future OBS version +whose `libobs/CMakeLists.txt` has changed shape. Verified locally (this is a +Linux sandbox, so only the CMake string-patching logic itself could be +checked, not a real Windows configure/build): ran the same `string(FIND)` +/`string(REPLACE)` sequence against the real `libobs/CMakeLists.txt` fetched +from the obs-studio 30.0.2 tag, confirmed it produces the intended +`if(OS_WINDOWS) / if(NOT TARGET ...) / add_subdirectory(...) / endif() / +include(...)` block, and confirmed a second run against the already-patched +file is a no-op. Whether this actually gets libobs through CMake generate and +building on a real Windows runner is the thing the next CI run needs to +prove — option 3 (dropping the from-source libobs on Windows for a prebuilt +SDK) remains the fallback if it does not. Two bugs of its own were found; the first is now proven fixed by `edb0c02` getting past it, the second is still unproven: diff --git a/cmake/common/buildspec_common.cmake b/cmake/common/buildspec_common.cmake index e5e35a2..feaff0b 100644 --- a/cmake/common/buildspec_common.cmake +++ b/cmake/common/buildspec_common.cmake @@ -24,6 +24,7 @@ # build tree, so it does not trip over the install rules of targets that # were deliberately never built. # 7. _resolve_versioned_macos_sdk exists at all -- see its own comment. +# 8. _patch_obs_studio_w32_pthreads exists at all -- see its own comment. # include_guard(GLOBAL) @@ -147,6 +148,70 @@ function(_resolve_versioned_macos_sdk out_path) endif() endfunction() +# _patch_obs_studio_w32_pthreads: Windows-only. Confirmed at every obs-studio +# tag from 30.0.2 through 31.1.1 (checked directly against +# libobs/cmake/os-windows.cmake and the top-level CMakeLists.txt at each tag): +# libobs/cmake/os-windows.cmake unconditionally links `OBS::w32-pthreads`, but +# that target is defined only by deps/w32-pthreads/CMakeLists.txt, and nothing +# in the OBS_CMAKE_VERSION>=3.0.0 ("modern") top-level CMakeLists.txt branch -- +# the one -DOBS_CMAKE_VERSION=3.0.0 selects -- ever adds deps/w32-pthreads. +# libobs/CMakeLists.txt itself only adds deps/libcaption and deps/uthash. +# +# Upstream obs-studio's own CI never hits this because it builds with +# ENABLE_UI left ON: UI/cmake/os-windows.cmake happens to add +# deps/w32-pthreads too (guarded by `if(NOT TARGET OBS::w32-pthreads)`), which +# satisfies libobs's link by the time CMake generates -- purely as a side +# effect of Qt still being in the build, not because anything wires +# w32-pthreads to libobs on purpose. This bootstrap deliberately builds with +# ENABLE_UI:BOOL=OFF (see the file header) specifically to avoid pulling in +# Qt6, so that side effect never happens here, and the gap is exposed. +# +# Rather than re-enable ENABLE_UI (and pay for a ~100 MB Qt6 download this +# plugin's plain obs_properties_* UI does not need) or vendor a full copy of +# deps/CMakeLists.txt, patch libobs/CMakeLists.txt to add the one missing +# subdirectory itself, using the exact same existence guard +# UI/cmake/os-windows.cmake already relies on. Idempotent: running this again +# against an already-patched tree is a no-op (the search text no longer +# matches), so it is safe to call on every configure regardless of whether +# .deps/ was freshly extracted or reused from a previous run. +function(_patch_obs_studio_w32_pthreads) + set(_cmakelists "${dependencies_dir}/${_obs_destination}/libobs/CMakeLists.txt") + if(NOT EXISTS "${_cmakelists}") + message(FATAL_ERROR "Cannot apply the w32-pthreads workaround: ${_cmakelists} does not exist.") + endif() + + file(READ "${_cmakelists}" _contents) + + string(FIND "${_contents}" "deps/w32-pthreads" _already_patched) + if(NOT _already_patched EQUAL -1) + message(STATUS "libobs/CMakeLists.txt already carries the w32-pthreads workaround - skipping") + return() + endif() + + set(_needle "if(OS_WINDOWS)\n include(cmake/os-windows.cmake)") + set(_replacement + "if(OS_WINDOWS)\n if(NOT TARGET OBS::w32-pthreads)\n add_subdirectory(\"\${CMAKE_SOURCE_DIR}/deps/w32-pthreads\" \"\${CMAKE_BINARY_DIR}/deps/w32-pthreads\")\n endif()\n include(cmake/os-windows.cmake)" + ) + + string(FIND "${_contents}" "${_needle}" _pos) + if(_pos EQUAL -1) + message( + FATAL_ERROR + "Could not find the expected 'if(OS_WINDOWS) / include(cmake/os-windows.cmake)' block in " + "${_cmakelists} to apply the w32-pthreads workaround. obs-studio's libobs/CMakeLists.txt " + "layout may have changed since this was written against 30.0.2." + ) + endif() + + string(REPLACE "${_needle}" "${_replacement}" _patched "${_contents}") + file(WRITE "${_cmakelists}" "${_patched}") + message( + STATUS + "Patched ${_cmakelists}: added deps/w32-pthreads so OBS::w32-pthreads exists " + "(obs-studio's modern Windows CMake path never defines it with ENABLE_UI=OFF)." + ) +endfunction() + # _setup_obs_studio: Create obs-studio build project, then build libobs and obs-frontend-api function(_setup_obs_studio) if(NOT libobs_DIR) @@ -366,5 +431,9 @@ function(_check_dependencies) set(CMAKE_PREFIX_PATH ${CMAKE_PREFIX_PATH} CACHE PATH "CMake prefix search path" FORCE) + if(OS_WINDOWS) + _patch_obs_studio_w32_pthreads() + endif() + _setup_obs_studio() endfunction()