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

Generate empty repo when path is not set #63

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

gferon
Copy link
Contributor

@gferon gferon commented Jan 31, 2024

Declare an empty repository with empty toolchains and an error message when the path to the NDK was missing.

Based on a comment from #61 (comment) and a very similar approach to what's done in the starlark version of rules_android.

Declare an empty repository with empty toolchains and an error message when the path to the NDK was missing.
@gferon gferon marked this pull request as ready for review January 31, 2024 21:19
@gferon gferon changed the title Soft-fail when path is not set Generate empty repo when path is not set Jan 31, 2024
@gferon
Copy link
Contributor Author

gferon commented Jan 31, 2024

Note: this doesn't exactly work yet as we'd like to, as it errors with the following message

While resolving toolchains for target @@bazel_tools//tools/cpp:current_cc_toolchain (2f4faa5): toolchain type @@bazel_tools//tools/cpp:toolchain_type resolved to target @@androidndk//:error_message, but that target does not provide ToolchainInfo

@gferon gferon marked this pull request as draft February 1, 2024 09:28
@ahumesky
Copy link
Collaborator

ahumesky commented Feb 1, 2024

It would be great to also look at how to add a test for this. It would probably require a new task in .bazelci/presubmit.yml because all the tests will set ANDROID_NDK_HOME

@ahumesky
Copy link
Collaborator

ahumesky commented Mar 8, 2024

Hi, any update on this change?

@gferon gferon force-pushed the main-empty-repo branch from 2c7d706 to 1039412 Compare July 31, 2024 13:41
@gferon gferon marked this pull request as ready for review July 31, 2024 13:42
@gferon
Copy link
Contributor Author

gferon commented Jul 31, 2024

@ahumesky I finally took the time to make it work as I wanted to. It involves adding a dummy_cc_toolchain like it is done in rules_rust for wasm32 (which doesn't have any CC toolchain).

I added two tests in .bazelci/presubmit.yml but don't know how to make the job expect a failure (for the hello-world-android-without-ndk variant). Could you fix that for me?

A failure will look like this (when building for Android platforms only):

bazel build //:all --platforms=//
:arm64-v8a --incompatible_enable_android_toolchain_resol
ution
INFO: Invocation ID: 45bffac4-903c-4be0-aae1-1949130af331
WARNING: Build option --platforms has changed, discarding analysis cache (this can be expensive, see https://bazel.build/advanced/performance/iteration-speed).
INFO: Analyzed target //:hello-world (83 packages loaded, 426 targets configured).
ERROR: /Volumes/bazel-cache/981d0988d8cc33709e99b10fe2f0cc90/external/rules_android_ndk~~android_ndk_repository_extension~androidndk/BUILD.bazel:48:8: Executing genrule @@rules_android_ndk~~android_ndk_repository_extension~androidndk//:invalid_android_ndk_repository_error [for tool] failed: (Exit 1): bash failed: error executing Genrule command (from target @@rules_android_ndk~~android_ndk_repository_extension~androidndk//:invalid_android_ndk_repository_error) /bin/bash -c ... (remaining 1 argument skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging

rules_android_ndk was used without a valid Android NDK being set: either the path attribute of android_ndk_repository or the ANDROID_NDK_HOME environment variable must be set.

Target //:hello-world failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 0.342s, Critical Path: 0.02s
INFO: 3 processes: 3 internal.
ERROR: Build did NOT complete successfully

@gferon gferon force-pushed the main-empty-repo branch from 1039412 to 785f09f Compare July 31, 2024 13:45
When building incrementally, any change to the value of the variable named by name will cause this repository to be re-fetched.
@ahumesky
Copy link
Collaborator

ahumesky commented Sep 18, 2024

Hmm yes expecting a failure in a test like this is tricky, I'm not sure that bazelci is setup up for negative tests. The alternative is to refactor the tests into "bazel within bazel test" integration tests: https://github.com/bazel-contrib/rules_bazel_integration_test

I'm not a big fan of those because they tend to very fragile and somewhat slow, but unless bazelci can do this or that's added to bazelci, might be the only option.

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.

2 participants