Skip to content

Commit

Permalink
Fix #1730 : Prevent binary files from being checked in using pre-comm…
Browse files Browse the repository at this point in the history
…it 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)).
  • Loading branch information
Rd4dev authored Sep 5, 2024
1 parent 16082aa commit 4730edb
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 0 deletions.
11 changes: 11 additions & 0 deletions .github/workflows/static_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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).
Expand Down
35 changes: 35 additions & 0 deletions scripts/pre-commit.sh
Original file line number Diff line number Diff line change
@@ -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
3 changes: 3 additions & 0 deletions scripts/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 4730edb

Please sign in to comment.