[RFC,PATCHv2] webp: switch recipe to cmake buildsystem

Besides the improvements that cmake offers compared to autotools,
the advantage would be that cmake config files get created.
This is useful so that other projects can find webp using cmake.

Remove EXTRA_OECONF settings because all of them are set like this
by default.

CMakeLists.txt doesn't provide an option for selecting neon but from
what I have tested it gets selected properly.

Add a patch that hides more dependencies behind options to avoid
floating dependencies.

Signed-off-by: Markus Volk <f_l_k@t-online.de>
---
 .../webp/libwebp/0001-cmake-add-options.patch | 120 ++++++++++++++++++
 meta/recipes-multimedia/webp/libwebp_1.3.2.bb |  40 ++----
 2 files changed, 133 insertions(+), 27 deletions(-)
 create mode 100644 meta/recipes-multimedia/webp/libwebp/0001-cmake-add-options.patch

Why is the patch marked inappropriate?

Please send upstream first, and only after upstream has expressed their
approval/disaporoval/request for changes we can take some form of it in
oe-core. Maybe they would simply want to turn these into hard dependencies
instead of adding even more options.

Until then, NAK.

Alex

On Tue 9. Jan 2024 at 18.10, Markus Volk <f_l_k@t-online.de> wrote:

> Besides the improvements that cmake offers compared to autotools,
> the advantage would be that cmake config files get created.
> This is useful so that other projects can find webp using cmake.
>
> Remove EXTRA_OECONF settings because all of them are set like this
> by default.
>
> CMakeLists.txt doesn't provide an option for selecting neon but from
> what I have tested it gets selected properly.
>
> Add a patch that hides more dependencies behind options to avoid
> floating dependencies.
>
> Signed-off-by: Markus Volk <f_l_k@t-online.de>
> ---
>  .../webp/libwebp/0001-cmake-add-options.patch | 120 ++++++++++++++++++
>  meta/recipes-multimedia/webp/libwebp_1.3.2.bb |  40 ++----
>  2 files changed, 133 insertions(+), 27 deletions(-)
>  create mode 100644
> meta/recipes-multimedia/webp/libwebp/0001-cmake-add-options.patch
>
> diff --git
> a/meta/recipes-multimedia/webp/libwebp/0001-cmake-add-options.patch
> b/meta/recipes-multimedia/webp/libwebp/0001-cmake-add-options.patch
> new file mode 100644
> index 0000000000..0a3e781097
> --- /dev/null
> +++ b/meta/recipes-multimedia/webp/libwebp/0001-cmake-add-options.patch
> @@ -0,0 +1,120 @@
> +From d126cdbe38cda7a4ab512738b2755942da33e43c Mon Sep 17 00:00:00 2001
> +From: Markus Volk <f_l_k@t-online.de>
> +Date: Tue, 9 Jan 2024 16:31:08 +0100
> +Subject: [PATCH] cmake: add more options
> +
> +By doing this gif,jpeg,png,tiff,opengl and sdl can be disabled to avoid
> floating dependencies.
> +
> +Upstream-Status: Inappropriate
> +
> +Signed-off-by: Markus Volk <f_l_k@t-online.de>
> +---
> + CMakeLists.txt   | 28 ++++++++++++++++++----------
> + cmake/deps.cmake | 24 +++++++++++++++---------
> + 2 files changed, 33 insertions(+), 19 deletions(-)
> +
> +diff --git a/CMakeLists.txt b/CMakeLists.txt
> +index ad5e14c3..c3b24f99 100644
> +--- a/CMakeLists.txt
> ++++ b/CMakeLists.txt
> +@@ -35,6 +35,12 @@ endif()
> + option(WEBP_ENABLE_SIMD "Enable any SIMD optimization."
> +        ${WEBP_ENABLE_SIMD_DEFAULT})
> + option(WEBP_BUILD_ANIM_UTILS "Build animation utilities." ON)
> ++option(WEBP_BUILD_GIF "Build gif support." OFF)
> ++option(WEBP_BUILD_TIFF "Build tiff support." OFF)
> ++option(WEBP_BUILD_PNG "Build png support." OFF)
> ++option(WEBP_BUILD_JPEG "Build jpeg support." OFF)
> ++option(WEBP_BUILD_OPENGL "Build opengl support." OFF)
> ++option(WEBP_BUILD_SDL "Build sdl support." OFF)
> + option(WEBP_BUILD_CWEBP "Build the cwebp command line tool." ON)
> + option(WEBP_BUILD_DWEBP "Build the dwebp command line tool." ON)
> + option(WEBP_BUILD_GIF2WEBP "Build the gif2webp conversion tool." ON)
> +@@ -562,7 +568,7 @@ if(WEBP_BUILD_IMG2WEBP)
> +   install(TARGETS img2webp RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR})
> + endif()
> +
> +-if(WEBP_BUILD_VWEBP)
> ++if(WEBP_BUILD_VWEBP AND WEBP_BUILD_OPENGL)
> +   # vwebp
> +   find_package(GLUT)
> +   if(GLUT_FOUND)
> +@@ -638,15 +644,17 @@ if(WEBP_BUILD_EXTRAS)
> +
>  ${CMAKE_CURRENT_BINARY_DIR})
> +
> +   # vwebp_sdl
> +-  find_package(SDL)
> +-  if(WEBP_BUILD_VWEBP AND SDL_FOUND)
> +-    add_executable(vwebp_sdl ${VWEBP_SDL_SRCS})
> +-    target_link_libraries(vwebp_sdl ${SDL_LIBRARY} imageioutil webp)
> +-    target_include_directories(
> +-      vwebp_sdl PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}
> ${CMAKE_CURRENT_BINARY_DIR}
> +-                        ${CMAKE_CURRENT_BINARY_DIR}/src
> ${SDL_INCLUDE_DIR})
> +-    set(WEBP_HAVE_SDL 1)
> +-    target_compile_definitions(vwebp_sdl PUBLIC WEBP_HAVE_SDL)
> ++  if(WEBP_BUILD_SDL)
> ++    find_package(SDL)
> ++    if(WEBP_BUILD_VWEBP AND SDL_FOUND)
> ++      add_executable(vwebp_sdl ${VWEBP_SDL_SRCS})
> ++      target_link_libraries(vwebp_sdl ${SDL_LIBRARY} imageioutil webp)
> ++      target_include_directories(
> ++        vwebp_sdl PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}
> ${CMAKE_CURRENT_BINARY_DIR}
> ++                          ${CMAKE_CURRENT_BINARY_DIR}/src
> ${SDL_INCLUDE_DIR})
> ++      set(WEBP_HAVE_SDL 1)
> ++      target_compile_definitions(vwebp_sdl PUBLIC WEBP_HAVE_SDL)
> ++    endif()
> +   endif()
> + endif()
> +
> +diff --git a/cmake/deps.cmake b/cmake/deps.cmake
> +index 0760ba92..cc978256 100644
> +--- a/cmake/deps.cmake
> ++++ b/cmake/deps.cmake
> +@@ -52,8 +52,10 @@ endif()
> + set(LT_OBJDIR ".libs/")
> +
> + # Only useful for vwebp, so useless for now.
> +-find_package(OpenGL)
> +-set(WEBP_HAVE_GL ${OPENGL_FOUND})
> ++if(WEBP_BUILD_OPENGL)
> ++  find_package(OpenGL)
> ++  set(WEBP_HAVE_GL ${OPENGL_FOUND})
> ++endif()
> +
> + # Check if we need to link to the C math library. We do not look for it
> as it is
> + # not found when cross-compiling, while it is here.
> +@@ -81,8 +83,10 @@ if(WEBP_FIND_IMG_LIBS)
> +       message(STATUS "TIFF is disabled when statically linking.")
> +       continue()
> +     endif()
> +-    find_package(${I_LIB})
> +-    set(WEBP_HAVE_${I_LIB} ${${I_LIB}_FOUND})
> ++    if(WEBP_BUILD_${I_LIB})
> ++      find_package(${I_LIB})
> ++      set(WEBP_HAVE_${I_LIB} ${${I_LIB}_FOUND})
> ++    endif()
> +     if(${I_LIB}_FOUND)
> +       list(APPEND WEBP_DEP_IMG_LIBRARIES ${${I_LIB}_LIBRARIES})
> +       list(APPEND WEBP_DEP_IMG_INCLUDE_DIRS ${${I_LIB}_INCLUDE_DIR}
> +@@ -94,11 +98,13 @@ if(WEBP_FIND_IMG_LIBS)
> +   endif()
> +
> +   # GIF detection, gifdec isn't part of the imageio lib.
> +-  include(CMakePushCheckState)
> +-  set(WEBP_DEP_GIF_LIBRARIES)
> +-  set(WEBP_DEP_GIF_INCLUDE_DIRS)
> +-  find_package(GIF)
> +-  set(WEBP_HAVE_GIF ${GIF_FOUND})
> ++  if(WEBP_BUILD_GIF)
> ++    include(CMakePushCheckState)
> ++    set(WEBP_DEP_GIF_LIBRARIES)
> ++    set(WEBP_DEP_GIF_INCLUDE_DIRS)
> ++    find_package(GIF)
> ++    set(WEBP_HAVE_GIF ${GIF_FOUND})
> ++  endif()
> +   if(GIF_FOUND)
> +     # GIF find_package only locates the header and library, it doesn't
> fail
> +     # compile tests when detecting the version, but falls back to 3 (as
> of at
> +--
> +2.43.0
> +
> diff --git a/meta/recipes-multimedia/webp/libwebp_1.3.2.bb
> b/meta/recipes-multimedia/webp/libwebp_1.3.2.bb
> index 63b0fd9a6c..13fe337660 100644
> --- a/meta/recipes-multimedia/webp/libwebp_1.3.2.bb
> +++ b/meta/recipes-multimedia/webp/libwebp_1.3.2.bb
> @@ -13,45 +13,31 @@ LICENSE = "BSD-3-Clause"
>  LIC_FILES_CHKSUM = "file://COPYING;md5=6e8dee932c26f2dab503abf70c96d8bb \
>                      file://PATENTS;md5=c6926d0cb07d296f886ab6e0cc5a85b7"
>
> -SRC_URI = "http://downloads.webmproject.org/releases/webp/${BP}.tar.gz"
> +SRC_URI = " \
> +       http://downloads.webmproject.org/releases/webp/${BP}.tar.gz \
> +       file://0001-cmake-add-options.patch \
> +"
>  SRC_URI[sha256sum] =
> "2a499607df669e40258e53d0ade8035ba4ec0175244869d1025d460562aa09b4"
>
>  UPSTREAM_CHECK_URI = "
> http://downloads.webmproject.org/releases/webp/index.html"
>
> -EXTRA_OECONF = " \
> -    --disable-wic \
> -    --enable-libwebpmux \
> -    --enable-libwebpdemux \
> -    --enable-threading \
> -"
> -
> -# Do not trust configure to determine if neon is available.
> -#
> -EXTRA_OECONF_ARM = " \
> -
> ${@bb.utils.contains("TUNE_FEATURES","neon","--enable-neon","--disable-neon",d)}
> \
> -"
> -EXTRA_OECONF:append:arm = " ${EXTRA_OECONF_ARM}"
> -EXTRA_OECONF:append:armeb = " ${EXTRA_OECONF_ARM}"
> +EXTRA_OECMAKE = "-DBUILD_SHARED_LIBS=ON"
>
> -inherit autotools lib_package
> -
> -PACKAGECONFIG ??= ""
> -
> -# libwebpdecoder is a subset of libwebp, don't build it unless requested
> -PACKAGECONFIG[decoder] =
> "--enable-libwebpdecoder,--disable-libwebpdecoder"
> +inherit cmake lib_package
>
> +PACKAGECONFIG ?= ""
>  # Apply for examples programs: cwebp and dwebp
> -PACKAGECONFIG[gif] = "--enable-gif,--disable-gif,giflib"
> -PACKAGECONFIG[jpeg] = "--enable-jpeg,--disable-jpeg,jpeg"
> -PACKAGECONFIG[png] = "--enable-png,--disable-png,,libpng"
> -PACKAGECONFIG[tiff] = "--enable-tiff,--disable-tiff,tiff"
> -
> +PACKAGECONFIG[gif] = "-DWEBP_BUILD_GIF=ON,-DWEBP_BUILD_GIF=OFF,giflib"
> +PACKAGECONFIG[jpeg] = "-DWEBP_BUILD_JPEG=ON,-DWEBP_BUILD_JPEG=OFF,jpeg"
> +PACKAGECONFIG[png] = "-DWEBP_BUILD_PNG=ON,-DWEBP_BUILD_PNG=OFF,libpng"
> +PACKAGECONFIG[tiff] = "-DWEBP_BUILD_TIFF=ON,-DWEBP_BUILD_TIFF=OFF,tiff"
>  # Apply only for example program vwebp
> -PACKAGECONFIG[gl] = "--enable-gl,--disable-gl,mesa-glut"
> +PACKAGECONFIG[opengl] =
> "-DWEBP_BUILD_OPENGL=ON,-DWEBP_BUILD_OPENGL=OFF,mesa-glut"
>
>  PACKAGES =+ "${PN}-gif2webp"
>
>  DESCRIPTION:${PN}-gif2webp = "Simple tool to convert animated GIFs to
> WebP"
>  FILES:${PN}-gif2webp = "${bindir}/gif2webp"
>
> +FILES:${PN} += "${datadir}"
>  BBCLASSEXTEND += "native nativesdk"
> --
> 2.43.0
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#193471):
> https://lists.openembedded.org/g/openembedded-core/message/193471
> Mute This Topic: https://lists.openembedded.org/mt/103623686/1686489
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [
> alex.kanavin@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>

On Fri, 12 Jan 2024 at 09:56, Markus Volk <mrksvolk@googlemail.com> wrote:
> > Why is the patch marked inappropriate?
> >
> > Please send upstream first, and only after upstream has expressed
> > their approval/disaporoval/request for changes we can take some form
> > of it in oe-core. Maybe they would simply want to turn these into hard
> > dependencies instead of adding even more options.
> I guess upstream simply wants to leave everything as it is, because the
> disadvantage of options would definitely be that everybody would have to
> adapt their build systems. And that for a problem that is rather
> yocto-specific. I'm not sure I wanted to suggest such a change upstream.
> Aside from the fact that I can't send suggestions to upstream at google
> anyway, because I'm not willing to accept their requirement for a google
> account with a gmail address.

In that case, please abstain from making patches that you are unable
to send upstream. It's a basic expectation in oe-core. Also please
don't make assumptions about what upstream wants; floating
dependencies is a real problem that they might be interested in
solving, and if not, we would still have a reference to their
position. It comes across as you trying to find reasons to avoid
interacting with them.

For this specific recipe, you can still rewrite it to use cmake,
without adding patches, if you follow my suggestion elsewhere (drop
the existing packageconfigs, and make a new set that strictly follows
the options that upstream offers right now, pulling in all the
optional libraries for each of them). It doesn't solve the floating
dependencies problem completely (it could silently regress to that on
version updates), but it would avoid it at this moment.

Alex

On Fri, 12 Jan 2024 at 11:26, Alexander Kanavin via
lists.openembedded.org <alex.kanavin=gmail.com@lists.openembedded.org>
wrote:
> > Aside from the fact that I can't send suggestions to upstream at google
> > anyway, because I'm not willing to accept their requirement for a google
> > account with a gmail address.

Also: this is not correct. Google account can be created with a
non-google email address (there is that option when creating it), and
if so it's not any different from a github or whatever account.

Alex