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

Native App Glue requires a C compiler #1906

Open
juan-lunarg opened this issue Jul 12, 2023 · 4 comments
Open

Native App Glue requires a C compiler #1906

juan-lunarg opened this issue Jul 12, 2023 · 4 comments

Comments

@juan-lunarg
Copy link

juan-lunarg commented Jul 12, 2023

Description

Our project only enables the CXX compiler by default.

project(VVL LANGUAGES CXX) # <- Are project is pure C++

However, for the Android build we have some awkward CMake code:

enable_language(C) # NOTE: We need to enable the C language for android_native_app_glue.c

set(native_app_glue_dir "${CMAKE_ANDROID_NDK}/sources/android/native_app_glue")

if (NOT EXISTS ${native_app_glue_dir})
    message(FATAL_ERROR "Couldn't find Android Native Glue directory!")
endif()

add_library(android_glue STATIC)

target_include_directories(android_glue PUBLIC ${native_app_glue_dir})
target_sources(android_glue PRIVATE
    ${native_app_glue_dir}/android_native_app_glue.c
    ${native_app_glue_dir}/android_native_app_glue.h
)

As you can see we need to enable a C compiler in order to use native app glue. This CMake isn't ideal and is confusing to look at unless you know what is going on. Ideally this process would be more streamlined.

Thoughts?

@juan-lunarg juan-lunarg changed the title [FR] Native App Glue requires CMake compiler [FR] Native App Glue requires a C compiler Jul 12, 2023
@juan-lunarg juan-lunarg changed the title [FR] Native App Glue requires a C compiler Native App Glue requires a C compiler Jul 12, 2023
@DanAlbert
Copy link
Member

What do you have in mind? This looks like the expected CMake behavior to me. You need to enable C support to build the C code.

@juan-lunarg
Copy link
Author

What do you have in mind?

Providing a CMakeLists.txt in the native_app_glue directory. It would look something like this:

project(ANDROID_NATIVE_APP_GLUE LANGUAGES C)

add_library(android_native_app_glue STATIC)
add_library(Android::NativeAppGlue ALIAS android_native_app_glue)

target_include_directories(android_native_app_glue PUBLIC .)
target_sources(android_native_app_glue PRIVATE
    android_native_app_glue.c
    android_native_app_glue.h
)

Then I could just call add_subdirectory and link against the static library.

add_subdirectory(${CMAKE_ANDROID_NDK}/sources/android/native_app_glue)
target_link_libraries(foobar PRIVATE Android::NativeAppGlue)

Thoughts?

@DanAlbert
Copy link
Member

Seems plausible. It might actually be more idiomatic to do something find_package() shaped. Either way, worth investigating. I can't say when we'll get to this though.

@juan-lunarg
Copy link
Author

In case others see this thread. I'd advise using an object library.

See this PR: https://github.com/KhronosGroup/Vulkan-ValidationLayers/pull/6343/files

add_library(android_glue OBJECT)
target_include_directories(android_glue PUBLIC ${native_app_glue_dir})
target_sources(android_glue PRIVATE
    ${native_app_glue_dir}/android_native_app_glue.c
    ${native_app_glue_dir}/android_native_app_glue.h
)

This fixed an issue with our symbol exporting logic allowing us to use a version-script.

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

No branches or pull requests

2 participants