Fix #1730 : Prevent binary files from being checked in using pre-commit hook #5525

Rd4dev · 2024-09-04T14:04:39Z

Explanation

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

CI re-check if the PR includes a binary - [ Fail - stack trace ] [ Pass - stack trace ]

Essential Checklist

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: ...".)
Any changes to scripts/assets files have their rationale included in the PR explanation.
The PR follows the style guide.
The PR does not contain any unnecessary code changes from Android Studio (reference).
The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
The PR is assigned to the appropriate reviewers (reference).

…-commit hook

Rd4dev · 2024-09-04T14:08:52Z

@BenHenning @adhiamboperes, would this be an appropriate way to prevent the binary files check in? Can you PTAL!

Also, this #1730 (comment) mentions

We may run into an issue with the CI check if it runs across the entirety of develop since there are some binary files in mainline, and we can't force push change those. We may need to build in exceptions for those files.

What does that exactly mean?

github-actions · 2024-09-04T14:16:07Z

Coverage Report

Results

Coverage Analysis: SKIP ⏭️

This PR did not introduce any changes to Kotlin source or test files.

To learn more, visit the Oppia Android Code Coverage wiki page

.github/workflows/static_checks.yml

BenHenning

Thanks @Rd4dev!

@BenHenning @adhiamboperes, would this be an appropriate way to prevent the binary files check in? Can you PTAL!

Also, this #1730 (comment) mentions

We may run into an issue with the CI check if it runs across the entirety of develop since there are some binary files in mainline, and we can't force push change those. We may need to build in exceptions for those files.

What does that exactly mean?

I think this was originally assuming that we'd make a Kotlin script that runs across all files in the codebase to ensure only the binary files we're aware of are ever checked in. That being said, I think what you're introducing here is a sufficient way to prevent binary files from being introduced.

.github/workflows/static_checks.yml

github-actions · 2024-09-05T02:50:31Z

APK & AAB differences analysis

Note that this is a summarized snapshot. See the CI artifacts for detailed differences.

Dev

Expand to see flavor specifics

Universal APK

APK file size: 19 MiB (old), 19 MiB (new), 4 bytes (Added)

APK download size (estimated): 17 MiB (old), 17 MiB (new), 13 bytes (Removed)

Method count: 259177 (old), 259177 (new), 0 (No change)

Features: 2 (old), 2 (new), 0 (No change)

Permissions: 6 (old), 6 (new), 0 (No change)

Resources: 6808 (old), 6808 (new), 0 (No change)

Anim: 43 (old), 43 (new), 0 (No change)
Animator: 26 (old), 26 (new), 0 (No change)
Array: 15 (old), 15 (new), 0 (No change)
Attr: 922 (old), 922 (new), 0 (No change)
Bool: 9 (old), 9 (new), 0 (No change)
Color: 967 (old), 967 (new), 0 (No change)
Dimen: 1048 (old), 1048 (new), 0 (No change)
Drawable: 379 (old), 379 (new), 0 (No change)
Id: 1272 (old), 1272 (new), 0 (No change)
Integer: 37 (old), 37 (new), 0 (No change)
Interpolator: 11 (old), 11 (new), 0 (No change)
Layout: 378 (old), 378 (new), 0 (No change)
Menu: 3 (old), 3 (new), 0 (No change)
Mipmap: 1 (old), 1 (new), 0 (No change)
Plurals: 10 (old), 10 (new), 0 (No change)
Raw: 2 (old), 2 (new), 0 (No change)
String: 848 (old), 848 (new), 0 (No change)
Style: 831 (old), 831 (new), 0 (No change)
Xml: 6 (old), 6 (new), 0 (No change)

Lesson assets: 111 (old), 111 (new), 0 (No change)

AAB differences

Expand to see AAB specifics

Supported configurations:

hdpi (same)
ldpi (same)
mdpi (same)
tvdpi (same)
xhdpi (same)
xxhdpi (same)
xxxhdpi (same)

Base APK

APK file size: 18 MiB (old), 18 MiB (new), 4 bytes (Added)
APK download size (estimated): 17 MiB (old), 17 MiB (new), 19 bytes (Removed)

Configuration hdpi

APK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change)
APK download size (estimated): 18 KiB (old), 18 KiB (new), 0 bytes (No change)

Configuration ldpi

APK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change)
APK download size (estimated): 14 KiB (old), 14 KiB (new), 0 bytes (No change)

Configuration mdpi

APK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change)
APK download size (estimated): 14 KiB (old), 14 KiB (new), 0 bytes (No change)

Configuration tvdpi

APK file size: 86 KiB (old), 86 KiB (new), 0 bytes (No change)
APK download size (estimated): 29 KiB (old), 29 KiB (new), 0 bytes (No change)

Configuration xhdpi

APK file size: 57 KiB (old), 57 KiB (new), 0 bytes (No change)
APK download size (estimated): 21 KiB (old), 21 KiB (new), 0 bytes (No change)

Configuration xxhdpi

APK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change)
APK download size (estimated): 29 KiB (old), 29 KiB (new), 0 bytes (No change)

Configuration xxxhdpi

APK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change)
APK download size (estimated): 28 KiB (old), 28 KiB (new), 0 bytes (No change)

Alpha

Expand to see flavor specifics

Universal APK

APK file size: 11 MiB (old), 11 MiB (new), 0 bytes (No change)

APK download size (estimated): 10 MiB (old), 10 MiB (new), 0 bytes (No change)

Method count: 115723 (old), 115723 (new), 0 (No change)

Features: 2 (old), 2 (new), 0 (No change)

Permissions: 6 (old), 6 (new), 0 (No change)

Resources: 5776 (old), 5776 (new), 0 (No change)

Anim: 33 (old), 33 (new), 0 (No change)
Animator: 24 (old), 24 (new), 0 (No change)
Array: 14 (old), 14 (new), 0 (No change)
Attr: 888 (old), 888 (new), 0 (No change)
Bool: 8 (old), 8 (new), 0 (No change)
Color: 820 (old), 820 (new), 0 (No change)
Dimen: 780 (old), 780 (new), 0 (No change)
Drawable: 341 (old), 341 (new), 0 (No change)
Id: 1218 (old), 1218 (new), 0 (No change)
Integer: 32 (old), 32 (new), 0 (No change)
Interpolator: 11 (old), 11 (new), 0 (No change)
Layout: 341 (old), 341 (new), 0 (No change)
Menu: 1 (old), 1 (new), 0 (No change)
Mipmap: 1 (old), 1 (new), 0 (No change)
Plurals: 10 (old), 10 (new), 0 (No change)
String: 781 (old), 781 (new), 0 (No change)
Style: 472 (old), 472 (new), 0 (No change)
Xml: 1 (old), 1 (new), 0 (No change)

Lesson assets: 111 (old), 111 (new), 0 (No change)

AAB differences

Expand to see AAB specifics

Supported configurations:

hdpi (same)
ldpi (same)
mdpi (same)
tvdpi (same)
xhdpi (same)
xxhdpi (same)
xxxhdpi (same)

Base APK

APK file size: 11 MiB (old), 11 MiB (new), 0 bytes (No change)
APK download size (estimated): 10 MiB (old), 10 MiB (new), 3 bytes (Removed)

Configuration hdpi

APK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change)
APK download size (estimated): 17 KiB (old), 17 KiB (new), 0 bytes (No change)

Configuration ldpi

APK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change)
APK download size (estimated): 13 KiB (old), 13 KiB (new), 0 bytes (No change)

Configuration mdpi

APK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change)
APK download size (estimated): 13 KiB (old), 13 KiB (new), 0 bytes (No change)

Configuration tvdpi

APK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Configuration xhdpi

APK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change)
APK download size (estimated): 20 KiB (old), 20 KiB (new), 0 bytes (No change)

Configuration xxhdpi

APK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change)
APK download size (estimated): 28 KiB (old), 28 KiB (new), 0 bytes (No change)

Configuration xxxhdpi

APK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Beta

Expand to see flavor specifics

Universal APK

APK file size: 11 MiB (old), 11 MiB (new), 0 bytes (No change)

APK download size (estimated): 10 MiB (old), 10 MiB (new), 3 bytes (Removed)

Method count: 115729 (old), 115729 (new), 0 (No change)

Features: 2 (old), 2 (new), 0 (No change)

Permissions: 6 (old), 6 (new), 0 (No change)

Resources: 5776 (old), 5776 (new), 0 (No change)

Anim: 33 (old), 33 (new), 0 (No change)
Animator: 24 (old), 24 (new), 0 (No change)
Array: 14 (old), 14 (new), 0 (No change)
Attr: 888 (old), 888 (new), 0 (No change)
Bool: 8 (old), 8 (new), 0 (No change)
Color: 820 (old), 820 (new), 0 (No change)
Dimen: 780 (old), 780 (new), 0 (No change)
Drawable: 341 (old), 341 (new), 0 (No change)
Id: 1218 (old), 1218 (new), 0 (No change)
Integer: 32 (old), 32 (new), 0 (No change)
Interpolator: 11 (old), 11 (new), 0 (No change)
Layout: 341 (old), 341 (new), 0 (No change)
Menu: 1 (old), 1 (new), 0 (No change)
Mipmap: 1 (old), 1 (new), 0 (No change)
Plurals: 10 (old), 10 (new), 0 (No change)
String: 781 (old), 781 (new), 0 (No change)
Style: 472 (old), 472 (new), 0 (No change)
Xml: 1 (old), 1 (new), 0 (No change)

Lesson assets: 111 (old), 111 (new), 0 (No change)

AAB differences

Expand to see AAB specifics

Supported configurations:

hdpi (same)
ldpi (same)
mdpi (same)
tvdpi (same)
xhdpi (same)
xxhdpi (same)
xxxhdpi (same)

Base APK

APK file size: 10 MiB (old), 10 MiB (new), 4 bytes (Added)
APK download size (estimated): 9 MiB (old), 9 MiB (new), 12 bytes (Removed)

Configuration hdpi

APK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change)
APK download size (estimated): 17 KiB (old), 17 KiB (new), 0 bytes (No change)

Configuration ldpi

APK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change)
APK download size (estimated): 13 KiB (old), 13 KiB (new), 0 bytes (No change)

Configuration mdpi

APK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change)
APK download size (estimated): 13 KiB (old), 13 KiB (new), 0 bytes (No change)

Configuration tvdpi

APK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Configuration xhdpi

APK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change)
APK download size (estimated): 20 KiB (old), 20 KiB (new), 0 bytes (No change)

Configuration xxhdpi

APK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change)
APK download size (estimated): 28 KiB (old), 28 KiB (new), 0 bytes (No change)

Configuration xxxhdpi

APK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Ga

Expand to see flavor specifics

Universal APK

APK file size: 11 MiB (old), 11 MiB (new), 0 bytes (No change)

APK download size (estimated): 10 MiB (old), 10 MiB (new), 3 bytes (Removed)

Method count: 115729 (old), 115729 (new), 0 (No change)

Features: 2 (old), 2 (new), 0 (No change)

Permissions: 6 (old), 6 (new), 0 (No change)

Resources: 5776 (old), 5776 (new), 0 (No change)

Anim: 33 (old), 33 (new), 0 (No change)
Animator: 24 (old), 24 (new), 0 (No change)
Array: 14 (old), 14 (new), 0 (No change)
Attr: 888 (old), 888 (new), 0 (No change)
Bool: 8 (old), 8 (new), 0 (No change)
Color: 820 (old), 820 (new), 0 (No change)
Dimen: 780 (old), 780 (new), 0 (No change)
Drawable: 341 (old), 341 (new), 0 (No change)
Id: 1218 (old), 1218 (new), 0 (No change)
Integer: 32 (old), 32 (new), 0 (No change)
Interpolator: 11 (old), 11 (new), 0 (No change)
Layout: 341 (old), 341 (new), 0 (No change)
Menu: 1 (old), 1 (new), 0 (No change)
Mipmap: 1 (old), 1 (new), 0 (No change)
Plurals: 10 (old), 10 (new), 0 (No change)
String: 781 (old), 781 (new), 0 (No change)
Style: 472 (old), 472 (new), 0 (No change)
Xml: 1 (old), 1 (new), 0 (No change)

Lesson assets: 111 (old), 111 (new), 0 (No change)

AAB differences

Expand to see AAB specifics

Supported configurations:

hdpi (same)
ldpi (same)
mdpi (same)
tvdpi (same)
xhdpi (same)
xxhdpi (same)
xxxhdpi (same)

Base APK

APK file size: 10 MiB (old), 10 MiB (new), 0 bytes (No change)
APK download size (estimated): 9 MiB (old), 9 MiB (new), 4 bytes (Added)

Configuration hdpi

APK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change)
APK download size (estimated): 17 KiB (old), 17 KiB (new), 0 bytes (No change)

Configuration ldpi

APK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change)
APK download size (estimated): 13 KiB (old), 13 KiB (new), 0 bytes (No change)

Configuration mdpi

APK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change)
APK download size (estimated): 13 KiB (old), 13 KiB (new), 0 bytes (No change)

Configuration tvdpi

APK file size: 73 KiB (old), 73 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Configuration xhdpi

APK file size: 50 KiB (old), 50 KiB (new), 0 bytes (No change)
APK download size (estimated): 20 KiB (old), 20 KiB (new), 0 bytes (No change)

Configuration xxhdpi

APK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change)
APK download size (estimated): 28 KiB (old), 28 KiB (new), 0 bytes (No change)

Configuration xxxhdpi

APK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

…dundant failure check for the PASS statement

github-actions · 2024-09-05T04:44:42Z

Coverage Report

Results

Coverage Analysis: SKIP ⏭️

This PR did not introduce any changes to Kotlin source or test files.

To learn more, visit the Oppia Android Code Coverage wiki page

… render the color codes properly with %b

…o pre_commit_hook_prevent_binary_checkin

Rd4dev · 2024-09-05T08:08:48Z

@BenHenning, thanks for the review. Linking the stack trace for the working ci check from fork (updated in PR desc).

Can you PTAL!

Minor change

Moved file printing into the iteration section to properly render them with color codes in the CI log.

Earlier:

Now:

github-actions · 2024-09-05T08:10:17Z

Coverage Report

Results

Coverage Analysis: SKIP ⏭️

This PR did not introduce any changes to Kotlin source or test files.

To learn more, visit the Oppia Android Code Coverage wiki page

oppiabot · 2024-09-05T08:11:52Z

Unassigning @Rd4dev since a re-review was requested. @Rd4dev, please make sure you have addressed all review comments. Thanks!

BenHenning

Thanks @Rd4dev! This LGTM. Glad to see this go in. :)

Fix oppia#1730 - Prevent binary files from being checked in using pre…

bfe4a99

…-commit hook

Rd4dev requested a review from a team as a code owner September 4, 2024 14:04

Rd4dev requested a review from adhiamboperes September 4, 2024 14:04

oppiabot bot assigned adhiamboperes and BenHenning Sep 4, 2024

Rd4dev commented Sep 4, 2024

View reviewed changes

.github/workflows/static_checks.yml Show resolved Hide resolved

BenHenning requested changes Sep 4, 2024

View reviewed changes

.github/workflows/static_checks.yml Outdated Show resolved Hide resolved

.github/workflows/static_checks.yml Show resolved Hide resolved

BenHenning assigned Rd4dev and unassigned BenHenning Sep 4, 2024

Adding the missed out !cancelled conditional check and removed the re…

3294c20

…dundant failure check for the PASS statement

Rd4dev added 2 commits September 5, 2024 13:28

Moved the print statement right into the check loop as the CI doesn't…

5a2bb70

… render the color codes properly with %b

Merge branch 'develop' of https://github.com/Rd4dev/oppia-android int…

fa5d521

…o pre_commit_hook_prevent_binary_checkin

oppiabot bot assigned BenHenning and unassigned Rd4dev Sep 5, 2024

BenHenning approved these changes Sep 5, 2024

View reviewed changes

BenHenning merged commit 4730edb into oppia:develop Sep 5, 2024
31 checks passed

Fix #1730 : Prevent binary files from being checked in using pre-commit hook #5525

Fix #1730 : Prevent binary files from being checked in using pre-commit hook #5525

Conversation

Rd4dev commented Sep 4, 2024 • edited Loading

Explanation

The PR includes

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

CI re-check if the PR includes a binary - [ Fail - stack trace ] [ Pass - stack trace ]

Essential Checklist

Rd4dev commented Sep 4, 2024

github-actions bot commented Sep 4, 2024

Coverage Report

Results

BenHenning left a comment

Choose a reason for hiding this comment

github-actions bot commented Sep 5, 2024

APK & AAB differences analysis

Dev

Universal APK

AAB differences

Base APK

Configuration hdpi

Configuration ldpi

Configuration mdpi

Configuration tvdpi

Configuration xhdpi

Configuration xxhdpi

Configuration xxxhdpi

Alpha

Universal APK

AAB differences

Base APK

Configuration hdpi

Configuration ldpi

Configuration mdpi

Configuration tvdpi

Configuration xhdpi

Configuration xxhdpi

Configuration xxxhdpi

Beta

Universal APK

AAB differences

Base APK

Configuration hdpi

Configuration ldpi

Configuration mdpi

Configuration tvdpi

Configuration xhdpi

Configuration xxhdpi

Configuration xxxhdpi

Ga

Universal APK

AAB differences

Base APK

Configuration hdpi

Configuration ldpi

Configuration mdpi

Configuration tvdpi

Configuration xhdpi

Configuration xxhdpi

Configuration xxxhdpi

github-actions bot commented Sep 5, 2024

Coverage Report

Results

Rd4dev commented Sep 5, 2024

Minor change

github-actions bot commented Sep 5, 2024

Coverage Report

Results

oppiabot bot commented Sep 5, 2024

BenHenning left a comment

Choose a reason for hiding this comment

Rd4dev commented Sep 4, 2024 •

edited

Loading