Skip to content

Commit

Permalink
Fix oppia#5370, part of oppia#59: Migrate to Bazel 6.5.0 (oppia#4886)
Browse files Browse the repository at this point in the history
## Explanation

Fixes oppia#5370
Fixes part of oppia#59

This PR updates the project to use Bazel 6.5.0 instead of 4.0.0.

Note that most of the changes done so far in addressing oppia#59 are centered
around the concept of simplifying the Bazel maintenance as much as
possible so that it's not too much more difficult than Gradle by the
time we fully remove Gradle support from the project. While Bazel will
always require more effort, there are many things that can be done to
narrow the gap. This is a major step in that process since Bazel 4.x
required using a custom Android toolchain
(https://github.com/oppia/oppia-bazel-tools) which is not at all user
friendly. Plus, there are many compatibility and performance
improvements in later versions of Bazel that we want to be able to
incorporate within the broader Oppia Android project.

Bazel 6.x was specifically chosen because:
- Bazel 4.x was missing support for the new D8 version which made it
impossible to upgrade past 29.0.2 build tools
bazelbuild/bazel#13989.
- Bazel 5.x had some additional compatibility issues with the D8 change,
so we weren't able to use it, either:
bazelbuild/bazel#15957.
- Bazel 7.x (which wasn't released when this work was originally done)
introduces new bzlmod support that causes some additional build
headaches that can be figured out later.
- Bazel 6.5.0 specifically was chosen since it's the latest 6.x version
(as of this edit) and seems to work correctly with existing unit tests.

Some other important details to note:
- rules_kotlin 1.7.x is needed at a minimum for Bazel 5.x+ support.
However, an additional fix was needed
(bazelbuild/rules_kotlin#940) in order to fix a
deviation in functionality that occurred starting in Bazel 5.x's
java_plugin support which led to some file duplication in rules_kotlin
(that was fortunately easy to fix). Unfortunately, this change wasn't
backported to 1.7.x so this PR makes use of a custom patch to
rules_kotlin 1.7.1 (https://github.com/oppia/rules_kotlin) that includes
the needed change. We'll get this change properly once we can upgrade to
1.8.x, though that will also require updating Kotlin itself to 1.8.x due
to bazelbuild/rules_kotlin#1019.
- Bazel 6.x (maybe 5.x) requires at least build tools 30.0.0 since it
completely removed support for the old D8 compat dexer. 32.0.0 was
chosen in this PR as it's simply a newer, more up-to-date build tools
(and removes D8 completely). With this upgrade to Bazel 6.x we'll be
able to update the build tools version more often (so long as it doesn't
introduce AGP incompatibilities since we can't upgrade Gradle).
- As of Bazel 6.x, we're able to reenable Java header compilation and
incremental dexing, both of which should have _significant_ performance
improvements for incremental builds of the app (and in fact we will have
build errors if we disable incremental dexing).
- In CI, we opted to **not** support build tools 29.0.2 or old builds of
the app. Instead, we'll rely on build tools failing for certain PRs as
an indicator that those PRs will require an update (once this PR is
merged) in order to have CI run correctly. This is a lot easier than
trying to figure out how to support before/after changes with some
fairly complex environment differences.
- There are a bunch of version updates that were needed to support the
minimum version of Kotlin for rules 1.7.x (1.6 I think) as well as JDK
11 (which I think was needed for Bazel 5.x), and these have largely been
taken care of in previous PRs to this one (though the JDK 11 update in
CI was done in this PR, along with wiki documentation updates to address
oppia#5370). One such case of a necessary version upgrade:
google/dagger#2511.
- There was a change needed for the databinding java_plugin declaration
to specify that it generates an API (in order for it to be used
correctly in builds).
- rules_java needed to be updated to support the newer version of Bazel.
- The desugaring hack needed for kotlinx-coroutines-core-jvm was removed
since it's no longer needed with the build tools & Bazel upgrade
introduced in this PR.
- This includes one small change in third-party to change all
single-export wrappers that don't have additional plugins being enabled
to aliases instead. This is more semantically correct as the wrappers
may lose information (which caused problems when investigating adding
Jetpack Compose support in oppia#5401). While this isn't directly required
for the Bazel upgrade, this is the last PR needed for Jetpack Compose
support so it's being added here for simplicity.
- ``.bazelrc`` was updated to configure tools, tests, and builds to all
use the remote JDK 11 available via Bazel rather than ever using the
user's local JDK. This should improve build hermeticity and consistency
across different user environments (see
https://bazel.build/docs/bazel-and-java).
- Setup docs were updated to remove setting up JDK 11 (or Java at all
for Linux & Mac) now that the user no longer needs to install Java (see
previous point) except for Windows. The Python instructions were also
removed since Bazel 6.x includes fixes for Android tools that previously
depended on Python 2.x.
- CI was unchanged for Java setup since, as far as I can tell, it's
still needed for sdkmanager.

There was also some small cleanup in unit_tests.yml that I noticed when
updating CI versions.

## Essential Checklist
- [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)).

## For UI-specific PRs only
N/A -- This is a build infrastructure change. It shouldn't impact the
end user experience.

---------

Co-authored-by: Adhiambo Peres <[email protected]>
Co-authored-by: Sean Lip <[email protected]>
  • Loading branch information
3 people authored Jun 13, 2024
1 parent 23def1b commit add9f74
Show file tree
Hide file tree
Showing 29 changed files with 176 additions and 417 deletions.
9 changes: 5 additions & 4 deletions .bazelrc
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
# Configurations for arguments that should automatically be added to Bazel commands.
# Configurations to ensure that Android-specific classes build correctly.
build --android_databinding_use_v3_4_args \
--experimental_android_databinding_v2 \
--java_header_compilation=false \
--noincremental_dexing \
--define=android_standalone_dexing_tool=d8_compat_dx \
--android_databinding_use_androidx

# Ensure that all builds use the same JDK for building & running (for better hermeticity and fewer
# inconsistencies across environment configurations).
build --java_runtime_version=remotejdk_11 --tool_java_runtime_version=remotejdk_11

# Ensure all built Java files treat warnings as errors (similar to the Kotlin configuration) to help
# reduce code smell & potential bugs during development.
build --javacopt="-Werror"
Expand Down
2 changes: 1 addition & 1 deletion .bazelversion
Original file line number Diff line number Diff line change
@@ -1 +1 @@
4.0.0
6.5.0
29 changes: 8 additions & 21 deletions .github/actions/set-up-android-bazel-build-environment/action.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Contains common operations to set up a hermetic Android + Bazel build environment for Oppia
# Android CI workflows. Action prerequisites:
# - JDK 9 must be installed & set as the default version via JAVA_HOME
# - Bazel must be installed, in the path, and be version 4.0.0
# - JDK 11 must be installed & set as the default version via JAVA_HOME
# - Bazel must be installed, in the path, and be version 6.5.0

# TODO(#1861): Revert SDK pinning for improved CI performance once Bazel is sufficiently stable that
# we can rely on the automatic SDK provided by GitHub's CI environment.
Expand All @@ -19,20 +19,20 @@ runs:
$JAVA_HOME/bin/java -version
# Verify that the correct version of Java is installed.
java -version 2>&1 | grep -q -E "1.9|9.0"
java -version 2>&1 | grep -q -E "11.0"
HAS_CORRECT_JAVA_VERSION=$(echo $?)
if [[ "$HAS_CORRECT_JAVA_VERSION" == 1 ]] ; then
echo "Expected Java 9 to be installed"
echo "Expected Java 11 to be installed"
exit 1
fi
shell: bash

- name: Verify Bazel version
run: |
bazel --version | grep -q 4.0.0
bazel --version | grep -q 6.5.0
HAS_CORRECT_BAZEL_VERSION=$(echo $?)
if [[ "$HAS_CORRECT_JAVA_VERSION" == 1 ]] ; then
echo "Expected Bazel 4.0.0 to be installed"
echo "Expected Bazel 6.5.0 to be installed"
exit 1
fi
shell: bash
Expand Down Expand Up @@ -77,26 +77,13 @@ runs:
$ANDROID_HOME/cmdline-tools/tools/bin/sdkmanager --install "platforms;android-33"
shell: bash

- name: Install build tools 29.0.2
- name: Install build tools 32.0.0
run: |
$ANDROID_HOME/cmdline-tools/tools/bin/sdkmanager --install "build-tools;29.0.2"
shell: bash

- name: Configure Bazel to use JDK 9 for building
run: |
echo "build --java_language_version=9" >> $HOME/.bazelrc
$ANDROID_HOME/cmdline-tools/tools/bin/sdkmanager --install "build-tools;32.0.0"
shell: bash

- name: Configure Bazel to use specific sandbox tmpfs
run: |
echo "build --enable_platform_specific_config" >> $HOME/.bazelrc
echo "build:linux --sandbox_tmpfs_path=/tmp" >> $HOME/.bazelrc
shell: bash

- name: Set up Oppia Bazel Android Tools
run: |
mkdir $HOME/opensource
cd $HOME/opensource
git clone https://github.com/oppia/oppia-bazel-tools
echo build --override_repository=android_tools="$(cd "$(dirname "$HOME/opensource/oppia-bazel-tools")"; pwd)/$(basename "$HOME/opensource/oppia-bazel-tools")" >> $HOME/.bazelrc
shell: bash
30 changes: 15 additions & 15 deletions .github/workflows/build_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ jobs:
with:
fetch-depth: 0

- name: Set up JDK 9
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: 9
java-version: 11

- name: Set up Bazel
uses: abhinavsingh/setup-bazel@v3
with:
version: 4.0.0
version: 6.5.0

- name: Set up build environment
uses: ./.github/actions/set-up-android-bazel-build-environment
Expand Down Expand Up @@ -168,15 +168,15 @@ jobs:
with:
fetch-depth: 0

- name: Set up JDK 9
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: 9
java-version: 11

- name: Set up Bazel
uses: abhinavsingh/setup-bazel@v3
with:
version: 4.0.0
version: 6.5.0

- name: Set up build environment
uses: ./.github/actions/set-up-android-bazel-build-environment
Expand Down Expand Up @@ -302,15 +302,15 @@ jobs:
with:
fetch-depth: 0

- name: Set up JDK 9
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: 9
java-version: 11

- name: Set up Bazel
uses: abhinavsingh/setup-bazel@v3
with:
version: 4.0.0
version: 6.5.0

- name: Set up build environment
uses: ./.github/actions/set-up-android-bazel-build-environment
Expand Down Expand Up @@ -449,15 +449,15 @@ jobs:
with:
fetch-depth: 0

- name: Set up JDK 9
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: 9
java-version: 11

- name: Set up Bazel
uses: abhinavsingh/setup-bazel@v3
with:
version: 4.0.0
version: 6.5.0

- name: Set up build environment
uses: ./.github/actions/set-up-android-bazel-build-environment
Expand Down Expand Up @@ -570,15 +570,15 @@ jobs:
with:
fetch-depth: 0

- name: Set up JDK 9
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: 9
java-version: 11

- name: Set up Bazel
uses: abhinavsingh/setup-bazel@v3
with:
version: 4.0.0
version: 6.5.0

- name: Set up build environment
uses: ./.github/actions/set-up-android-bazel-build-environment
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/issue_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
- name: Set up Bazel
uses: abhinavsingh/setup-bazel@v3
with:
version: 4.0.0
version: 6.5.0

- name: TODO Issue Resolved Check
id: todoIssueResolvedCheck
Expand Down
9 changes: 4 additions & 5 deletions .github/workflows/static_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,10 @@ jobs:
- name: Create oppia android tools directory
run: mkdir -p $HOME/oppia-android-tools

# Java 11 is specifically needed for Checkstyle.
- name: Set up JDK 1.11
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: 1.11
java-version: 11

- name: Download Checkstyle
run: |
Expand Down Expand Up @@ -104,7 +103,7 @@ jobs:
- name: Set up Bazel
uses: abhinavsingh/setup-bazel@v3
with:
version: 4.0.0
version: 6.5.0

- uses: actions/cache@v2
id: scripts_cache
Expand Down Expand Up @@ -196,7 +195,7 @@ jobs:
- name: Set up Bazel
uses: abhinavsingh/setup-bazel@v3
with:
version: 4.0.0
version: 6.5.0

- name: Maven Repin Check
if: always()
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/stats.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,15 @@ jobs:
run: |
echo "PR $PR_NUMBER is merging into $PR_BASE_REF_NAME from https://github.com/$PR_HEAD branch $PR_HEAD_REF_NAME."
- name: Set up JDK 9
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: 9
java-version: 11

- name: Set up Bazel
uses: abhinavsingh/setup-bazel@v3
with:
version: 4.0.0
version: 6.5.0

# For reference on this & the later cache actions, see:
# https://github.com/actions/cache/issues/239#issuecomment-606950711 &
Expand Down
18 changes: 9 additions & 9 deletions .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
runs-on: ubuntu-20.04
outputs:
matrix: ${{ steps.compute-test-matrix.outputs.matrix }}
have_tests_to_run: ${{ steps.compute-test-matrix.outputs.have_tests_to_run }}
can_skip_tests: ${{ steps.compute-test-matrix.outputs.can_skip_tests }}
env:
CACHE_DIRECTORY: ~/.bazel_cache
steps:
Expand All @@ -29,7 +29,7 @@ jobs:
- name: Set up Bazel
uses: abhinavsingh/setup-bazel@v3
with:
version: 4.0.0
version: 6.5.0

- uses: actions/cache@v2
id: scripts_cache
Expand Down Expand Up @@ -86,16 +86,16 @@ jobs:
echo "Affected tests (note that this might be all tests if configured to run all or on the develop branch): $TEST_BUCKET_LIST"
echo "::set-output name=matrix::{\"affected-tests-bucket-base64-encoded-shard\":[$TEST_BUCKET_LIST]}"
if [[ ! -z "$TEST_BUCKET_LIST" ]]; then
echo "::set-output name=have_tests_to_run::true"
echo "::set-output name=can_skip_tests::false"
else
echo "::set-output name=have_tests_to_run::false"
echo "::set-output name=can_skip_tests::true"
echo "No tests are detected as affected by this change. If this is wrong, you can add '[RunAllTests]' to the PR title to force a run."
fi
bazel_run_test:
name: Run Bazel Test
needs: bazel_compute_affected_targets
if: ${{ needs.bazel_compute_affected_targets.outputs.have_tests_to_run == 'true' }}
if: ${{ needs.bazel_compute_affected_targets.outputs.can_skip_tests != 'true' }}
runs-on: ubuntu-20.04
strategy:
fail-fast: false
Expand All @@ -107,15 +107,15 @@ jobs:
steps:
- uses: actions/checkout@v2

- name: Set up JDK 9
- name: Set up JDK 11
uses: actions/setup-java@v1
with:
java-version: 9
java-version: 11

- name: Set up Bazel
uses: abhinavsingh/setup-bazel@v3
with:
version: 4.0.0
version: 6.5.0

- uses: actions/cache@v2
id: scripts_cache
Expand Down Expand Up @@ -325,5 +325,5 @@ jobs:
steps:
# This step will be skipped if there are no tests to run, so the overall job should pass.
- name: Check tests passed (for tests that ran)
if: ${{ needs.bazel_compute_affected_targets.outputs.have_tests_to_run == 'true' && needs.bazel_run_test.result != 'success' }}
if: ${{ needs.bazel_compute_affected_targets.outputs.can_skip_tests != 'true' && needs.bazel_run_test.result != 'success' }}
run: exit 1
17 changes: 2 additions & 15 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ http_archive(
# Add support for Kotlin: https://github.com/bazelbuild/rules_kotlin.
http_archive(
name = "io_bazel_rules_kotlin",
patches = ["//tools/kotlin:add_kotlinc_optin_support.patch"],
patches = ["//tools/kotlin:remove_processor_duplicates.patch"],
sha256 = HTTP_DEPENDENCY_VERSIONS["rules_kotlin"]["sha"],
urls = ["https://github.com/bazelbuild/rules_kotlin/releases/download/%s/rules_kotlin_release.tgz" % HTTP_DEPENDENCY_VERSIONS["rules_kotlin"]["version"]],
)

load("@io_bazel_rules_kotlin//kotlin:repositories.bzl", "kotlin_repositories", "kotlinc_version")

# Use the 1.6 compiler since rules_kotlin 1.5 defaults to the 1.5 compiler.
# Use the 1.6 compiler since Kotlin 1.6 is the current supported version in the repository.
kotlin_repositories(
compiler_release = kotlinc_version(
release = "1.6.10",
Expand Down Expand Up @@ -212,7 +212,6 @@ maven_install(
maven_install_json = "//third_party:maven_install.json",
override_targets = {
"com.google.guava:guava": "@//third_party:com_google_guava_guava",
"org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm": "@//third_party:kotlinx-coroutines-core-jvm",
},
repositories = DAGGER_REPOSITORIES + MAVEN_REPOSITORIES,
strict_visibility = True,
Expand Down Expand Up @@ -240,15 +239,3 @@ pinned_maven_install()
"jre",
]
]

http_jar(
name = "kotlinx-coroutines-core-jvm",
sha256 = HTTP_DEPENDENCY_VERSIONS["kotlinx-coroutines-core-jvm"]["sha"],
urls = [
"{0}/org/jetbrains/kotlinx/kotlinx-coroutines-core-jvm/{1}/kotlinx-coroutines-core-jvm-{1}.jar".format(
url_base,
HTTP_DEPENDENCY_VERSIONS["kotlinx-coroutines-core-jvm"]["version"],
)
for url_base in DAGGER_REPOSITORIES + MAVEN_REPOSITORIES
],
)
2 changes: 1 addition & 1 deletion build_vars.bzl
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
BUILD_SDK_VERSION = 33
BUILD_TOOLS_VERSION = "29.0.2"
BUILD_TOOLS_VERSION = "32.0.0"
1 change: 0 additions & 1 deletion config/kitkat_main_dex_class_list.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ androidx/multidex/ZipUtil.class
androidx/work/Configuration$Provider.class
androidx/work/Configuration.class
androidx/work/WorkManager.class
com/google/firebase/FirebaseApp.class
javax/inject/Provider.class
kotlin/Function.class
kotlin/jvm/functions/Function0.class
Expand Down
2 changes: 1 addition & 1 deletion scripts/assets/file_content_validation_checks.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ file_content_checks {
}
file_content_checks {
file_path_regex: ".+?\\.kt"
prohibited_content_regex: "(format|getString|getStringArray|getQuantityString|getQuantityText|toLowerCase|toUpperCase|capitalize|decapitalize|lowercase|uppercase)\\("
prohibited_content_regex: "(format|getString|getStringArray|getQuantityString|getQuantityText|toLowerCase|toUpperCase|capitalize|decapitalize|lowercase|uppercase|replaceFirstChar)\\("
failure_message: "String formatting and resource retrieval should go through AppLanguageResourceHandler, OppiaLocale.DisplayLocale, or OppiaLocale.MachineLocale depending on the context (see each class's documentation for details on when each should be used)."
exempted_file_name: "app/src/main/java/org/oppia/android/app/translation/AppLanguageResourceHandler.kt"
exempted_file_name: "domain/src/main/java/org/oppia/android/domain/util/JsonExtensions.kt"
Expand Down
11 changes: 11 additions & 0 deletions scripts/assets/maven_dependencies.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -1680,6 +1680,17 @@ maven_dependency {
}
}
}
maven_dependency {
artifact_name: "org.jetbrains.kotlinx:kotlinx-coroutines-core-jvm:1.6.4"
artifact_version: "1.6.4"
license {
license_name: "The Apache Software License, Version 2.0"
original_link: "https://www.apache.org/licenses/LICENSE-2.0.txt"
scrapable_link {
url: "https://www.apache.org/licenses/LICENSE-2.0.txt"
}
}
}
maven_dependency {
artifact_name: "org.jetbrains.kotlinx:kotlinx-coroutines-guava:1.6.4"
artifact_version: "1.6.4"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import org.junit.rules.TemporaryFolder
import java.io.File

/** The version of Bazel to use in tests that set up Bazel workspaces. */
const val BAZEL_VERSION = "4.0.0"
const val BAZEL_VERSION = "6.5.0"

/**
* Test utility for generating various test & library targets in the specified [TemporaryFolder].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ class AndroidBuildSdkPropertiesTest {
fun testBuildToolsVersion_isTheCorrectVersion() {
val properties = AndroidBuildSdkProperties()

assertThat(properties.buildToolsVersion).isEqualTo("29.0.2")
assertThat(properties.buildToolsVersion).isEqualTo("32.0.0")
}
}
Loading

0 comments on commit add9f74

Please sign in to comment.