From 6560b8b45156a5a327310aab491d153e432f9330 Mon Sep 17 00:00:00 2001 From: Alan George Date: Thu, 23 Apr 2026 12:30:07 -0600 Subject: [PATCH 01/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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/10] 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()) {