-
Notifications
You must be signed in to change notification settings - Fork 1
Initial cpp-tools version #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
24 commits
Select commit
Hold shift + click to select a range
2bc6880
Add shared C++ tooling scripts
alan-george-lk 9f3067c
Add reusable C++ checks workflow
alan-george-lk 7d5c71d
Cleaner layout for easier use, updated README
alan-george-lk f2dbc22
Harden shared C++ checks workflow
alan-george-lk 53a90dc
Undo doxygen
alan-george-lk 12c9451
Cleaner install
alan-george-lk cdfd744
Don't make pre-commit hook required
alan-george-lk 257865d
Separate AGENTS.md
alan-george-lk ce85deb
New section on clangd
alan-george-lk 0f2e3f4
Another agents md update
alan-george-lk e9468a3
AGENTS.md adjustments
alan-george-lk 9b5b95a
More inputs/descriptions
alan-george-lk d0934bb
Regex not required
alan-george-lk 2828658
Fix clang-format directory scans
alan-george-lk 7077627
Add consumer tooling runners
alan-george-lk 1e2d2cd
Remove shared workflow
alan-george-lk 493e1a0
Add clang-tidy doc
alan-george-lk e85feaf
README updates
alan-george-lk 41bf0d3
Update README
alan-george-lk ebb47ff
Another README update
alan-george-lk 2cc7268
Add CODEOWNERS
alan-george-lk 846464b
Basic CI job
alan-george-lk 05ce3e8
Actually use analyzer
alan-george-lk 43fe313
Add style section
alan-george-lk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| --- | ||
| BasedOnStyle: Google | ||
| Language: Cpp | ||
| Standard: c++17 | ||
|
|
||
| # LiveKit modifications to Google style below | ||
|
|
||
| ColumnLimit: 120 # more width on modern screens | ||
| SpacesBeforeTrailingComments: 1 # one space for trailing namespace comments, e.g. `} // namespace foo` (Google uses 2) | ||
| AccessModifierOffset: -2 # left-align public/protected/private with the class keyword (Google indents by 1) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| Checks: > | ||
| -*, | ||
| clang-analyzer-*, | ||
| bugprone-*, | ||
| misc-const-correctness, | ||
| performance-*, | ||
| modernize-*, | ||
| readability-misleading-indentation, | ||
| readability-redundant-smartptr-get, | ||
| readability-identifier-naming, | ||
| -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, | ||
| -performance-enum-size, | ||
| -readability-braces-around-statements, | ||
|
|
||
| # These warnings have determined to be critical and are as such treated as errors | ||
| WarningsAsErrors: > | ||
| clang-analyzer-*, | ||
| bugprone-use-after-move, | ||
| bugprone-dangling-handle, | ||
| bugprone-infinite-loop, | ||
| bugprone-narrowing-conversions, | ||
| bugprone-undefined-memory-manipulation, | ||
| bugprone-move-forwarding-reference, | ||
| bugprone-incorrect-roundings, | ||
| bugprone-sizeof-expression, | ||
| bugprone-string-literal-with-embedded-nul, | ||
| bugprone-suspicious-memset-usage, | ||
|
|
||
| FormatStyle: file | ||
|
|
||
| CheckOptions: | ||
| - key: modernize-use-nullptr.NullMacros | ||
| value: 'NULL' | ||
| - key: readability-identifier-naming.ClassCase | ||
| value: CamelCase | ||
| - key: readability-identifier-naming.MethodCase | ||
| value: camelBack |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| * @alan-george-lk @stephen-derosa |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,64 @@ | ||
| name: CI | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| pull_request: | ||
| push: | ||
| branches: ["main"] | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| jobs: | ||
| lint: | ||
| name: Lint | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
|
|
||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
|
|
||
| - name: Install ShellCheck | ||
| run: | | ||
| sudo apt-get update | ||
| sudo apt-get install -y shellcheck | ||
|
|
||
| - name: Check shell scripts | ||
| run: | | ||
| bash -n ./*.sh | ||
| shellcheck ./*.sh | ||
|
|
||
| - name: Check Markdown | ||
| run: npx --yes markdownlint-cli2@0.23.1 README.md AGENTS.md 'docs/**/*.md' | ||
|
|
||
| link-check: | ||
| name: Link Check | ||
| runs-on: ubuntu-latest | ||
| timeout-minutes: 10 | ||
|
|
||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 | ||
|
|
||
| - name: Restore lychee cache | ||
| uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 | ||
| with: | ||
| path: .lycheecache | ||
| key: cache-lychee-${{ github.sha }} | ||
| restore-keys: cache-lychee- | ||
|
|
||
| - name: Run lychee | ||
| uses: lycheeverse/lychee-action@8646ba30535128ac92d33dfc9133794bfdd9b411 # v2.8.0 | ||
| with: | ||
| args: >- | ||
| --verbose | ||
| --no-progress | ||
| --cache | ||
| --max-cache-age 1d | ||
| --root-dir . | ||
| README.md | ||
| AGENTS.md | ||
| 'docs/**/*.md' | ||
| fail: true | ||
| jobSummary: false |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| .lycheecache |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| { | ||
| "config": { | ||
| "MD013": false | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| # AGENTS.md — Shared C++ Engineering Baseline | ||
|
|
||
| ## Scope | ||
|
|
||
| These rules apply to LiveKit C++ projects that consume `cpp-tools`. If a consuming | ||
| repository has an `AGENTS.md` with conflicting rules they should take priority. | ||
|
|
||
| ## Safety and Determinism | ||
|
|
||
| - Design for predictable memory use, execution time, and failure behavior. | ||
| - Avoid heap allocation when practical. In real-time, callback, and steady-state | ||
| paths, allocate resources during initialization and reuse them. | ||
| - Prefer stack storage, RAII, fixed-capacity storage, and bounded pools or | ||
| queues. Document unavoidable dynamic allocation in time-sensitive code. | ||
| - Put explicit bounds on queues, retries, buffers, worker counts, and waits. | ||
| Define observable behavior for exhaustion and overload. | ||
| - Do not block real-time or callback threads with file/network I/O, sleeps, | ||
| unbounded work, allocation, or contended locks. Offload work through bounded | ||
| mechanisms with clear back-pressure. | ||
| - Avoid unbounded recursion and large stack objects. Account for stack limits on | ||
| embedded targets. | ||
|
|
||
| ## Errors and Failure Modes | ||
|
|
||
| - Use return values for expected failures instead of exceptions: | ||
| - `std::optional<T>` when absence is expected and needs no diagnostic. | ||
| - `bool` for a simple success/failure result. | ||
| - `Result<T, E>`, `expected` (C++23 or higher), or equivalent when callers need a typed error. | ||
| - Callers must inspect status-bearing return values. Mark important results | ||
| `[[nodiscard]]` where practical. | ||
| - Do not throw through C, FFI, callback, destructor, real-time, or | ||
| resource-constrained boundaries. | ||
| - Reserve exceptions for genuinely exceptional failures when the consuming | ||
| project permits them. Catch them at a well-defined boundary and convert them | ||
| to the project's error model. | ||
| - Validate external inputs and cross-boundary data. Keep state valid on failure | ||
| and prefer fail-safe behavior over partial updates. | ||
|
|
||
| ## Memory, Ownership, and Lifetime | ||
|
|
||
| - Make ownership explicit. Prefer values and RAII types; do not use raw owning | ||
| pointers. | ||
| - Use `std::unique_ptr` for exclusive dynamic ownership and `std::shared_ptr` | ||
| only when ownership is genuinely shared. | ||
| - Avoid heap allocations as much as possible. If code is heap allocating in a loop | ||
| or a high-frequency path, second guess the design and consider alternatives. | ||
| - Keep object lifetimes and teardown order deterministic. Destructors must not | ||
| throw. | ||
| - Avoid hidden copies of large buffers. Make copy and move behavior intentional, | ||
| especially for media, sensor, and message data. | ||
| - Declare data at the smallest useful scope and initialize it before use. | ||
|
|
||
| ## Types, Arithmetic, and Units | ||
|
|
||
| - Prefer STL types over third-party dependencies when possible. | ||
| - Prefer fixed-width integers from `<cstdint>` when width or signedness matters, | ||
| including serialization, FFI, hardware, timestamps, IDs, and public APIs. | ||
| - Use platform-sized primitive integers only when the value is intentionally | ||
| platform-sized or compatibility requires it. | ||
| - Avoid implicit narrowing and mixed signed/unsigned arithmetic. Validate ranges | ||
| before conversions and arithmetic that can overflow. | ||
| - Represent durations and time points with `std::chrono`; use a monotonic clock | ||
| for elapsed time, deadlines, and timeouts. | ||
| - Make physical units explicit with strong or clearly named types, such as `_us` | ||
| for microseconds. Do not pass ambiguous raw numeric values across interfaces. | ||
|
|
||
| ## Concurrency | ||
|
|
||
| - Document which threads call an API and whether each type is thread-safe. | ||
| - Minimize shared mutable state. Protect it with clear synchronization and keep | ||
| critical sections short. | ||
| - Never call user code while holding an internal lock. | ||
| - Use bounded waits and define cancellation and shutdown behavior. Join worker | ||
| threads outside locks. | ||
| - Treat atomics and lock-free code as specialized tools; document memory-order | ||
| reasoning and test concurrency paths under stress. | ||
|
|
||
| ## Design and Readability | ||
|
|
||
| - Keep functions focused and short, roughly 60 lines or fewer when practical. | ||
| - Prefer straightforward control flow over clever abstractions. Document any | ||
| deliberate tradeoff between readability, determinism, and performance. | ||
| - Use `enum class`, `nullptr`, explicit conversions, and const-correct | ||
| interfaces. | ||
| - Check non-void return values and make ignored results explicit. | ||
| - Use `git mv` when moving or renaming tracked files. | ||
|
|
||
| ## Portability | ||
|
|
||
| - Avoid undefined behavior, compiler-specific assumptions, and dependence on | ||
| host endianness, alignment, or primitive widths. | ||
| - Keep cross-platform and cross-architecture boundaries explicit. Test all | ||
| supported targets defined by the consuming repository. | ||
| - Keep third-party implementation details out of public headers and ABI | ||
| boundaries. | ||
|
|
||
| ## Style | ||
|
|
||
| - Add the LiveKit copyright header with the correct year to new code files. | ||
| - Prefer the constructor initializer list rather than variable declaration | ||
| and assignment in the constructor body. | ||
| - For Doxygen/doc comments, prefer `///` comment style and use @brief, | ||
| @param, @return, @throw, @ref, @note, @warning as applicable. | ||
|
|
||
| ## Project-Owned clangd Configuration | ||
|
|
||
| - Each consuming project must provide its own `.clangd`; compilation database | ||
| locations and flags are project-specific and are not shared by `cpp-tools`. | ||
| - Verify `.clangd` points clangd at the project's generated compilation database | ||
| before relying on IDE diagnostics. | ||
| - clang-tidy does not read `.clangd`. Before running clang-tidy, generate a valid | ||
| `compile_commands.json` and pass its build directory to `clang-tidy.sh`. | ||
|
|
||
| ## Verification | ||
|
|
||
| - Adhere to the shared `.clang-format` and `.clang-tidy` configurations. | ||
| - After C++ changes, run `./cpp-tools/clang-format.sh` with the consuming | ||
| project's paths. Use `--fix` when needed, then rerun the check. | ||
| - Generate the consuming project's compilation database and run | ||
| `./cpp-tools/clang-tidy.sh` with its documented build directory and filters. | ||
| - Do not bypass formatter or static-analysis failures. Keep suppressions narrow, | ||
| local, and justified in code. | ||
| - Add deterministic tests for normal, boundary, overload, timeout, cancellation, | ||
| and failure behavior. Avoid timing-only sleeps when a condition or simulated | ||
| clock can be used. | ||
| - Benchmark or stress-test new time-sensitive or resource-sensitive behavior and | ||
| verify that configured limits are enforced. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,138 @@ | ||
| # cpp-tools | ||
| Standardized set of tools for use in LiveKit C++ projects. | ||
|
|
||
| Standardized tools for LiveKit C++ projects. This repository provides: | ||
|
|
||
| - [clang-format](https://clang.llvm.org/docs/ClangFormat.html): Code styling consistency across projects | ||
| - [clang-tidy](https://clang.llvm.org/extra/clang-tidy/): Standardized static analysis and bug catching checks. See [docs/clang-tidy.md](./docs/clang-tidy.md) | ||
| - Base [AGENTS.md](./AGENTS.md) with C++ best practices | ||
| - Helper scripts and GitHub actions reporting support | ||
|
|
||
| This repository is intended to be consumed as a git submodule. | ||
|
|
||
| ## Quick start | ||
|
|
||
| Add this repository as a submodule from the consuming repository root: | ||
|
|
||
| ```bash | ||
| git submodule add https://github.com/livekit/cpp-tools.git cpp-tools | ||
| ``` | ||
|
|
||
| Install the shared configuration symlinks: | ||
|
|
||
| ```bash | ||
| ./cpp-tools/install.sh # Installs .clang-format and .clang-tidy symlinks to repo root | ||
| ``` | ||
|
|
||
| Optionally install a precommit hook that automatically runs `clang-format` before commits: | ||
|
|
||
| ```bash | ||
| ./cpp-tools/install.sh precommit-hook # Installs precommit hook | ||
| ``` | ||
|
|
||
| Run the tools from the repository root: | ||
|
|
||
| ```bash | ||
| # Check formatting or rewrite files in place. | ||
| ./cpp-tools/clang-format.sh --path path/to/sources | ||
| ./cpp-tools/clang-format.sh --path path/to/sources --fix | ||
|
|
||
| # Run static analysis after generating compile_commands.json. | ||
| ./cpp-tools/clang-tidy.sh --file-regex '.*\.(c|cpp|cc|cxx)$' | ||
| ./cpp-tools/clang-tidy.sh --file-regex '.*\.(c|cpp|cc|cxx)$' --fail-on-warning | ||
| ``` | ||
|
|
||
| > Note: It's recommended to wrap these tool scripts in consuming repository scripts such that arguments and options can be passed in and re-used in CI. See below section for examples. | ||
|
|
||
| Update existing `AGENTS.md` file to reference this one: | ||
|
|
||
| ```markdown | ||
| ## Shared C++ baseline | ||
| Follow `cpp-tools/AGENTS.md` for shared LiveKit C++ engineering guidance. | ||
| Instructions in this file are project-specific and take precedence if they | ||
| conflict with the shared baseline. | ||
| ``` | ||
|
|
||
| ## clang-format | ||
|
|
||
| Each consumer supplies the tracked paths that `clang-format.sh` should check. | ||
| Pass `--path` repeatedly for repositories with multiple source trees: | ||
|
|
||
| ```bash | ||
| ./cpp-tools/clang-format.sh \ | ||
| --path path/to/sources \ | ||
| --path path/to/headers | ||
| ``` | ||
|
|
||
| The `CLANG_FORMAT_PATHS` environment variable provides the same configuration | ||
| as a colon-separated list. Positional file paths restrict the check to those | ||
| files. | ||
|
|
||
| ## clang-tidy | ||
|
|
||
| See [docs/clang-tidy.md](docs/clang-tidy.md) for the enabled checks, | ||
| exclusions, and the reasoning behind them. | ||
|
|
||
| The consuming repository must generate `compile_commands.json` before running | ||
| `clang-tidy.sh`. Project-specific behavior is configured with command-line | ||
| flags or their corresponding environment variables: | ||
|
|
||
| ```bash | ||
| ./cpp-tools/clang-tidy.sh \ | ||
| --build-dir build-release \ | ||
| --file-regex '.*\.(c|cpp|cc|cxx)$' | ||
| ``` | ||
|
|
||
| Additional `run-clang-tidy` arguments can be passed after `--`. | ||
|
|
||
| ## Tool wrappers | ||
|
|
||
| Consuming repositories should provide thin project-owned entrypoints such as | ||
| `scripts/clang-format.sh` and `scripts/clang-tidy.sh`. The wrappers encode the | ||
| repository's paths, filters, and build directory, then `exec` the shared script. | ||
| This gives developers a zero-argument command without copying the formatting, | ||
| diagnostic, or GitHub summary implementation. | ||
|
|
||
| For example: | ||
|
|
||
| ```bash | ||
| #!/usr/bin/env bash | ||
| set -euo pipefail | ||
|
|
||
| repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd -P)" | ||
| export CLANG_FORMAT_FIX_COMMAND="./scripts/clang-format.sh --fix" | ||
| exec "${repo_root}/cpp-tools/clang-format.sh" \ | ||
| --repo-root "${repo_root}" \ | ||
| --path src \ | ||
| --path include \ | ||
| "$@" | ||
| ``` | ||
|
|
||
| Repository-owned CI workflows should invoke the same project entrypoints so | ||
| local and CI file selection cannot drift. Repositories that do not use wrappers | ||
| can call the shared scripts with explicit arguments. | ||
|
|
||
| ## Pre-commit hook | ||
|
|
||
| The hook formats staged C++ files and re-stages files rewritten by `clang-format`. | ||
|
|
||
| ## GitHub Actions | ||
|
|
||
| The scripts automatically enable GitHub annotations and step summaries when | ||
| `GITHUB_ACTIONS=true`. Consumer repositories own checkout, tool installation, | ||
| and project-specific build preparation, then call their project wrappers: | ||
|
|
||
| ```yaml | ||
| - name: Run clang-format | ||
| env: | ||
| FORMAT_BLOB_SHA: ${{ github.event.pull_request.head.sha || github.sha }} | ||
| run: ./scripts/clang-format.sh | ||
|
|
||
| - name: Run clang-tidy | ||
| env: | ||
| TIDY_BLOB_SHA: ${{ github.event.pull_request.head.sha || github.sha }} | ||
| run: ./scripts/clang-tidy.sh --fail-on-warning | ||
| ``` | ||
|
|
||
| `FORMAT_BLOB_SHA` and `TIDY_BLOB_SHA` make source links target the pull | ||
| request's head commit such that links to violating files render correctly. | ||
| The scripts fall back to `GITHUB_SHA` when these values are not supplied. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fmu what is this doing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since JSON doesn't have comments a bit unfortunate I can't add context, but for Markdownlint rule
MD013defines an80character width limit which is just annoying so this disables it: https://github.com/DavidAnson/markdownlint/blob/main/doc/md013.md