Adding Valgrind workflow for GitHub Actions

.dockerignore

@@ -1 +1,31 @@
-/build
+# Build directories
+build
+build/
+build/**
+bin/
+lib/
+
+# Python
+test/venv/
+__pycache__/
+*.pyc
+*.pyo
+*.pyd
+.pytest_cache/
+
+# Valgrind logs
+valgrind_logs/
+
+# Git
+.git/
+
+# IDE
+.vscode/
+.idea/
+*.swp
+*.swo
+*~
+
+# OS
+.DS_Store
+Thumbs.db

.github/workflows/pr.yml

@@ -25,3 +25,5 @@ jobs:
   test:
     uses: ./.github/workflows/test.yml
     needs: build
+  valgrind:
+    uses: ./.github/workflows/valgrind.yml

.github/workflows/valgrind.yml

@@ -0,0 +1,146 @@
+name: Valgrind Memory Leak Check
+
+on:
+  pull_request:
+    branches: [ main ]
+    paths:
+      - 'src/**'
+      - 'extern/**'
+      - 'test/**'
+      - 'Dockerfile.valgrind'
+      - 'CMakeLists.txt'
+      - '.github/workflows/valgrind.yml'
+  workflow_dispatch:
+
+permissions:
+  contents: read
+  pull-requests: write
+
+concurrency:
+  group: valgrind-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  valgrind:
+    runs-on: ubuntu-24.04
+    timeout-minutes: 60
+
+    steps:
+      - name: Check out repository code
+        uses: actions/checkout@v6
+        with:
+          submodules: true
+
+      - name: Build Valgrind Docker image
+        run: |
+          docker build \
+            --build-arg USER_ID=$(id -u) \
+            --build-arg GROUP_ID=$(id -g) \
+            -t mpqcli-valgrind \
+            -f Dockerfile.valgrind \
+            .
+
+      - name: Run Valgrind on test suite
+        run: |
+          mkdir -p valgrind_logs
+          docker run --rm \
+            -v "$(pwd)/valgrind_logs:/mpqcli/valgrind_logs:rw" \
+            mpqcli-valgrind bash -c '
+            mv /mpqcli/build/bin/mpqcli /mpqcli/build/bin/mpqcli.real
+            cat > /mpqcli/build/bin/mpqcli << "EOF"
+          #!/bin/bash
+          exec valgrind --leak-check=full --show-leak-kinds=all --track-origins=yes \
+            --log-file=/mpqcli/valgrind_logs/valgrind_$$.log \
+            /mpqcli/build/bin/mpqcli.real "$@"
+          EOF
+            chmod +x /mpqcli/build/bin/mpqcli
+            cd /mpqcli && test/venv/bin/python -m pytest test -v --tb=short
+          '
+
+      - name: Analyze Valgrind results
+        id: analyze
+        run: |
+          total_logs=$(find valgrind_logs -maxdepth 1 -type f -name '*.log' 2>/dev/null | wc -l)
+          logs_with_errors=$(grep -lE "ERROR SUMMARY: [1-9][0-9]* errors" valgrind_logs/*.log 2>/dev/null | wc -l)
+          logs_with_leaks=$(grep -lE "(definitely|indirectly) lost: [1-9][0-9,]* bytes" valgrind_logs/*.log 2>/dev/null | wc -l)
+
+          echo "Total logs analyzed: $total_logs"
+          echo "Logs with errors:    $logs_with_errors"
+          echo "Logs with leaks:     $logs_with_leaks"
+
+          echo "## Valgrind Memory Leak Analysis" >> $GITHUB_STEP_SUMMARY
+          echo "" >> $GITHUB_STEP_SUMMARY
+
+          if [ "$logs_with_errors" -eq 0 ] && [ "$logs_with_leaks" -eq 0 ]; then
+            echo "✅ **No memory leaks detected.**" >> $GITHUB_STEP_SUMMARY
+            echo "" >> $GITHUB_STEP_SUMMARY
+            echo "- Logs analyzed: $total_logs" >> $GITHUB_STEP_SUMMARY
+            echo "status=success" >> $GITHUB_OUTPUT
+          else
+            echo "⚠️ **Potential memory issues detected**" >> $GITHUB_STEP_SUMMARY
+            echo "" >> $GITHUB_STEP_SUMMARY
+            echo "- Logs analyzed:    $total_logs" >> $GITHUB_STEP_SUMMARY
+            echo "- Logs with errors: $logs_with_errors" >> $GITHUB_STEP_SUMMARY
+            echo "- Logs with leaks:  $logs_with_leaks" >> $GITHUB_STEP_SUMMARY
+            echo "" >> $GITHUB_STEP_SUMMARY
+            echo "#### Logs with leaks" >> $GITHUB_STEP_SUMMARY
+            echo "" >> $GITHUB_STEP_SUMMARY
+            echo '```' >> $GITHUB_STEP_SUMMARY
+            grep -lE "(definitely|indirectly) lost: [1-9][0-9,]* bytes" valgrind_logs/*.log 2>/dev/null >> $GITHUB_STEP_SUMMARY || true
+            echo '```' >> $GITHUB_STEP_SUMMARY
+            echo "" >> $GITHUB_STEP_SUMMARY
+            echo "Please review the Valgrind logs for details." >> $GITHUB_STEP_SUMMARY
+            echo "status=warning" >> $GITHUB_OUTPUT
+          fi
+
+      - name: Upload Valgrind logs
+        if: always()
+        uses: actions/upload-artifact@v7
+        with:
+          name: valgrind-logs
+          path: |
+            valgrind_logs/
+          retention-days: 30
+
+      - name: Comment on PR
+        if: github.event_name == 'pull_request'
+        uses: actions/github-script@v9
+        with:
+          script: |
+            const marker = '<!-- valgrind-leak-check -->';
+            const status = '${{ steps.analyze.outputs.status }}';
+            const runUrl = `${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}`;
+
+            const { data: comments } = await github.rest.issues.listComments({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              issue_number: context.issue.number,
+            });
+            const existing = comments.find(c => c.body && c.body.startsWith(marker));
+
+            if (status === 'warning') {
+              const body = `${marker}\n⚠️ **Valgrind detected potential memory leaks**\n\nPlease review the [Valgrind logs](${runUrl}) for details.\n\n*Note: This check is informational and does not block merging.*`;
+              if (existing) {
+                await github.rest.issues.updateComment({
+                  owner: context.repo.owner,
+                  repo: context.repo.repo,
+                  comment_id: existing.id,
+                  body,
+                });
+              } else {
+                await github.rest.issues.createComment({
+                  owner: context.repo.owner,
+                  repo: context.repo.repo,
+                  issue_number: context.issue.number,
+                  body,
+                });
+              }
+            } else if (existing) {
+              const body = `${marker}\n✅ **No Valgrind memory leaks detected.**\n\nIssues reported by an earlier run on this PR have been resolved. See the latest [Valgrind logs](${runUrl}).`;
+              await github.rest.issues.updateComment({
+                owner: context.repo.owner,
+                repo: context.repo.repo,
+                comment_id: existing.id,
+                body,
+              });
+            }

Dockerfile.valgrind

@@ -0,0 +1,47 @@
+FROM ubuntu:22.04
+
+# Install build dependencies and Valgrind
+RUN apt-get update
+RUN apt-get install -y \
+    build-essential \
+    cmake \
+    git \
+    valgrind \
+    python3 \
+    python3-pip \
+    python3-venv \
+    sudo
+RUN rm -rf /var/lib/apt/lists/*
+
+# Create a user with the same UID/GID as the host user (passed as build args)
+ARG USER_ID=1000
+ARG GROUP_ID=1000
+RUN groupadd -g ${GROUP_ID} mpquser || true && \
+    useradd -m -u ${USER_ID} -g ${GROUP_ID} -s /bin/bash mpquser && \
+    echo "mpquser ALL=(ALL) NOPASSWD:ALL" >> /etc/sudoers
+
+WORKDIR /mpqcli
+
+# Copy the entire project
+COPY . .
+
+# Remove any existing build directory and fix ownership
+RUN rm -rf build && \
+    chown -R mpquser:mpquser /mpqcli
+
+# Switch to non-root user
+USER mpquser
+
+# Build with debug symbols (needed by Valgrind)
+RUN cmake -B build \
+    -DCMAKE_BUILD_TYPE=Debug \
+    -DBUILD_MPQCLI=ON
+
+RUN cmake --build build
+
+# Set up Python test environment
+RUN python3 -m venv test/venv
+RUN test/venv/bin/pip install -r test/requirements.txt
+
+# Default command runs Valgrind on the binary
+CMD ["valgrind", "--leak-check=full", "--show-leak-kinds=all", "--track-origins=yes", "--verbose", "./build/bin/mpqcli", "version"]

scripts/run_valgrind_tests.sh

@@ -0,0 +1,147 @@
+#!/bin/bash
+
+# Script to run pytest tests under Valgrind for comprehensive memory leak detection
+# This wraps the mpqcli binary with Valgrind during test execution
+
+set -e
+
+SCRIPT_DIR="$(dirname "$(readlink -fm "$0")")"
+PROJECT_DIR="$(dirname "$SCRIPT_DIR")"
+
+# Colors for output
+RED='\033[0;31m'
+GREEN='\033[0;32m'
+YELLOW='\033[1;33m'
+NC='\033[0m' # No Color
+
+# Default values
+USE_DOCKER=false
+BUILD_DIR="build"
+VALGRIND_LOG_DIR="$PROJECT_DIR/valgrind_logs"
+
+# Parse arguments
+while [[ $# -gt 0 ]]; do
+    case $1 in
+        -d|--docker)
+            USE_DOCKER=true
+            shift
+            ;;
+        -h|--help)
+            echo "Usage: $0 [OPTIONS]"
+            echo ""
+            echo "Run pytest tests with Valgrind memory leak detection"
+            echo ""
+            echo "Options:"
+            echo "  -d, --docker    Run in Docker container"
+            echo "  -h, --help      Show this help message"
+            echo ""
+            echo "This script creates a wrapper that runs mpqcli under Valgrind"
+            echo "during pytest execution, providing comprehensive memory leak detection."
+            exit 0
+            ;;
+        *)
+            echo -e "${RED}Unknown option: $1${NC}"
+            exit 1
+            ;;
+    esac
+done
+
+# Create Valgrind log directory
+mkdir -p "$VALGRIND_LOG_DIR"
+
+if [ "$USE_DOCKER" = true ]; then
+    echo -e "${GREEN}Running tests with Valgrind in Docker...${NC}"
+
+    # Build Docker image with current user's UID/GID
+    echo -e "${YELLOW}Building Docker image...${NC}"
+    docker build \
+        --build-arg USER_ID="$(id -u)"\
+        --build-arg GROUP_ID="$(id -g)" \
+        -t mpqcli-valgrind \
+        -f "$PROJECT_DIR/Dockerfile.valgrind" \
+        "$PROJECT_DIR"
+
+    # Run tests in Docker with Valgrind wrapper
+    echo -e "${YELLOW}Running pytest with Valgrind...${NC}"
+    docker run --rm -v "$VALGRIND_LOG_DIR":/mpqcli/valgrind_logs mpqcli-valgrind bash -c "
+        # Replace the binary in place with a Valgrind wrapper
+        mv /mpqcli/build/bin/mpqcli /mpqcli/build/bin/mpqcli.real
+        cat > /mpqcli/build/bin/mpqcli << 'EOF'
+#!/bin/bash
+valgrind --leak-check=full \
+         --show-leak-kinds=all \
+         --track-origins=yes \
+         --log-file=/mpqcli/valgrind_logs/valgrind_%p.log \
+         /mpqcli/build/bin/mpqcli.real \"\$@\"
+EOF
+        chmod +x /mpqcli/build/bin/mpqcli
+
+        # Run tests
+        cd /mpqcli
+        . test/venv/bin/activate
+        python3 -m pytest test -s
+    "
+else
+    echo -e "${GREEN}Running tests with Valgrind locally...${NC}"
+
+    # Check if Valgrind is installed
+    if ! command -v valgrind &> /dev/null; then
+        echo -e "${RED}Error: Valgrind is not installed${NC}"
+        echo "Install it with: sudo apt-get install valgrind"
+        exit 1
+    fi
+
+    # Check if binary exists
+    if [ ! -f "$PROJECT_DIR/$BUILD_DIR/bin/mpqcli" ]; then
+        echo -e "${YELLOW}Binary not found. Building with debug symbols...${NC}"
+        cd "$PROJECT_DIR"
+        cmake -B "$BUILD_DIR" -DCMAKE_BUILD_TYPE=Debug -DBUILD_MPQCLI=ON
+        cmake --build "$BUILD_DIR"
+    fi
+
+    # Check if Python venv exists
+    if [ ! -d "$PROJECT_DIR/test/venv" ]; then
+        echo -e "${YELLOW}Setting up Python test environment...${NC}"
+        python3 -m venv "$PROJECT_DIR/test/venv"
+        . "$PROJECT_DIR/test/venv/bin/activate"
+        pip3 install -r "$PROJECT_DIR/test/requirements.txt"
+    else
+        . "$PROJECT_DIR/test/venv/bin/activate"
+    fi
+
+    # Replace the binary in place with a Valgrind wrapper
+    BIN="$PROJECT_DIR/$BUILD_DIR/bin/mpqcli"
+    REAL_BIN="$BIN.real"
+    mv "$BIN" "$REAL_BIN"
+    cat > "$BIN" << EOF
+#!/bin/bash
+valgrind --leak-check=full \
+         --show-leak-kinds=all \
+         --track-origins=yes \
+         --log-file=$VALGRIND_LOG_DIR/valgrind_\$\$.log \
+         "$REAL_BIN" "\$@"
+EOF
+    chmod +x "$BIN"
+
+    # Restore the real binary on exit
+    trap 'mv "$REAL_BIN" "$BIN"' EXIT
+
+    # Run tests
+    echo -e "${YELLOW}Running pytest with Valgrind wrapper...${NC}"
+    cd "$PROJECT_DIR"
+    python3 -m pytest test -s
+fi
+
+echo -e "${GREEN}Tests complete!${NC}"
+echo -e "${YELLOW}Valgrind logs saved to: $VALGRIND_LOG_DIR${NC}"
+echo ""
+echo -e "${YELLOW}Analyzing results...${NC}"
+
+# Check for memory leaks in logs
+if grep -q "definitely lost" "$VALGRIND_LOG_DIR"/*.log 2>/dev/null; then
+    echo -e "${RED}Memory leaks detected! Check logs in $VALGRIND_LOG_DIR${NC}"
+    grep -H "definitely lost" "$VALGRIND_LOG_DIR"/*.log | head -20
+    exit 1
+else
+    echo -e "${GREEN}No definite memory leaks detected!${NC}"
+fi