-
Notifications
You must be signed in to change notification settings - Fork 703
sre tidy changes #3108
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
Open
artyom-activeloop
wants to merge
16
commits into
main
Choose a base branch
from
sre_tidy_changes
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+95
−105
Open
sre tidy changes #3108
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
1da1763
Introduce clang tidy and fix part of the issues.
khustup2 69a13ee
Fix compilation.
khustup2 b7786eb
Fixed clang-tidy install.
khustup2 c849a3c
Fixed running clang-tidy.
khustup2 c8c4e3c
More tidy issues fixed.
khustup2 eee8d71
Fix linter script.
khustup2 161e9e1
edits
artyom-activeloop 56e3155
Merge branch 'main' into sre_tidy_changes
artyom-activeloop 6e6bd1f
edits
artyom-activeloop 517f1aa
edits
artyom-activeloop 24090b5
edits
artyom-activeloop 0e139e2
edits
artyom-activeloop f364692
edits
artyom-activeloop ad73223
edits
artyom-activeloop f84c705
edits
artyom-activeloop 3bbb00d
edits
artyom-activeloop 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
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
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,98 +1,92 @@ | ||
| #!/bin/bash | ||
| #!/usr/bin/env bash | ||
|
|
||
| set -e | ||
|
|
||
| # Script to run clang-tidy on pg_deeplake source files | ||
| # Usage: ./scripts/run_clang_tidy.sh [build_dir] | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" | ||
| PROJECT_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)" | ||
| BUILD_DIR="${1:-${PROJECT_ROOT}/builds/deeplake-pg-dev}" | ||
|
|
||
| # Convert BUILD_DIR to absolute path if it's relative | ||
| if [[ "$BUILD_DIR" != /* ]]; then | ||
| BUILD_DIR="${PROJECT_ROOT}/${BUILD_DIR}" | ||
| fi | ||
| trap "rm -rf ${TEMP_DIR}" EXIT | ||
|
|
||
| echo "Running clang-tidy on cpp/deeplake_pg..." | ||
| echo "Build directory: ${BUILD_DIR}" | ||
| function run_tidy() { | ||
| local output file_warnings file_errors | ||
| output=$(clang-tidy --header-filter='.*deeplake_pg.*' -p "$BUILD_DIR" "$1" 2>&1 || true) | ||
| file_warnings=$(echo "$output" | grep -c "deeplake_pg/.*warning:" || true) | ||
| file_errors=$(echo "$output" | grep -c "deeplake_pg/.*error:" || true) | ||
| echo "$file|$file_warnings|$file_errors" >"${TEMP_DIR}/${1}.count" | ||
| if [ "$file_errors" -gt 0 ]; then | ||
| echo "$output" | grep "deeplake_pg/.*error:" >"${TEMP_DIR}/${1}.output" | ||
| elif [ "$file_warnings" -gt 0 ]; then | ||
| echo "$output" | grep "deeplake_pg/.*warning:" >"${TEMP_DIR}/${1}.output" | ||
| fi | ||
| } | ||
|
|
||
| function log() { | ||
| local level ts | ||
| level="$1" | ||
| ts="$(date --utc -Iseconds)" | ||
| shift | ||
| printf "[%s] - [%s] - \"%s\"\n" "${level^^}" "${ts}" "$*" | ||
| } | ||
|
|
||
| SCRIPT_DIR="$(realpath "$(dirname "${BASH_SOURCE[0]}")")" | ||
| PROJECT_ROOT="$(cd $SCRIPT_DIR && realpath ..)" | ||
| BUILD_DIR="$(realpath "${1:-${PROJECT_ROOT}/builds/deeplake-pg-dev}")" | ||
|
|
||
| log info "running clang-tidy on cpp/deeplake_pg..." | ||
| log info "build directory: ${BUILD_DIR}" | ||
|
|
||
| if [ ! -f "${BUILD_DIR}/compile_commands.json" ]; then | ||
| echo "Error: compile_commands.json not found in ${BUILD_DIR}" | ||
| echo "Please build the project first to generate compile_commands.json" | ||
| exit 1 | ||
| log error "compile_commands.json not found in ${BUILD_DIR}, please build the project first to generate compile_commands.json" | ||
| exit 1 | ||
| fi | ||
|
|
||
| cd "${PROJECT_ROOT}/cpp/deeplake_pg" | ||
|
|
||
| # Create temp directory for parallel output | ||
| TEMP_DIR=$(mktemp -d) | ||
| trap "rm -rf ${TEMP_DIR}" EXIT | ||
|
|
||
| echo "Running clang-tidy in parallel..." | ||
| log info "running clang-tidy in parallel..." | ||
|
|
||
| # Run clang-tidy for each file in parallel | ||
| worker_count="$(nproc)" | ||
| for file in *.cpp; do | ||
| ( | ||
| OUTPUT=$(clang-tidy --header-filter='.*deeplake_pg.*' -p "$BUILD_DIR" "$file" 2>&1 || true) | ||
|
|
||
| # Count warnings in this file (only in deeplake_pg directory) | ||
| FILE_WARNINGS=$(echo "$OUTPUT" | grep -c "deeplake_pg/.*warning:" || true) | ||
| # Count errors in this file | ||
| FILE_ERRORS=$(echo "$OUTPUT" | grep -c "deeplake_pg/.*error:" || true) | ||
|
|
||
| # Save results to temp file | ||
| echo "$file|$FILE_WARNINGS|$FILE_ERRORS" > "${TEMP_DIR}/${file}.count" | ||
|
|
||
| if [ "$FILE_ERRORS" -gt 0 ]; then | ||
| echo "$OUTPUT" | grep "deeplake_pg/.*error:" > "${TEMP_DIR}/${file}.output" | ||
| elif [ "$FILE_WARNINGS" -gt 0 ]; then | ||
| echo "$OUTPUT" | grep "deeplake_pg/.*warning:" > "${TEMP_DIR}/${file}.output" | ||
| fi | ||
| ) & | ||
| run_tidy $file & | ||
| while [ "$(jobs | wc -l)" -ge "$worker_count" ]; do | ||
| sleep 0.1 | ||
| done | ||
| done | ||
|
|
||
| # Wait for all parallel jobs to complete | ||
| wait | ||
|
|
||
| echo "" | ||
| echo "Processing results..." | ||
| log info "processing results..." | ||
|
|
||
| WARNINGS=0 | ||
| ERRORS=0 | ||
|
|
||
| # Process results in order | ||
| for file in *.cpp; do | ||
| if [ -f "${TEMP_DIR}/${file}.count" ]; then | ||
| IFS='|' read -r fname FILE_WARNINGS FILE_ERRORS < "${TEMP_DIR}/${file}.count" | ||
|
|
||
| if [ "$FILE_ERRORS" -gt 0 ]; then | ||
| echo "❌ $file - has $FILE_ERRORS errors" | ||
| cat "${TEMP_DIR}/${file}.output" | ||
| ERRORS=$((ERRORS + FILE_ERRORS)) | ||
| elif [ "$FILE_WARNINGS" -gt 0 ]; then | ||
| echo "⚠ $file - has $FILE_WARNINGS warnings" | ||
| cat "${TEMP_DIR}/${file}.output" | ||
| WARNINGS=$((WARNINGS + FILE_WARNINGS)) | ||
| else | ||
| echo "✓ $file - no issues" | ||
| fi | ||
| if [ -f "${TEMP_DIR}/${file}.count" ]; then | ||
| IFS='|' read -r fname FILE_WARNINGS FILE_ERRORS <"${TEMP_DIR}/${file}.count" | ||
|
|
||
| if [ "$FILE_ERRORS" -gt 0 ]; then | ||
| log error "❌ $file - has $FILE_ERRORS errors" | ||
| cat "${TEMP_DIR}/${file}.output" | ||
| ERRORS=$((ERRORS + FILE_ERRORS)) | ||
| elif [ "$FILE_WARNINGS" -gt 0 ]; then | ||
| log warn "⚠ $file - has $FILE_WARNINGS warnings" | ||
| cat "${TEMP_DIR}/${file}.output" | ||
| WARNINGS=$((WARNINGS + FILE_WARNINGS)) | ||
| else | ||
| log info "✓ $file - no issues" | ||
| fi | ||
| fi | ||
| done | ||
|
|
||
| echo "" | ||
| echo "====================" | ||
| echo "Clang-Tidy Summary" | ||
| echo "====================" | ||
| echo "Total warnings: $WARNINGS" | ||
| echo "Total errors: $ERRORS" | ||
| log info "clang-tidy summary: warnings=$WARNINGS errors=$ERRORS" | ||
|
|
||
| if [ $ERRORS -gt 0 ]; then | ||
| echo "❌ Clang-tidy found $ERRORS errors!" | ||
| exit 1 | ||
| log error "❌ Clang-tidy found $ERRORS errors!" | ||
| exit 1 | ||
| elif [ $WARNINGS -gt 0 ]; then | ||
| echo "⚠ Clang-tidy found $WARNINGS warnings (non-blocking)" | ||
| exit 0 | ||
| log warn "⚠ Clang-tidy found $WARNINGS warnings (non-blocking)" | ||
| exit 0 | ||
| else | ||
| echo "✅ No issues found!" | ||
| exit 0 | ||
| log info "✅ No issues found!" | ||
| exit 0 | ||
| fi | ||
|
|
||
Submodule vcpkg
added at
6f29f1
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.
The log function does not use the provided arguments correctly. The shift command removes the level argument, but then the function tries to print
${msg}which is never assigned. The remaining arguments after shift should be concatenated intomsgor the printf statement should use"$*"to print all remaining arguments.