Skip to content

Commit 85e1ec6

Browse files
hanatyan128hanatyan128
committed
refactor: Apply review findings across vendor request implementation
1 parent ed757e0 commit 85e1ec6

15 files changed

Lines changed: 387 additions & 109 deletions

‎.github/workflows/build-project.yaml‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,49 @@ jobs:
256256
name: ${{ steps.setup.outputs.pluginName }}-${{ steps.setup.outputs.pluginVersion }}-ubuntu-22.04-x86_64-${{ needs.check-event.outputs.commitHash }}-dbgsym
257257
path: ${{ github.workspace }}/release/${{ steps.setup.outputs.pluginName }}-${{ steps.setup.outputs.pluginVersion }}-x86_64*-dbgsym.ddeb
258258

259+
ubuntu-build-no-websocket:
260+
# Verifies the ENABLE_OBS_WEBSOCKET=OFF compile path. The checkout omits `submodules`
261+
# because the obs-websocket vendor sources are not consumed when the option is OFF.
262+
name: Verify ENABLE_OBS_WEBSOCKET=OFF Compile 🐧
263+
runs-on: ubuntu-22.04
264+
needs: check-event
265+
defaults:
266+
run:
267+
shell: bash
268+
steps:
269+
- uses: actions/checkout@v4
270+
with:
271+
fetch-depth: 1
272+
273+
- name: Install obs-studio Build Dependencies 🛍️
274+
run: |
275+
: Install obs-studio Build Dependencies 🛍️
276+
if [[ "${RUNNER_DEBUG}" ]]; then set -x; fi
277+
sudo add-apt-repository --yes ppa:obsproject/obs-studio
278+
sudo apt-get update
279+
sudo apt-get install --no-install-recommends -y \
280+
build-essential \
281+
ninja-build \
282+
libgles2-mesa-dev \
283+
obs-studio \
284+
qt6-base-dev \
285+
libqt6svg6-dev \
286+
qt6-base-private-dev
287+
288+
- name: Configure with ENABLE_OBS_WEBSOCKET=OFF 🛠️
289+
run: |
290+
: Configure with ENABLE_OBS_WEBSOCKET=OFF 🛠️
291+
if [[ "${RUNNER_DEBUG}" ]]; then set -x; fi
292+
cmake --preset linux-ci-x86_64 \
293+
-DENABLE_OBS_WEBSOCKET=OFF \
294+
-DCMAKE_INSTALL_PREFIX=/usr
295+
296+
- name: Build 🧱
297+
run: |
298+
: Build 🧱
299+
if [[ "${RUNNER_DEBUG}" ]]; then set -x; fi
300+
cmake --build --preset linux-ci-x86_64 --parallel
301+
259302
windows-build:
260303
name: Build for Windows 🪟
261304
runs-on: windows-2022

‎.github/workflows/check-format.yaml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,23 @@ jobs:
55
clang-format:
66
runs-on: ubuntu-24.04
77
steps:
8+
# `submodules: recursive` is intentionally omitted: faster checkout, and
9+
# the formatter source list does not include lib/, so vendored submodule
10+
# sources are not formatted regardless.
811
- uses: actions/checkout@v4
912
with:
1013
fetch-depth: 0
14+
- name: Verify Vendored Submodule Is Absent 🚫
15+
# Hard-asserts the no-submodule precondition. If a future change starts checking out
16+
# submodules, the formatter exclusion filters in build-aux/.run-format.zsh become the
17+
# only line of defense and a regression there would silently format vendored sources
18+
# in CI but not locally. Catch the drift early.
19+
run: |
20+
if [ -e lib/obs-websocket/lib/obs-websocket-api.h ]; then
21+
echo "::error::lib/obs-websocket/ submodule is present in the format-check checkout but should not be." >&2
22+
echo "::error::Either remove 'submodules' from the checkout step or update this guard." >&2
23+
exit 1
24+
fi
1125
- name: clang-format check 🐉
1226
id: clang-format
1327
uses: ./.github/actions/run-clang-format
@@ -17,9 +31,23 @@ jobs:
1731
cmake-format:
1832
runs-on: ubuntu-22.04
1933
steps:
34+
# `submodules: recursive` is intentionally omitted: faster checkout, and
35+
# the formatter source list does not include lib/, so vendored submodule
36+
# sources are not formatted regardless.
2037
- uses: actions/checkout@v4
2138
with:
2239
fetch-depth: 0
40+
- name: Verify Vendored Submodule Is Absent 🚫
41+
# Hard-asserts the no-submodule precondition. If a future change starts checking out
42+
# submodules, the formatter exclusion filters in build-aux/.run-format.zsh become the
43+
# only line of defense and a regression there would silently format vendored sources
44+
# in CI but not locally. Catch the drift early.
45+
run: |
46+
if [ -e lib/obs-websocket/lib/obs-websocket-api.h ]; then
47+
echo "::error::lib/obs-websocket/ submodule is present in the format-check checkout but should not be." >&2
48+
echo "::error::Either remove 'submodules' from the checkout step or update this guard." >&2
49+
exit 1
50+
fi
2351
- name: cmake-format check 🎛️
2452
id: cmake-format
2553
uses: ./.github/actions/run-cmake-format

‎.gitignore‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
!screenshot1.jpg
3030
!screenshot2.jpg
3131
!build.ps1
32+
# Submodules under lib/ use an explicit allowlist: `/lib/*` re-excludes everything under
33+
# lib/, so each new submodule must be re-included with its own `!/lib/<name>` line below.
34+
# Forgetting that line silently drops the submodule from `git add .`.
3235
!/lib
3336
/lib/*
3437
!/lib/obs-websocket

‎.gitmodules‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
[submodule "lib/obs-websocket"]
22
path = lib/obs-websocket
33
url = https://github.com/obsproject/obs-websocket.git
4-
# Pinned to tag: 5.7.3 (commit 1fcb95b)
5-
# Tag-pinned: do NOT use `git submodule update --remote`.
6-
# To update, fetch and checkout a specific release tag manually
7-
# (see https://github.com/obsproject/obs-websocket/tags).
4+
# Pinned to a specific release tag; do NOT use `git submodule update --remote`.
5+
# To bump the pinned tag, see the "Prerequisites" section in README.md.

‎CMakeLists.txt‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,15 @@ include(helpers)
1313

1414
option(ENABLE_OBS_WEBSOCKET "Enable obs-websocket vendor request integration" ON)
1515

16+
# When the submodule is missing, only this configure run is downgraded to OFF (a non-cache local variable). Writing OFF
17+
# to the cache here would persist past `git submodule update --init` until the user remembers to pass `--fresh` or to
18+
# delete the build dir.
1619
if(ENABLE_OBS_WEBSOCKET AND NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/lib/obs-websocket/lib/obs-websocket-api.h")
1720
message(
18-
FATAL_ERROR
19-
"ENABLE_OBS_WEBSOCKET is ON but lib/obs-websocket submodule is missing. Run: git submodule update --init --recursive (or pass -DENABLE_OBS_WEBSOCKET=OFF to disable obs-websocket vendor request integration)"
21+
WARNING
22+
"ENABLE_OBS_WEBSOCKET is ON but lib/obs-websocket submodule is missing; disabling integration for this configure run. Run: git submodule update --init lib/obs-websocket then re-run cmake with --fresh (or delete the build dir) to enable obs-websocket vendor request integration."
2023
)
24+
set(ENABLE_OBS_WEBSOCKET OFF)
2125
endif()
2226

2327
add_library(${CMAKE_PROJECT_NAME} MODULE)
@@ -44,6 +48,8 @@ if(ENABLE_QT)
4448
endif()
4549

4650
if(ENABLE_OBS_WEBSOCKET)
51+
# Consumed surface is `obs-websocket-api.h` only; the whole `lib/obs-websocket/lib` directory is intentionally exposed
52+
# as a SYSTEM include path because that header lives there and SYSTEM suppresses upstream warnings.
4753
target_include_directories(${CMAKE_PROJECT_NAME} SYSTEM PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/lib/obs-websocket/lib)
4854
target_compile_definitions(${CMAKE_PROJECT_NAME} PRIVATE ENABLE_OBS_WEBSOCKET)
4955
target_sources(${CMAKE_PROJECT_NAME} PRIVATE src/plugin-websocket.cpp)

‎build-aux/.run-format.zsh‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,15 @@ invoke_formatter() {
7070

7171
local -a source_files=(**/(CMakeLists.txt|*.cmake)(.N))
7272
source_files=(${source_files:#(build_*)/*})
73-
# Exclude vendored submodules: drop every file at any depth under lib/
74-
source_files=(${source_files:#lib/**/*})
73+
# Exclude vendored submodules. `${var:#pattern}` matches against the current shell's
74+
# option set (not this function's local `setopt`), so a manual prefix loop avoids
75+
# surprises if the matching logic is ever reused from a caller without EXTENDED_GLOB.
76+
local -a filtered=()
77+
local f
78+
for f (${source_files}) {
79+
if [[ ${f} != lib/* ]] filtered+=(${f})
80+
}
81+
source_files=(${filtered})
7582

7683
local -a format_args=()
7784
if (( _loglevel > 2 )) format_args+=(--log-level debug)

‎build.ps1‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,31 @@ try {
1111

1212
$OutputName = "${ProductName}-${ProductVersion}-windows-x64"
1313

14-
if (-not (Test-Path "lib/obs-websocket/lib/obs-websocket-api.h")) {
15-
git submodule update --init --recursive lib/obs-websocket
14+
# Source-archive builds have no `.git`; fall back to a header existence check
15+
# so `git submodule status` is not invoked outside a working git tree.
16+
$hasGit = $null -ne (Get-Command git -ErrorAction SilentlyContinue)
17+
if ((Test-Path ".git") -and $hasGit) {
18+
# Detect submodule state via the leading status char.
19+
# ' ' = up to date, '-' = uninitialized, '+' = out of date, 'U' = merge conflict.
20+
$submoduleStatus = git submodule status lib/obs-websocket
1621
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
22+
if ($submoduleStatus) {
23+
$statusChar = $submoduleStatus[0]
24+
if ($statusChar -eq 'U') {
25+
Write-Error "lib/obs-websocket submodule has merge conflicts; resolve them before building."
26+
exit 1
27+
} elseif ($statusChar -eq '-' -or $statusChar -eq '+') {
28+
git submodule update --init lib/obs-websocket
29+
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
30+
}
31+
}
32+
} elseif (-not (Test-Path "lib/obs-websocket/lib/obs-websocket-api.h")) {
33+
if ((Test-Path ".git") -and -not $hasGit) {
34+
Write-Error "git was not found on PATH; install git to update the lib/obs-websocket submodule, or pre-populate lib/obs-websocket/lib/obs-websocket-api.h before running build.ps1."
35+
} else {
36+
Write-Error "lib/obs-websocket sources are missing and this tree is not a git checkout. Configure with -DENABLE_OBS_WEBSOCKET=OFF or build from a tree that includes the submodule."
37+
}
38+
exit 1
1739
}
1840

1941
cmake --fresh -S . -B build_x64 -Wdev -Wdeprecated -DCMAKE_SYSTEM_VERSION="10.0.18363.657" -G "Visual Studio 17 2022" -A x64
@@ -29,4 +51,4 @@ try {
2951
}
3052
} finally {
3153
Pop-Location
32-
}
54+
}

‎cmake/linux/defaults.cmake‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,26 @@ set(CPACK_SOURCE_IGNORE_FILES
3636
\\.gitignore
3737
build_.*
3838
cmake/\\.CMakeBuildNumber
39-
lib/
39+
# Vendored obs-websocket: ship only `lib/obs-websocket/lib/` (the consumed header) plus `LICENSE` (required for
40+
# redistribution); exclude everything else.
41+
lib/obs-websocket/\\.clang-format
42+
lib/obs-websocket/\\.cmake-format\\.json
43+
lib/obs-websocket/\\.editorconfig
44+
lib/obs-websocket/\\.git
45+
lib/obs-websocket/\\.github/
46+
lib/obs-websocket/\\.gitignore
47+
lib/obs-websocket/\\.gitmodules
48+
lib/obs-websocket/\\.markdownlintignore
49+
lib/obs-websocket/\\.markdownlintrc
50+
lib/obs-websocket/CI/
51+
lib/obs-websocket/cmake/
52+
lib/obs-websocket/CMakeLists\\.txt
53+
lib/obs-websocket/data/
54+
lib/obs-websocket/deps/
55+
lib/obs-websocket/docs/
56+
lib/obs-websocket/lib/example/
57+
lib/obs-websocket/README\\.md
58+
lib/obs-websocket/src/
4059
release/)
4160

4261
set(CPACK_VERBATIM_VARIABLES YES)

‎src/UI/output-status-dock.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ with this program. If not, see <https://www.gnu.org/licenses/>
3232
#include <QMouseEvent>
3333
#include <QDesktopServices>
3434
#include <QSet>
35+
#include <QThread>
3536

3637
#include "../plugin-main.hpp"
3738
#include "output-status-dock.hpp"
@@ -369,6 +370,8 @@ void BranchOutputStatusDock::addRow(
369370

370371
void BranchOutputStatusDock::addFilter(BranchOutputFilter *filter)
371372
{
373+
Q_ASSERT(thread() == QThread::currentThread());
374+
372375
// Ensure filter removed
373376
removeFilter(filter);
374377

@@ -412,6 +415,8 @@ void BranchOutputStatusDock::addFilter(BranchOutputFilter *filter)
412415

413416
void BranchOutputStatusDock::removeFilter(BranchOutputFilter *filter)
414417
{
418+
Q_ASSERT(thread() == QThread::currentThread());
419+
415420
// DO NOT access filter resources at this time (It may be already deleted)
416421
foreach (auto row, outputTableRows) {
417422
if (row->filter == filter) {
@@ -1553,6 +1558,8 @@ void StatusCell::setTextValue(const QString &textValue)
15531558

15541559
QList<BranchOutputFilterInfo> BranchOutputStatusDock::getFilterList() const
15551560
{
1561+
Q_ASSERT(thread() == QThread::currentThread());
1562+
15561563
QList<BranchOutputFilterInfo> list;
15571564
QSet<obs_source_t *> seen;
15581565

‎src/UI/output-status-dock.hpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -233,9 +233,10 @@ class BranchOutputStatusDock : public QFrame {
233233
QTimer timer;
234234
QTableWidget *outputTable = nullptr;
235235
// Invariant: touched only from the Qt UI thread. Cross-thread callers
236-
// (addFilter / removeFilter / onFilterRenamed / getFilterList) route
237-
// through QMetaObject::invokeMethod, so no mutex is needed. Preserve
238-
// this invariant when adding new access paths.
236+
// (addFilter / removeFilter / getFilterList) route through
237+
// QMetaObject::invokeMethod, so no mutex is needed. Preserve this
238+
// invariant when adding new access paths; entry-point methods enforce it
239+
// with Q_ASSERT(thread() == QThread::currentThread()) in debug builds.
239240
QList<OutputTableRow *> outputTableRows;
240241
QLabel *applyToAllLabel = nullptr;
241242
QToolButton *enableAllButton = nullptr;

0 commit comments

Comments
 (0)