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.

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

Test Android in CI #730

Closed
wants to merge 3 commits into from
Closed

Test Android in CI #730

wants to merge 3 commits into from

Conversation

fmeum
Copy link
Contributor

@fmeum fmeum commented Apr 25, 2023

No description provided.

Replaces the custom `_ANDROID` macro.
The flags should be ignored when building non-Android targets.
@fmeum
Copy link
Contributor Author

fmeum commented Apr 25, 2023

@marktefftech With regular builds, we use --java_runtime_version=remotejdk_17 paired with --javacopt=-target --javacopt=8 to build Java 8 compatible jars with a modern JDK. Is there a way we could make this work for Android builds as well instead of forcing JDK 8?

@marktefftech
Copy link
Contributor

I'm looking into this and will circle back around shortly.

@marktefftech
Copy link
Contributor

@fmeum Does PR#734 - Fixed Android build breaks #734 Address your concerns with JDK versioning, or should we revisit this open PR?

@fmeum
Copy link
Contributor Author

fmeum commented May 22, 2023

@marktefftech I would still like to get the current PR merged if it doesn't break the Android build. Ideally, we wouldn't need a separate android config at all.

@fmeum fmeum marked this pull request as ready for review May 22, 2023 15:06
build:android_arm --incompatible_enable_android_toolchain_resolution
build:android_arm --android_platforms=//:android_arm64
build:android_arm --copt=-D_ANDROID
build:android_arm --java_runtime_version=8
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I tested locally, everything worked except this value still needed to be "=local_jdk" to find the correct toolchain

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking. That probably means that this is blocked onhttps://github.com/bazelbuild/bazel/pull/18262. I will keep it open and we can test it again after that change has landed in Bazel.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fmeum Thanks so much for following up with this, we tried many toolchain workarounds for --java_runtime_version flag but couldn't get it to work. Hopefully the above PR will unblock this feature.

In the meantime, we are still hoping to add a basic CI test to ensure the critical targets that we have will continue to build.

Currently we rely heavily on the following two commands:

bazelisk build launcher/android:jazzer_android --config=android_arm
bazelisk build src/main/java/com/code_intelligence/jazzer/android:jazzer_standalone_android --config=android_arm

Can you please offer some guidance on how we can incorporate these commands into the current pipeline? Apologies for the rudimentary question, but I don't have much bazel testing experience yet.

We really just need to make sure that those two commands pass to ensure that Android is in a good state in Jazzer. I think it is important to note that even though the below test commands still execute as expected and the test passes, trying to build these targets individually will still fail:

bazelisk test --config=ci --remote_header=x-buildbuddy-api-key=*** --java_runtime_version=local_jdk_8 --disk_cache=/home/runner/.cache/bazel-disk //launcher/android:jazzer_android --build_tag_filters="-no-linux-jdk8,-no-jdk8" --test_tag_filters="-no-linux-jdk8,-no-jdk8" //...

Thanks in advance, I am happy to implement the test, just need some guidance on where to start.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@marktefftech Could you try adding --config=android_arm as well as the missing label to

bazel_args: "//launcher/android:jazzer_android"
?

@fmeum fmeum closed this Aug 1, 2023
@fmeum fmeum deleted the android-ci branch August 1, 2023 13:38
@fmeum
Copy link
Contributor Author

fmeum commented Aug 1, 2023

Superseded by #807

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants