Improve Android build (Clang + tidyness) #6958
Merged
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
RandomShaper
force-pushed
the
android-clang-tidy
branch
from
941022c to
e6f1aaf
Compare
RandomShaper
force-pushed
the
android-clang-tidy
branch
2 times, most recently
from
0214061 to
3aa624d
Compare
|
Fails on travis: |
|
Yes, on Darwin I'm having an issue with the --sysroot linker option. I'll
keep pushing until I get it to work since I haven't a Mac build environment.
On Windows it works flawlessly for me.
If anbody could test on Linux and report, it would be appreciated.
|
RandomShaper
force-pushed
the
android-clang-tidy
branch
11 times, most recently
from
11cb26e to
bd37dbf
Compare
|
It fails building for all platforms on travis, but I'm not sure why. |
|
Ah now I know why, you're forcing the build to stop unless it's the android one :) |
RandomShaper
force-pushed
the
android-clang-tidy
branch
2 times, most recently
from
c4b3074 to
0b2eac1
Compare
|
Now, unless STL support is enabled, RTTI is also disabled as it looks safe Godot-wide. I only have needed to make regex aware. I've setup a local Mac dev environment and there the build has been correct. Let's hope Travis thinks the same… |
RandomShaper
force-pushed
the
android-clang-tidy
branch
2 times, most recently
from
2ff4a20 to
12e37e0
Compare
|
Rebased. |
leezh
reviewed
Nov 1, 2016
| @@ -1312,7 +1332,11 @@ Error RegEx::compile(const String& p_pattern) { | |||
|
|
|||
| if (min_val != max_val) | |||
| for (int i = 0; i < stack.size(); ++i) | |||
| #ifdef NO_SAFE_CAST | |||
| if (stack[i]->is_look_behind()) | |||
There was a problem hiding this comment.
I think from a maintenance standpoint it'd be better to use is_look_behind() on all builds, taking away the #ifdef and the dynamic cast.
There was a problem hiding this comment.
Pushed with preprocessor talk dropped.
RandomShaper
force-pushed
the
android-clang-tidy
branch
from
12e37e0 to
66dd0c6
Compare
|
I'm getting this error on Linux: |
|
Could you add |
|
|
Thanks! Hmm… I wonder why such a weird path to the assembler ("/..//bin/as"). Sorry to bother you, but could you put this at the end?: If that doesn't shed any light I will suggest you upgrading your NDK to the latest release. |
|
Here's the dump: http://hastebin.com/umufuleruq.go I'll try a newer NDK yeah. |
|
Same issue with latest NDK :/ |
RandomShaper
force-pushed
the
android-clang-tidy
branch
3 times, most recently
from
9b02232 to
19ab970
Compare
|
I've pinpointed a mistake: passing I'm also trying to enable link-time optimization. (If I weren't able to make this work soon I'd leave it for another PR.) I've pused both changes. |
|
It seems to work much better for me :) |
RandomShaper
force-pushed
the
android-clang-tidy
branch
2 times, most recently
from
4312d0b to
e400a0f
Compare
|
Well, I've finally dropped my attempt at LTO and pushed again. |
Closed
RandomShaper
force-pushed
the
android-clang-tidy
branch
from
e400a0f to
8baf5c1
Compare
RandomShaper
force-pushed
the
android-clang-tidy
branch
from
8baf5c1 to
b18ff94
Compare
akien-mga
added a commit
to akien-mga/godot
that referenced
this pull request
May 19, 2021
We found that this flag causes this error on PR godotengine#48812 which does not add any fancy inline assembly: ``` /tmp/tile_set-ce236a.s: Assembler messages: /tmp/tile_set-ce236a.s:34676: Error: selected processor does not support `bfc x0,godotengine#32,godotengine#32' clang++: error: assembler command failed with exit code 1 (use -v to see invocation) ``` That flag is mentioned in various errors related to assembler failures on arm64v8 with Clang from the Android NDK. It was added in Godot in godotengine#6958 when migrating from GCC to Clang, and is indeed referenced in the NDK's Clang migration guide: https://android.googlesource.com/platform/ndk/+/master/docs/ClangMigration.md > Especially for ARM and ARM64, Clang is much stricter about assembler rules > than GCC/GAS. Use `-fno-integrated-as` if Clang reports errors in inline > assembly or assembly files that you don't wish to modernize. We don't get those errors nowadays so it seems the flag is no longer needed.
akien-mga
added a commit
that referenced
this pull request
May 19, 2021
We found that this flag causes this error on PR #48812 which does not add any fancy inline assembly: ``` /tmp/tile_set-ce236a.s: Assembler messages: /tmp/tile_set-ce236a.s:34676: Error: selected processor does not support `bfc x0,#32,#32' clang++: error: assembler command failed with exit code 1 (use -v to see invocation) ``` That flag is mentioned in various errors related to assembler failures on arm64v8 with Clang from the Android NDK. It was added in Godot in #6958 when migrating from GCC to Clang, and is indeed referenced in the NDK's Clang migration guide: https://android.googlesource.com/platform/ndk/+/master/docs/ClangMigration.md > Especially for ARM and ARM64, Clang is much stricter about assembler rules > than GCC/GAS. Use `-fno-integrated-as` if Clang reports errors in inline > assembly or assembly files that you don't wish to modernize. We don't get those errors nowadays so it seems the flag is no longer needed. (cherry picked from commit 23f7c75)
akien-mga
added a commit
that referenced
this pull request
May 19, 2021
We found that this flag causes this error on PR #48812 which does not add any fancy inline assembly: ``` /tmp/tile_set-ce236a.s: Assembler messages: /tmp/tile_set-ce236a.s:34676: Error: selected processor does not support `bfc x0,#32,#32' clang++: error: assembler command failed with exit code 1 (use -v to see invocation) ``` That flag is mentioned in various errors related to assembler failures on arm64v8 with Clang from the Android NDK. It was added in Godot in #6958 when migrating from GCC to Clang, and is indeed referenced in the NDK's Clang migration guide: https://android.googlesource.com/platform/ndk/+/master/docs/ClangMigration.md > Especially for ARM and ARM64, Clang is much stricter about assembler rules > than GCC/GAS. Use `-fno-integrated-as` if Clang reports errors in inline > assembly or assembly files that you don't wish to modernize. We don't get those errors nowadays so it seems the flag is no longer needed. (cherry picked from commit 23f7c75)
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.
This is something I wanted to do for a long time.
This must be heavily tested (different combinations of target architectures and build variants, etc.).
Not possible on Windows until #6844 is solved. For testing on Windows, merge #6959 first.### WARNING: This is not ready to be tested/merged until I fix the Mac build since I will be pushing test commits until I get it to work. I'll remove this notice as soon as that's done.NDK_TARGET/NDK_TARGET_X86since only one Clang version is included, thus no choice possible. Regarding binutils and gnu-libstdc++ the NDK also includes only one version (4.9), so no choice there either.android_stl=yes.Fix cases of bad indentation.(Superseeded by the new PEP8 effort).