Add Android NDK Vulkan validation layers libraries #10095
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is my first foray in Bazel source code, there might be a better way to do this, all feedback very welcome. |
hevrard
force-pushed
the
android-ndk-vk-layer
branch
from
fc10a9d to
8edb41a
Compare
|
Hi @hevrard , thank you for the pull request! |
|
I tried locally and the generated BUILD.bazel file for the NDK seems clean: The log of failing tests refer to Also, in #10094 (comment) , @jin points out that Bazel is mocking an NDK for tests: if so, where should I add mocks for the validation layers? Thanks, |
|
Ping. Any luck with the failing tests? |
hevrard
force-pushed
the
android-ndk-vk-layer
branch
from
8edb41a to
bbc9f3d
Compare
|
@aiuto thanks for the follow-up, I just rebased on master, and I confirm it builds on my machine using bazel-3.1.0 to build. Looking at https://bazel.build/contributing.html, I can't find the command to run all the tests, can you remind me of it (and maybe add it to this doc?) |
|
@aiuto Looking at the CI logs, I can repro the failing tests with: It fails with: But the logs are not very helpful: Could you give me some hints of where to look at to understand why the tests fail? |
|
It looks like the Android NDK hasn't been wired up in the Bazel workspace. Try this: diff --git a/WORKSPACE b/WORKSPACE
index 1f2ff74978..3a6b62314a 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -78,8 +78,8 @@ list_source_repository(name = "local_bazel_source_list")
# 2. Set the $ANDROID_HOME and $ANDROID_NDK_HOME environment variables
# 3. Uncomment the two lines below
#
-# android_sdk_repository(name = "androidsdk")
-# android_ndk_repository(name = "androidndk")
+android_sdk_repository(name = "androidsdk")
+android_ndk_repository(name = "androidndk")Then run the tests again. |
|
Thanks @jin , with this I have meaningful test logs. I'll try to fix the tests now. |
|
The CI machines use NDK r15c, so that could be a factor.
…On Thu, Apr 23, 2020 at 6:33 PM Hugues Evrard ***@***.***> wrote:
Thanks @jin <https://github.com/jin> , with this I have meaningful test
logs. I'll try to fix the tests now.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#10095 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACU6DUBT53IOHOWPE446QDROAKP3ANCNFSM4JES6PEQ>
.
|
|
@jin I have NDK r21 on my desktop, where the test fails with: Do you have any idea how to fix this? Also, NDK r15c seems to have some Vulkan layers, but it has deprecated for a long time. Is there anything preventing an update to a more recent NDK on the CI? |
|
The tests are hardcoded with the compiler versions shipped with NDK r15c. it can be updated with the latest versions of the NDK, but the CI machines will need to be reimaged and the rest of the tests updated. Without blocking you on that, the errors look similar across all of the failing tests: My guess is that your newly added targets are duplicated in the outer nested for loop. |
jin
reviewed
Apr 23, 2020
| // Create the Vulkan validation layers libraries | ||
| for (CToolchain toolchain : crosstool.getToolchainList()) { | ||
| vulkanValidationLayers.append( | ||
| vulkanValidationLayersTemplate.replace("%cpu%", toolchain.getTargetCpu())); |
There was a problem hiding this comment.
This is not a unique name, if toolchain.getTargetCpu() is common between the outer crosstools loop.
There was a problem hiding this comment.
Thanks for this, I've changed the rule name to contain the toolchain name, and it seems to have fixed the tests.
jin
approved these changes
Apr 23, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #10094