From bfe4a99694c373348b471da6ee1bd98460d0c441 Mon Sep 17 00:00:00 2001 From: Rd Date: Wed, 4 Sep 2024 19:17:03 +0530 Subject: [PATCH 1/3] Fix #1730 - Prevent binary files from being checked in using pre-commit hook --- .github/workflows/static_checks.yml | 12 ++++++++++ scripts/pre-commit.sh | 36 +++++++++++++++++++++++++++++ scripts/setup.sh | 3 +++ 3 files changed, 51 insertions(+) create mode 100644 scripts/pre-commit.sh diff --git a/.github/workflows/static_checks.yml b/.github/workflows/static_checks.yml index 15dd3e28e76..d1bec7127b6 100644 --- a/.github/workflows/static_checks.yml +++ b/.github/workflows/static_checks.yml @@ -107,6 +107,8 @@ jobs: CACHE_DIRECTORY: ~/.bazel_cache steps: - uses: actions/checkout@v2 + with: + fetch-depth: 0 - name: Set up Bazel uses: abhinavsingh/setup-bazel@v3 @@ -205,6 +207,16 @@ jobs: run: | bazel run //scripts:string_resource_validation_check -- $(pwd) + - name: Binary files check + run: | + bash /home/runner/work/oppia-android/oppia-android/scripts/pre-commit.sh + exit_code=$? + + if [ $exit_code -eq 0 ]; then + echo "No binary files found" + echo "BINARY FILES CHECK PASSED" + fi + # Note that caching is intentionally not enabled for this check since licenses should always be # verified without any potential influence from earlier builds (i.e. always from a clean build to # ensure the results exactly match the current state of the repository). diff --git a/scripts/pre-commit.sh b/scripts/pre-commit.sh new file mode 100644 index 00000000000..4057aeb019a --- /dev/null +++ b/scripts/pre-commit.sh @@ -0,0 +1,36 @@ +#!/bin/bash + +# Pre-commit hook to check for binary files. + +# Find the common ancestor between develop and the current branch +base_commit=$(git merge-base 'origin/develop' HEAD) + +# Get the list of staged changes (files ready to be committed) +staged_files=$(git diff --cached --name-only) + +# Get the list of changed files compared to the base commit +changed_files=$(git diff --name-only "$base_commit" HEAD) + +# Combine both lists of files, ensuring no duplicates +all_files=$(echo -e "$staged_files\n$changed_files" | sort -u) + +function checkForBinaries() { + binaryFilesCount=0 + + # Iterate over all files (both staged and changed) + for file in $all_files; do + if [ -f "$file" ] && file --mime "$file" | grep -q 'binary'; then + binaryFiles+="${file}\n" + ((binaryFilesCount++)) + fi + done + + if [[ -n "${binaryFiles}" && "${binaryFilesCount}" -gt 0 ]]; then + printf "\nPlease remove the following binary file(s):\n\n" + printf "\033[33m%b\033[0m\n" "$binaryFiles" + printf "BINARY FILES CHECK FAILED\n" + exit 1 + fi +} + +checkForBinaries diff --git a/scripts/setup.sh b/scripts/setup.sh index 8c3ef595e1d..9f822095f32 100644 --- a/scripts/setup.sh +++ b/scripts/setup.sh @@ -13,6 +13,9 @@ # Move file from script folder to .git/hooks folder cp scripts/pre-push.sh .git/hooks/pre-push +# Copy the pre-commit hook from script to .git/hooks folder +cp scripts/pre-commit.sh .git/hooks/pre-commit + # Create a folder where all the set up files will be downloaded mkdir -p ../oppia-android-tools cd ../oppia-android-tools From 3294c2003475dcd66af8881e9bf820ffb33080ac Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 5 Sep 2024 10:03:53 +0530 Subject: [PATCH 2/3] Adding the missed out !cancelled conditional check and removed the redundant failure check for the PASS statement --- .github/workflows/static_checks.yml | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/.github/workflows/static_checks.yml b/.github/workflows/static_checks.yml index d1bec7127b6..3079870061b 100644 --- a/.github/workflows/static_checks.yml +++ b/.github/workflows/static_checks.yml @@ -208,14 +208,13 @@ jobs: bazel run //scripts:string_resource_validation_check -- $(pwd) - name: Binary files check + # The expression if: ${{ !cancelled() }} runs a job or step regardless of its success or failure while responding to cancellations, + # serving as a cancellation-compliant alternative to if: ${{ always() }} in concurrent workflows. + if: ${{ !cancelled() }} run: | bash /home/runner/work/oppia-android/oppia-android/scripts/pre-commit.sh - exit_code=$? - - if [ $exit_code -eq 0 ]; then - echo "No binary files found" - echo "BINARY FILES CHECK PASSED" - fi + echo "No binary files found" + echo "BINARY FILES CHECK PASSED" # Note that caching is intentionally not enabled for this check since licenses should always be # verified without any potential influence from earlier builds (i.e. always from a clean build to From 5a2bb708b8c95c1592c28a94257454124436bf49 Mon Sep 17 00:00:00 2001 From: Rd Date: Thu, 5 Sep 2024 13:28:58 +0530 Subject: [PATCH 3/3] Moved the print statement right into the check loop as the CI doesn't render the color codes properly with %b --- .github/workflows/static_checks.yml | 2 +- scripts/pre-commit.sh | 9 ++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/.github/workflows/static_checks.yml b/.github/workflows/static_checks.yml index 3079870061b..ae04da9c0f4 100644 --- a/.github/workflows/static_checks.yml +++ b/.github/workflows/static_checks.yml @@ -213,7 +213,7 @@ jobs: if: ${{ !cancelled() }} run: | bash /home/runner/work/oppia-android/oppia-android/scripts/pre-commit.sh - echo "No binary files found" + echo "No binary files found in commit" echo "BINARY FILES CHECK PASSED" # Note that caching is intentionally not enabled for this check since licenses should always be diff --git a/scripts/pre-commit.sh b/scripts/pre-commit.sh index 4057aeb019a..26061ef5c09 100644 --- a/scripts/pre-commit.sh +++ b/scripts/pre-commit.sh @@ -20,15 +20,14 @@ function checkForBinaries() { # Iterate over all files (both staged and changed) for file in $all_files; do if [ -f "$file" ] && file --mime "$file" | grep -q 'binary'; then - binaryFiles+="${file}\n" ((binaryFilesCount++)) + printf "\n\033[33m%s\033[0m" "$file" fi done - if [[ -n "${binaryFiles}" && "${binaryFilesCount}" -gt 0 ]]; then - printf "\nPlease remove the following binary file(s):\n\n" - printf "\033[33m%b\033[0m\n" "$binaryFiles" - printf "BINARY FILES CHECK FAILED\n" + if [[ "${binaryFilesCount}" -gt 0 ]]; then + printf "\n\nPlease remove the %d detected binary file(s)." "$binaryFilesCount" + printf "\nBINARY FILES CHECK FAILED" exit 1 fi }