Skip to content

Commit

Permalink
Fix Release opt flags for legacy toolchain.
Browse files Browse the repository at this point in the history
Bug: android/ndk#1740
Test: expanded existing test to cover more configurations
Change-Id: If1986d549edc19bbf21c6b94d2c84af2b1c5dcd1
  • Loading branch information
DanAlbert committed Jul 28, 2022
1 parent 381cdfc commit 4ad4cb4
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 11 deletions.
1 change: 1 addition & 0 deletions build/cmake/android-legacy.toolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ list(APPEND ANDROID_LINKER_FLAGS -Wl,--gc-sections)
list(APPEND ANDROID_LINKER_FLAGS_EXE -Wl,--gc-sections)

# Debug and release flags.
list(APPEND ANDROID_COMPILER_FLAGS_RELEASE -O3)
list(APPEND ANDROID_COMPILER_FLAGS_RELEASE -DNDEBUG)
if(ANDROID_TOOLCHAIN STREQUAL clang)
list(APPEND ANDROID_COMPILER_FLAGS_DEBUG -fno-limit-debug-info)
Expand Down
11 changes: 11 additions & 0 deletions docs/changelogs/Changelog-r25.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,17 @@ directly, see the [build system maintainers guide].
[Android Studio site]: http://tools.android.com/filing-bugs
[build system maintainers guide]: https://android.googlesource.com/platform/ndk/+/master/docs/BuildSystemMaintainers.md


## r25b

* [Issue 1740]: Fixed the legacy toolchain when using CMake's `Release` build
configuration. Since r23b it has not be receiving any optimization flag. It
will now receive `-O3`. If you're building with AGP and haven't overridden
AGP's default CMake modes, this change does not affect you, as AGP uses
`RelWithDebInfo` by default.

[Issue 1740]: https://github.com/android/ndk/issues/1740

## Changes

* Includes Android 13 Beta 2 APIs.
Expand Down
43 changes: 32 additions & 11 deletions tests/build/cmake_default_flags/test.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,38 @@
from typing import Optional

from ndk.test.spec import BuildConfiguration
from ndk.testing.flag_verifier import FlagVerifier
from ndk.testing.flag_verifier import FlagVerifier, FlagVerifierResult


def run_test(ndk_path: str,
config: BuildConfiguration) -> tuple[bool, Optional[str]]:
"""Check that the CMake toolchain uses the correct default flags.
def check_configuration(
ndk_path: str,
build_config: BuildConfiguration,
cmake_config: str,
expected_flags: list[str],
unexpected_flags: list[str],
) -> FlagVerifierResult:
verifier = FlagVerifier(Path("project"), Path(ndk_path), build_config)
for flag in expected_flags:
verifier.expect_flag(flag)
for flag in unexpected_flags:
verifier.expect_not_flag(flag)
return verifier.verify_cmake([f"-DCMAKE_BUILD_TYPE={cmake_config}"])

Currently this only tests the optimization flags for RelWithDebInfo, but
it's probably worth expanding in the future.
"""
verifier = FlagVerifier(Path('project'), Path(ndk_path), config)
verifier.expect_flag('-O2')
return verifier.verify_cmake(['-DCMAKE_BUILD_TYPE=RelWithDebInfo'
]).make_test_result_tuple()

def run_test(ndk_path: str, config: BuildConfiguration) -> tuple[bool, Optional[str]]:
"""Check that the CMake toolchain uses the correct default flags."""
verify_configs: dict[str, tuple[list[str], list[str]]] = {
# No flag is the same as -O0. As long as no other opt flag is used, the default
# is fine.
"Debug": ([], ["-O1", "-O2", "-O3", "-Os", "-Oz"]),
"MinSizeRel": (["-Os"], ["-O0", "-O1", "-O2", "-O3", "-Oz"]),
"Release": (["-O3"], ["-O0", "-O1", "-O2", "-Os", "-Oz"]),
"RelWithDebInfo": (["-O2"], ["-O0", "-O1", "-O3", "-Os", "-Oz"]),
}
for cmake_config, (expected_flags, unexpected_flags) in verify_configs.items():
result = check_configuration(
ndk_path, config, cmake_config, expected_flags, unexpected_flags
)
if result.failed():
return result.make_test_result_tuple()
return result.make_test_result_tuple()

0 comments on commit 4ad4cb4

Please sign in to comment.