-
Notifications
You must be signed in to change notification settings - Fork 541
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.
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
Required Bazel build CI check now failing for everyone #3510
Comments
Continuing the discussion from #3509 (comment). Given that we don't have an explicit workflow to set up the Android SDK in CI, we're probably relying on the SDK provided by GitHub. If GitHub updated to use a newer platform tools, it's likely causing this failure. We may need to set up a CI action to force a specific version (which is probably better, anyway; this change is breaking CI environment hermeticity). |
This is only reproduced when we try to build android_binary |
@FareesHussain I think that's expected because only android_binary runs the dexer portion of the build pipeline. |
So trying to find a way locally to determine what actually updated in GitHub's environment to break us. It seems this is caused by build tools being updated from 29.0.2 (or, at least, that's my local version) to 31.0.0. I'm guessing this means the prerelease build tools are now officially supported that include the breaking dexer changes. #3024 has more context on the reason why the build tools upgrade breaks us. |
So per #3024 we have a workaround: https://github.com/oppia/oppia-android/wiki/Bazel-Setup-Instructions-for-Windows#3-installing-the-android-sdk explains how to install specific build tools versions. I'm looking for a CI-compatible way to do this since we need to be able to accept the licenses which I don't think can happen automatically. |
https://stackoverflow.com/a/45782695 & https://github.com/android-actions/setup-android/blob/main/src/main.ts#L14 as references seem to demonstrate that this is possible. Sending out a PR momentarily to update all Bazel workflows to set up a hermetic SDK environment to both mitigate this issue & avoid it in the future. |
…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).
See #3509 for initial context.
The text was updated successfully, but these errors were encountered: