Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for build tools 31.x+ #3024

Closed
BenHenning opened this issue Mar 29, 2021 · 7 comments
Closed

Add support for build tools 31.x+ #3024

BenHenning opened this issue Mar 29, 2021 · 7 comments
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Low Low perceived user impact (e.g. edge cases). Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@BenHenning
Copy link
Sponsor Member

#3023 has some context, but it seems that we cannot support the newest Android build tools version due to it removing D8. Bazel is supposed to have a D8 compatibility layer embedded, but it doesn't seem to be working as expected.

That being said, we should move to R8 instead of using the D8 compatibility layer, however per #3023 this doesn't seem to work in our build because we rely on API desugaring (or rather, dependencies that we use do). I haven't yet found any Bazel issues tied to this, so this may require additional investigation & coordination with the Bazel team to find a root cause.
@BenHenning
Copy link
Sponsor Member Author

Note that this may cause some confusion among contributors if they inadvertently update their build tools past 31.x & try to build the app using Bazel.

@fsharpasharp as FYI.

@BenHenning BenHenning mentioned this issue Jul 21, 2021
Closed
@BenHenning
Copy link
Sponsor Member Author

This has become much more urgent due to #3510. We might need to find a way to mitigate the problem until an actual fix can be used.

bazelbuild/bazel#10240 is tracking the fix on the Bazel side of things for a long-term fix.

@BenHenning
Copy link
Sponsor Member Author

Confirming that https://github.com/oppia/oppia-android/wiki/Bazel-Setup-Instructions-for-Windows#3-installing-the-android-sdk works as a mitigation to force the local SDK to have an older version (if people ever need to downgrade). Switching back over the other issue thread to determine how to make this work in CI.

@BenHenning
Copy link
Sponsor Member Author

BenHenning commented Jul 21, 2021 •
edited
Loading

For folks looking to mitigate this, reference the instructions listed here with the following modifications:

  1. Install the cmdline tools into your existing Android SDK directory
  2. Rather than updating your PATH to point to the bin directory, just reference it relatively, e.g.: $ANDROID_HOME/cmdline-tools/tools/bin/sdkmanager
  3. Only install the 29.0.2 build tools; the other bits probably don't need to be installed since they should already be set up by Android Studio
  4. Remove any non-29.0.2 build tools from your $ANDROID_HOME/build-tools directory (the build system will always default to the newest version)

@BenHenning
Copy link
Sponsor Member Author

Added mitigation instructions to https://github.com/oppia/oppia-android/wiki/Oppia-Bazel-Setup-Instructions#known-issues (it just explains the issue & points back to this thread).

@BenHenning BenHenning mentioned this issue Jul 22, 2021
Merged
8 tasks
BenHenning added a commit that referenced this issue Jul 22, 2021
@BenHenning
[RunAllTests] Fix #3510: Set up Android SDK hermetically in GitHub Ac…
e49edc7 
…tions to workaround #3024 (#3513)

* Refactor common build env setup to local action.

This attempts to move all common build environment setup bits to a
common local action using GitHub composite actions per
https://github.community/t/path-to-action-in-the-same-repository-as-workflow/16952/2,
https://github.blog/changelog/2020-08-07-github-actions-composite-run-steps/,
and
https://docs.github.com/en/actions/creating-actions/creating-a-composite-run-steps-action.

This does not include any actual changes to the CI environment yet other
than temporarily disabling the other expensive checks as part of testing
these changes.

* Update local action to only use shell commands.

* Set up JDK 9 manually for build tests.

* Fix Java version checking.

OpenJDK can use "9.0" as the version string instead of "1.9".

* Fix Java exit from version check.

* Fix incorrect check.

* Add Bazel setup.

* Fix syntax error & add Bazel version check.

* Add SDK setup bits.

* Avoid mv warning/error.

* Add debug lines.

* Attempt to fix pathing for new SDK.

* Attempt to workaround unexpected failure from sdkmanager.

* Add debug lines.

* Try to run license bit in a subshell to avoid exit

* Add codeowners, TODO, cleanup, and integrate.

This integrates the new action with Bazel tests in addition to the
build. While the tests work fine with the current build tools, we should
be consistent in how the environment is set up for CI workflows.

* Make local action name clearer.

The new name specifically clarifies why this action isn't needed for the
Bazel-only script runs (since they don't require the Android SDK bits).
@BenHenning BenHenning mentioned this issue Oct 12, 2021
Open
@BenHenning
Copy link
Sponsor Member Author

bazelbuild/bazel#13738 means we can update our min Bazel version to 4.2.0 & probably drop our custom Android tools repository, but we'll need to update build-tools pin to 30.0.0. It's a start for this issue, but doesn't fully address the issue since Bazel still needs to fix the D8 compatibility issue.

This might help with #3371 though.

@BenHenning BenHenning added this to the Beta MR2 milestone Jun 11, 2022
@Broppia Broppia added issue_type_infrastructure Impact: Low Low perceived user impact (e.g. edge cases). labels Jul 29, 2022
@BenHenning BenHenning added issue_user_developer Z-ibt Temporary label for Ben to keep track of issues he's triaged. labels Sep 16, 2022
@BenHenning BenHenning removed this from the Beta MR2 milestone Sep 16, 2022
@seanlip seanlip added bug End user-perceivable behaviors which are not desirable. and removed issue_type_infrastructure labels Mar 28, 2023
@BenHenning
Copy link
Sponsor Member Author

We're now using build tools 32.0.0, so this is technically completed:

oppia-android/build_vars.bzl

Line 2 in 0ef7a46

BUILD_TOOLS_VERSION = "32.0.0"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Low Low perceived user impact (e.g. edge cases). Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Task Force] Migrate to Bazel - Android
Archived in project
Milestone
No milestone
Development

No branches or pull requests
3 participants
@seanlip @BenHenning @Broppia