From 4730edb0263135e88dab08ab0f6dc724a1585a67 Mon Sep 17 00:00:00 2001
From: RD Rama Devi <122200035+Rd4dev@users.noreply.github.com>
Date: Thu, 5 Sep 2024 22:52:56 +0530
Subject: [PATCH] Fix #1730 : Prevent binary files from being checked in using
 pre-commit hook (#5525)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

Fixes #1730

### The PR includes
- A pre-commit hook that identifies binary files in both
  - staged changes (for local checks before committing)
  - committed files (for CI checks after pushing to a PR)
- If binary files are found, the commit/check is blocked, and the
offending files are listed for removal.
- The pre-commit hook is integrated via a setup script.
- The same script is utilized for the CI pipeline.
- The 'Pass' statement is only included in the CI checks to keep the
local commit process cleaner.

### Local block as pre-commit hook when a binary file is detected

![image](https://github.com/user-attachments/assets/b953b2d7-55ec-46e6-a0b9-00967200509c)

### CI re-check if the PR includes a binary - [ Fail - [stack
trace](https://github.com/Rd4dev/oppia-android/actions/runs/10715972847/job/29712474511#step:7:15)
] [ Pass - [stack
trace](https://github.com/Rd4dev/oppia-android/actions/runs/10716040065/job/29712695824?pr=11#step:7:10)
]

![image](https://github.com/user-attachments/assets/64352473-86cb-49e0-b888-d3247286e9e5)

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).
---
 .github/workflows/static_checks.yml | 11 +++++++++
 scripts/pre-commit.sh               | 35 +++++++++++++++++++++++++++++
 scripts/setup.sh                    |  3 +++
 3 files changed, 49 insertions(+)
 create mode 100644 scripts/pre-commit.sh

diff --git a/.github/workflows/static_checks.yml b/.github/workflows/static_checks.yml
index 15dd3e28e76..ae04da9c0f4 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,15 @@ jobs:
         run: |
           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
+          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
   # 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..26061ef5c09
--- /dev/null
+++ b/scripts/pre-commit.sh
@@ -0,0 +1,35 @@
+#!/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
+      ((binaryFilesCount++))
+      printf "\n\033[33m%s\033[0m" "$file"
+    fi
+  done
+
+  if [[ "${binaryFilesCount}" -gt 0 ]]; then
+    printf "\n\nPlease remove the %d detected binary file(s)." "$binaryFilesCount"
+    printf "\nBINARY FILES CHECK FAILED"
+    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