apol requested changes to this revision.
apol added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> ECMAndroidDeployQt.cmake:41
> if (NOT VALUE STREQUAL "")
> - string(FIND "${VALUE}" ".so\"" OUT)
> - math(EXPR OUT "${OUT}+4")
> - string(SUBSTRING "${VALUE}" 0 ${OUT} OUTSTR)
> - file(WRITE ${CMAKE_BINARY_DIR}/stl "${OUTSTR}")
> + string (REGEX MATCH "\".+shared\.so\"" OUT ${VALUE})
> + file(WRITE ${CMAKE_BINARY_DIR}/stl "${OUT}")
Please split into a separate patch.
> ECMAndroidDeployQt.cmake:41
> if (NOT VALUE STREQUAL "")
> - string(FIND "${VALUE}" ".so\"" OUT)
> - math(EXPR OUT "${OUT}+4")
> - string(SUBSTRING "${VALUE}" 0 ${OUT} OUTSTR)
> - file(WRITE ${CMAKE_BINARY_DIR}/stl "${OUTSTR}")
> + string (REGEX MATCH "\".+shared\.so\"" OUT ${VALUE})
> + file(WRITE ${CMAKE_BINARY_DIR}/stl "${OUT}")
what if it's using an stl version that isn't called `.+shared.so`?
> specifydependencies.cmake:24
>
> +function(duplicate libpath)
> + get_filename_component (name ${libpath} NAME)
I'd call the function `contains_library`, as is it looks that it's duplicating
something.
Also pass the output value as an argument instead of magically declaring
IS_EQUAL.
> specifydependencies.cmake:40
> + message (STATUS "found duplicate, skipping: " ${extralib})
> + set (IS_EQUAL FALSE)
> + else()
It shouldn't be necessary to set it back again.
REPOSITORY
R240 Extra CMake Modules
REVISION DETAIL
https://phabricator.kde.org/D20509
To: sh-zam, apol, vkrause
Cc: kde-buildsystem, kde-frameworks-devel, bencreasy, michaelh, ngraham, bruns
