Skip to content

Commit

Permalink
Fix part of #4391: Fix failing build on m1 macs (#5111)
Browse files Browse the repository at this point in the history
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
Fix part of #4391 : Fix failing build on M1 Macs
## 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.
  -->
Some M1 Macs fail to build the Oppia app because of unused imports in
some of the proto files in the project. This PR attempts to fix this
issue by removing unused imports in proto files. Files affected by this
fix are;
- profile.proto
- deprecation.proto

This is a temporary fix and more investigation will need to be done to
properly understand why this behavior is erratic and why it only happens
in some instances and not in others. More checks are also needed to
ensure that proto files only have imports that are used in the proto
file before they are merged.

Besides build fixes, this PR also;
- Includes a fix for the `buildifier_download.sh` which previously did
not work for MacOS users.
- Introduces a `static_checks.sh` file to allow contributors to run all
static checks locally before pushing the code to Git.
- Adds a wiki entry in the Static-Analysis-Checks wiki. The entry
contains instructions on how to run static checks locally with the
static_checks.sh file.

## 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)).

---------

Co-authored-by: Kenneth Murerwa <[email protected]>
Co-authored-by: Adhiambo Peres <[email protected]>
  • Loading branch information
3 people authored Aug 3, 2023
1 parent 2b3213c commit d55b7e6
Show file tree
Hide file tree
Showing 6 changed files with 127 additions and 7 deletions.
2 changes: 0 additions & 2 deletions model/src/main/proto/deprecation.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ syntax = "proto3";

package model;

import "version.proto";

option java_package = "org.oppia.android.app.model";
option java_multiple_files = true;

Expand Down
2 changes: 0 additions & 2 deletions model/src/main/proto/profile.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ syntax = "proto3";

package model;

import "languages.proto";

option java_package = "org.oppia.android.app.model";
option java_multiple_files = true;

Expand Down
12 changes: 10 additions & 2 deletions scripts/buildifier_download.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,14 @@
# Download buildifier
BUILDIFIER="3.4.0"
echo Using Buildifier version $BUILDIFIER
curl -sSLO https://github.com/bazelbuild/buildtools/releases/download/$BUILDIFIER/buildifier > buildifier
chmod a+x buildifier
if [[ "$OSTYPE" == "darwin"* ]];
then
curl -sSLO https://github.com/bazelbuild/buildtools/releases/download/$BUILDIFIER/buildifier.mac > buildifier.mac
chmod a+x buildifier.mac
mv buildifier.mac buildifier
else
curl -sSLO https://github.com/bazelbuild/buildtools/releases/download/$BUILDIFIER/buildifier > buildifier
chmod a+x buildifier
fi

echo Buildifier file downloaded
3 changes: 3 additions & 0 deletions scripts/setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,6 @@ bash ../oppia-android/scripts/buf_download.sh

# Add protobuf platform for M1 Mac
bash ../oppia-android/scripts/buf_m1_mac_setup.sh

# Download buildifier
bash ../oppia-android/scripts/buildifier_download.sh
108 changes: 108 additions & 0 deletions scripts/static_checks.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
#!/bin/bash

# INSTRUCTIONS
# This script will run all script checks locally to make
# sure that all script checks will still pass when run on
# CI
#
# To run this check run the script from the Oppia-android root folder:
#
# bash scripts/static_checks.sh
#

# LINT CHECKS
# Run Java lint check
bash scripts/checkstyle_lint_check.sh
echo ""

# Run Kotlin lint check
bash scripts/ktlint_lint_check.sh
echo ""

# Run protobuf lint checks
bash scripts/buf_lint_check.sh
echo ""

# Download Buildifier in oppia-android-tools folder (pre-requisite for buildifier checks)
echo "********************************"
echo "Downloading buildifier"
echo "********************************"
cd ../oppia-android-tools/
bash ../oppia-android/scripts/buildifier_download.sh
cd ../oppia-android/
echo ""

# Run Bazel Build file lint checks (buildifier checks)
bash scripts/buildifier_lint_check.sh
echo ""


# SCRIPT CHECKS
# These checks run on Bazel. Ensure Bazel is installed and configured correctly.

# Run regex pattern checks
echo "********************************"
echo "Running regex pattern checks"
echo "********************************"
bazel run //scripts:regex_pattern_validation_check -- $(pwd)
echo ""

# Run XML Syntax check validation
echo "********************************"
echo "Running XML Syntax validation checks"
echo "********************************"
bazel run //scripts:xml_syntax_check -- $(pwd)
echo ""

# Run Testfile Presence Check
echo "********************************"
echo "Running Testfile presence checks"
echo "********************************"
bazel run //scripts:test_file_check -- $(pwd)
echo ""

# Run Accessibility label Check
echo "********************************"
echo "Running Accessibility label checks"
echo "********************************"
bazel run //scripts:accessibility_label_check -- $(pwd) scripts/assets/accessibility_label_exemptions.pb app/src/main/AndroidManifest.xml
echo ""

# Run KDoc Validation Check
echo "********************************"
echo "Running KDoc validation checks"
echo "********************************"
bazel run //scripts:kdoc_validity_check -- $(pwd) scripts/assets/kdoc_validity_exemptions.pb
echo ""

# Run String resource validation check
echo "********************************"
echo "Running resource validation checks"
echo "********************************"
bazel run //scripts:string_resource_validation_check -- $(pwd)
echo ""


# THIRD PARTY DEPENDENCY CHECKS
# These are checks for third party dependencies

# Maven Repin Check
echo "********************************"
echo "Running Maven repin checks"
echo "********************************"
REPIN=1 bazel run @unpinned_maven//:pin
echo ""

# Maven Dependencies Update Check
echo "********************************"
echo "Running maven dependencies update checks"
echo "********************************"
bazel run //scripts:maven_dependencies_list_check -- $(pwd) third_party/maven_install.json scripts/assets/maven_dependencies.pb
echo ""

# License Texts Check
echo "********************************"
echo "Running license texts checks"
echo "********************************"
bazel run //scripts:license_texts_check -- $(pwd)/app/src/main/res/values/third_party_dependencies.xml

7 changes: 6 additions & 1 deletion wiki/Static-Analysis-Checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -230,9 +230,14 @@ To fix the failures for this check: resolve the TODO items and then close the is
// TODO([#3690](https://github.com/oppia/oppia-android/issues/3690)): Complete static checks Wiki

# How to run static checks locally
There are two ways to run static checks locally. You can either run all checks locally with the command `bash scripts/static_checks.sh` or run individual commands based on the name of the failing check on GitHub CI.

Running `bash scripts/static_checks.sh` should take around 10 minutes for a clean run (after running `bazel clean`) and around 2-5 minutes for subsequent runs.

To fix failing tests from GitHub CI individually, follow the steps below.
- Go to the failing CI check in your GitHub PR.
- Scroll to the top of the failing CI check logs, and find the Bazel command that was run for this script.
- Alternatively, in Android Studio, go to the `.github` folder and find the [static_checks.yml](https://github.com/oppia/oppia-android/blob/develop/.github/workflows/static_checks.yml) file. Search for the line that corresponds to the name of the job that failed. You can then run the same script on your local terminal.
- You can also go to scripts/static_checks.sh to view the failing check and run it locally.

Note: Before running the script command in your local terminal, make sure you have Bazel installed. To learn how to set up Bazel for Oppia Android, follow these [instructions](https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions).
Note: Before running the script command in your local terminal, make sure you have Bazel installed. To learn how to set up Bazel for Oppia Android, follow these [instructions](https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions). Also make sure you have oppia-android-tools installed since static checks rely on these tools to be able to perform some of the checks. To install oppia-android-tools, run `bash scripts/setup.sh` in the oppia-android directory.

0 comments on commit d55b7e6

Please sign in to comment.