From 6560b8b45156a5a327310aab491d153e432f9330 Mon Sep 17 00:00:00 2001 From: Alan George Date: Thu, 23 Apr 2026 12:30:07 -0600 Subject: [PATCH 01/21] Trying a build without cpp-linter ignore --- .github/workflows/builds.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index e580b73d..95305920 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -599,9 +599,9 @@ jobs: tidy-checks: '' database: build-release files-changed-only: false - lines-changed-only: true + lines-changed-only: false extensions: 'c,cpp,cc,cxx' - ignore: 'build-*|cpp-example-collection|client-sdk-rust|vcpkg_installed|src/tests|bridge|src/room_event_converter.cpp' + # ignore: file-annotations: true thread-comments: update step-summary: true From b9b4a35baca0203168fbd66a658983a2a16dcf39 Mon Sep 17 00:00:00 2001 From: Alan George Date: Thu, 23 Apr 2026 13:28:24 -0600 Subject: [PATCH 02/21] See if CI matches local --- .clang-tidy | 3 +- .github/workflows/builds.yml | 2 +- tidy.sh | 59 ++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) create mode 100755 tidy.sh diff --git a/.clang-tidy b/.clang-tidy index 44918701..65029ad6 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -29,7 +29,8 @@ WarningsAsErrors: > bugprone-string-literal-with-embedded-nul, bugprone-suspicious-memset-usage, -HeaderFilterRegex: '(include/livekit|src|examples)' +HeaderFilterRegex: '.*/(include/livekit|src)/.*\.(h|hpp)$' +ExcludeHeaderFilterRegex: '(.*/src/tests/.*)|(.*/_deps/.*)|(.*/build-[^/]*/.*)' FormatStyle: file diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 95305920..0226cf36 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -601,7 +601,7 @@ jobs: files-changed-only: false lines-changed-only: false extensions: 'c,cpp,cc,cxx' - # ignore: + ignore: 'build-*|cpp-example-collection|client-sdk-rust|vcpkg_installed|src/tests|bridge|examples|docker|data|docs' file-annotations: true thread-comments: update step-summary: true diff --git a/tidy.sh b/tidy.sh new file mode 100755 index 00000000..b06f1210 --- /dev/null +++ b/tidy.sh @@ -0,0 +1,59 @@ +#!/usr/bin/env bash +# +# tidy.sh -- Run clang-tidy locally using the same file set and config as CI. +# +# Matches the file filter used by the cpp-linter GitHub Action in +# .github/workflows/builds.yml: only src/**/*.{c,cpp,cc,cxx} excluding +# src/tests/. Picks up checks from the repo-root .clang-tidy automatically. +# +# Usage: +# ./tidy.sh # run on full src/ tree +# ./tidy.sh -j 4 # override parallelism +# ./tidy.sh -fix # auto-apply fixes (forwarded to run-clang-tidy) +# +# Requires CMake to have generated build-release/compile_commands.json. +# Run once: cmake --preset macos-release (or linux-release) + +set -euo pipefail + +BUILD_DIR="build-release" +# Positive match for top-level src/*.{c,cpp,cc,cxx}; negative lookahead excludes +# dep paths (_deps/, build-*/, -src/src/) and other top-level dirs that CI's +# cpp-linter `ignore:` list filters out. Python regex (PCRE-ish) supports +# lookahead; this regex is evaluated by run-clang-tidy. +FILE_REGEX='^(?!.*/(_deps|build-[^/]*|bridge|examples|client-sdk-rust|cpp-example-collection|vcpkg_installed|docker|docs|data)/).*/src/(?!tests/).*\.(c|cpp|cc|cxx)$' + +if [[ ! -f "${BUILD_DIR}/compile_commands.json" ]]; then + echo "ERROR: ${BUILD_DIR}/compile_commands.json not found." >&2 + echo "Run: cmake --preset macos-release (or linux-release)" >&2 + exit 1 +fi + +if ! command -v run-clang-tidy >/dev/null 2>&1; then + echo "ERROR: run-clang-tidy not found in PATH." >&2 + echo "Install LLVM: brew install llvm (macOS)" >&2 + echo " apt install clang-tidy (Linux)" >&2 + exit 1 +fi + +extra_args=() +if [[ "$(uname)" == "Darwin" ]]; then + sdk_path="$(xcrun --show-sdk-path 2>/dev/null || true)" + if [[ -n "${sdk_path}" ]]; then + extra_args+=(-extra-arg="-isysroot${sdk_path}") + fi +fi + +if command -v nproc >/dev/null 2>&1; then + jobs=$(nproc) +else + jobs=$(sysctl -n hw.ncpu 2>/dev/null || echo 4) +fi + +run-clang-tidy \ + -p "${BUILD_DIR}" \ + -quiet \ + -j "${jobs}" \ + "${extra_args[@]}" \ + "$@" \ + "${FILE_REGEX}" From 2336dca36f811ac787be714a5238f8c02f190c4d Mon Sep 17 00:00:00 2001 From: Alan George Date: Thu, 23 Apr 2026 16:26:31 -0600 Subject: [PATCH 03/21] Additional fixes --- .gitignore | 2 +- src/audio_stream.cpp | 12 ++-- src/data_stream.cpp | 12 ++-- src/data_track_stream.cpp | 10 +-- src/ffi_client.cpp | 10 +-- src/local_participant.cpp | 6 +- src/logging.cpp | 6 +- src/room.cpp | 92 +++++++++++++------------- src/subscription_thread_dispatcher.cpp | 40 +++++------ src/trace/event_tracer.cpp | 12 ++-- src/video_stream.cpp | 12 ++-- 11 files changed, 107 insertions(+), 107 deletions(-) diff --git a/.gitignore b/.gitignore index dc29c3fe..2a54c6ab 100644 --- a/.gitignore +++ b/.gitignore @@ -30,7 +30,7 @@ lib/ *.dylib *.dll *.exe -livekit.log +*.log web/ *trace.json compile_commands.json diff --git a/src/audio_stream.cpp b/src/audio_stream.cpp index 1f2a5798..07b1e693 100644 --- a/src/audio_stream.cpp +++ b/src/audio_stream.cpp @@ -55,7 +55,7 @@ AudioStream::fromParticipant(Participant &participant, TrackSource track_source, AudioStream::~AudioStream() { close(); } AudioStream::AudioStream(AudioStream &&other) noexcept { - std::lock_guard lock(other.mutex_); + const std::scoped_lock lock(other.mutex_); queue_ = std::move(other.queue_); capacity_ = other.capacity_; eof_ = other.eof_; @@ -76,8 +76,8 @@ AudioStream &AudioStream::operator=(AudioStream &&other) noexcept { close(); { - std::lock_guard lock_this(mutex_); - std::lock_guard lock_other(other.mutex_); + const std::scoped_lock lock_this(mutex_); + const std::scoped_lock lock_other(other.mutex_); queue_ = std::move(other.queue_); capacity_ = other.capacity_; @@ -110,7 +110,7 @@ bool AudioStream::read(AudioFrameEvent &out_event) { void AudioStream::close() { { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (closed_) { return; } @@ -221,7 +221,7 @@ void AudioStream::onFfiEvent(const FfiEvent &event) { void AudioStream::pushFrame(AudioFrameEvent &&ev) { { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (closed_ || eof_) { return; @@ -239,7 +239,7 @@ void AudioStream::pushFrame(AudioFrameEvent &&ev) { void AudioStream::pushEos() { { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (eof_) { return; } diff --git a/src/data_stream.cpp b/src/data_stream.cpp index 8b1a6d73..78df1e98 100644 --- a/src/data_stream.cpp +++ b/src/data_stream.cpp @@ -109,7 +109,7 @@ TextStreamReader::TextStreamReader(TextStreamInfo info) void TextStreamReader::onChunkUpdate(const std::string &text) { { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (closed_) return; queue_.push_back(text); @@ -120,7 +120,7 @@ void TextStreamReader::onChunkUpdate(const std::string &text) { void TextStreamReader::onStreamClose( const std::map &trailer_attrs) { { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); for (const auto &kv : trailer_attrs) { info_.attributes[kv.first] = kv.second; } @@ -154,7 +154,7 @@ ByteStreamReader::ByteStreamReader(ByteStreamInfo info) void ByteStreamReader::onChunkUpdate(const std::vector &bytes) { { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (closed_) return; queue_.push_back(bytes); @@ -165,7 +165,7 @@ void ByteStreamReader::onChunkUpdate(const std::vector &bytes) { void ByteStreamReader::onStreamClose( const std::map &trailer_attrs) { { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); for (const auto &kv : trailer_attrs) { info_.attributes[kv.first] = kv.second; } @@ -308,7 +308,7 @@ TextStreamWriter::TextStreamWriter( } void TextStreamWriter::write(const std::string &text) { - std::lock_guard lock(write_mutex_); + const std::scoped_lock lock(write_mutex_); if (closed_) throw std::runtime_error("Cannot write to closed TextStreamWriter"); @@ -339,7 +339,7 @@ ByteStreamWriter::ByteStreamWriter( } void ByteStreamWriter::write(const std::vector &data) { - std::lock_guard lock(write_mutex_); + const std::scoped_lock lock(write_mutex_); if (closed_) throw std::runtime_error("Cannot write to closed ByteStreamWriter"); diff --git a/src/data_track_stream.cpp b/src/data_track_stream.cpp index 94188284..30a355c8 100644 --- a/src/data_track_stream.cpp +++ b/src/data_track_stream.cpp @@ -38,7 +38,7 @@ void DataTrackStream::init(FfiHandle subscription_handle) { bool DataTrackStream::read(DataTrackFrame &out) { { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (closed_ || eof_) { return false; } @@ -70,7 +70,7 @@ bool DataTrackStream::read(DataTrackFrame &out) { void DataTrackStream::close() { std::int32_t listener_id = -1; { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (closed_) { return; } @@ -94,7 +94,7 @@ void DataTrackStream::onFfiEvent(const FfiEvent &event) { const auto &dts = event.data_track_stream_event(); { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (closed_ || dts.stream_handle() != static_cast(subscription_handle_.get())) { return; @@ -111,7 +111,7 @@ void DataTrackStream::onFfiEvent(const FfiEvent &event) { } void DataTrackStream::pushFrame(DataTrackFrame &&frame) { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (closed_ || eof_) { return; @@ -128,7 +128,7 @@ void DataTrackStream::pushFrame(DataTrackFrame &&frame) { void DataTrackStream::pushEos() { { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (eof_) { return; } diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index c2d1829a..f80c7c63 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -172,14 +172,14 @@ bool FfiClient::isInitialized() const noexcept { FfiClient::ListenerId FfiClient::AddListener(const FfiClient::Listener &listener) { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); FfiClient::ListenerId id = next_listener_id++; listeners_[id] = listener; return id; } void FfiClient::RemoveListener(ListenerId id) { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); listeners_.erase(id); } @@ -216,7 +216,7 @@ void FfiClient::PushEvent(const proto::FfiEvent &event) const { std::unique_ptr to_complete; std::vector listeners_copy; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); // Complete pending future if this event is a callback with async_id if (auto async_id = ExtractAsyncId(event)) { @@ -259,7 +259,7 @@ FfiClient::AsyncId FfiClient::generateAsyncId() { bool FfiClient::cancelPendingByAsyncId(AsyncId async_id) { std::unique_ptr to_cancel; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); auto it = pending_by_id_.find(async_id); if (it != pending_by_id_.end()) { to_cancel = std::move(it->second); @@ -283,7 +283,7 @@ std::future FfiClient::registerAsync( pending->match = std::move(match); pending->handler = std::move(handler); { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); pending_by_id_.emplace(async_id, std::move(pending)); } return fut; diff --git a/src/local_participant.cpp b/src/local_participant.cpp index 8fe31129..8c0d3b52 100644 --- a/src/local_participant.cpp +++ b/src/local_participant.cpp @@ -416,7 +416,7 @@ void LocalParticipant::handleRpcMethodInvocation( // Track this invocation and check if we're shutting down { - std::lock_guard lock(state->mutex); + const std::scoped_lock lock(state->mutex); if (state->shutting_down) { // Already shutting down, don't process new invocations return; @@ -430,7 +430,7 @@ void LocalParticipant::handleRpcMethodInvocation( struct InvocationGuard { std::shared_ptr state; ~InvocationGuard() { - std::lock_guard lock(state->mutex); + const std::scoped_lock lock(state->mutex); state->active_invocations--; if (state->active_invocations == 0) { state->cv.notify_all(); @@ -465,7 +465,7 @@ void LocalParticipant::handleRpcMethodInvocation( // Check again if shutdown started during handler execution { - std::lock_guard lock(state->mutex); + const std::scoped_lock lock(state->mutex); if (state->shutting_down) { // Shutdown started, don't send response - handle may be invalid return; diff --git a/src/logging.cpp b/src/logging.cpp index e11af570..f8001e7b 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -91,7 +91,7 @@ std::shared_ptr createDefaultLogger() { namespace detail { std::shared_ptr getLogger() { - std::lock_guard lock(loggerMutex()); + const std::scoped_lock lock(loggerMutex()); auto &logger = loggerStorage(); if (!logger) { logger = createDefaultLogger(); @@ -101,7 +101,7 @@ std::shared_ptr getLogger() { } void shutdownLogger() { - std::lock_guard lock(loggerMutex()); + const std::scoped_lock lock(loggerMutex()); auto &logger = loggerStorage(); if (logger) { spdlog::drop(kLoggerName); @@ -118,7 +118,7 @@ void setLogLevel(LogLevel level) { LogLevel getLogLevel() { return fromSpdlogLevel(detail::getLogger()->level()); } void setLogCallback(LogCallback callback) { - std::lock_guard lock(loggerMutex()); + const std::scoped_lock lock(loggerMutex()); auto &logger = loggerStorage(); auto current_level = logger ? logger->level() : spdlog::level::info; diff --git a/src/room.cpp b/src/room.cpp index f735150a..d079b2ab 100644 --- a/src/room.cpp +++ b/src/room.cpp @@ -80,7 +80,7 @@ Room::~Room() { int listener_to_remove = 0; std::unique_ptr local_participant_to_cleanup; { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); listener_to_remove = listener_id_; listener_id_ = 0; // Move local participant out for cleanup outside the lock @@ -102,7 +102,7 @@ Room::~Room() { } void Room::setDelegate(RoomDelegate *delegate) { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); delegate_ = delegate; } @@ -111,7 +111,7 @@ bool Room::Connect(const std::string &url, const std::string &token, TRACE_EVENT0("livekit", "Room::Connect"); { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); if (connection_state_ != ConnectionState::Disconnected) { throw std::runtime_error("already connected"); } @@ -154,7 +154,7 @@ bool Room::Connect(const std::string &url, const std::string &token, new_remote_participants; { const auto &participants = connectCb.result().participants(); - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); for (const auto &pt : participants) { const auto &owned = pt.participant(); auto rp = createRemoteParticipant(owned); @@ -180,7 +180,7 @@ bool Room::Connect(const std::string &url, const std::string &token, // Publish all state atomically under lock { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); room_handle_ = std::move(new_room_handle); room_info_ = std::move(new_room_info); local_participant_ = std::move(new_local_participant); @@ -193,7 +193,7 @@ bool Room::Connect(const std::string &url, const std::string &token, auto listenerId = FfiClient::instance().AddListener( [this](const proto::FfiEvent &e) { OnEvent(e); }); { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); listener_id_ = listenerId; } @@ -207,24 +207,24 @@ bool Room::Connect(const std::string &url, const std::string &token, } RoomInfoData Room::room_info() const { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); return room_info_; } LocalParticipant *Room::localParticipant() const { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); return local_participant_.get(); } RemoteParticipant *Room::remoteParticipant(const std::string &identity) const { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); auto it = remote_participants_.find(identity); return it == remote_participants_.end() ? nullptr : it->second.get(); } std::vector> Room::remoteParticipants() const { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); std::vector> out; out.reserve(remote_participants_.size()); for (const auto &kv : remote_participants_) { @@ -234,13 +234,13 @@ Room::remoteParticipants() const { } E2EEManager *Room::e2eeManager() const { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); return e2ee_manager_.get(); } void Room::registerTextStreamHandler(const std::string &topic, TextStreamHandler handler) { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); auto [it, inserted] = text_stream_handlers_.emplace(topic, std::move(handler)); if (!inserted) { @@ -250,13 +250,13 @@ void Room::registerTextStreamHandler(const std::string &topic, } void Room::unregisterTextStreamHandler(const std::string &topic) { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); text_stream_handlers_.erase(topic); } void Room::registerByteStreamHandler(const std::string &topic, ByteStreamHandler handler) { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); auto [it, inserted] = byte_stream_handlers_.emplace(topic, std::move(handler)); if (!inserted) { @@ -266,7 +266,7 @@ void Room::registerByteStreamHandler(const std::string &topic, } void Room::unregisterByteStreamHandler(const std::string &topic) { - std::lock_guard g(lock_); + const std::scoped_lock g(lock_); byte_stream_handlers_.erase(topic); } @@ -368,7 +368,7 @@ void Room::OnEvent(const FfiEvent &event) { // lock. RoomDelegate *delegate_snapshot = nullptr; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); delegate_snapshot = delegate_; } @@ -378,7 +378,7 @@ void Room::OnEvent(const FfiEvent &event) { LocalParticipant *lp = nullptr; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); if (!local_participant_) { return; } @@ -407,7 +407,7 @@ void Room::OnEvent(const FfiEvent &event) { // Check if this event is for our room handle { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); if (!room_handle_ || re.room_handle() != static_cast(room_handle_->get())) { return; @@ -418,7 +418,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kParticipantConnected: { std::shared_ptr new_participant; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &owned = re.participant_connected().info(); // createRemoteParticipant takes proto::OwnedParticipant new_participant = createRemoteParticipant(owned); @@ -436,7 +436,7 @@ void Room::OnEvent(const FfiEvent &event) { std::shared_ptr removed; DisconnectReason reason = DisconnectReason::Unknown; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &pd = re.participant_disconnected(); const std::string &identity = pd.participant_identity(); reason = toDisconnectReason(pd.disconnect_reason()); @@ -466,7 +466,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kLocalTrackPublished: { LocalTrackPublishedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); if (!local_participant_) { LK_LOG_ERROR("kLocalTrackPublished: local_participant_ is nullptr"); break; @@ -490,7 +490,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kLocalTrackUnpublished: { LocalTrackUnpublishedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); if (!local_participant_) { LK_LOG_ERROR("kLocalTrackUnpublished: local_participant_ is nullptr"); break; @@ -514,7 +514,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kLocalTrackSubscribed: { LocalTrackSubscribedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); if (!local_participant_) { break; } @@ -538,7 +538,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kTrackPublished: { TrackPublishedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &tp = re.track_published(); const std::string &identity = tp.participant_identity(); auto it = remote_participants_.find(identity); @@ -567,7 +567,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kTrackUnpublished: { TrackUnpublishedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &tu = re.track_unpublished(); const std::string &identity = tu.participant_identity(); const std::string &pub_sid = tu.publication_sid(); @@ -605,7 +605,7 @@ void Room::OnEvent(const FfiEvent &event) { RemoteParticipant *rparticipant = nullptr; std::shared_ptr remote_track; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); // Find participant auto pit = remote_participants_.find(identity); if (pit == remote_participants_.end()) { @@ -660,7 +660,7 @@ void Room::OnEvent(const FfiEvent &event) { TrackSource unsub_source = TrackSource::SOURCE_UNKNOWN; std::string unsub_identity; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &tu = re.track_unsubscribed(); unsub_identity = tu.participant_identity(); const std::string &track_sid = tu.track_sid(); @@ -704,7 +704,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kTrackSubscriptionFailed: { TrackSubscriptionFailedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &tsf = re.track_subscription_failed(); const std::string &identity = tsf.participant_identity(); auto pit = remote_participants_.find(identity); @@ -756,7 +756,7 @@ void Room::OnEvent(const FfiEvent &event) { TrackMutedEvent ev; bool success = false; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &tm = re.track_muted(); const std::string &identity = tm.participant_identity(); const std::string &sid = tm.track_sid(); @@ -795,7 +795,7 @@ void Room::OnEvent(const FfiEvent &event) { TrackUnmutedEvent ev; bool success = false; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &tu = re.track_unmuted(); const std::string &identity = tu.participant_identity(); const std::string &sid = tu.track_sid(); @@ -838,7 +838,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kActiveSpeakersChanged: { ActiveSpeakersChangedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &asc = re.active_speakers_changed(); for (const auto &identity : asc.participant_identities()) { Participant *participant = nullptr; @@ -864,7 +864,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kRoomMetadataChanged: { RoomMetadataChangedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto old_metadata = room_info_.metadata; room_info_.metadata = re.room_metadata_changed().metadata(); ev.old_metadata = old_metadata; @@ -878,7 +878,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kRoomSidChanged: { RoomSidChangedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); room_info_.sid = re.room_sid_changed().sid(); ev.sid = room_info_.sid.value_or(std::string{}); } @@ -890,7 +890,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kParticipantMetadataChanged: { ParticipantMetadataChangedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &pm = re.participant_metadata_changed(); const std::string &identity = pm.participant_identity(); Participant *participant = nullptr; @@ -923,7 +923,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kParticipantNameChanged: { ParticipantNameChangedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &pn = re.participant_name_changed(); const std::string &identity = pn.participant_identity(); Participant *participant = nullptr; @@ -954,7 +954,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kParticipantAttributesChanged: { ParticipantAttributesChangedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &pa = re.participant_attributes_changed(); const std::string &identity = pa.participant_identity(); Participant *participant = nullptr; @@ -993,7 +993,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kParticipantEncryptionStatusChanged: { ParticipantEncryptionStatusChangedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &pe = re.participant_encryption_status_changed(); const std::string &identity = pe.participant_identity(); Participant *participant = nullptr; @@ -1023,7 +1023,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kConnectionQualityChanged: { ConnectionQualityChangedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &cq = re.connection_quality_changed(); const std::string &identity = cq.participant_identity(); Participant *participant = nullptr; @@ -1057,7 +1057,7 @@ void Room::OnEvent(const FfiEvent &event) { const auto &dp = re.data_packet_received(); RemoteParticipant *rp = nullptr; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); auto it = remote_participants_.find(dp.participant_identity()); if (it != remote_participants_.end()) { rp = it->second.get(); @@ -1082,7 +1082,7 @@ void Room::OnEvent(const FfiEvent &event) { E2eeStateChangedEvent ev; { LK_LOG_DEBUG("e2ee_state_changed for participant"); - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &es = re.e2ee_state_changed(); const std::string &identity = es.participant_identity(); Participant *participant = nullptr; @@ -1116,7 +1116,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kConnectionStateChanged: { ConnectionStateChangedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &cs = re.connection_state_changed(); // TODO, maybe we should update our |connection_state_| // correspoindingly, but the this kConnectionStateChanged event is never @@ -1173,7 +1173,7 @@ void Room::OnEvent(const FfiEvent &event) { old_byte_readers; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); listener_to_remove = listener_id_; listener_id_ = 0; @@ -1218,7 +1218,7 @@ void Room::OnEvent(const FfiEvent &event) { std::shared_ptr text_reader; std::shared_ptr byte_reader; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); // Determine stream type from oneof in protobuf // Adjust these names if your generated C++ uses different ones @@ -1265,7 +1265,7 @@ void Room::OnEvent(const FfiEvent &event) { std::shared_ptr text_reader; std::shared_ptr byte_reader; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); auto itT = text_stream_readers_.find(chunk.stream_id()); if (itT != text_stream_readers_.end()) { text_reader = itT->second; @@ -1297,7 +1297,7 @@ void Room::OnEvent(const FfiEvent &event) { trailer_attrs.emplace(kv.first, kv.second); } { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); auto itT = text_stream_readers_.find(trailer.stream_id()); if (itT != text_stream_readers_.end()) { text_reader = itT->second; @@ -1356,7 +1356,7 @@ void Room::OnEvent(const FfiEvent &event) { case proto::RoomEvent::kParticipantsUpdated: { ParticipantsUpdatedEvent ev; { - std::lock_guard guard(lock_); + const std::scoped_lock guard(lock_); const auto &pu = re.participants_updated(); for (const auto &info : pu.participants()) { const std::string &identity = info.identity(); diff --git a/src/subscription_thread_dispatcher.cpp b/src/subscription_thread_dispatcher.cpp index b1d2b603..d5e88a92 100644 --- a/src/subscription_thread_dispatcher.cpp +++ b/src/subscription_thread_dispatcher.cpp @@ -55,7 +55,7 @@ void SubscriptionThreadDispatcher::setOnAudioFrameCallback( const std::string &participant_identity, TrackSource source, AudioFrameCallback callback, AudioStream::Options opts) { const CallbackKey key{participant_identity, source, ""}; - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); const bool replacing = audio_callbacks_.find(key) != audio_callbacks_.end(); audio_callbacks_[key] = RegisteredAudioCallback{std::move(callback), std::move(opts)}; @@ -70,7 +70,7 @@ void SubscriptionThreadDispatcher::setOnAudioFrameCallback( AudioFrameCallback callback, AudioStream::Options opts) { const CallbackKey key{participant_identity, TrackSource::SOURCE_UNKNOWN, track_name}; - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); const bool replacing = audio_callbacks_.find(key) != audio_callbacks_.end(); audio_callbacks_[key] = RegisteredAudioCallback{std::move(callback), std::move(opts)}; @@ -84,7 +84,7 @@ void SubscriptionThreadDispatcher::setOnVideoFrameCallback( const std::string &participant_identity, TrackSource source, VideoFrameCallback callback, VideoStream::Options opts) { const CallbackKey key{participant_identity, source, ""}; - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); const bool replacing = video_callbacks_.find(key) != video_callbacks_.end(); video_callbacks_[key] = RegisteredVideoCallback{std::move(callback), opts}; @@ -99,7 +99,7 @@ void SubscriptionThreadDispatcher::setOnVideoFrameCallback( VideoFrameCallback callback, VideoStream::Options opts) { const CallbackKey key{participant_identity, TrackSource::SOURCE_UNKNOWN, track_name}; - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); const bool replacing = video_callbacks_.find(key) != video_callbacks_.end(); video_callbacks_[key] = RegisteredVideoCallback{std::move(callback), opts}; @@ -115,7 +115,7 @@ void SubscriptionThreadDispatcher::clearOnAudioFrameCallback( std::thread old_thread; bool removed_callback = false; { - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); removed_callback = audio_callbacks_.erase(key) > 0; old_thread = extractReaderThreadLocked(key); LK_LOG_DEBUG( @@ -136,7 +136,7 @@ void SubscriptionThreadDispatcher::clearOnAudioFrameCallback( std::thread old_thread; bool removed_callback = false; { - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); removed_callback = audio_callbacks_.erase(key) > 0; old_thread = extractReaderThreadLocked(key); LK_LOG_DEBUG( @@ -156,7 +156,7 @@ void SubscriptionThreadDispatcher::clearOnVideoFrameCallback( std::thread old_thread; bool removed_callback = false; { - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); removed_callback = video_callbacks_.erase(key) > 0; old_thread = extractReaderThreadLocked(key); LK_LOG_DEBUG( @@ -177,7 +177,7 @@ void SubscriptionThreadDispatcher::clearOnVideoFrameCallback( std::thread old_thread; bool removed_callback = false; { - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); removed_callback = video_callbacks_.erase(key) > 0; old_thread = extractReaderThreadLocked(key); LK_LOG_DEBUG( @@ -211,7 +211,7 @@ void SubscriptionThreadDispatcher::handleTrackSubscribed( const CallbackKey fallback_key{participant_identity, source, ""}; std::thread old_thread; { - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); if ((track->kind() == TrackKind::KIND_AUDIO && audio_callbacks_.find(key) == audio_callbacks_.end()) || (track->kind() == TrackKind::KIND_VIDEO && @@ -234,7 +234,7 @@ void SubscriptionThreadDispatcher::handleTrackUnsubscribed( std::thread old_thread; std::thread fallback_old_thread; { - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); old_thread = extractReaderThreadLocked(key); fallback_old_thread = extractReaderThreadLocked(fallback_key); LK_LOG_DEBUG("Handling unsubscribed track for participant={} source={} " @@ -260,7 +260,7 @@ DataFrameCallbackId SubscriptionThreadDispatcher::addOnDataFrameCallback( std::thread old_thread; DataFrameCallbackId id; { - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); id = next_data_callback_id_++; const DataCallbackKey key{participant_identity, track_name}; data_callbacks_[id] = RegisteredDataCallback{key, std::move(callback)}; @@ -281,7 +281,7 @@ void SubscriptionThreadDispatcher::removeOnDataFrameCallback( DataFrameCallbackId id) { std::thread old_thread; { - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); data_callbacks_.erase(id); old_thread = extractDataReaderThreadLocked(id); } @@ -303,7 +303,7 @@ void SubscriptionThreadDispatcher::handleDataTrackPublished( std::vector old_threads; { - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); const DataCallbackKey key{track->publisherIdentity(), track->info().name}; remote_data_tracks_[key] = track; @@ -327,13 +327,13 @@ void SubscriptionThreadDispatcher::handleDataTrackUnpublished( std::vector old_threads; { - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); for (auto it = active_data_readers_.begin(); it != active_data_readers_.end();) { auto &reader = it->second; if (reader->remote_track && reader->remote_track->info().sid == sid) { { - const std::lock_guard sub_guard(reader->sub_mutex); + const std::scoped_lock sub_guard(reader->sub_mutex); if (reader->stream) { reader->stream->close(); } @@ -362,7 +362,7 @@ void SubscriptionThreadDispatcher::handleDataTrackUnpublished( void SubscriptionThreadDispatcher::stopAll() { std::vector threads; { - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); LK_LOG_DEBUG("Stopping all subscription readers active_readers={} " "active_data_readers={} audio_callbacks={} " "video_callbacks={} data_callbacks={}", @@ -387,7 +387,7 @@ void SubscriptionThreadDispatcher::stopAll() { for (auto &[id, reader] : active_data_readers_) { { - const std::lock_guard sub_guard(reader->sub_mutex); + const std::scoped_lock sub_guard(reader->sub_mutex); if (reader->stream) { reader->stream->close(); } @@ -589,7 +589,7 @@ std::thread SubscriptionThreadDispatcher::extractDataReaderThreadLocked( auto reader = std::move(it->second); active_data_readers_.erase(it); { - const std::lock_guard guard(reader->sub_mutex); + const std::scoped_lock guard(reader->sub_mutex); if (reader->stream) { reader->stream->close(); } @@ -608,7 +608,7 @@ std::thread SubscriptionThreadDispatcher::extractDataReaderThreadLocked( auto reader = std::move(it->second); active_data_readers_.erase(it); { - const std::lock_guard guard(reader->sub_mutex); + const std::scoped_lock guard(reader->sub_mutex); if (reader->stream) { reader->stream->close(); } @@ -660,7 +660,7 @@ std::thread SubscriptionThreadDispatcher::startDataReaderLocked( identity, track_name); { - const std::lock_guard guard(reader->sub_mutex); + const std::scoped_lock guard(reader->sub_mutex); reader->stream = stream; } diff --git a/src/trace/event_tracer.cpp b/src/trace/event_tracer.cpp index cfcc267c..66f5caa7 100644 --- a/src/trace/event_tracer.cpp +++ b/src/trace/event_tracer.cpp @@ -257,7 +257,7 @@ void WriterThreadFunc() { // Exit if shutdown requested and queue is empty if (g_shutdown_requested.load()) { - std::lock_guard lock(g_mutex); + const std::scoped_lock lock(g_mutex); if (g_event_queue.empty()) { break; } @@ -269,7 +269,7 @@ void WriterThreadFunc() { void SetupEventTracer(GetCategoryEnabledPtr get_category_enabled_ptr, AddTraceEventPtr add_trace_event_ptr) { - std::lock_guard lock(g_mutex); + const std::scoped_lock lock(g_mutex); g_custom_get_category_enabled = get_category_enabled_ptr; g_custom_add_trace_event = add_trace_event_ptr; } @@ -286,7 +286,7 @@ const unsigned char *EventTracer::GetCategoryEnabled(const char *name) { } // Check if category is enabled - std::lock_guard lock(g_mutex); + const std::scoped_lock lock(g_mutex); // Empty enabled set means all categories are enabled if (g_enabled_categories.empty()) { @@ -366,7 +366,7 @@ void EventTracer::AddTraceEvent(char phase, // Queue the event for the writer thread { - std::lock_guard lock(g_mutex); + const std::scoped_lock lock(g_mutex); g_event_queue.push(std::move(event)); } g_cv.notify_one(); @@ -376,7 +376,7 @@ namespace internal { bool StartTracing(const std::string &file_path, const std::vector &categories) { - std::lock_guard lock(g_mutex); + const std::scoped_lock lock(g_mutex); // Don't start if already running if (g_tracing_enabled.load()) { @@ -426,7 +426,7 @@ void StopTracing() { } // Close the file with JSON footer - std::lock_guard lock(g_mutex); + const std::scoped_lock lock(g_mutex); if (g_trace_file.is_open()) { g_trace_file << R"(],"displayTimeUnit":"ms"})"; g_trace_file.close(); diff --git a/src/video_stream.cpp b/src/video_stream.cpp index 90992331..49b4163f 100644 --- a/src/video_stream.cpp +++ b/src/video_stream.cpp @@ -50,7 +50,7 @@ VideoStream::fromParticipant(Participant &participant, TrackSource track_source, VideoStream::~VideoStream() { close(); } VideoStream::VideoStream(VideoStream &&other) noexcept { - std::lock_guard lock(other.mutex_); + const std::scoped_lock lock(other.mutex_); queue_ = std::move(other.queue_); capacity_ = other.capacity_; eof_ = other.eof_; @@ -69,8 +69,8 @@ VideoStream &VideoStream::operator=(VideoStream &&other) noexcept { close(); { - std::lock_guard lock_this(mutex_); - std::lock_guard lock_other(other.mutex_); + const std::scoped_lock lock_this(mutex_); + const std::scoped_lock lock_other(other.mutex_); queue_ = std::move(other.queue_); capacity_ = other.capacity_; @@ -104,7 +104,7 @@ bool VideoStream::read(VideoFrameEvent &out) { void VideoStream::close() { { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (closed_) { return; } @@ -210,7 +210,7 @@ void VideoStream::onFfiEvent(const proto::FfiEvent &event) { void VideoStream::pushFrame(VideoFrameEvent &&ev) { { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (closed_ || eof_) { return; @@ -228,7 +228,7 @@ void VideoStream::pushFrame(VideoFrameEvent &&ev) { void VideoStream::pushEos() { { - std::lock_guard lock(mutex_); + const std::scoped_lock lock(mutex_); if (eof_) { return; } From 2a57f85075100daab6c0f445a1ca0b54bc8f6cf0 Mon Sep 17 00:00:00 2001 From: Alan George Date: Thu, 23 Apr 2026 16:57:00 -0600 Subject: [PATCH 04/21] Try custom clang CI job --- .github/workflows/builds.yml | 47 ++++--------- tidy.sh | 130 +++++++++++++++++++++++++++++++---- 2 files changed, 131 insertions(+), 46 deletions(-) diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 0226cf36..5c380e2c 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -553,7 +553,6 @@ jobs: continue-on-error: false permissions: contents: read - pull-requests: write steps: - name: Checkout (with submodules) @@ -568,8 +567,16 @@ jobs: sudo apt-get update sudo apt-get install -y \ build-essential cmake ninja-build pkg-config \ - llvm-dev libclang-dev clang clang-tidy \ + llvm-dev libclang-dev clang clang-tidy clang-tools \ libssl-dev + # run-clang-tidy ships as run-clang-tidy- on apt; expose an + # unsuffixed name so tidy.sh works unchanged across environments. + if ! command -v run-clang-tidy >/dev/null 2>&1; then + rct_versioned=$(ls /usr/bin/run-clang-tidy-* 2>/dev/null | sort -V | tail -1) + if [ -n "${rct_versioned}" ]; then + sudo ln -sf "${rct_versioned}" /usr/local/bin/run-clang-tidy + fi + fi - name: Install Rust (stable) uses: dtolnay/rust-toolchain@3c5f7ea28cd621ae0bf5283f0e981fb97b8a7af9 @@ -590,33 +597,9 @@ jobs: run: cmake --build build-release --target livekit_proto - name: Run clang-tidy - uses: cpp-linter/cpp-linter-action@77c390c5ba9c947ebc185a3e49cc754f1558abb5 # v2.18.0 - id: linter - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - with: - style: '' - tidy-checks: '' - database: build-release - files-changed-only: false - lines-changed-only: false - extensions: 'c,cpp,cc,cxx' - ignore: 'build-*|cpp-example-collection|client-sdk-rust|vcpkg_installed|src/tests|bridge|examples|docker|data|docs' - file-annotations: true - thread-comments: update - step-summary: true - tidy-review: false - no-lgtm: true - jobs: 0 # 0 == use all available CPU cores - - - name: Check warning thresholds - env: - TIDY_FINDINGS: ${{ steps.linter.outputs.clang-tidy-checks-failed }} - MAX_TIDY_FINDINGS: '0' - run: | - echo "clang-tidy findings: ${TIDY_FINDINGS}" - if [ "${TIDY_FINDINGS}" -gt "${MAX_TIDY_FINDINGS}" ]; then - echo "::warning::clang-tidy found ${TIDY_FINDINGS} issue(s), threshold is ${MAX_TIDY_FINDINGS}" - exit 1 - fi - echo "clang-tidy findings within threshold" + # tidy.sh auto-detects $GITHUB_ACTIONS and emits ::warning/::error + # workflow commands so findings surface as PR file annotations. + # It exits non-zero only when a finding is promoted to an error via + # WarningsAsErrors in .clang-tidy; pure warnings annotate but don't + # fail the build. + run: ./tidy.sh diff --git a/tidy.sh b/tidy.sh index b06f1210..0ff403cd 100755 --- a/tidy.sh +++ b/tidy.sh @@ -1,15 +1,23 @@ #!/usr/bin/env bash # -# tidy.sh -- Run clang-tidy locally using the same file set and config as CI. -# -# Matches the file filter used by the cpp-linter GitHub Action in -# .github/workflows/builds.yml: only src/**/*.{c,cpp,cc,cxx} excluding -# src/tests/. Picks up checks from the repo-root .clang-tidy automatically. +# tidy.sh -- Run clang-tidy locally and in CI using the same file set and +# config. Picks up checks from the repo-root .clang-tidy automatically. # # Usage: -# ./tidy.sh # run on full src/ tree -# ./tidy.sh -j 4 # override parallelism -# ./tidy.sh -fix # auto-apply fixes (forwarded to run-clang-tidy) +# ./tidy.sh # run on full src/ tree +# ./tidy.sh -j 4 # override parallelism +# ./tidy.sh --github-actions # force GitHub Actions annotation mode +# ./tidy.sh -fix # forwarded to run-clang-tidy +# +# In GitHub Actions (auto-detected via $GITHUB_ACTIONS=true, or forced with +# --github-actions), this script additionally: +# - Emits ::warning/::error workflow commands so findings appear as PR file +# annotations (yellow for warnings, red for errors). Severity comes from +# clang-tidy itself -- errors are findings promoted by WarningsAsErrors +# in .clang-tidy. +# - Writes a short markdown summary to $GITHUB_STEP_SUMMARY. +# - Exits non-zero only when run-clang-tidy does (i.e. only on errors); +# warnings annotate but do not fail the build. # # Requires CMake to have generated build-release/compile_commands.json. # Run once: cmake --preset macos-release (or linux-release) @@ -18,11 +26,29 @@ set -euo pipefail BUILD_DIR="build-release" # Positive match for top-level src/*.{c,cpp,cc,cxx}; negative lookahead excludes -# dep paths (_deps/, build-*/, -src/src/) and other top-level dirs that CI's -# cpp-linter `ignore:` list filters out. Python regex (PCRE-ish) supports -# lookahead; this regex is evaluated by run-clang-tidy. +# dep paths (_deps/, build-*/, -src/src/) and every other top-level dir. Python +# regex (PCRE-ish) supports lookahead; this regex is evaluated by run-clang-tidy. FILE_REGEX='^(?!.*/(_deps|build-[^/]*|bridge|examples|client-sdk-rust|cpp-example-collection|vcpkg_installed|docker|docs|data)/).*/src/(?!tests/).*\.(c|cpp|cc|cxx)$' +CI_MODE=0 +if [[ "${GITHUB_ACTIONS:-}" == "true" ]]; then + CI_MODE=1 +fi + +forward_args=() +while (($#)); do + case "$1" in + --github-actions|--gh) + CI_MODE=1 + shift + ;; + *) + forward_args+=("$1") + shift + ;; + esac +done + if [[ ! -f "${BUILD_DIR}/compile_commands.json" ]]; then echo "ERROR: ${BUILD_DIR}/compile_commands.json not found." >&2 echo "Run: cmake --preset macos-release (or linux-release)" >&2 @@ -32,7 +58,7 @@ fi if ! command -v run-clang-tidy >/dev/null 2>&1; then echo "ERROR: run-clang-tidy not found in PATH." >&2 echo "Install LLVM: brew install llvm (macOS)" >&2 - echo " apt install clang-tidy (Linux)" >&2 + echo " apt install clang-tools-NN (Linux)" >&2 exit 1 fi @@ -50,10 +76,86 @@ else jobs=$(sysctl -n hw.ncpu 2>/dev/null || echo 4) fi +# Emit GitHub Actions workflow commands for each clang-tidy diagnostic line +# in the given log. Notes (`path:L:C: note: ...`) are deliberately skipped -- +# they belong to the preceding warning/error and would produce noisy extra +# annotations. Severity (::warning vs ::error) mirrors clang-tidy's prefix. +emit_annotations() { + local log="$1" + local workspace="${GITHUB_WORKSPACE:-${PWD}}" + local line path lineno col severity message check rel_path + + while IFS= read -r line; do + [[ "${line}" =~ ^(.+):([0-9]+):([0-9]+):[[:space:]]+(warning|error):[[:space:]]+(.+)[[:space:]]\[([^]]+)\][[:space:]]*$ ]] || continue + path="${BASH_REMATCH[1]}" + lineno="${BASH_REMATCH[2]}" + col="${BASH_REMATCH[3]}" + severity="${BASH_REMATCH[4]}" + message="${BASH_REMATCH[5]}" + check="${BASH_REMATCH[6]}" + + rel_path="${path#${workspace}/}" + + message="${message//$'%'/%25}" + message="${message//$'\r'/%0D}" + message="${message//$'\n'/%0A}" + + printf '::%s file=%s,line=%s,col=%s,title=clang-tidy (%s)::%s\n' \ + "${severity}" "${rel_path}" "${lineno}" "${col}" "${check}" "${message}" + done < "${log}" +} + +# Append a small markdown summary (counts + top checks) to $GITHUB_STEP_SUMMARY +# so the GitHub job page surfaces totals without needing to scan the log. +write_step_summary() { + local log="$1" + local summary_file="${GITHUB_STEP_SUMMARY:-}" + [[ -n "${summary_file}" ]] || return 0 + + local warnings errors + warnings=$(grep -Ec '^.+:[0-9]+:[0-9]+:[[:space:]]+warning:[[:space:]]' "${log}" || true) + errors=$(grep -Ec '^.+:[0-9]+:[0-9]+:[[:space:]]+error:[[:space:]]' "${log}" || true) + + { + echo "## clang-tidy results" + echo + echo "| Severity | Count |" + echo "|----------|-------|" + echo "| Errors | ${errors} |" + echo "| Warnings | ${warnings} |" + echo + + if (( warnings + errors > 0 )); then + echo "### Top checks" + echo + echo '| Check | Count |' + echo '|-------|-------|' + grep -Eo '\[[a-zA-Z0-9._,-]+\]$' "${log}" \ + | sort | uniq -c | sort -rn | head -5 \ + | awk '{ n = $1; $1 = ""; sub(/^ /, ""); gsub(/[\[\]]/, "", $0); printf("| `%s` | %d |\n", $0, n) }' + echo + fi + } >> "${summary_file}" +} + +log="$(mktemp -t tidy-log.XXXXXX)" +trap 'rm -f "${log}"' EXIT + +set +e run-clang-tidy \ -p "${BUILD_DIR}" \ -quiet \ -j "${jobs}" \ "${extra_args[@]}" \ - "$@" \ - "${FILE_REGEX}" + "${forward_args[@]}" \ + "${FILE_REGEX}" \ + 2>&1 | tee "${log}" +rc="${PIPESTATUS[0]}" +set -e + +if [[ "${CI_MODE}" == "1" ]]; then + emit_annotations "${log}" + write_step_summary "${log}" +fi + +exit "${rc}" From 897fcfaeb5c52bf285b74873b5b633d28392a5f5 Mon Sep 17 00:00:00 2001 From: Alan George Date: Thu, 23 Apr 2026 17:32:17 -0600 Subject: [PATCH 05/21] Another run with issues fixed --- .github/workflows/builds.yml | 32 +++++++++++++------- tidy.sh | 57 +++++++++++++++++++++++++++++++----- 2 files changed, 71 insertions(+), 18 deletions(-) diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 5c380e2c..618b679c 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -567,16 +567,28 @@ jobs: sudo apt-get update sudo apt-get install -y \ build-essential cmake ninja-build pkg-config \ - llvm-dev libclang-dev clang clang-tidy clang-tools \ - libssl-dev - # run-clang-tidy ships as run-clang-tidy- on apt; expose an - # unsuffixed name so tidy.sh works unchanged across environments. - if ! command -v run-clang-tidy >/dev/null 2>&1; then - rct_versioned=$(ls /usr/bin/run-clang-tidy-* 2>/dev/null | sort -V | tail -1) - if [ -n "${rct_versioned}" ]; then - sudo ln -sf "${rct_versioned}" /usr/local/bin/run-clang-tidy - fi - fi + llvm-dev libclang-dev clang \ + libssl-dev wget ca-certificates gnupg + + - name: Install clang-tidy 19 (for ExcludeHeaderFilterRegex support) + run: | + set -eux + # Ubuntu 24.04 apt ships clang-tidy 18, which doesn't understand + # ExcludeHeaderFilterRegex (added in 19). Pull clang-tidy 19 from + # the upstream LLVM apt repository and pin the unversioned names. + sudo install -m 0755 -d /etc/apt/keyrings + wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key \ + | sudo tee /etc/apt/keyrings/llvm.asc >/dev/null + sudo chmod a+r /etc/apt/keyrings/llvm.asc + codename=$(lsb_release -cs) + echo "deb [signed-by=/etc/apt/keyrings/llvm.asc] http://apt.llvm.org/${codename}/ llvm-toolchain-${codename}-19 main" \ + | sudo tee /etc/apt/sources.list.d/llvm-19.list >/dev/null + sudo apt-get update + sudo apt-get install -y clang-tidy-19 clang-tools-19 + sudo ln -sf /usr/bin/clang-tidy-19 /usr/local/bin/clang-tidy + sudo ln -sf /usr/bin/run-clang-tidy-19 /usr/local/bin/run-clang-tidy + clang-tidy --version + run-clang-tidy --help | head -1 || true - name: Install Rust (stable) uses: dtolnay/rust-toolchain@3c5f7ea28cd621ae0bf5283f0e981fb97b8a7af9 diff --git a/tidy.sh b/tidy.sh index 0ff403cd..2f87f8ba 100755 --- a/tidy.sh +++ b/tidy.sh @@ -105,16 +105,41 @@ emit_annotations() { done < "${log}" } -# Append a small markdown summary (counts + top checks) to $GITHUB_STEP_SUMMARY -# so the GitHub job page surfaces totals without needing to scan the log. +# Append a markdown summary (counts + top checks + full finding list) to +# $GITHUB_STEP_SUMMARY so the GitHub job page surfaces every finding without +# needing to scan the raw log. Counts require a [check-name] suffix so +# clang-tidy's own config-parse errors can't inflate the totals. write_step_summary() { local log="$1" local summary_file="${GITHUB_STEP_SUMMARY:-}" [[ -n "${summary_file}" ]] || return 0 - local warnings errors - warnings=$(grep -Ec '^.+:[0-9]+:[0-9]+:[[:space:]]+warning:[[:space:]]' "${log}" || true) - errors=$(grep -Ec '^.+:[0-9]+:[0-9]+:[[:space:]]+error:[[:space:]]' "${log}" || true) + local workspace="${GITHUB_WORKSPACE:-${PWD}}" + local findings_tsv + findings_tsv="$(mktemp -t tidy-findings.XXXXXX)" + + # Extract every real finding (severity must be followed by [check-name]) + # as tab-separated severity\tpath\tline\tcol\tcheck\tmessage. Use bash's + # regex engine (same as emit_annotations) for portability -- BSD awk and + # mawk don't support gawk's 3-argument match(). + local sline spath slineno scol sseverity smessage scheck + while IFS= read -r sline; do + [[ "${sline}" =~ ^(.+):([0-9]+):([0-9]+):[[:space:]]+(warning|error):[[:space:]]+(.+)[[:space:]]\[([^]]+)\][[:space:]]*$ ]] || continue + spath="${BASH_REMATCH[1]#${workspace}/}" + slineno="${BASH_REMATCH[2]}" + scol="${BASH_REMATCH[3]}" + sseverity="${BASH_REMATCH[4]}" + smessage="${BASH_REMATCH[5]}" + scheck="${BASH_REMATCH[6]}" + printf '%s\t%s\t%s\t%s\t%s\t%s\n' \ + "${sseverity}" "${spath}" "${slineno}" "${scol}" "${scheck}" "${smessage}" \ + >> "${findings_tsv}" + done < "${log}" + + local warnings errors total + warnings=$(awk -F'\t' '$1=="warning"{c++} END{print c+0}' "${findings_tsv}") + errors=$(awk -F'\t' '$1=="error"{c++} END{print c+0}' "${findings_tsv}") + total=$((warnings + errors)) { echo "## clang-tidy results" @@ -125,17 +150,33 @@ write_step_summary() { echo "| Warnings | ${warnings} |" echo - if (( warnings + errors > 0 )); then + if (( total > 0 )); then echo "### Top checks" echo echo '| Check | Count |' echo '|-------|-------|' - grep -Eo '\[[a-zA-Z0-9._,-]+\]$' "${log}" \ + awk -F'\t' '{print $5}' "${findings_tsv}" \ | sort | uniq -c | sort -rn | head -5 \ - | awk '{ n = $1; $1 = ""; sub(/^ /, ""); gsub(/[\[\]]/, "", $0); printf("| `%s` | %d |\n", $0, n) }' + | awk '{ n = $1; $1 = ""; sub(/^ /, ""); printf("| `%s` | %d |\n", $0, n) }' + echo + + echo "
All ${total} findings" + echo + echo '| Severity | File | Check | Message |' + echo '|----------|------|-------|---------|' + awk -F'\t' '{ + sev = $1; path = $2; lineno = $3; check = $5; msg = $6 + gsub(/\|/, "\\|", msg) + icon = (sev == "error") ? "error" : "warning" + printf("| %s | `%s:%s` | `%s` | %s |\n", icon, path, lineno, check, msg) + }' "${findings_tsv}" + echo + echo "
" echo fi } >> "${summary_file}" + + rm -f "${findings_tsv}" } log="$(mktemp -t tidy-log.XXXXXX)" From 73f9d26d5ca9502d7af9d4c94cd144a9d9ab17f0 Mon Sep 17 00:00:00 2001 From: Alan George Date: Thu, 23 Apr 2026 19:01:14 -0600 Subject: [PATCH 06/21] Guard against missing protobuf files --- tidy.sh | 50 +++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 47 insertions(+), 3 deletions(-) diff --git a/tidy.sh b/tidy.sh index 2f87f8ba..5ab4524d 100755 --- a/tidy.sh +++ b/tidy.sh @@ -51,7 +51,28 @@ done if [[ ! -f "${BUILD_DIR}/compile_commands.json" ]]; then echo "ERROR: ${BUILD_DIR}/compile_commands.json not found." >&2 - echo "Run: cmake --preset macos-release (or linux-release)" >&2 + echo "Run: ./build.sh release (configures + builds, generates protobuf headers)" >&2 + exit 1 +fi + +# Protobuf sanity check. `cmake --preset` only configures -- it doesn't invoke +# the Rust/protoc chain that writes livekit's generated headers into +# build-release/generated/. If those headers are missing or stale, every TU +# that #includes "room.pb.h" / "ffi.pb.h" / etc. fails with dozens of +# clang-diagnostic-error diagnostics (e.g. "no member named 'FrameMetadata' in +# namespace 'livekit::proto'") that drown out the real findings. Detect the +# "configured but never built" state early and point the user at ./build.sh. +proto_dir="${BUILD_DIR}/generated" +if [[ ! -d "${proto_dir}" ]] || ! compgen -G "${proto_dir}/*.pb.h" >/dev/null; then + echo "ERROR: no generated protobuf headers found in ${proto_dir}/." >&2 + echo "clang-tidy needs .pb.h files that are produced during the build step," >&2 + echo "not by 'cmake --preset' alone. To generate them, run:" >&2 + echo "" >&2 + echo " ./build.sh release # or release-tests / release-all, as needed" >&2 + echo "" >&2 + echo "If you previously bumped client-sdk-rust, also run 'clean-all' first:" >&2 + echo "" >&2 + echo " ./build.sh clean-all && ./build.sh release" >&2 exit 1 fi @@ -62,6 +83,14 @@ if ! command -v run-clang-tidy >/dev/null 2>&1; then exit 1 fi +# On macOS, the C++ standard library headers (, , ...) live +# inside the active Xcode / Command Line Tools SDK rather than on the default +# include path. Homebrew's clang-tidy doesn't know where that SDK is, so +# without -isysroot it fails every TU with "'cstdint' file not found" before +# any real check runs. `xcrun --show-sdk-path` resolves the currently selected +# SDK and we forward it to every clang-tidy invocation via --extra-arg. Linux +# CI doesn't need this -- the system clang-tidy already finds libstdc++/libc++ +# through its built-in resource dir. extra_args=() if [[ "$(uname)" == "Darwin" ]]; then sdk_path="$(xcrun --show-sdk-path 2>/dev/null || true)" @@ -70,12 +99,19 @@ if [[ "$(uname)" == "Darwin" ]]; then fi fi +# run-clang-tidy parallelizes across TUs via -j. Default to one worker per +# logical CPU so local runs aren't artificially slow. `nproc` is the Linux +# coreutils tool; macOS doesn't ship it, so fall back to `sysctl hw.ncpu`, +# and finally to a conservative 4 if neither is available (e.g. a minimal +# container). if command -v nproc >/dev/null 2>&1; then jobs=$(nproc) else jobs=$(sysctl -n hw.ncpu 2>/dev/null || echo 4) fi +# -------- Begin GitHub Actions annotations -------- + # Emit GitHub Actions workflow commands for each clang-tidy diagnostic line # in the given log. Notes (`path:L:C: note: ...`) are deliberately skipped -- # they belong to the preceding warning/error and would produce noisy extra @@ -179,8 +215,14 @@ write_step_summary() { rm -f "${findings_tsv}" } -log="$(mktemp -t tidy-log.XXXXXX)" -trap 'rm -f "${log}"' EXIT +# --------- End GitHub Actions annotations --------- + +# Capture clang-tidy's combined stdout+stderr to a stable, repo-local path so +# it can be re-parsed after the run (for annotations and the step summary) and +# re-read by the user afterwards (e.g. `grep misc-const tidy.log`). `*.log` is +# gitignored so this file never gets committed. Each run overwrites the +# previous log via `tee` (no -a), keeping the path predictable. +log="tidy.log" set +e run-clang-tidy \ @@ -199,4 +241,6 @@ if [[ "${CI_MODE}" == "1" ]]; then write_step_summary "${log}" fi +echo "Results written to: $(cd "$(dirname "${log}")" && pwd)/$(basename "${log}")" + exit "${rc}" From 47d5b3ceefcebd81c1ffbb2f5ca4664f7d9e055c Mon Sep 17 00:00:00 2001 From: Alan George Date: Thu, 23 Apr 2026 19:06:11 -0600 Subject: [PATCH 07/21] Add link to table --- tidy.sh | 43 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 36 insertions(+), 7 deletions(-) diff --git a/tidy.sh b/tidy.sh index 5ab4524d..2ee6ab3c 100755 --- a/tidy.sh +++ b/tidy.sh @@ -177,6 +177,32 @@ write_step_summary() { errors=$(awk -F'\t' '$1=="error"{c++} END{print c+0}' "${findings_tsv}") total=$((warnings + errors)) + # Render a check name as a markdown link to its official clang-tidy docs page + # (mirrors what cpp-linter-action used to do). The canonical URL layout is + # https://clang.llvm.org/extra/clang-tidy/checks//.html + # where is everything up to the first '-'. Two categories don't + # follow that layout: + # * clang-diagnostic-* -- compiler diagnostics, no per-check doc page + # * clang-analyzer-* -- static analyzer, documented on a single page + # Those fall back to plain code formatting / the analyzer index page. + check_link() { + local name="$1" + local module="${name%%-*}" + local rest="${name#*-}" + case "${name}" in + clang-diagnostic-*) + printf '`%s`' "${name}" + ;; + clang-analyzer-*) + printf '[`%s`](https://clang.llvm.org/docs/analyzer/checkers.html)' "${name}" + ;; + *) + printf '[`%s`](https://clang.llvm.org/extra/clang-tidy/checks/%s/%s.html)' \ + "${name}" "${module}" "${rest}" + ;; + esac + } + { echo "## clang-tidy results" echo @@ -193,19 +219,22 @@ write_step_summary() { echo '|-------|-------|' awk -F'\t' '{print $5}' "${findings_tsv}" \ | sort | uniq -c | sort -rn | head -5 \ - | awk '{ n = $1; $1 = ""; sub(/^ /, ""); printf("| `%s` | %d |\n", $0, n) }' + | while read -r count name; do + printf '| %s | %d |\n' "$(check_link "${name}")" "${count}" + done echo echo "
All ${total} findings" echo echo '| Severity | File | Check | Message |' echo '|----------|------|-------|---------|' - awk -F'\t' '{ - sev = $1; path = $2; lineno = $3; check = $5; msg = $6 - gsub(/\|/, "\\|", msg) - icon = (sev == "error") ? "error" : "warning" - printf("| %s | `%s:%s` | `%s` | %s |\n", icon, path, lineno, check, msg) - }' "${findings_tsv}" + while IFS=$'\t' read -r sev path lineno col check msg; do + msg="${msg//|/\\|}" + local icon + icon=$([[ "${sev}" == "error" ]] && echo error || echo warning) + printf '| %s | `%s:%s` | %s | %s |\n' \ + "${icon}" "${path}" "${lineno}" "$(check_link "${check}")" "${msg}" + done < "${findings_tsv}" echo echo "
" echo From 12572fd7cb1a9b694c6b0d1979bfee87f410962d Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 24 Apr 2026 10:45:39 -0600 Subject: [PATCH 08/21] Try linking to issues --- .github/workflows/builds.yml | 8 ++++++++ tidy.sh | 32 +++++++++++++++++++++++++++++--- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 618b679c..2349fc85 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -614,4 +614,12 @@ jobs: # It exits non-zero only when a finding is promoted to an error via # WarningsAsErrors in .clang-tidy; pure warnings annotate but don't # fail the build. + env: + # Linkify findings in the step summary against the PR head commit + # rather than GITHUB_SHA. On pull_request events GITHUB_SHA is the + # ephemeral refs/pull/N/merge commit, whose blob URLs 404 once the + # PR closes; the head SHA stays resolvable. For push / + # workflow_dispatch / schedule runs this expression resolves to + # github.sha, which is the pushed/selected commit. + TIDY_BLOB_SHA: ${{ github.event.pull_request.head.sha || github.sha }} run: ./tidy.sh diff --git a/tidy.sh b/tidy.sh index 2ee6ab3c..610948bf 100755 --- a/tidy.sh +++ b/tidy.sh @@ -177,6 +177,22 @@ write_step_summary() { errors=$(awk -F'\t' '$1=="error"{c++} END{print c+0}' "${findings_tsv}") total=$((warnings + errors)) + # Resolve a blob URL prefix so findings in the table below become clickable + # links to github.com. Prefer TIDY_BLOB_SHA (set by the workflow to the PR + # head SHA) over GITHUB_SHA -- on pull_request events GITHUB_SHA points at + # the ephemeral refs/pull/N/merge commit, whose blob URLs stop resolving + # once the PR closes. On push / workflow_dispatch / schedule runs + # TIDY_BLOB_SHA is unset and we fall through to GITHUB_SHA, which is the + # pushed / selected commit respectively. When neither is set (local runs), + # repo_url stays empty and the file column renders as plain code. + local repo_url="" + if [[ -n "${GITHUB_SERVER_URL:-}" && -n "${GITHUB_REPOSITORY:-}" ]]; then + local blob_sha="${TIDY_BLOB_SHA:-${GITHUB_SHA:-}}" + if [[ -n "${blob_sha}" ]]; then + repo_url="${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/blob/${blob_sha}" + fi + fi + # Render a check name as a markdown link to its official clang-tidy docs page # (mirrors what cpp-linter-action used to do). The canonical URL layout is # https://clang.llvm.org/extra/clang-tidy/checks//.html @@ -230,10 +246,20 @@ write_step_summary() { echo '|----------|------|-------|---------|' while IFS=$'\t' read -r sev path lineno col check msg; do msg="${msg//|/\\|}" - local icon + local icon file_cell icon=$([[ "${sev}" == "error" ]] && echo error || echo warning) - printf '| %s | `%s:%s` | %s | %s |\n' \ - "${icon}" "${path}" "${lineno}" "$(check_link "${check}")" "${msg}" + # Link to github.com when we have a blob URL and a repo-relative path. + # Absolute paths (leading '/') are system headers that leaked past the + # note filter -- rendering them as a github.com link would 404, so + # fall back to plain code formatting. GitHub blob anchors support + # #L but not columns, so the column is kept in the label only. + if [[ -n "${repo_url}" && "${path}" != /* ]]; then + file_cell="[\`${path}:${lineno}\`](${repo_url}/${path}#L${lineno})" + else + file_cell="\`${path}:${lineno}\`" + fi + printf '| %s | %s | %s | %s |\n' \ + "${icon}" "${file_cell}" "$(check_link "${check}")" "${msg}" done < "${findings_tsv}" echo echo "" From 156f47e48b5acbf8250424ebdf0bc343ef8d8892 Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 24 Apr 2026 13:59:58 -0600 Subject: [PATCH 09/21] No type traits, actually stream clang-tidy output locally, easy fixes --- .clang-tidy | 1 + src/data_stream.cpp | 6 +++--- src/ffi_client.cpp | 10 +++++----- src/local_participant.cpp | 10 +++++----- src/room.cpp | 6 +++--- src/subscription_thread_dispatcher.cpp | 6 +++--- src/trace/event_tracer.cpp | 6 +++--- tidy.sh | 8 +++++++- 8 files changed, 30 insertions(+), 23 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 65029ad6..aa215469 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -9,6 +9,7 @@ Checks: > -bugprone-easily-swappable-parameters, -modernize-use-trailing-return-type, -modernize-avoid-c-arrays, + -modernize-type-traits, -modernize-use-auto, -modernize-use-nodiscard, -modernize-return-braced-init-list, diff --git a/src/data_stream.cpp b/src/data_stream.cpp index 78df1e98..9f846a27 100644 --- a/src/data_stream.cpp +++ b/src/data_stream.cpp @@ -38,7 +38,7 @@ std::string generateRandomId(std::size_t bytes = 16) { out.reserve(bytes * 2); const char *hex = "0123456789abcdef"; for (std::size_t i = 0; i < bytes; ++i) { - int v = dist(rng); + const int v = dist(rng); out.push_back(hex[(v >> 4) & 0xF]); out.push_back(hex[v & 0xF]); } @@ -314,7 +314,7 @@ void TextStreamWriter::write(const std::string &text) { for (const auto &chunk_str : splitUtf8(text, kStreamChunkSize)) { const auto *p = reinterpret_cast(chunk_str.data()); - std::vector bytes(p, p + chunk_str.size()); + const std::vector bytes(p, p + chunk_str.size()); LK_LOG_DEBUG("sending chunk"); sendChunk(bytes); } @@ -348,7 +348,7 @@ void ByteStreamWriter::write(const std::vector &data) { const std::size_t n = std::min(kStreamChunkSize, data.size() - offset); auto it = data.begin() + static_cast(offset); - std::vector chunk(it, it + static_cast(n)); + const std::vector chunk(it, it + static_cast(n)); sendChunk(chunk); offset += n; } diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index f80c7c63..1f08d824 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -173,7 +173,7 @@ bool FfiClient::isInitialized() const noexcept { FfiClient::ListenerId FfiClient::AddListener(const FfiClient::Listener &listener) { const std::scoped_lock guard(lock_); - FfiClient::ListenerId id = next_listener_id++; + const FfiClient::ListenerId id = next_listener_id++; listeners_[id] = listener; return id; } @@ -191,7 +191,7 @@ FfiClient::sendRequest(const proto::FfiRequest &request) const { } const uint8_t *resp_ptr = nullptr; size_t resp_len = 0; - FfiHandleId handle = + const FfiHandleId handle = livekit_ffi_request(reinterpret_cast(bytes.data()), bytes.size(), &resp_ptr, &resp_len); if (handle == INVALID_HANDLE) { @@ -455,7 +455,7 @@ FfiClient::getTrackStatsAsync(uintptr_t track_handle) { get_stats_req->set_request_async_id(async_id); try { - proto::FfiResponse resp = sendRequest(req); + const proto::FfiResponse resp = sendRequest(req); if (!resp.has_get_stats()) { logAndThrow("FfiResponse missing get_stats"); } @@ -502,7 +502,7 @@ FfiClient::publishTrackAsync(std::uint64_t local_participant_handle, } const proto::OwnedTrackPublication &pub = cb.publication(); - pr.set_value(std::move(pub)); + pr.set_value(pub); }); // Build and send the request @@ -701,7 +701,7 @@ FfiClient::subscribeDataTrack(std::uint64_t track_handle, } try { - proto::FfiResponse resp = sendRequest(req); + const proto::FfiResponse resp = sendRequest(req); if (!resp.has_subscribe_data_track()) { return Result::failure(SubscribeDataTrackError{ diff --git a/src/local_participant.cpp b/src/local_participant.cpp index 8c0d3b52..d33b7f2a 100644 --- a/src/local_participant.cpp +++ b/src/local_participant.cpp @@ -99,7 +99,7 @@ void LocalParticipant::publishDtmf(int code, const std::string &digit) { } // TODO, should we take destination as inputs? - std::vector destination_identities; + const std::vector destination_identities; auto fut = FfiClient::instance().publishSipDtmfAsync( static_cast(handle_id), static_cast(code), digit, destination_identities); @@ -212,7 +212,7 @@ void LocalParticipant::publishTrack(const std::shared_ptr &track, static_cast(track_handle), options); // Will throw if the async op fails (error in callback). - proto::OwnedTrackPublication owned_pub = fut.get(); + const proto::OwnedTrackPublication owned_pub = fut.get(); // Construct a LocalTrackPublication from the proto publication. auto publication = std::make_shared(owned_pub); @@ -436,12 +436,12 @@ void LocalParticipant::handleRpcMethodInvocation( state->cv.notify_all(); } } - } guard{state}; + } const guard{state}; std::optional response_error; std::optional response_payload; - RpcInvocationData params{request_id, caller_identity, payload, - response_timeout_sec}; + const RpcInvocationData params{request_id, caller_identity, payload, + response_timeout_sec}; auto it = rpc_handlers_.find(method); if (it == rpc_handlers_.end()) { // No handler registered → built-in UNSUPPORTED_METHOD diff --git a/src/room.cpp b/src/room.cpp index c339c7b0..5153097d 100644 --- a/src/room.cpp +++ b/src/room.cpp @@ -918,7 +918,7 @@ void Room::OnEvent(const FfiEvent &event) { identity); break; } - std::string old_metadata = participant->metadata(); + const std::string old_metadata = participant->metadata(); participant->set_metadata(pm.metadata()); ev.participant = participant; ev.old_metadata = old_metadata; @@ -950,7 +950,7 @@ void Room::OnEvent(const FfiEvent &event) { identity); break; } - std::string old_name = participant->name(); + const std::string old_name = participant->name(); participant->set_name(pn.name()); ev.participant = participant; ev.old_name = old_name; @@ -1292,7 +1292,7 @@ void Room::OnEvent(const FfiEvent &event) { } else if (byte_reader) { // Convert string bytes -> vector const std::string &s = chunk.content(); - std::vector bytes(s.begin(), s.end()); + const std::vector bytes(s.begin(), s.end()); byte_reader->onChunkUpdate(bytes); } break; diff --git a/src/subscription_thread_dispatcher.cpp b/src/subscription_thread_dispatcher.cpp index 4e758804..1d2eb744 100644 --- a/src/subscription_thread_dispatcher.cpp +++ b/src/subscription_thread_dispatcher.cpp @@ -118,7 +118,7 @@ void SubscriptionThreadDispatcher::setOnVideoFrameCallback( VideoFrameCallback callback, const VideoStream::Options &opts) { const CallbackKey key{participant_identity, TrackSource::SOURCE_UNKNOWN, track_name}; - const std::lock_guard lock(lock_); + const std::scoped_lock lock(lock_); const bool replacing = video_callbacks_.find(key) != video_callbacks_.end(); video_callbacks_[key] = RegisteredVideoCallback{ std::move(callback), @@ -651,8 +651,8 @@ std::thread SubscriptionThreadDispatcher::startDataReaderLocked( const DataFrameCallback &cb) { auto old_thread = extractDataReaderThreadLocked(id); - int total_active = static_cast(active_readers_.size()) + - static_cast(active_data_readers_.size()); + const int total_active = static_cast(active_readers_.size()) + + static_cast(active_data_readers_.size()); if (total_active >= kMaxActiveReaders) { LK_LOG_ERROR("Cannot start data reader for {} track={}: active reader " "limit ({}) reached", diff --git a/src/trace/event_tracer.cpp b/src/trace/event_tracer.cpp index 66f5caa7..cff9a3a0 100644 --- a/src/trace/event_tracer.cpp +++ b/src/trace/event_tracer.cpp @@ -92,7 +92,7 @@ struct TraceEventData { // Escape a string for JSON std::string JsonEscape(const std::string &s) { std::ostringstream oss; - for (char c : s) { + for (const char c : s) { switch (c) { case '"': oss << "\\\""; @@ -294,7 +294,7 @@ const unsigned char *EventTracer::GetCategoryEnabled(const char *name) { } // Check if this specific category is enabled - std::string category_name(name); + const std::string category_name(name); if (g_enabled_categories.count(category_name) > 0) { return &g_enabled_byte; } @@ -302,7 +302,7 @@ const unsigned char *EventTracer::GetCategoryEnabled(const char *name) { // Check for wildcard matches (e.g., "livekit.*" matches "livekit.connect") for (const auto &pattern : g_enabled_categories) { if (pattern.back() == '*') { - std::string prefix = pattern.substr(0, pattern.size() - 1); + const std::string prefix = pattern.substr(0, pattern.size() - 1); if (category_name.compare(0, prefix.size(), prefix) == 0) { return &g_enabled_byte; } diff --git a/tidy.sh b/tidy.sh index 610948bf..df956d1e 100755 --- a/tidy.sh +++ b/tidy.sh @@ -280,7 +280,13 @@ write_step_summary() { log="tidy.log" set +e -run-clang-tidy \ +# run-clang-tidy is a Python script that doesn't flush stdout in its per-file +# loop; it only flushes once at exit. When its stdout is a pipe (the `| tee` +# below), Python's stdio defaults to block-buffered mode, so diagnostics +# accumulate in an ~8 KB buffer and the user sees nothing until the run is +# essentially over. PYTHONUNBUFFERED=1 forces line/unbuffered writes so each +# file's findings appear as soon as that file's clang-tidy finishes. +PYTHONUNBUFFERED=1 run-clang-tidy \ -p "${BUILD_DIR}" \ -quiet \ -j "${jobs}" \ From b087b08842efc9eb50ebfa4aae1197ea2bfaba76 Mon Sep 17 00:00:00 2001 From: Alan George Date: Fri, 24 Apr 2026 14:07:58 -0600 Subject: [PATCH 10/21] Induce error to see how CI performs --- src/ffi_client.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index 1f08d824..7e84a985 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -45,6 +45,25 @@ inline void logAndThrow(const std::string &error_msg) { throw std::runtime_error(error_msg); } +// TEMP: INTENTIONAL clang-tidy triggers to demo how GitHub CI renders red +// (error-level) annotations. Each line below hits a check promoted to +// WarningsAsErrors in .clang-tidy. Revert this block before merging. +// NOLINTBEGIN(misc-const-correctness) +[[maybe_unused]] void debug_tidy_error_markers() { + std::string s = "hello"; + std::string t = std::move(s); + (void)s.size(); // bugprone-use-after-move + (void)t; + + int i = 0; + while (i < 10) { // bugprone-infinite-loop + (void)t; + } + + (void)sizeof(sizeof(int)); // bugprone-sizeof-expression +} +// NOLINTEND(misc-const-correctness) + std::optional ExtractAsyncId(const proto::FfiEvent &event) { using E = proto::FfiEvent; switch (event.message_case()) { From ed9408c67a5fa15cc7690f2ae4ecc8f74bb2bdde Mon Sep 17 00:00:00 2001 From: Alan George Date: Mon, 27 Apr 2026 09:37:48 -0600 Subject: [PATCH 11/21] Fix formatting --- tidy.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tidy.sh b/tidy.sh index df956d1e..ab1d1dbd 100755 --- a/tidy.sh +++ b/tidy.sh @@ -31,6 +31,7 @@ BUILD_DIR="build-release" FILE_REGEX='^(?!.*/(_deps|build-[^/]*|bridge|examples|client-sdk-rust|cpp-example-collection|vcpkg_installed|docker|docs|data)/).*/src/(?!tests/).*\.(c|cpp|cc|cxx)$' CI_MODE=0 +# Automatically detect CI mode if in GitHub actions environment if [[ "${GITHUB_ACTIONS:-}" == "true" ]]; then CI_MODE=1 fi @@ -129,6 +130,12 @@ emit_annotations() { severity="${BASH_REMATCH[4]}" message="${BASH_REMATCH[5]}" check="${BASH_REMATCH[6]}" + # clang-tidy appends ",-warnings-as-errors" to the bracket suffix when a + # diagnostic is promoted from warning to error via WarningsAsErrors. Strip + # it so the title, docs link, and Top-checks bucket are identical for the + # warning and error code paths -- severity is already conveyed by the + # ::warning / ::error workflow command prefix below. + check="${check%,-warnings-as-errors}" rel_path="${path#${workspace}/}" @@ -167,6 +174,10 @@ write_step_summary() { sseverity="${BASH_REMATCH[4]}" smessage="${BASH_REMATCH[5]}" scheck="${BASH_REMATCH[6]}" + # Strip the ",-warnings-as-errors" promotion marker so error rows bucket + # under the same check name as their warning counterparts (see + # emit_annotations for the rationale). + scheck="${scheck%,-warnings-as-errors}" printf '%s\t%s\t%s\t%s\t%s\t%s\n' \ "${sseverity}" "${spath}" "${slineno}" "${scol}" "${scheck}" "${smessage}" \ >> "${findings_tsv}" From 9b10e20f4f362f2f1aeb57c295e83e80bca10313 Mon Sep 17 00:00:00 2001 From: Alan George Date: Mon, 27 Apr 2026 09:54:11 -0600 Subject: [PATCH 12/21] Induce warnings for CI --- src/ffi_client.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index 7e84a985..924a4b73 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -64,6 +64,25 @@ inline void logAndThrow(const std::string &error_msg) { } // NOLINTEND(misc-const-correctness) +// TEMP: INTENTIONAL clang-tidy triggers to demo how GitHub CI renders yellow +// (warning-level) annotations alongside the error block above. Each line +// below hits a check that is enabled in .clang-tidy but NOT promoted to +// WarningsAsErrors. Revert this block before merging. +[[maybe_unused]] void debug_tidy_warning_markers() { + typedef int LegacyInt; // modernize-use-using + LegacyInt n = 0; + ++n; + (void)n; + + // NOLINTNEXTLINE(misc-const-correctness) + void *p = NULL; // modernize-use-nullptr (NULL is a configured NullMacro) + p = &n; + (void)p; + + int unmodified = 42; // misc-const-correctness + (void)unmodified; +} + std::optional ExtractAsyncId(const proto::FfiEvent &event) { using E = proto::FfiEvent; switch (event.message_case()) { From a63bd3663adb850ff394db6e7c398a548f08c232 Mon Sep 17 00:00:00 2001 From: Alan George Date: Mon, 27 Apr 2026 12:47:18 -0600 Subject: [PATCH 13/21] Try iconography --- tidy.sh | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/tidy.sh b/tidy.sh index ab1d1dbd..669d5334 100755 --- a/tidy.sh +++ b/tidy.sh @@ -231,7 +231,7 @@ write_step_summary() { } { - echo "## clang-tidy results" + echo "## Analysis results" echo echo "| Severity | Count |" echo "|----------|-------|" @@ -255,10 +255,20 @@ write_step_summary() { echo echo '| Severity | File | Check | Message |' echo '|----------|------|-------|---------|' - while IFS=$'\t' read -r sev path lineno col check msg; do + # Stream errors first, then warnings, by running two awk passes over the + # TSV. This preserves log-discovery order within each severity bucket + # without relying on `sort -s` (whose stable-sort semantics differ + # subtly between GNU and BSD coreutils). + { + awk -F'\t' '$1=="error"' "${findings_tsv}" + awk -F'\t' '$1=="warning"' "${findings_tsv}" + } | while IFS=$'\t' read -r sev path lineno col check msg; do msg="${msg//|/\\|}" local icon file_cell - icon=$([[ "${sev}" == "error" ]] && echo error || echo warning) + # GFM emoji shortcodes render in step summaries; :x: (red X) and + # :warning: (yellow triangle) are the closest visual analogs to + # GitHub's native annotation pills shown in the PR review UI. + icon=$([[ "${sev}" == "error" ]] && echo ':x:' || echo ':warning:') # Link to github.com when we have a blob URL and a repo-relative path. # Absolute paths (leading '/') are system headers that leaked past the # note filter -- rendering them as a github.com link would 404, so @@ -269,9 +279,9 @@ write_step_summary() { else file_cell="\`${path}:${lineno}\`" fi - printf '| %s | %s | %s | %s |\n' \ - "${icon}" "${file_cell}" "$(check_link "${check}")" "${msg}" - done < "${findings_tsv}" + printf '| %s %s | %s | %s | %s |\n' \ + "${icon}" "${sev}" "${file_cell}" "$(check_link "${check}")" "${msg}" + done echo echo "" echo From 64cb6cfb81ba02b2548a4df4633ba11e6c0a01de Mon Sep 17 00:00:00 2001 From: Alan George Date: Mon, 27 Apr 2026 13:40:16 -0600 Subject: [PATCH 14/21] Adjust script name/location/output --- .github/workflows/builds.yml | 12 +- AGENTS.md | 1 + CMakePresets.json | 1018 +++++++++++++++--------------- README.md | 21 +- tidy.sh => scripts/clang-tidy.sh | 52 +- 5 files changed, 568 insertions(+), 536 deletions(-) rename tidy.sh => scripts/clang-tidy.sh (87%) diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 2349fc85..4e0d6b61 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -609,11 +609,11 @@ jobs: run: cmake --build build-release --target livekit_proto - name: Run clang-tidy - # tidy.sh auto-detects $GITHUB_ACTIONS and emits ::warning/::error - # workflow commands so findings surface as PR file annotations. - # It exits non-zero only when a finding is promoted to an error via - # WarningsAsErrors in .clang-tidy; pure warnings annotate but don't - # fail the build. + # scripts/clang-tidy.sh auto-detects $GITHUB_ACTIONS and emits + # ::warning/::error workflow commands so findings surface as PR file + # annotations. It exits non-zero only when a finding is promoted to + # an error via WarningsAsErrors in .clang-tidy; pure warnings + # annotate but don't fail the build. env: # Linkify findings in the step summary against the PR head commit # rather than GITHUB_SHA. On pull_request events GITHUB_SHA is the @@ -622,4 +622,4 @@ jobs: # workflow_dispatch / schedule runs this expression resolves to # github.sha, which is the pushed/selected commit. TIDY_BLOB_SHA: ${{ github.event.pull_request.head.sha || github.sha }} - run: ./tidy.sh + run: ./scripts/clang-tidy.sh diff --git a/AGENTS.md b/AGENTS.md index 7656e301..f6a0d883 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -79,6 +79,7 @@ Be sure to update the directory layout in this file if the directory layout chan | `client-sdk-rust/livekit-ffi/protocol/*.proto` | FFI contract (protobuf definitions, read-only reference) | | `cmake/` | Build helpers (`protobuf.cmake`, `spdlog.cmake`, `LiveKitConfig.cmake.in`) | | `docker/` | Dockerfile for CI and SDK distribution images | +| `scripts/` | Developer / CI helper scripts (e.g. `clang-tidy.sh`) | | `.github/workflows/` | GitHub Actions CI workflows | ### Key Types diff --git a/CMakePresets.json b/CMakePresets.json index 3c863d7c..5646e5c7 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -1,509 +1,509 @@ -{ - "version": 6, - "cmakeMinimumRequired": { - "major": 3, - "minor": 20, - "patch": 0 - }, - "configurePresets": [ - { - "name": "vcpkg", - "hidden": true, - "cacheVariables": { - "CMAKE_TOOLCHAIN_FILE": { - "type": "FILEPATH", - "value": "$env{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake" - }, - "VCPKG_INSTALLED_DIR": "${sourceDir}/vcpkg_installed", - "LIVEKIT_USE_VCPKG": "ON", - "VCPKG_TARGET_TRIPLET": "x64-windows-static-md", - "VCPKG_HOST_TRIPLET": "x64-windows-static-md" - } - }, - { - "name": "windows-base", - "hidden": true, - "inherits": "vcpkg", - "generator": "Visual Studio 17 2022", - "architecture": { - "value": "x64", - "strategy": "set" - }, - "cacheVariables": { - "VCPKG_TARGET_TRIPLET": "x64-windows-static-md" - } - }, - { - "name": "linux-base", - "hidden": true, - "generator": "Ninja", - "cacheVariables": { - "LIVEKIT_USE_VCPKG": "OFF", - "CMAKE_EXPORT_COMPILE_COMMANDS": "ON" - }, - "condition": { - "type": "equals", - "lhs": "${hostSystemName}", - "rhs": "Linux" - } - }, - { - "name": "macos-base", - "hidden": true, - "generator": "Ninja", - "cacheVariables": { - "LIVEKIT_USE_VCPKG": "OFF", - "CMAKE_EXPORT_COMPILE_COMMANDS": "ON" - }, - "condition": { - "type": "equals", - "lhs": "${hostSystemName}", - "rhs": "Darwin" - } - }, - { - "name": "windows-release", - "displayName": "Windows x64 Release", - "description": "Build for Windows x64 Release (vcpkg)", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "windows-debug", - "displayName": "Windows x64 Debug", - "description": "Build for Windows x64 Debug (vcpkg)", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "windows-release-examples", - "displayName": "Windows x64 Release with Examples", - "description": "Build for Windows x64 Release with example applications", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "OFF", - "VCPKG_MANIFEST_FEATURES": "examples" - } - }, - { - "name": "windows-debug-examples", - "displayName": "Windows x64 Debug with Examples", - "description": "Build for Windows x64 Debug with example applications", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "OFF", - "VCPKG_MANIFEST_FEATURES": "examples" - } - }, - { - "name": "linux-release", - "displayName": "Linux Release", - "description": "Build for Linux Release (system packages)", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "linux-debug", - "displayName": "Linux Debug", - "description": "Build for Linux Debug (system packages)", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "linux-release-examples", - "displayName": "Linux Release with Examples", - "description": "Build for Linux Release with example applications", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "linux-debug-examples", - "displayName": "Linux Debug with Examples", - "description": "Build for Linux Debug with example applications", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "macos-release", - "displayName": "macOS Release", - "description": "Build for macOS Release (Homebrew packages)", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "macos-debug", - "displayName": "macOS Debug", - "description": "Build for macOS Debug (Homebrew packages)", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "macos-release-examples", - "displayName": "macOS Release with Examples", - "description": "Build for macOS Release with example applications", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "macos-debug-examples", - "displayName": "macOS Debug with Examples", - "description": "Build for macOS Debug with example applications", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "windows-release-all", - "displayName": "Windows x64 Release with Tests + Examples", - "description": "Build for Windows x64 Release with tests and examples", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "ON", - "VCPKG_MANIFEST_FEATURES": "examples" - } - }, - { - "name": "windows-debug-all", - "displayName": "Windows x64 Debug with Tests + Examples", - "description": "Build for Windows x64 Debug with tests and examples", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "ON", - "VCPKG_MANIFEST_FEATURES": "examples" - } - }, - { - "name": "windows-release-tests", - "displayName": "Windows x64 Release with Tests", - "description": "Build for Windows x64 Release with test suite", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "windows-debug-tests", - "displayName": "Windows x64 Debug with Tests", - "description": "Build for Windows x64 Debug with test suite", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "linux-release-all", - "displayName": "Linux Release with Tests + Examples", - "description": "Build for Linux Release with tests and examples", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "linux-debug-all", - "displayName": "Linux Debug with Tests + Examples", - "description": "Build for Linux Debug with tests and examples", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "linux-release-tests", - "displayName": "Linux Release with Tests", - "description": "Build for Linux Release with test suite", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "linux-debug-tests", - "displayName": "Linux Debug with Tests", - "description": "Build for Linux Debug with test suite", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "macos-release-all", - "displayName": "macOS Release with Tests + Examples", - "description": "Build for macOS Release with tests and examples", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "macos-debug-all", - "displayName": "macOS Debug with Tests + Examples", - "description": "Build for macOS Debug with tests and examples", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "macos-release-tests", - "displayName": "macOS Release with Tests", - "description": "Build for macOS Release with test suite", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "macos-debug-tests", - "displayName": "macOS Debug with Tests", - "description": "Build for macOS Debug with test suite", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "ON" - } - } - ], - "buildPresets": [ - { - "name": "windows-release", - "configurePreset": "windows-release", - "configuration": "Release" - }, - { - "name": "windows-debug", - "configurePreset": "windows-debug", - "configuration": "Debug" - }, - { - "name": "windows-release-examples", - "configurePreset": "windows-release-examples", - "configuration": "Release" - }, - { - "name": "windows-debug-examples", - "configurePreset": "windows-debug-examples", - "configuration": "Debug" - }, - { - "name": "linux-release", - "configurePreset": "linux-release" - }, - { - "name": "linux-debug", - "configurePreset": "linux-debug" - }, - { - "name": "linux-release-examples", - "configurePreset": "linux-release-examples" - }, - { - "name": "linux-debug-examples", - "configurePreset": "linux-debug-examples" - }, - { - "name": "macos-release", - "configurePreset": "macos-release" - }, - { - "name": "macos-debug", - "configurePreset": "macos-debug" - }, - { - "name": "macos-release-examples", - "configurePreset": "macos-release-examples" - }, - { - "name": "macos-debug-examples", - "configurePreset": "macos-debug-examples" - }, - { - "name": "windows-release-all", - "configurePreset": "windows-release-all", - "configuration": "Release" - }, - { - "name": "windows-debug-all", - "configurePreset": "windows-debug-all", - "configuration": "Debug" - }, - { - "name": "linux-release-all", - "configurePreset": "linux-release-all" - }, - { - "name": "linux-debug-all", - "configurePreset": "linux-debug-all" - }, - { - "name": "macos-release-all", - "configurePreset": "macos-release-all" - }, - { - "name": "macos-debug-all", - "configurePreset": "macos-debug-all" - }, - { - "name": "windows-release-tests", - "configurePreset": "windows-release-tests", - "configuration": "Release" - }, - { - "name": "windows-debug-tests", - "configurePreset": "windows-debug-tests", - "configuration": "Debug" - }, - { - "name": "linux-release-tests", - "configurePreset": "linux-release-tests" - }, - { - "name": "linux-debug-tests", - "configurePreset": "linux-debug-tests" - }, - { - "name": "macos-release-tests", - "configurePreset": "macos-release-tests" - }, - { - "name": "macos-debug-tests", - "configurePreset": "macos-debug-tests" - } - ], - "testPresets": [ - { - "name": "windows-release-tests", - "configurePreset": "windows-release-tests", - "configuration": "Release", - "output": { - "outputOnFailure": true - } - }, - { - "name": "windows-debug-tests", - "configurePreset": "windows-debug-tests", - "configuration": "Debug", - "output": { - "outputOnFailure": true - } - }, - { - "name": "linux-release-tests", - "configurePreset": "linux-release-tests", - "output": { - "outputOnFailure": true - } - }, - { - "name": "linux-debug-tests", - "configurePreset": "linux-debug-tests", - "output": { - "outputOnFailure": true - } - }, - { - "name": "macos-release-tests", - "configurePreset": "macos-release-tests", - "output": { - "outputOnFailure": true - } - }, - { - "name": "macos-debug-tests", - "configurePreset": "macos-debug-tests", - "output": { - "outputOnFailure": true - } - } - ] -} +{ + "version": 6, + "cmakeMinimumRequired": { + "major": 3, + "minor": 20, + "patch": 0 + }, + "configurePresets": [ + { + "name": "vcpkg", + "hidden": true, + "cacheVariables": { + "CMAKE_TOOLCHAIN_FILE": { + "type": "FILEPATH", + "value": "$env{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake" + }, + "VCPKG_INSTALLED_DIR": "${sourceDir}/vcpkg_installed", + "LIVEKIT_USE_VCPKG": "ON", + "VCPKG_TARGET_TRIPLET": "x64-windows-static-md", + "VCPKG_HOST_TRIPLET": "x64-windows-static-md" + } + }, + { + "name": "windows-base", + "hidden": true, + "inherits": "vcpkg", + "generator": "Visual Studio 17 2022", + "architecture": { + "value": "x64", + "strategy": "set" + }, + "cacheVariables": { + "VCPKG_TARGET_TRIPLET": "x64-windows-static-md" + } + }, + { + "name": "linux-base", + "hidden": true, + "generator": "Ninja", + "cacheVariables": { + "LIVEKIT_USE_VCPKG": "OFF", + "CMAKE_EXPORT_COMPILE_COMMANDS": "ON" + }, + "condition": { + "type": "equals", + "lhs": "${hostSystemName}", + "rhs": "Linux" + } + }, + { + "name": "macos-base", + "hidden": true, + "generator": "Ninja", + "cacheVariables": { + "LIVEKIT_USE_VCPKG": "OFF", + "CMAKE_EXPORT_COMPILE_COMMANDS": "ON" + }, + "condition": { + "type": "equals", + "lhs": "${hostSystemName}", + "rhs": "Darwin" + } + }, + { + "name": "windows-release", + "displayName": "Windows x64 Release", + "description": "Build for Windows x64 Release (vcpkg)", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "windows-debug", + "displayName": "Windows x64 Debug", + "description": "Build for Windows x64 Debug (vcpkg)", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "windows-release-examples", + "displayName": "Windows x64 Release with Examples", + "description": "Build for Windows x64 Release with example applications", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "OFF", + "VCPKG_MANIFEST_FEATURES": "examples" + } + }, + { + "name": "windows-debug-examples", + "displayName": "Windows x64 Debug with Examples", + "description": "Build for Windows x64 Debug with example applications", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "OFF", + "VCPKG_MANIFEST_FEATURES": "examples" + } + }, + { + "name": "linux-release", + "displayName": "Linux Release", + "description": "Build for Linux Release (system packages)", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "linux-debug", + "displayName": "Linux Debug", + "description": "Build for Linux Debug (system packages)", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "linux-release-examples", + "displayName": "Linux Release with Examples", + "description": "Build for Linux Release with example applications", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "linux-debug-examples", + "displayName": "Linux Debug with Examples", + "description": "Build for Linux Debug with example applications", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "macos-release", + "displayName": "macOS Release", + "description": "Build for macOS Release (Homebrew packages)", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "macos-debug", + "displayName": "macOS Debug", + "description": "Build for macOS Debug (Homebrew packages)", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "macos-release-examples", + "displayName": "macOS Release with Examples", + "description": "Build for macOS Release with example applications", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "macos-debug-examples", + "displayName": "macOS Debug with Examples", + "description": "Build for macOS Debug with example applications", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "windows-release-all", + "displayName": "Windows x64 Release with Tests + Examples", + "description": "Build for Windows x64 Release with tests and examples", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "ON", + "VCPKG_MANIFEST_FEATURES": "examples" + } + }, + { + "name": "windows-debug-all", + "displayName": "Windows x64 Debug with Tests + Examples", + "description": "Build for Windows x64 Debug with tests and examples", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "ON", + "VCPKG_MANIFEST_FEATURES": "examples" + } + }, + { + "name": "windows-release-tests", + "displayName": "Windows x64 Release with Tests", + "description": "Build for Windows x64 Release with test suite", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "windows-debug-tests", + "displayName": "Windows x64 Debug with Tests", + "description": "Build for Windows x64 Debug with test suite", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "linux-release-all", + "displayName": "Linux Release with Tests + Examples", + "description": "Build for Linux Release with tests and examples", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "linux-debug-all", + "displayName": "Linux Debug with Tests + Examples", + "description": "Build for Linux Debug with tests and examples", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "linux-release-tests", + "displayName": "Linux Release with Tests", + "description": "Build for Linux Release with test suite", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "linux-debug-tests", + "displayName": "Linux Debug with Tests", + "description": "Build for Linux Debug with test suite", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "macos-release-all", + "displayName": "macOS Release with Tests + Examples", + "description": "Build for macOS Release with tests and examples", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "macos-debug-all", + "displayName": "macOS Debug with Tests + Examples", + "description": "Build for macOS Debug with tests and examples", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "macos-release-tests", + "displayName": "macOS Release with Tests", + "description": "Build for macOS Release with test suite", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "macos-debug-tests", + "displayName": "macOS Debug with Tests", + "description": "Build for macOS Debug with test suite", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "ON" + } + } + ], + "buildPresets": [ + { + "name": "windows-release", + "configurePreset": "windows-release", + "configuration": "Release" + }, + { + "name": "windows-debug", + "configurePreset": "windows-debug", + "configuration": "Debug" + }, + { + "name": "windows-release-examples", + "configurePreset": "windows-release-examples", + "configuration": "Release" + }, + { + "name": "windows-debug-examples", + "configurePreset": "windows-debug-examples", + "configuration": "Debug" + }, + { + "name": "linux-release", + "configurePreset": "linux-release" + }, + { + "name": "linux-debug", + "configurePreset": "linux-debug" + }, + { + "name": "linux-release-examples", + "configurePreset": "linux-release-examples" + }, + { + "name": "linux-debug-examples", + "configurePreset": "linux-debug-examples" + }, + { + "name": "macos-release", + "configurePreset": "macos-release" + }, + { + "name": "macos-debug", + "configurePreset": "macos-debug" + }, + { + "name": "macos-release-examples", + "configurePreset": "macos-release-examples" + }, + { + "name": "macos-debug-examples", + "configurePreset": "macos-debug-examples" + }, + { + "name": "windows-release-all", + "configurePreset": "windows-release-all", + "configuration": "Release" + }, + { + "name": "windows-debug-all", + "configurePreset": "windows-debug-all", + "configuration": "Debug" + }, + { + "name": "linux-release-all", + "configurePreset": "linux-release-all" + }, + { + "name": "linux-debug-all", + "configurePreset": "linux-debug-all" + }, + { + "name": "macos-release-all", + "configurePreset": "macos-release-all" + }, + { + "name": "macos-debug-all", + "configurePreset": "macos-debug-all" + }, + { + "name": "windows-release-tests", + "configurePreset": "windows-release-tests", + "configuration": "Release" + }, + { + "name": "windows-debug-tests", + "configurePreset": "windows-debug-tests", + "configuration": "Debug" + }, + { + "name": "linux-release-tests", + "configurePreset": "linux-release-tests" + }, + { + "name": "linux-debug-tests", + "configurePreset": "linux-debug-tests" + }, + { + "name": "macos-release-tests", + "configurePreset": "macos-release-tests" + }, + { + "name": "macos-debug-tests", + "configurePreset": "macos-debug-tests" + } + ], + "testPresets": [ + { + "name": "windows-release-tests", + "configurePreset": "windows-release-tests", + "configuration": "Release", + "output": { + "outputOnFailure": true + } + }, + { + "name": "windows-debug-tests", + "configurePreset": "windows-debug-tests", + "configuration": "Debug", + "output": { + "outputOnFailure": true + } + }, + { + "name": "linux-release-tests", + "configurePreset": "linux-release-tests", + "output": { + "outputOnFailure": true + } + }, + { + "name": "linux-debug-tests", + "configurePreset": "linux-debug-tests", + "output": { + "outputOnFailure": true + } + }, + { + "name": "macos-release-tests", + "configurePreset": "macos-release-tests", + "output": { + "outputOnFailure": true + } + }, + { + "name": "macos-debug-tests", + "configurePreset": "macos-debug-tests", + "output": { + "outputOnFailure": true + } + } + ] +} diff --git a/README.md b/README.md index e9228565..4c17446d 100644 --- a/README.md +++ b/README.md @@ -522,13 +522,28 @@ sudo apt-get install clang-format clang-tidy clang-tools To run: -1. Run a build script command, such that `compile_command.json` is present at the root of the repository -2. Run: +1. Generate `compile_commands.json` and the protobuf headers via a release build: ```bash -clang-tidy -p . src/*.cpp +./build.sh release ``` +2. Run the clang-tidy wrapper, which uses the same file set, regex filters, and `.clang-tidy` config as CI: + +```bash +./scripts/clang-tidy.sh +``` + +The wrapper forwards extra arguments to `run-clang-tidy`, so you can narrow the run, change parallelism, or auto-apply fixes: + +```bash +./scripts/clang-tidy.sh -j 4 +./scripts/clang-tidy.sh -checks='-*,misc-const-correctness' +./scripts/clang-tidy.sh -fix +``` + +Findings are captured to `clang-tidy.log` at the repo root for `grep`-friendly post-run inspection. + ### Memory Checks Run valgrind on various examples or tests to check for memory leaks and other issues. diff --git a/tidy.sh b/scripts/clang-tidy.sh similarity index 87% rename from tidy.sh rename to scripts/clang-tidy.sh index 669d5334..a1097ef2 100755 --- a/tidy.sh +++ b/scripts/clang-tidy.sh @@ -1,13 +1,13 @@ #!/usr/bin/env bash # -# tidy.sh -- Run clang-tidy locally and in CI using the same file set and -# config. Picks up checks from the repo-root .clang-tidy automatically. +# clang-tidy.sh -- Run clang-tidy locally and in CI using the same file set +# and config. Picks up checks from the repo-root .clang-tidy automatically. # -# Usage: -# ./tidy.sh # run on full src/ tree -# ./tidy.sh -j 4 # override parallelism -# ./tidy.sh --github-actions # force GitHub Actions annotation mode -# ./tidy.sh -fix # forwarded to run-clang-tidy +# Usage (from anywhere; the script self-anchors to the repo root): +# ./scripts/clang-tidy.sh # run on full src/ tree +# ./scripts/clang-tidy.sh -j 4 # override parallelism +# ./scripts/clang-tidy.sh --github-actions # force GitHub Actions annotation mode +# ./scripts/clang-tidy.sh -fix # forwarded to run-clang-tidy # # In GitHub Actions (auto-detected via $GITHUB_ACTIONS=true, or forced with # --github-actions), this script additionally: @@ -20,10 +20,17 @@ # warnings annotate but do not fail the build. # # Requires CMake to have generated build-release/compile_commands.json. -# Run once: cmake --preset macos-release (or linux-release) +# Run once: ./build.sh release (configures, builds, generates protobuf headers) set -euo pipefail +# Anchor every relative path (BUILD_DIR, clang-tidy.log, etc.) to the repo root +# regardless of how/where this script is invoked. Without this, calling the +# script from a subdirectory or via an absolute path would break the +# compile_commands.json / generated/ checks below. +script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)" +cd "${script_dir}/.." + BUILD_DIR="build-release" # Positive match for top-level src/*.{c,cpp,cc,cxx}; negative lookahead excludes # dep paths (_deps/, build-*/, -src/src/) and every other top-level dir. Python @@ -235,8 +242,10 @@ write_step_summary() { echo echo "| Severity | Count |" echo "|----------|-------|" - echo "| Errors | ${errors} |" - echo "| Warnings | ${warnings} |" + # Same GFM shortcodes used in the detailed findings table below, so the + # count summary and per-finding severity column read consistently. + echo "| :x: Error | ${errors} |" + echo "| :warning: Warning | ${warnings} |" echo if (( total > 0 )); then @@ -264,11 +273,18 @@ write_step_summary() { awk -F'\t' '$1=="warning"' "${findings_tsv}" } | while IFS=$'\t' read -r sev path lineno col check msg; do msg="${msg//|/\\|}" - local icon file_cell + local icon label file_cell # GFM emoji shortcodes render in step summaries; :x: (red X) and # :warning: (yellow triangle) are the closest visual analogs to - # GitHub's native annotation pills shown in the PR review UI. - icon=$([[ "${sev}" == "error" ]] && echo ':x:' || echo ':warning:') + # GitHub's native annotation pills shown in the PR review UI. The + # title-cased label matches the count table at the top of the summary. + if [[ "${sev}" == "error" ]]; then + icon=':x:' + label='Error' + else + icon=':warning:' + label='Warning' + fi # Link to github.com when we have a blob URL and a repo-relative path. # Absolute paths (leading '/') are system headers that leaked past the # note filter -- rendering them as a github.com link would 404, so @@ -280,7 +296,7 @@ write_step_summary() { file_cell="\`${path}:${lineno}\`" fi printf '| %s %s | %s | %s | %s |\n' \ - "${icon}" "${sev}" "${file_cell}" "$(check_link "${check}")" "${msg}" + "${icon}" "${label}" "${file_cell}" "$(check_link "${check}")" "${msg}" done echo echo "" @@ -295,10 +311,10 @@ write_step_summary() { # Capture clang-tidy's combined stdout+stderr to a stable, repo-local path so # it can be re-parsed after the run (for annotations and the step summary) and -# re-read by the user afterwards (e.g. `grep misc-const tidy.log`). `*.log` is -# gitignored so this file never gets committed. Each run overwrites the -# previous log via `tee` (no -a), keeping the path predictable. -log="tidy.log" +# re-read by the user afterwards (e.g. `grep misc-const clang-tidy.log`). +# `*.log` is gitignored so this file never gets committed. Each run overwrites +# the previous log via `tee` (no -a), keeping the path predictable. +log="clang-tidy.log" set +e # run-clang-tidy is a Python script that doesn't flush stdout in its per-file From 4d1fbb78458bffecb4d29da10bc843ddf9235f54 Mon Sep 17 00:00:00 2001 From: Alan George Date: Tue, 28 Apr 2026 13:25:41 -0600 Subject: [PATCH 15/21] Many clang fixes, reduce bash version required on script, added unit test for track publication --- .clang-tidy | 7 +- include/livekit/local_track_publication.h | 8 +- include/livekit/remote_track_publication.h | 8 +- include/livekit/result.h | 13 +++ .../livekit/subscription_thread_dispatcher.h | 5 +- include/livekit/track.h | 7 ++ scripts/clang-tidy.sh | 9 +- src/ffi_client.cpp | 38 ------- src/ffi_client.h | 11 ++- src/local_track_publication.cpp | 6 -- src/remote_track_publication.cpp | 6 -- src/room.cpp | 2 + src/subscription_thread_dispatcher.cpp | 91 +++++++++++------ src/tests/unit/test_track_publication.cpp | 98 +++++++++++++++++++ src/trace/event_tracer.cpp | 2 + src/trace/event_tracer_internal.h | 8 +- 16 files changed, 217 insertions(+), 102 deletions(-) create mode 100644 src/tests/unit/test_track_publication.cpp diff --git a/.clang-tidy b/.clang-tidy index aa215469..46cc6271 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -31,7 +31,12 @@ WarningsAsErrors: > bugprone-suspicious-memset-usage, HeaderFilterRegex: '.*/(include/livekit|src)/.*\.(h|hpp)$' -ExcludeHeaderFilterRegex: '(.*/src/tests/.*)|(.*/_deps/.*)|(.*/build-[^/]*/.*)' +# src/trace/{trace_event,event_tracer}.h are derived from Chromium and WebRTC +# (see "Based on" notice in their headers). Per AGENTS.md we don't restyle +# externally-pulled files, so they're excluded from clang-tidy entirely. +# event_tracer_internal.h and event_tracer.cpp are LiveKit-authored and +# remain in scope. +ExcludeHeaderFilterRegex: '(.*/src/tests/.*)|(.*/_deps/.*)|(.*/build-[^/]*/.*)|(.*/src/trace/(trace_event|event_tracer)\.h$)' FormatStyle: file diff --git a/include/livekit/local_track_publication.h b/include/livekit/local_track_publication.h index d00d863f..404381d5 100644 --- a/include/livekit/local_track_publication.h +++ b/include/livekit/local_track_publication.h @@ -24,16 +24,16 @@ namespace proto { class OwnedTrackPublication; } -class Track; - class LocalTrackPublication : public TrackPublication { public: /// Note, this LocalTrackPublication is constructed internally only; /// safe to accept proto::OwnedTrackPublication. explicit LocalTrackPublication(const proto::OwnedTrackPublication &owned); - /// Typed accessor for the attached LocalTrack (if any). - std::shared_ptr track() const noexcept; + // The track accessor is inherited from `TrackPublication::track()`. It + // returns `std::shared_ptr`; if a typed `LocalTrack` accessor is + // ever needed, add a differently-named method (e.g. `localTrack()`) so the + // base accessor remains source-stable for existing consumers. }; } // namespace livekit diff --git a/include/livekit/remote_track_publication.h b/include/livekit/remote_track_publication.h index aa394081..a98b5609 100644 --- a/include/livekit/remote_track_publication.h +++ b/include/livekit/remote_track_publication.h @@ -24,16 +24,16 @@ namespace proto { class OwnedTrackPublication; } -class Track; - class RemoteTrackPublication : public TrackPublication { public: /// Note, this RemoteTrackPublication is constructed internally only; /// safe to accept proto::OwnedTrackPublication. explicit RemoteTrackPublication(const proto::OwnedTrackPublication &owned); - /// Typed accessor for the attached RemoteTrack (if any). - std::shared_ptr track() const noexcept; + // The track accessor is inherited from `TrackPublication::track()`. It + // returns `std::shared_ptr`; if a typed `RemoteTrack` accessor is + // ever needed, add a differently-named method (e.g. `remoteTrack()`) so the + // base accessor remains source-stable for existing consumers. bool subscribed() const noexcept { return subscribed_; } diff --git a/include/livekit/result.h b/include/livekit/result.h index bc7f1338..dce0e874 100644 --- a/include/livekit/result.h +++ b/include/livekit/result.h @@ -64,49 +64,62 @@ template class [[nodiscard]] Result { /// Allows `if (result)` style success checks. explicit operator bool() const noexcept { return ok(); } + // TODO (AEG): clang-tidy flagged these accessors because the signatures are marked + // noexcept, but std::get can throw a std::bad_variant_access exception on + // std::variant specifically. Investigate if this is actually a concern or if the types + // are safe within this class (unit test ideal). + /// Access the success value. Requires `ok() == true`. + // NOLINTNEXTLINE(bugprone-exception-escape) T &value() & noexcept { assert(ok()); return std::get<0>(storage_); } /// Access the success value. Requires `ok() == true`. + // NOLINTNEXTLINE(bugprone-exception-escape) const T &value() const & noexcept { assert(ok()); return std::get<0>(storage_); } /// Move the success value out. Requires `ok() == true`. + // NOLINTNEXTLINE(bugprone-exception-escape) T &&value() && noexcept { assert(ok()); return std::get<0>(std::move(storage_)); } /// Move the success value out. Requires `ok() == true`. + // NOLINTNEXTLINE(bugprone-exception-escape) const T &&value() const && noexcept { assert(ok()); return std::get<0>(std::move(storage_)); } /// Access the error value. Requires `has_error() == true`. + // NOLINTNEXTLINE(bugprone-exception-escape) E &error() & noexcept { assert(has_error()); return std::get<1>(storage_); } /// Access the error value. Requires `has_error() == true`. + // NOLINTNEXTLINE(bugprone-exception-escape) const E &error() const & noexcept { assert(has_error()); return std::get<1>(storage_); } /// Move the error value out. Requires `has_error() == true`. + // NOLINTNEXTLINE(bugprone-exception-escape) E &&error() && noexcept { assert(has_error()); return std::get<1>(std::move(storage_)); } /// Move the error value out. Requires `has_error() == true`. + // NOLINTNEXTLINE(bugprone-exception-escape) const E &&error() const && noexcept { assert(has_error()); return std::get<1>(std::move(storage_)); diff --git a/include/livekit/subscription_thread_dispatcher.h b/include/livekit/subscription_thread_dispatcher.h index c647c565..273efb6e 100644 --- a/include/livekit/subscription_thread_dispatcher.h +++ b/include/livekit/subscription_thread_dispatcher.h @@ -470,8 +470,9 @@ class SubscriptionThreadDispatcher { std::unordered_map active_readers_; - /// Next auto-increment ID for data frame callbacks. - DataFrameCallbackId next_data_callback_id_; + /// Next auto-increment ID for data frame callbacks. Starts at 1 so 0 can + /// remain a sentinel "unset" value if needed downstream. + DataFrameCallbackId next_data_callback_id_{1}; /// Registered data frame callbacks keyed by opaque callback ID. std::unordered_map diff --git a/include/livekit/track.h b/include/livekit/track.h index eeff571a..7f7c82ed 100644 --- a/include/livekit/track.h +++ b/include/livekit/track.h @@ -85,6 +85,13 @@ class Track { std::optional simulcasted() const noexcept { return simulcasted_; } std::optional width() const noexcept { return width_; } std::optional height() const noexcept { return height_; } + // Returns by value (a string copy) rather than const-ref to keep the + // shape of the surrounding optional accessors. The string copy can + // theoretically throw std::bad_alloc, so the noexcept qualifier is a + // mild lie; preserved here for source/ABI compatibility with existing + // consumers that may query noexcept(t.mime_type()) or take the address + // of this overload. + // NOLINTNEXTLINE(bugprone-exception-escape) std::optional mime_type() const noexcept { return mime_type_; } // Handle access diff --git a/scripts/clang-tidy.sh b/scripts/clang-tidy.sh index a1097ef2..62214803 100755 --- a/scripts/clang-tidy.sh +++ b/scripts/clang-tidy.sh @@ -323,12 +323,17 @@ set +e # accumulate in an ~8 KB buffer and the user sees nothing until the run is # essentially over. PYTHONUNBUFFERED=1 forces line/unbuffered writes so each # file's findings appear as soon as that file's clang-tidy finishes. +# Empty-array expansion under `set -u` is treated as "unbound" on bash <4.4 +# (notably macOS's system /bin/bash 3.2 and minimal Linux containers). The +# `${arr[@]+"${arr[@]}"}` idiom expands to nothing when the array is empty +# and to the quoted elements when non-empty, sidestepping the issue +# portably back to bash 3.2. PYTHONUNBUFFERED=1 run-clang-tidy \ -p "${BUILD_DIR}" \ -quiet \ -j "${jobs}" \ - "${extra_args[@]}" \ - "${forward_args[@]}" \ + ${extra_args[@]+"${extra_args[@]}"} \ + ${forward_args[@]+"${forward_args[@]}"} \ "${FILE_REGEX}" \ 2>&1 | tee "${log}" rc="${PIPESTATUS[0]}" diff --git a/src/ffi_client.cpp b/src/ffi_client.cpp index 924a4b73..1f08d824 100644 --- a/src/ffi_client.cpp +++ b/src/ffi_client.cpp @@ -45,44 +45,6 @@ inline void logAndThrow(const std::string &error_msg) { throw std::runtime_error(error_msg); } -// TEMP: INTENTIONAL clang-tidy triggers to demo how GitHub CI renders red -// (error-level) annotations. Each line below hits a check promoted to -// WarningsAsErrors in .clang-tidy. Revert this block before merging. -// NOLINTBEGIN(misc-const-correctness) -[[maybe_unused]] void debug_tidy_error_markers() { - std::string s = "hello"; - std::string t = std::move(s); - (void)s.size(); // bugprone-use-after-move - (void)t; - - int i = 0; - while (i < 10) { // bugprone-infinite-loop - (void)t; - } - - (void)sizeof(sizeof(int)); // bugprone-sizeof-expression -} -// NOLINTEND(misc-const-correctness) - -// TEMP: INTENTIONAL clang-tidy triggers to demo how GitHub CI renders yellow -// (warning-level) annotations alongside the error block above. Each line -// below hits a check that is enabled in .clang-tidy but NOT promoted to -// WarningsAsErrors. Revert this block before merging. -[[maybe_unused]] void debug_tidy_warning_markers() { - typedef int LegacyInt; // modernize-use-using - LegacyInt n = 0; - ++n; - (void)n; - - // NOLINTNEXTLINE(misc-const-correctness) - void *p = NULL; // modernize-use-nullptr (NULL is a configured NullMacro) - p = &n; - (void)p; - - int unmodified = 42; // misc-const-correctness - (void)unmodified; -} - std::optional ExtractAsyncId(const proto::FfiEvent &event) { using E = proto::FfiEvent; switch (event.message_case()) { diff --git a/src/ffi_client.h b/src/ffi_client.h index bc5fd3e7..dd39d76c 100644 --- a/src/ffi_client.h +++ b/src/ffi_client.h @@ -32,6 +32,7 @@ #include "livekit/data_track_error.h" #include "livekit/result.h" #include "livekit/stats.h" +#include "lk_log.h" #include "room.pb.h" namespace livekit { @@ -188,8 +189,14 @@ class FfiClient { try { promise.set_exception(std::make_exception_ptr( std::runtime_error("Async operation cancelled"))); - } catch (const std::future_error &) { - // already satisfied + } catch (const std::future_error &e) { + // The promise was already satisfied -- the async op completed (or its + // exception was already set) just before this cancel() arrived. This + // race is benign and intentionally not propagated; logging at DEBUG + // surfaces it for diagnostics without polluting the default log + // level. See bugprone-empty-catch. + LK_LOG_DEBUG("FfiClient::cancel: promise already satisfied: {}", + e.what()); } } }; diff --git a/src/local_track_publication.cpp b/src/local_track_publication.cpp index 24fc0de9..b98ead82 100644 --- a/src/local_track_publication.cpp +++ b/src/local_track_publication.cpp @@ -16,7 +16,6 @@ #include "livekit/local_track_publication.h" -#include "livekit/track.h" #include "track_proto_converter.h" namespace livekit { @@ -32,9 +31,4 @@ LocalTrackPublication::LocalTrackPublication( static_cast(owned.info().encryption_type()), convertAudioFeatures(owned.info().audio_features())) {} -std::shared_ptr LocalTrackPublication::track() const noexcept { - auto base = TrackPublication::track(); - return std::static_pointer_cast(base); -} - } // namespace livekit diff --git a/src/remote_track_publication.cpp b/src/remote_track_publication.cpp index 10cf9623..c57b5586 100644 --- a/src/remote_track_publication.cpp +++ b/src/remote_track_publication.cpp @@ -18,7 +18,6 @@ #include "ffi.pb.h" #include "ffi_client.h" -#include "livekit/track.h" #include "track_proto_converter.h" namespace livekit { @@ -34,11 +33,6 @@ RemoteTrackPublication::RemoteTrackPublication( static_cast(owned.info().encryption_type()), convertAudioFeatures(owned.info().audio_features())) {} -std::shared_ptr RemoteTrackPublication::track() const noexcept { - auto base = TrackPublication::track(); - return std::static_pointer_cast(base); -} - void RemoteTrackPublication::setSubscribed(bool subscribed) { if (ffiHandleId() == 0) { throw std::runtime_error( diff --git a/src/room.cpp b/src/room.cpp index 5153097d..9eae941d 100644 --- a/src/room.cpp +++ b/src/room.cpp @@ -851,6 +851,8 @@ void Room::OnEvent(const FfiEvent &event) { const std::scoped_lock guard(lock_); const auto &asc = re.active_speakers_changed(); for (const auto &identity : asc.participant_identities()) { + // Appears to be clang-tidy false positive + // NOLINTNEXTLINE(misc-const-correctness) Participant *participant = nullptr; if (local_participant_ && local_participant_->identity() == identity) { diff --git a/src/subscription_thread_dispatcher.cpp b/src/subscription_thread_dispatcher.cpp index 1d2eb744..39ddde2c 100644 --- a/src/subscription_thread_dispatcher.cpp +++ b/src/subscription_thread_dispatcher.cpp @@ -43,13 +43,20 @@ const char *trackKindName(TrackKind kind) { } // namespace -SubscriptionThreadDispatcher::SubscriptionThreadDispatcher() - : next_data_callback_id_(1) {} +SubscriptionThreadDispatcher::SubscriptionThreadDispatcher() = default; SubscriptionThreadDispatcher::~SubscriptionThreadDispatcher() { - LK_LOG_DEBUG("Destroying SubscriptionThreadDispatcher"); - stopAll(); + try { + LK_LOG_DEBUG("Destroying SubscriptionThreadDispatcher"); + stopAll(); + } catch (const std::exception &e) { + LK_LOG_ERROR("Exception during ~SubscriptionThreadDispatcher: {}", + e.what()); + } catch (...) { + LK_LOG_ERROR("Unknown exception during ~SubscriptionThreadDispatcher"); + } } +// NOLINTEND(bugprone-exception-escape) void SubscriptionThreadDispatcher::setOnAudioFrameCallback( const std::string &participant_identity, TrackSource source, @@ -520,22 +527,34 @@ std::thread SubscriptionThreadDispatcher::startAudioReaderLocked( reader.audio_stream = stream; const std::string participant_identity = key.participant_identity; const TrackSource source = key.source; - // NOLINTBEGIN(bugprone-lambda-function-name) + // NOLINTBEGIN(bugprone-lambda-function-name,bugprone-exception-escape) + // Outer try/catch contains anything escaping the per-frame try/catch + // (stream->read, LK_LOG formatting, etc.) so an exception in this reader + // thread cannot std::terminate the process. clang-tidy still flags a + // residual escape path through spdlog's own formatter; that's a logger + // fault, not application logic -- suppressed at the lambda level. reader.thread = std::thread([stream, cb, participant_identity, source]() { - LK_LOG_DEBUG("Audio reader thread started for participant={} source={}", - participant_identity, static_cast(source)); - AudioFrameEvent ev; - while (stream->read(ev)) { - try { - cb(ev.frame); - } catch (const std::exception &e) { - LK_LOG_ERROR("Audio frame callback exception: {}", e.what()); + try { + LK_LOG_DEBUG("Audio reader thread started for participant={} source={}", + participant_identity, static_cast(source)); + AudioFrameEvent ev; + while (stream->read(ev)) { + try { + cb(ev.frame); + } catch (const std::exception &e) { + LK_LOG_ERROR("Audio frame callback exception: {}", e.what()); + } } + LK_LOG_DEBUG("Audio reader thread exiting for participant={} source={}", + participant_identity, static_cast(source)); + } catch (const std::exception &e) { + LK_LOG_ERROR("Audio reader thread terminating due to exception: {}", + e.what()); + } catch (...) { + LK_LOG_ERROR("Audio reader thread terminating due to unknown exception"); } - LK_LOG_DEBUG("Audio reader thread exiting for participant={} source={}", - participant_identity, static_cast(source)); }); - // NOLINTEND(bugprone-lambda-function-name) + // NOLINTEND(bugprone-lambda-function-name,bugprone-exception-escape) active_readers_[key] = std::move(reader); LK_LOG_DEBUG("Started audio reader for participant={} source={} " "active_readers={}", @@ -573,27 +592,37 @@ std::thread SubscriptionThreadDispatcher::startVideoReaderLocked( auto event_cb = callback.event_callback; const std::string participant_identity = key.participant_identity; const TrackSource source = key.source; - // NOLINTBEGIN(bugprone-lambda-function-name) + // NOLINTBEGIN(bugprone-lambda-function-name,bugprone-exception-escape) + // Mirrors the audio reader: outer try/catch contains escapes from + // stream->read, LK_LOG, etc. Residual diagnostic from spdlog's own + // formatter is an unrelated logger-fault path and is suppressed. reader.thread = std::thread([stream = std::move(stream), legacy_cb, event_cb, participant_identity, source]() { - LK_LOG_DEBUG("Video reader thread started for participant={} source={}", - participant_identity, static_cast(source)); - VideoFrameEvent ev; - while (stream->read(ev)) { - try { - if (event_cb) { - event_cb(ev); - } else if (legacy_cb) { - legacy_cb(ev.frame, ev.timestamp_us); + try { + LK_LOG_DEBUG("Video reader thread started for participant={} source={}", + participant_identity, static_cast(source)); + VideoFrameEvent ev; + while (stream->read(ev)) { + try { + if (event_cb) { + event_cb(ev); + } else if (legacy_cb) { + legacy_cb(ev.frame, ev.timestamp_us); + } + } catch (const std::exception &e) { + LK_LOG_ERROR("Video frame callback exception: {}", e.what()); } - } catch (const std::exception &e) { - LK_LOG_ERROR("Video frame callback exception: {}", e.what()); } + LK_LOG_DEBUG("Video reader thread exiting for participant={} source={}", + participant_identity, static_cast(source)); + } catch (const std::exception &e) { + LK_LOG_ERROR("Video reader thread terminating due to exception: {}", + e.what()); + } catch (...) { + LK_LOG_ERROR("Video reader thread terminating due to unknown exception"); } - LK_LOG_DEBUG("Video reader thread exiting for participant={} source={}", - participant_identity, static_cast(source)); }); - // NOLINTEND(bugprone-lambda-function-name) + // NOLINTEND(bugprone-lambda-function-name,bugprone-exception-escape) active_readers_[key] = std::move(reader); LK_LOG_DEBUG("Started video reader for participant={} source={} " "active_readers={}", diff --git a/src/tests/unit/test_track_publication.cpp b/src/tests/unit/test_track_publication.cpp new file mode 100644 index 00000000..a0e74bf3 --- /dev/null +++ b/src/tests/unit/test_track_publication.cpp @@ -0,0 +1,98 @@ +/* + * Copyright 2026 LiveKit + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +#include + +#include "livekit/local_track_publication.h" +#include "livekit/remote_track_publication.h" +#include "livekit/track.h" +#include "livekit/track_publication.h" +#include "track.pb.h" + +namespace livekit::test { +namespace { + +// Concrete `Track` for testing. The base ctor is `protected`, so a thin +// subclass is needed to construct one. The handle is intentionally zero so +// no FFI drop is invoked on destruction. +class FakeTrack : public Track { +public: + FakeTrack() + : Track(FfiHandle(), "fake-sid", "fake-name", TrackKind::KIND_AUDIO, + StreamState::STATE_ACTIVE, + /*muted=*/false, /*remote=*/false) {} +}; + +// Default-construct the proto: the publication ctor only reads via getters, +// which return well-defined defaults for unset fields. Using a zero handle +// keeps the `FfiHandle` a no-op on destruction. +proto::OwnedTrackPublication makeOwnedPub() { + return proto::OwnedTrackPublication{}; +} + +template +class TrackPublicationTest : public ::testing::Test {}; + +using PublicationTypes = + ::testing::Types; +TYPED_TEST_SUITE(TrackPublicationTest, PublicationTypes); + +} // namespace + +TYPED_TEST(TrackPublicationTest, TrackIsNullByDefault) { + TypeParam pub(makeOwnedPub()); + EXPECT_EQ(pub.track(), nullptr); +} + +TYPED_TEST(TrackPublicationTest, SetTrackRoundTrips) { + TypeParam pub(makeOwnedPub()); + + auto track = std::make_shared(); + pub.setTrack(track); + + EXPECT_EQ(pub.track().get(), track.get()); + // Publication holds a strong reference alongside the local one. + EXPECT_GE(track.use_count(), 2); +} + +TYPED_TEST(TrackPublicationTest, SetTrackNullClears) { + TypeParam pub(makeOwnedPub()); + + pub.setTrack(std::make_shared()); + ASSERT_NE(pub.track(), nullptr); + + pub.setTrack(nullptr); + EXPECT_EQ(pub.track(), nullptr); +} + +TYPED_TEST(TrackPublicationTest, ReplacingTrackReleasesPrevious) { + TypeParam pub(makeOwnedPub()); + + auto first = std::make_shared(); + pub.setTrack(first); + ASSERT_EQ(pub.track().get(), first.get()); + + auto second = std::make_shared(); + pub.setTrack(second); + + EXPECT_EQ(pub.track().get(), second.get()); + // The publication no longer references `first`; only the local handle does. + EXPECT_EQ(first.use_count(), 1); +} + +} // namespace livekit::test diff --git a/src/trace/event_tracer.cpp b/src/trace/event_tracer.cpp index cff9a3a0..ec57de33 100644 --- a/src/trace/event_tracer.cpp +++ b/src/trace/event_tracer.cpp @@ -211,6 +211,8 @@ std::unordered_set g_enabled_categories; std::atomic g_tracing_enabled{false}; std::atomic g_shutdown_requested{false}; std::thread g_writer_thread; +// This is internal test code, not worth changing to avoid throw risk +// NOLINTNEXTLINE(bugprone-throwing-static-initialization) std::ofstream g_trace_file; uint64_t g_start_time = 0; bool g_first_event = true; diff --git a/src/trace/event_tracer_internal.h b/src/trace/event_tracer_internal.h index 01884b85..e01d76b3 100644 --- a/src/trace/event_tracer_internal.h +++ b/src/trace/event_tracer_internal.h @@ -21,9 +21,7 @@ #include #include -namespace livekit { -namespace trace { -namespace internal { +namespace livekit::trace::internal { /** * Start tracing and write events to the specified file. @@ -54,8 +52,6 @@ void StopTracing(); */ bool IsTracingEnabled(); -} // namespace internal -} // namespace trace -} // namespace livekit +} // namespace livekit::trace::internal #endif // LIVEKIT_TRACE_EVENT_TRACER_INTERNAL_H_ From 29d4451ffc364e41c6f830c5193d56d235b060ab Mon Sep 17 00:00:00 2001 From: Alan George Date: Tue, 28 Apr 2026 16:13:55 -0600 Subject: [PATCH 16/21] Final fixes/CI adjustments --- .github/workflows/builds.yml | 8 +-- include/livekit/audio_stream.h | 4 ++ scripts/clang-tidy.sh | 95 ++++++++++++++++++++++++-- src/subscription_thread_dispatcher.cpp | 3 + 4 files changed, 99 insertions(+), 11 deletions(-) diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 4e0d6b61..04c54a49 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -611,9 +611,9 @@ jobs: - name: Run clang-tidy # scripts/clang-tidy.sh auto-detects $GITHUB_ACTIONS and emits # ::warning/::error workflow commands so findings surface as PR file - # annotations. It exits non-zero only when a finding is promoted to - # an error via WarningsAsErrors in .clang-tidy; pure warnings - # annotate but don't fail the build. + # annotations. --fail-on-warning escalates any finding (warning or + # error) to a non-zero exit so the job fails -- this prevents + # warning-level tech debt from accumulating silently. env: # Linkify findings in the step summary against the PR head commit # rather than GITHUB_SHA. On pull_request events GITHUB_SHA is the @@ -622,4 +622,4 @@ jobs: # workflow_dispatch / schedule runs this expression resolves to # github.sha, which is the pushed/selected commit. TIDY_BLOB_SHA: ${{ github.event.pull_request.head.sha || github.sha }} - run: ./scripts/clang-tidy.sh + run: ./scripts/clang-tidy.sh --fail-on-warning diff --git a/include/livekit/audio_stream.h b/include/livekit/audio_stream.h index 334d7615..d6dce270 100644 --- a/include/livekit/audio_stream.h +++ b/include/livekit/audio_stream.h @@ -41,9 +41,13 @@ class FfiEvent; * This struct wraps an AudioFrame and is used as the output type when * reading from an AudioStream. */ +// NOLINTBEGIN(bugprone-exception-escape) +// AudioFrame can throw in various places monitored by bugprone-exception-escape +// Suppressing for now, would require significant refactor to fix struct AudioFrameEvent { AudioFrame frame; ///< The decoded PCM audio frame. }; +// NOLINTEND(bugprone-exception-escape) /** * Represents a pull-based stream of decoded PCM audio frames coming from diff --git a/scripts/clang-tidy.sh b/scripts/clang-tidy.sh index 62214803..79217ee3 100755 --- a/scripts/clang-tidy.sh +++ b/scripts/clang-tidy.sh @@ -4,10 +4,14 @@ # and config. Picks up checks from the repo-root .clang-tidy automatically. # # Usage (from anywhere; the script self-anchors to the repo root): -# ./scripts/clang-tidy.sh # run on full src/ tree -# ./scripts/clang-tidy.sh -j 4 # override parallelism -# ./scripts/clang-tidy.sh --github-actions # force GitHub Actions annotation mode -# ./scripts/clang-tidy.sh -fix # forwarded to run-clang-tidy +# ./scripts/clang-tidy.sh # run on full src/ tree +# ./scripts/clang-tidy.sh -j 4 # override parallelism +# ./scripts/clang-tidy.sh --github-actions # force GitHub Actions annotation mode +# ./scripts/clang-tidy.sh --fail-on-warning # exit non-zero on any finding (warning OR error) +# ./scripts/clang-tidy.sh -fix # forwarded to run-clang-tidy +# +# Every run prints a concise stdout summary at the end with the warning and +# error counts (and a per-check breakdown when there are findings). # # In GitHub Actions (auto-detected via $GITHUB_ACTIONS=true, or forced with # --github-actions), this script additionally: @@ -15,9 +19,15 @@ # annotations (yellow for warnings, red for errors). Severity comes from # clang-tidy itself -- errors are findings promoted by WarningsAsErrors # in .clang-tidy. -# - Writes a short markdown summary to $GITHUB_STEP_SUMMARY. -# - Exits non-zero only when run-clang-tidy does (i.e. only on errors); -# warnings annotate but do not fail the build. +# - Writes a markdown summary to $GITHUB_STEP_SUMMARY. +# +# Exit code: +# - Non-zero when run-clang-tidy itself reports errors (findings promoted +# to errors via WarningsAsErrors in .clang-tidy). +# - Also non-zero when --fail-on-warning is set and at least one warning +# was emitted. This is what CI uses to gate merges so warning-level +# tech debt cannot accumulate silently. +# - Zero otherwise. # # Requires CMake to have generated build-release/compile_commands.json. # Run once: ./build.sh release (configures, builds, generates protobuf headers) @@ -43,6 +53,11 @@ if [[ "${GITHUB_ACTIONS:-}" == "true" ]]; then CI_MODE=1 fi +# When set, the script exits non-zero on any finding (warning or error), +# not just on errors. Off by default so local edit/run cycles aren't blocked +# by in-progress code; CI opts in via the workflow file. +FAIL_ON_WARNING=0 + forward_args=() while (($#)); do case "$1" in @@ -50,6 +65,10 @@ while (($#)); do CI_MODE=1 shift ;; + --fail-on-warning|--strict) + FAIL_ON_WARNING=1 + shift + ;; *) forward_args+=("$1") shift @@ -307,6 +326,54 @@ write_step_summary() { rm -f "${findings_tsv}" } +# Print a one-line summary plus a small per-check breakdown to stdout. Always +# runs (regardless of CI_MODE) so local invocations get the same headline view +# the GitHub step summary provides. Sets two globals consumed by the exit-code +# logic below: __TIDY_WARNINGS and __TIDY_ERRORS. The parsing regex matches +# the one used by emit_annotations / write_step_summary so the three views +# can't drift apart. +print_stdout_summary() { + local log="$1" + local workspace="${GITHUB_WORKSPACE:-${PWD}}" + local checks_tsv + checks_tsv="$(mktemp -t tidy-stdout.XXXXXX)" + + local line severity check + while IFS= read -r line; do + [[ "${line}" =~ ^(.+):([0-9]+):([0-9]+):[[:space:]]+(warning|error):[[:space:]]+(.+)[[:space:]]\[([^]]+)\][[:space:]]*$ ]] || continue + severity="${BASH_REMATCH[4]}" + check="${BASH_REMATCH[6]}" + # Match the annotation/summary normalization: strip the + # ",-warnings-as-errors" promotion suffix so a check buckets under one + # name regardless of severity. + check="${check%,-warnings-as-errors}" + printf '%s\t%s\n' "${severity}" "${check}" >> "${checks_tsv}" + done < "${log}" + + local warnings errors + warnings=$(awk -F'\t' '$1=="warning"{c++} END{print c+0}' "${checks_tsv}") + errors=$(awk -F'\t' '$1=="error"{c++} END{print c+0}' "${checks_tsv}") + + echo "------------------------------------------------------------" + if (( warnings == 0 && errors == 0 )); then + echo "clang-tidy summary: clean (0 warnings, 0 errors)" + else + printf 'clang-tidy summary: %d warning(s), %d error(s)\n' \ + "${warnings}" "${errors}" + echo " by check:" + awk -F'\t' '{print $2}' "${checks_tsv}" \ + | sort | uniq -c | sort -rn \ + | while read -r count name; do + printf ' %-50s %d\n' "${name}" "${count}" + done + fi + echo "------------------------------------------------------------" + + rm -f "${checks_tsv}" + __TIDY_WARNINGS="${warnings}" + __TIDY_ERRORS="${errors}" +} + # --------- End GitHub Actions annotations --------- # Capture clang-tidy's combined stdout+stderr to a stable, repo-local path so @@ -346,4 +413,18 @@ fi echo "Results written to: $(cd "$(dirname "${log}")" && pwd)/$(basename "${log}")" +# Always emit the concise headline summary to stdout. Sets __TIDY_WARNINGS / +# __TIDY_ERRORS globals which the strict-mode escalation below consumes. +__TIDY_WARNINGS=0 +__TIDY_ERRORS=0 +print_stdout_summary "${log}" + +# Strict mode: any warning escalates to a non-zero exit. Errors already make +# run-clang-tidy itself exit non-zero (rc != 0), so this only changes the +# warnings-only case. Used by CI to gate merges on a clean clang-tidy result. +if [[ "${FAIL_ON_WARNING}" == "1" && "${rc}" == "0" && "${__TIDY_WARNINGS}" -gt 0 ]]; then + echo "clang-tidy: --fail-on-warning is set and ${__TIDY_WARNINGS} warning(s) were emitted; exiting non-zero." >&2 + rc=1 +fi + exit "${rc}" diff --git a/src/subscription_thread_dispatcher.cpp b/src/subscription_thread_dispatcher.cpp index 39ddde2c..1e94f550 100644 --- a/src/subscription_thread_dispatcher.cpp +++ b/src/subscription_thread_dispatcher.cpp @@ -45,6 +45,9 @@ const char *trackKindName(TrackKind kind) { SubscriptionThreadDispatcher::SubscriptionThreadDispatcher() = default; +// NOLINTBEGIN(bugprone-exception-escape) +// spdlog can throw in this desctuctor, and clang flags as an exception escape +// suppressing for now SubscriptionThreadDispatcher::~SubscriptionThreadDispatcher() { try { LK_LOG_DEBUG("Destroying SubscriptionThreadDispatcher"); From d7da2e03c51b19437ff6f0a798cedce5cdff1b6a Mon Sep 17 00:00:00 2001 From: Alan George Date: Tue, 28 Apr 2026 17:27:11 -0600 Subject: [PATCH 17/21] Adjustments from self-PR review, reducing AI-noise --- .clang-tidy | 7 +------ .github/workflows/builds.yml | 14 +++----------- README.md | 12 ++++++------ include/livekit/local_track_publication.h | 5 ----- include/livekit/remote_track_publication.h | 7 +------ include/livekit/subscription_thread_dispatcher.h | 5 ++--- include/livekit/track.h | 7 +------ src/ffi_client.h | 7 ++----- src/subscription_thread_dispatcher.cpp | 15 ++++----------- src/trace/event_tracer.h | 5 +++++ src/trace/trace_event.h | 6 ++++++ 11 files changed, 31 insertions(+), 59 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 46cc6271..aa215469 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -31,12 +31,7 @@ WarningsAsErrors: > bugprone-suspicious-memset-usage, HeaderFilterRegex: '.*/(include/livekit|src)/.*\.(h|hpp)$' -# src/trace/{trace_event,event_tracer}.h are derived from Chromium and WebRTC -# (see "Based on" notice in their headers). Per AGENTS.md we don't restyle -# externally-pulled files, so they're excluded from clang-tidy entirely. -# event_tracer_internal.h and event_tracer.cpp are LiveKit-authored and -# remain in scope. -ExcludeHeaderFilterRegex: '(.*/src/tests/.*)|(.*/_deps/.*)|(.*/build-[^/]*/.*)|(.*/src/trace/(trace_event|event_tracer)\.h$)' +ExcludeHeaderFilterRegex: '(.*/src/tests/.*)|(.*/_deps/.*)|(.*/build-[^/]*/.*)' FormatStyle: file diff --git a/.github/workflows/builds.yml b/.github/workflows/builds.yml index 04c54a49..784687d2 100644 --- a/.github/workflows/builds.yml +++ b/.github/workflows/builds.yml @@ -609,17 +609,9 @@ jobs: run: cmake --build build-release --target livekit_proto - name: Run clang-tidy - # scripts/clang-tidy.sh auto-detects $GITHUB_ACTIONS and emits - # ::warning/::error workflow commands so findings surface as PR file - # annotations. --fail-on-warning escalates any finding (warning or - # error) to a non-zero exit so the job fails -- this prevents - # warning-level tech debt from accumulating silently. env: - # Linkify findings in the step summary against the PR head commit - # rather than GITHUB_SHA. On pull_request events GITHUB_SHA is the - # ephemeral refs/pull/N/merge commit, whose blob URLs 404 once the - # PR closes; the head SHA stays resolvable. For push / - # workflow_dispatch / schedule runs this expression resolves to - # github.sha, which is the pushed/selected commit. TIDY_BLOB_SHA: ${{ github.event.pull_request.head.sha || github.sha }} + # This script is intended to be run locally and in CI. It will auto-detect + # the GHA environment and add PR annotations and a run summary. + # As of writing all warnings are treateded as errors to avoid tech debt build up run: ./scripts/clang-tidy.sh --fail-on-warning diff --git a/README.md b/README.md index 4c17446d..0cee6160 100644 --- a/README.md +++ b/README.md @@ -528,21 +528,21 @@ To run: ./build.sh release ``` -2. Run the clang-tidy wrapper, which uses the same file set, regex filters, and `.clang-tidy` config as CI: +1. Run the clang-tidy wrapper, which uses the same file set, regex filters, and `.clang-tidy` config as CI: ```bash ./scripts/clang-tidy.sh ``` -The wrapper forwards extra arguments to `run-clang-tidy`, so you can narrow the run, change parallelism, or auto-apply fixes: +The wrapper forwards extra arguments to `run-clang-tidy`, examples below: ```bash -./scripts/clang-tidy.sh -j 4 -./scripts/clang-tidy.sh -checks='-*,misc-const-correctness' -./scripts/clang-tidy.sh -fix +./scripts/clang-tidy.sh -j 4 # Change number of cores used +./scripts/clang-tidy.sh -checks='-*,misc-const-correctness' # Only run certain checks +./scripts/clang-tidy.sh -fix # Apply fixes automatically ``` -Findings are captured to `clang-tidy.log` at the repo root for `grep`-friendly post-run inspection. +Output is captured to `clang-tidy.log` at the repo root. This is done as a convenience feature, as often times the terminal buffer is not large enough for all the output. ### Memory Checks diff --git a/include/livekit/local_track_publication.h b/include/livekit/local_track_publication.h index 404381d5..f2ccd346 100644 --- a/include/livekit/local_track_publication.h +++ b/include/livekit/local_track_publication.h @@ -29,11 +29,6 @@ class LocalTrackPublication : public TrackPublication { /// Note, this LocalTrackPublication is constructed internally only; /// safe to accept proto::OwnedTrackPublication. explicit LocalTrackPublication(const proto::OwnedTrackPublication &owned); - - // The track accessor is inherited from `TrackPublication::track()`. It - // returns `std::shared_ptr`; if a typed `LocalTrack` accessor is - // ever needed, add a differently-named method (e.g. `localTrack()`) so the - // base accessor remains source-stable for existing consumers. }; } // namespace livekit diff --git a/include/livekit/remote_track_publication.h b/include/livekit/remote_track_publication.h index a98b5609..212eabd3 100644 --- a/include/livekit/remote_track_publication.h +++ b/include/livekit/remote_track_publication.h @@ -29,12 +29,7 @@ class RemoteTrackPublication : public TrackPublication { /// Note, this RemoteTrackPublication is constructed internally only; /// safe to accept proto::OwnedTrackPublication. explicit RemoteTrackPublication(const proto::OwnedTrackPublication &owned); - - // The track accessor is inherited from `TrackPublication::track()`. It - // returns `std::shared_ptr`; if a typed `RemoteTrack` accessor is - // ever needed, add a differently-named method (e.g. `remoteTrack()`) so the - // base accessor remains source-stable for existing consumers. - + bool subscribed() const noexcept { return subscribed_; } void setSubscribed(bool subscribed); diff --git a/include/livekit/subscription_thread_dispatcher.h b/include/livekit/subscription_thread_dispatcher.h index 273efb6e..8e5fa65c 100644 --- a/include/livekit/subscription_thread_dispatcher.h +++ b/include/livekit/subscription_thread_dispatcher.h @@ -470,9 +470,8 @@ class SubscriptionThreadDispatcher { std::unordered_map active_readers_; - /// Next auto-increment ID for data frame callbacks. Starts at 1 so 0 can - /// remain a sentinel "unset" value if needed downstream. - DataFrameCallbackId next_data_callback_id_{1}; + /// Next auto-increment ID for data frame callbacks. + DataFrameCallbackId next_data_callback_id_{0}; /// Registered data frame callbacks keyed by opaque callback ID. std::unordered_map diff --git a/include/livekit/track.h b/include/livekit/track.h index 7f7c82ed..6d1e98c4 100644 --- a/include/livekit/track.h +++ b/include/livekit/track.h @@ -85,12 +85,7 @@ class Track { std::optional simulcasted() const noexcept { return simulcasted_; } std::optional width() const noexcept { return width_; } std::optional height() const noexcept { return height_; } - // Returns by value (a string copy) rather than const-ref to keep the - // shape of the surrounding optional accessors. The string copy can - // theoretically throw std::bad_alloc, so the noexcept qualifier is a - // mild lie; preserved here for source/ABI compatibility with existing - // consumers that may query noexcept(t.mime_type()) or take the address - // of this overload. + // std::string can actually throw, suppressing for now to maintain API compatibility // NOLINTNEXTLINE(bugprone-exception-escape) std::optional mime_type() const noexcept { return mime_type_; } diff --git a/src/ffi_client.h b/src/ffi_client.h index dd39d76c..182dc143 100644 --- a/src/ffi_client.h +++ b/src/ffi_client.h @@ -190,11 +190,8 @@ class FfiClient { promise.set_exception(std::make_exception_ptr( std::runtime_error("Async operation cancelled"))); } catch (const std::future_error &e) { - // The promise was already satisfied -- the async op completed (or its - // exception was already set) just before this cancel() arrived. This - // race is benign and intentionally not propagated; logging at DEBUG - // surfaces it for diagnostics without polluting the default log - // level. See bugprone-empty-catch. + // Unlikely to throw here as the promise should be satisfied before cancel() + // Logging a debug message to avoid clang empty catch warning LK_LOG_DEBUG("FfiClient::cancel: promise already satisfied: {}", e.what()); } diff --git a/src/subscription_thread_dispatcher.cpp b/src/subscription_thread_dispatcher.cpp index 1e94f550..91877877 100644 --- a/src/subscription_thread_dispatcher.cpp +++ b/src/subscription_thread_dispatcher.cpp @@ -46,18 +46,11 @@ const char *trackKindName(TrackKind kind) { SubscriptionThreadDispatcher::SubscriptionThreadDispatcher() = default; // NOLINTBEGIN(bugprone-exception-escape) -// spdlog can throw in this desctuctor, and clang flags as an exception escape -// suppressing for now +// Exceptions can be thrown by stopAll() in this desctuctor, and clang flags as +// an exception escape suppressing for now SubscriptionThreadDispatcher::~SubscriptionThreadDispatcher() { - try { - LK_LOG_DEBUG("Destroying SubscriptionThreadDispatcher"); - stopAll(); - } catch (const std::exception &e) { - LK_LOG_ERROR("Exception during ~SubscriptionThreadDispatcher: {}", - e.what()); - } catch (...) { - LK_LOG_ERROR("Unknown exception during ~SubscriptionThreadDispatcher"); - } + LK_LOG_DEBUG("Destroying SubscriptionThreadDispatcher"); + stopAll(); } // NOLINTEND(bugprone-exception-escape) diff --git a/src/trace/event_tracer.h b/src/trace/event_tracer.h index 0b7aae28..181abf22 100644 --- a/src/trace/event_tracer.h +++ b/src/trace/event_tracer.h @@ -18,6 +18,9 @@ * Use of this source code is governed by a BSD-style license. */ +// NOLINTBEGIN +// External code: this header is derived from WebRTC's event_tracer.h + // This file defines the interface for event tracing in LiveKit SDK. // // Event log handlers are set through SetupEventTracer(). User of this API will @@ -89,4 +92,6 @@ namespace webrtc { using namespace livekit::trace; } // namespace webrtc +// NOLINTEND + #endif // LIVEKIT_TRACE_EVENT_TRACER_H_ diff --git a/src/trace/trace_event.h b/src/trace/trace_event.h index 9efda258..6944ba16 100644 --- a/src/trace/trace_event.h +++ b/src/trace/trace_event.h @@ -18,6 +18,9 @@ * Use of this source code is governed by a BSD-style license. */ +// NOLINTBEGIN +// External code: this header is derived from Chromium's trace_event.h + #ifndef LIVEKIT_TRACE_TRACE_EVENT_H_ #define LIVEKIT_TRACE_TRACE_EVENT_H_ @@ -859,4 +862,7 @@ class TraceEndOnScopeClose { }; } // namespace trace_event_internal } // namespace webrtc + +// NOLINTEND + #endif // LIVEKIT_TRACE_TRACE_EVENT_H_ From 815ff9ae9a60a587b9340fb67b59f9e9781ba05e Mon Sep 17 00:00:00 2001 From: Alan George Date: Tue, 28 Apr 2026 18:40:27 -0600 Subject: [PATCH 18/21] Fix unit test --- include/livekit/subscription_thread_dispatcher.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/livekit/subscription_thread_dispatcher.h b/include/livekit/subscription_thread_dispatcher.h index 8e5fa65c..c5900ee3 100644 --- a/include/livekit/subscription_thread_dispatcher.h +++ b/include/livekit/subscription_thread_dispatcher.h @@ -471,7 +471,7 @@ class SubscriptionThreadDispatcher { active_readers_; /// Next auto-increment ID for data frame callbacks. - DataFrameCallbackId next_data_callback_id_{0}; + DataFrameCallbackId next_data_callback_id_{1}; /// Registered data frame callbacks keyed by opaque callback ID. std::unordered_map From 62f010d2857732245ab688210cf57dbe72ac2a3d Mon Sep 17 00:00:00 2001 From: Alan George Date: Tue, 28 Apr 2026 19:14:14 -0600 Subject: [PATCH 19/21] Revert unintended line-ending change in CMakePresets.json Commit 64cb6cf inadvertently rewrote CMakePresets.json from CRLF to LF (no content change). Restore the file to match origin/main so the PR diff stays focused on the clang-tidy work. Made-with: Cursor --- CMakePresets.json | 1018 ++++++++++++++++++++++----------------------- 1 file changed, 509 insertions(+), 509 deletions(-) diff --git a/CMakePresets.json b/CMakePresets.json index 5646e5c7..3c863d7c 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -1,509 +1,509 @@ -{ - "version": 6, - "cmakeMinimumRequired": { - "major": 3, - "minor": 20, - "patch": 0 - }, - "configurePresets": [ - { - "name": "vcpkg", - "hidden": true, - "cacheVariables": { - "CMAKE_TOOLCHAIN_FILE": { - "type": "FILEPATH", - "value": "$env{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake" - }, - "VCPKG_INSTALLED_DIR": "${sourceDir}/vcpkg_installed", - "LIVEKIT_USE_VCPKG": "ON", - "VCPKG_TARGET_TRIPLET": "x64-windows-static-md", - "VCPKG_HOST_TRIPLET": "x64-windows-static-md" - } - }, - { - "name": "windows-base", - "hidden": true, - "inherits": "vcpkg", - "generator": "Visual Studio 17 2022", - "architecture": { - "value": "x64", - "strategy": "set" - }, - "cacheVariables": { - "VCPKG_TARGET_TRIPLET": "x64-windows-static-md" - } - }, - { - "name": "linux-base", - "hidden": true, - "generator": "Ninja", - "cacheVariables": { - "LIVEKIT_USE_VCPKG": "OFF", - "CMAKE_EXPORT_COMPILE_COMMANDS": "ON" - }, - "condition": { - "type": "equals", - "lhs": "${hostSystemName}", - "rhs": "Linux" - } - }, - { - "name": "macos-base", - "hidden": true, - "generator": "Ninja", - "cacheVariables": { - "LIVEKIT_USE_VCPKG": "OFF", - "CMAKE_EXPORT_COMPILE_COMMANDS": "ON" - }, - "condition": { - "type": "equals", - "lhs": "${hostSystemName}", - "rhs": "Darwin" - } - }, - { - "name": "windows-release", - "displayName": "Windows x64 Release", - "description": "Build for Windows x64 Release (vcpkg)", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "windows-debug", - "displayName": "Windows x64 Debug", - "description": "Build for Windows x64 Debug (vcpkg)", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "windows-release-examples", - "displayName": "Windows x64 Release with Examples", - "description": "Build for Windows x64 Release with example applications", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "OFF", - "VCPKG_MANIFEST_FEATURES": "examples" - } - }, - { - "name": "windows-debug-examples", - "displayName": "Windows x64 Debug with Examples", - "description": "Build for Windows x64 Debug with example applications", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "OFF", - "VCPKG_MANIFEST_FEATURES": "examples" - } - }, - { - "name": "linux-release", - "displayName": "Linux Release", - "description": "Build for Linux Release (system packages)", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "linux-debug", - "displayName": "Linux Debug", - "description": "Build for Linux Debug (system packages)", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "linux-release-examples", - "displayName": "Linux Release with Examples", - "description": "Build for Linux Release with example applications", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "linux-debug-examples", - "displayName": "Linux Debug with Examples", - "description": "Build for Linux Debug with example applications", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "macos-release", - "displayName": "macOS Release", - "description": "Build for macOS Release (Homebrew packages)", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "macos-debug", - "displayName": "macOS Debug", - "description": "Build for macOS Debug (Homebrew packages)", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "macos-release-examples", - "displayName": "macOS Release with Examples", - "description": "Build for macOS Release with example applications", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "macos-debug-examples", - "displayName": "macOS Debug with Examples", - "description": "Build for macOS Debug with example applications", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "OFF" - } - }, - { - "name": "windows-release-all", - "displayName": "Windows x64 Release with Tests + Examples", - "description": "Build for Windows x64 Release with tests and examples", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "ON", - "VCPKG_MANIFEST_FEATURES": "examples" - } - }, - { - "name": "windows-debug-all", - "displayName": "Windows x64 Debug with Tests + Examples", - "description": "Build for Windows x64 Debug with tests and examples", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "ON", - "VCPKG_MANIFEST_FEATURES": "examples" - } - }, - { - "name": "windows-release-tests", - "displayName": "Windows x64 Release with Tests", - "description": "Build for Windows x64 Release with test suite", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "windows-debug-tests", - "displayName": "Windows x64 Debug with Tests", - "description": "Build for Windows x64 Debug with test suite", - "inherits": "windows-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "linux-release-all", - "displayName": "Linux Release with Tests + Examples", - "description": "Build for Linux Release with tests and examples", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "linux-debug-all", - "displayName": "Linux Debug with Tests + Examples", - "description": "Build for Linux Debug with tests and examples", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "linux-release-tests", - "displayName": "Linux Release with Tests", - "description": "Build for Linux Release with test suite", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "linux-debug-tests", - "displayName": "Linux Debug with Tests", - "description": "Build for Linux Debug with test suite", - "inherits": "linux-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "macos-release-all", - "displayName": "macOS Release with Tests + Examples", - "description": "Build for macOS Release with tests and examples", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "macos-debug-all", - "displayName": "macOS Debug with Tests + Examples", - "description": "Build for macOS Debug with tests and examples", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "ON", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "macos-release-tests", - "displayName": "macOS Release with Tests", - "description": "Build for macOS Release with test suite", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-release", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Release", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "ON" - } - }, - { - "name": "macos-debug-tests", - "displayName": "macOS Debug with Tests", - "description": "Build for macOS Debug with test suite", - "inherits": "macos-base", - "binaryDir": "${sourceDir}/build-debug", - "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "LIVEKIT_BUILD_EXAMPLES": "OFF", - "LIVEKIT_BUILD_TESTS": "ON" - } - } - ], - "buildPresets": [ - { - "name": "windows-release", - "configurePreset": "windows-release", - "configuration": "Release" - }, - { - "name": "windows-debug", - "configurePreset": "windows-debug", - "configuration": "Debug" - }, - { - "name": "windows-release-examples", - "configurePreset": "windows-release-examples", - "configuration": "Release" - }, - { - "name": "windows-debug-examples", - "configurePreset": "windows-debug-examples", - "configuration": "Debug" - }, - { - "name": "linux-release", - "configurePreset": "linux-release" - }, - { - "name": "linux-debug", - "configurePreset": "linux-debug" - }, - { - "name": "linux-release-examples", - "configurePreset": "linux-release-examples" - }, - { - "name": "linux-debug-examples", - "configurePreset": "linux-debug-examples" - }, - { - "name": "macos-release", - "configurePreset": "macos-release" - }, - { - "name": "macos-debug", - "configurePreset": "macos-debug" - }, - { - "name": "macos-release-examples", - "configurePreset": "macos-release-examples" - }, - { - "name": "macos-debug-examples", - "configurePreset": "macos-debug-examples" - }, - { - "name": "windows-release-all", - "configurePreset": "windows-release-all", - "configuration": "Release" - }, - { - "name": "windows-debug-all", - "configurePreset": "windows-debug-all", - "configuration": "Debug" - }, - { - "name": "linux-release-all", - "configurePreset": "linux-release-all" - }, - { - "name": "linux-debug-all", - "configurePreset": "linux-debug-all" - }, - { - "name": "macos-release-all", - "configurePreset": "macos-release-all" - }, - { - "name": "macos-debug-all", - "configurePreset": "macos-debug-all" - }, - { - "name": "windows-release-tests", - "configurePreset": "windows-release-tests", - "configuration": "Release" - }, - { - "name": "windows-debug-tests", - "configurePreset": "windows-debug-tests", - "configuration": "Debug" - }, - { - "name": "linux-release-tests", - "configurePreset": "linux-release-tests" - }, - { - "name": "linux-debug-tests", - "configurePreset": "linux-debug-tests" - }, - { - "name": "macos-release-tests", - "configurePreset": "macos-release-tests" - }, - { - "name": "macos-debug-tests", - "configurePreset": "macos-debug-tests" - } - ], - "testPresets": [ - { - "name": "windows-release-tests", - "configurePreset": "windows-release-tests", - "configuration": "Release", - "output": { - "outputOnFailure": true - } - }, - { - "name": "windows-debug-tests", - "configurePreset": "windows-debug-tests", - "configuration": "Debug", - "output": { - "outputOnFailure": true - } - }, - { - "name": "linux-release-tests", - "configurePreset": "linux-release-tests", - "output": { - "outputOnFailure": true - } - }, - { - "name": "linux-debug-tests", - "configurePreset": "linux-debug-tests", - "output": { - "outputOnFailure": true - } - }, - { - "name": "macos-release-tests", - "configurePreset": "macos-release-tests", - "output": { - "outputOnFailure": true - } - }, - { - "name": "macos-debug-tests", - "configurePreset": "macos-debug-tests", - "output": { - "outputOnFailure": true - } - } - ] -} +{ + "version": 6, + "cmakeMinimumRequired": { + "major": 3, + "minor": 20, + "patch": 0 + }, + "configurePresets": [ + { + "name": "vcpkg", + "hidden": true, + "cacheVariables": { + "CMAKE_TOOLCHAIN_FILE": { + "type": "FILEPATH", + "value": "$env{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake" + }, + "VCPKG_INSTALLED_DIR": "${sourceDir}/vcpkg_installed", + "LIVEKIT_USE_VCPKG": "ON", + "VCPKG_TARGET_TRIPLET": "x64-windows-static-md", + "VCPKG_HOST_TRIPLET": "x64-windows-static-md" + } + }, + { + "name": "windows-base", + "hidden": true, + "inherits": "vcpkg", + "generator": "Visual Studio 17 2022", + "architecture": { + "value": "x64", + "strategy": "set" + }, + "cacheVariables": { + "VCPKG_TARGET_TRIPLET": "x64-windows-static-md" + } + }, + { + "name": "linux-base", + "hidden": true, + "generator": "Ninja", + "cacheVariables": { + "LIVEKIT_USE_VCPKG": "OFF", + "CMAKE_EXPORT_COMPILE_COMMANDS": "ON" + }, + "condition": { + "type": "equals", + "lhs": "${hostSystemName}", + "rhs": "Linux" + } + }, + { + "name": "macos-base", + "hidden": true, + "generator": "Ninja", + "cacheVariables": { + "LIVEKIT_USE_VCPKG": "OFF", + "CMAKE_EXPORT_COMPILE_COMMANDS": "ON" + }, + "condition": { + "type": "equals", + "lhs": "${hostSystemName}", + "rhs": "Darwin" + } + }, + { + "name": "windows-release", + "displayName": "Windows x64 Release", + "description": "Build for Windows x64 Release (vcpkg)", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "windows-debug", + "displayName": "Windows x64 Debug", + "description": "Build for Windows x64 Debug (vcpkg)", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "windows-release-examples", + "displayName": "Windows x64 Release with Examples", + "description": "Build for Windows x64 Release with example applications", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "OFF", + "VCPKG_MANIFEST_FEATURES": "examples" + } + }, + { + "name": "windows-debug-examples", + "displayName": "Windows x64 Debug with Examples", + "description": "Build for Windows x64 Debug with example applications", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "OFF", + "VCPKG_MANIFEST_FEATURES": "examples" + } + }, + { + "name": "linux-release", + "displayName": "Linux Release", + "description": "Build for Linux Release (system packages)", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "linux-debug", + "displayName": "Linux Debug", + "description": "Build for Linux Debug (system packages)", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "linux-release-examples", + "displayName": "Linux Release with Examples", + "description": "Build for Linux Release with example applications", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "linux-debug-examples", + "displayName": "Linux Debug with Examples", + "description": "Build for Linux Debug with example applications", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "macos-release", + "displayName": "macOS Release", + "description": "Build for macOS Release (Homebrew packages)", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "macos-debug", + "displayName": "macOS Debug", + "description": "Build for macOS Debug (Homebrew packages)", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "macos-release-examples", + "displayName": "macOS Release with Examples", + "description": "Build for macOS Release with example applications", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "macos-debug-examples", + "displayName": "macOS Debug with Examples", + "description": "Build for macOS Debug with example applications", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "OFF" + } + }, + { + "name": "windows-release-all", + "displayName": "Windows x64 Release with Tests + Examples", + "description": "Build for Windows x64 Release with tests and examples", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "ON", + "VCPKG_MANIFEST_FEATURES": "examples" + } + }, + { + "name": "windows-debug-all", + "displayName": "Windows x64 Debug with Tests + Examples", + "description": "Build for Windows x64 Debug with tests and examples", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "ON", + "VCPKG_MANIFEST_FEATURES": "examples" + } + }, + { + "name": "windows-release-tests", + "displayName": "Windows x64 Release with Tests", + "description": "Build for Windows x64 Release with test suite", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "windows-debug-tests", + "displayName": "Windows x64 Debug with Tests", + "description": "Build for Windows x64 Debug with test suite", + "inherits": "windows-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "linux-release-all", + "displayName": "Linux Release with Tests + Examples", + "description": "Build for Linux Release with tests and examples", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "linux-debug-all", + "displayName": "Linux Debug with Tests + Examples", + "description": "Build for Linux Debug with tests and examples", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "linux-release-tests", + "displayName": "Linux Release with Tests", + "description": "Build for Linux Release with test suite", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "linux-debug-tests", + "displayName": "Linux Debug with Tests", + "description": "Build for Linux Debug with test suite", + "inherits": "linux-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "macos-release-all", + "displayName": "macOS Release with Tests + Examples", + "description": "Build for macOS Release with tests and examples", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "macos-debug-all", + "displayName": "macOS Debug with Tests + Examples", + "description": "Build for macOS Debug with tests and examples", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "ON", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "macos-release-tests", + "displayName": "macOS Release with Tests", + "description": "Build for macOS Release with test suite", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-release", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Release", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "ON" + } + }, + { + "name": "macos-debug-tests", + "displayName": "macOS Debug with Tests", + "description": "Build for macOS Debug with test suite", + "inherits": "macos-base", + "binaryDir": "${sourceDir}/build-debug", + "cacheVariables": { + "CMAKE_BUILD_TYPE": "Debug", + "LIVEKIT_BUILD_EXAMPLES": "OFF", + "LIVEKIT_BUILD_TESTS": "ON" + } + } + ], + "buildPresets": [ + { + "name": "windows-release", + "configurePreset": "windows-release", + "configuration": "Release" + }, + { + "name": "windows-debug", + "configurePreset": "windows-debug", + "configuration": "Debug" + }, + { + "name": "windows-release-examples", + "configurePreset": "windows-release-examples", + "configuration": "Release" + }, + { + "name": "windows-debug-examples", + "configurePreset": "windows-debug-examples", + "configuration": "Debug" + }, + { + "name": "linux-release", + "configurePreset": "linux-release" + }, + { + "name": "linux-debug", + "configurePreset": "linux-debug" + }, + { + "name": "linux-release-examples", + "configurePreset": "linux-release-examples" + }, + { + "name": "linux-debug-examples", + "configurePreset": "linux-debug-examples" + }, + { + "name": "macos-release", + "configurePreset": "macos-release" + }, + { + "name": "macos-debug", + "configurePreset": "macos-debug" + }, + { + "name": "macos-release-examples", + "configurePreset": "macos-release-examples" + }, + { + "name": "macos-debug-examples", + "configurePreset": "macos-debug-examples" + }, + { + "name": "windows-release-all", + "configurePreset": "windows-release-all", + "configuration": "Release" + }, + { + "name": "windows-debug-all", + "configurePreset": "windows-debug-all", + "configuration": "Debug" + }, + { + "name": "linux-release-all", + "configurePreset": "linux-release-all" + }, + { + "name": "linux-debug-all", + "configurePreset": "linux-debug-all" + }, + { + "name": "macos-release-all", + "configurePreset": "macos-release-all" + }, + { + "name": "macos-debug-all", + "configurePreset": "macos-debug-all" + }, + { + "name": "windows-release-tests", + "configurePreset": "windows-release-tests", + "configuration": "Release" + }, + { + "name": "windows-debug-tests", + "configurePreset": "windows-debug-tests", + "configuration": "Debug" + }, + { + "name": "linux-release-tests", + "configurePreset": "linux-release-tests" + }, + { + "name": "linux-debug-tests", + "configurePreset": "linux-debug-tests" + }, + { + "name": "macos-release-tests", + "configurePreset": "macos-release-tests" + }, + { + "name": "macos-debug-tests", + "configurePreset": "macos-debug-tests" + } + ], + "testPresets": [ + { + "name": "windows-release-tests", + "configurePreset": "windows-release-tests", + "configuration": "Release", + "output": { + "outputOnFailure": true + } + }, + { + "name": "windows-debug-tests", + "configurePreset": "windows-debug-tests", + "configuration": "Debug", + "output": { + "outputOnFailure": true + } + }, + { + "name": "linux-release-tests", + "configurePreset": "linux-release-tests", + "output": { + "outputOnFailure": true + } + }, + { + "name": "linux-debug-tests", + "configurePreset": "linux-debug-tests", + "output": { + "outputOnFailure": true + } + }, + { + "name": "macos-release-tests", + "configurePreset": "macos-release-tests", + "output": { + "outputOnFailure": true + } + }, + { + "name": "macos-debug-tests", + "configurePreset": "macos-debug-tests", + "output": { + "outputOnFailure": true + } + } + ] +} From 0430a0f8f364aafa78f3450b22afc6beaf42588f Mon Sep 17 00:00:00 2001 From: Alan George Date: Wed, 29 Apr 2026 10:57:48 -0600 Subject: [PATCH 20/21] Add copyright/help --- scripts/clang-tidy.sh | 93 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/scripts/clang-tidy.sh b/scripts/clang-tidy.sh index 79217ee3..edcca5b9 100755 --- a/scripts/clang-tidy.sh +++ b/scripts/clang-tidy.sh @@ -1,4 +1,19 @@ #!/usr/bin/env bash +# +# Copyright 2026 LiveKit +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + # # clang-tidy.sh -- Run clang-tidy locally and in CI using the same file set # and config. Picks up checks from the repo-root .clang-tidy automatically. @@ -41,6 +56,59 @@ set -euo pipefail script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd -P)" cd "${script_dir}/.." +# Usage banner. Printed by --help and referenced by the EXIT trap below so +# users who hit a pre-flight error or a bad argument get pointed at this. +usage() { + cat <<'EOF' +Usage: ./scripts/clang-tidy.sh [OPTIONS] [run-clang-tidy args...] + +Run clang-tidy locally or in CI using the same file set and config that the +repo's CI workflow uses. Picks up checks from the repo-root .clang-tidy. + +Options: + -h, --help, -? + Show this help and exit. + --github-actions, --gh + Force GitHub Actions annotation + step-summary mode. + Auto-detected when GITHUB_ACTIONS=true. + --fail-on-warning, --strict + Exit non-zero when any warning is emitted (errors already exit + non-zero on their own). Off by default for local edit/run cycles; + CI opts in via the workflow file. + +Any other arguments are forwarded verbatim to run-clang-tidy. Common ones: + -j N worker count (defaults to nproc / sysctl hw.ncpu) + -fix apply fixes in place + -checks=... override the .clang-tidy check list for this run + +Examples: + ./scripts/clang-tidy.sh # full src/ tree + ./scripts/clang-tidy.sh -j 4 # override parallelism + ./scripts/clang-tidy.sh --github-actions # force CI annotation mode + ./scripts/clang-tidy.sh --fail-on-warning # exit non-zero on any finding + ./scripts/clang-tidy.sh -fix # forwarded to run-clang-tidy + +Pre-requisites: + CMake must have generated build-release/compile_commands.json AND the + protobuf headers under build-release/generated/. Run once: + ./build.sh release +EOF +} + +# Print a one-line "see --help" hint on any non-zero exit during pre-flight +# (argument parsing, missing build artifacts, missing run-clang-tidy, etc.). +# It is suppressed once we hand off to run-clang-tidy itself, so legitimate +# clang-tidy findings don't trigger the hint. +__tidy_hint_active=1 +__tidy_print_hint() { + local rc=$? + if (( rc != 0 )) && (( __tidy_hint_active )); then + echo >&2 + echo "Run './scripts/clang-tidy.sh --help' for usage." >&2 + fi +} +trap __tidy_print_hint EXIT + BUILD_DIR="build-release" # Positive match for top-level src/*.{c,cpp,cc,cxx}; negative lookahead excludes # dep paths (_deps/, build-*/, -src/src/) and every other top-level dir. Python @@ -61,6 +129,11 @@ FAIL_ON_WARNING=0 forward_args=() while (($#)); do case "$1" in + -h|--help|-\?) + usage + __tidy_hint_active=0 + exit 0 + ;; --github-actions|--gh) CI_MODE=1 shift @@ -69,6 +142,21 @@ while (($#)); do FAIL_ON_WARNING=1 shift ;; + --) + # Explicit forwarding separator: everything after `--` goes straight + # to run-clang-tidy without further interpretation. + shift + forward_args+=("$@") + break + ;; + --*) + # Long options we don't recognize are user typos far more often than + # genuine run-clang-tidy long options (run-clang-tidy uses single-dash + # long options like -fix, -j, -checks=...). Reject and surface usage. + echo "ERROR: unknown option: $1" >&2 + usage >&2 + exit 2 + ;; *) forward_args+=("$1") shift @@ -383,6 +471,11 @@ print_stdout_summary() { # the previous log via `tee` (no -a), keeping the path predictable. log="clang-tidy.log" +# Past pre-flight: any non-zero exit from here on is a clang-tidy result +# (findings, internal error, etc.), not a user-facing argument/usage error, +# so suppress the "see --help" hint installed via the EXIT trap above. +__tidy_hint_active=0 + set +e # run-clang-tidy is a Python script that doesn't flush stdout in its per-file # loop; it only flushes once at exit. When its stdout is a pipe (the `| tee` From fea9018d2af66ff66917b60121f524eee52d3262 Mon Sep 17 00:00:00 2001 From: Alan George Date: Wed, 29 Apr 2026 12:29:53 -0600 Subject: [PATCH 21/21] Fix for subscription data frame callback ID --- include/livekit/subscription_thread_dispatcher.h | 2 +- src/tests/unit/test_subscription_thread_dispatcher.cpp | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/include/livekit/subscription_thread_dispatcher.h b/include/livekit/subscription_thread_dispatcher.h index c5900ee3..8e5fa65c 100644 --- a/include/livekit/subscription_thread_dispatcher.h +++ b/include/livekit/subscription_thread_dispatcher.h @@ -471,7 +471,7 @@ class SubscriptionThreadDispatcher { active_readers_; /// Next auto-increment ID for data frame callbacks. - DataFrameCallbackId next_data_callback_id_{1}; + DataFrameCallbackId next_data_callback_id_{0}; /// Registered data frame callbacks keyed by opaque callback ID. std::unordered_map diff --git a/src/tests/unit/test_subscription_thread_dispatcher.cpp b/src/tests/unit/test_subscription_thread_dispatcher.cpp index bb3bdfd1..a3864c64 100644 --- a/src/tests/unit/test_subscription_thread_dispatcher.cpp +++ b/src/tests/unit/test_subscription_thread_dispatcher.cpp @@ -562,8 +562,15 @@ TEST_F(SubscriptionThreadDispatcherTest, "alice", "my-track", [](const std::vector &, std::optional) {}); - EXPECT_NE(id, 0u); + EXPECT_EQ(id, 0u); EXPECT_EQ(dataCallbacks(dispatcher).size(), 1u); + + // Add a second one to confirm size and IDs are correct + auto id2 = dispatcher.addOnDataFrameCallback( + "alice", "my-track", + [](const std::vector &, std::optional) {}); + EXPECT_EQ(id2, 1u); + EXPECT_EQ(dataCallbacks(dispatcher).size(), 2u); } TEST_F(SubscriptionThreadDispatcherTest,