Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] package/mpv: use meson instead of waf
@ 2024-12-21 17:19 Thomas Bonnefille via buildroot
  2024-12-22 15:18 ` Thomas Petazzoni via buildroot
  2024-12-26 15:39 ` Thomas Petazzoni via buildroot
  0 siblings, 2 replies; 7+ messages in thread
From: Thomas Bonnefille via buildroot @ 2024-12-21 17:19 UTC (permalink / raw)
  To: buildroot
  Cc: Thomas Petazzoni, Miquèl Raynal, Mahyar Koshkouei,
	Eric Le Bihan, Thomas Bonnefille

Waf has been entirely removed from mpv since commit f2cce5f [1] in favor
of meson.

This commit is a rework of mpv makefile to use meson.

Note that the meson package compilation itself supports the
static/dynamic compilation setting.

Fixes:
    https://autobuild.buildroot.org/results/68d42441fc0da34e1bf2a4247726f5f4ec3b8e77/

[1]: https://github.com/mpv-player/mpv/commit/f2cce5f38f4031bf1a4b4919ec90e4e8f8c66a77

Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
---
 package/mpv/mpv.mk | 122 +++++++++++++++++++++++++----------------------------
 1 file changed, 57 insertions(+), 65 deletions(-)

diff --git a/package/mpv/mpv.mk b/package/mpv/mpv.mk
index cf37091186a2ffa67db964cc3d1c9dd7dda05e6e..a91849036299bdbac2a2fcf4fd7e6b7455216999 100644
--- a/package/mpv/mpv.mk
+++ b/package/mpv/mpv.mk
@@ -14,197 +14,189 @@ MPV_LICENSE_FILES = LICENSE.GPL
 MPV_CPE_ID_VENDOR = mpv
 MPV_INSTALL_STAGING = YES
 
-MPV_NEEDS_EXTERNAL_WAF = YES
-
 # Some of these options need testing and/or tweaks
 MPV_CONF_OPTS = \
 	--prefix=/usr \
-	--disable-android \
-	--disable-caca \
-	--disable-cocoa \
-	--disable-coreaudio \
-	--disable-cuda-hwaccel \
-	--disable-opensles \
-	--disable-rubberband \
-	--disable-uchardet \
-	--disable-vapoursynth
+	-Dcaca=disabled \
+	-Dcocoa=disabled \
+	-Dcoreaudio=disabled \
+	-Dcuda-hwaccel=disabled \
+	-Dlibmpv=true \
+	-Dopensles=disabled \
+	-Drubberband=disabled \
+	-Duchardet=disabled \
+	-Dvapoursynth=disabled
 
 ifeq ($(BR2_REPRODUCIBLE),y)
-MPV_CONF_OPTS += --disable-build-date
-endif
-
-ifeq ($(BR2_STATIC_LIBS),y)
-MPV_CONF_OPTS += --disable-libmpv-shared --enable-libmpv-static
-else
-MPV_CONF_OPTS += --enable-libmpv-shared --disable-libmpv-static
+MPV_CONF_OPTS += -Dbuild-date=disabled
 endif
 
 ifeq ($(BR2_PACKAGE_ALSA_LIB),y)
-MPV_CONF_OPTS += --enable-alsa
+MPV_CONF_OPTS += -Dalsa=enabled
 MPV_DEPENDENCIES += alsa-lib
 else
-MPV_CONF_OPTS += --disable-alsa
+MPV_CONF_OPTS += -Dalsa=disabled
 endif
 
 ifeq ($(BR2_PACKAGE_MESA3D_GBM),y)
-MPV_CONF_OPTS += --enable-gbm
+MPV_CONF_OPTS += -Dgbm=enabled
 MPV_DEPENDENCIES += mesa3d
 ifeq ($(BR2_PACKAGE_LIBDRM),y)
-MPV_CONF_OPTS += --enable-egl-drm
+MPV_CONF_OPTS += -Degl-drm=enabled
 else
-MPV_CONF_OPTS += --disable-egl-drm
+MPV_CONF_OPTS += -Degl-drm=disabled
 endif
 else
-MPV_CONF_OPTS += --disable-gbm --disable-egl-drm
+MPV_CONF_OPTS += -Dgbm=disabled -Degl-drm=disabled
 endif
 
 # jack support
 # It also requires 64-bit sync intrinsics
 ifeq ($(BR2_TOOLCHAIN_HAS_SYNC_8)$(BR2_PACKAGE_JACK2),yy)
-MPV_CONF_OPTS += --enable-jack
+MPV_CONF_OPTS += -Djack=enabled
 MPV_DEPENDENCIES += jack2
 else
-MPV_CONF_OPTS += --disable-jack
+MPV_CONF_OPTS += -Djack=disabled
 endif
 
 # jpeg support
 ifeq ($(BR2_PACKAGE_JPEG),y)
-MPV_CONF_OPTS += --enable-jpeg
+MPV_CONF_OPTS += -Djpeg=enabled
 MPV_DEPENDENCIES += jpeg
 else
-MPV_CONF_OPTS += --disable-jpeg
+MPV_CONF_OPTS += -Djpeg=disabled
 endif
 
 # lcms2 support
 ifeq ($(BR2_PACKAGE_LCMS2),y)
-MPV_CONF_OPTS += --enable-lcms2
+MPV_CONF_OPTS += -Dlcms2=enabled
 MPV_DEPENDENCIES += lcms2
 else
-MPV_CONF_OPTS += --disable-lcms2
+MPV_CONF_OPTS += -Dlcms2=disabled
 endif
 
 # libarchive support
 ifeq ($(BR2_PACKAGE_LIBARCHIVE),y)
-MPV_CONF_OPTS += --enable-libarchive
+MPV_CONF_OPTS += -Dlibarchive=enabled
 MPV_DEPENDENCIES += libarchive
 else
-MPV_CONF_OPTS += --disable-libarchive
+MPV_CONF_OPTS += -Dlibarchive=disabled
 endif
 
 # bluray support
 ifeq ($(BR2_PACKAGE_LIBBLURAY),y)
-MPV_CONF_OPTS += --enable-libbluray
+MPV_CONF_OPTS += -Dlibbluray=enabled
 MPV_DEPENDENCIES += libbluray
 else
-MPV_CONF_OPTS += --disable-libbluray
+MPV_CONF_OPTS += -Dlibbluray=disabled
 endif
 
 # libcdio-paranoia
 ifeq ($(BR2_PACKAGE_LIBCDIO_PARANOIA),y)
-MPV_CONF_OPTS += --enable-cdda
+MPV_CONF_OPTS += -Dcdda=enabled
 MPV_DEPENDENCIES += libcdio-paranoia
 else
-MPV_CONF_OPTS += --disable-cdda
+MPV_CONF_OPTS += -Dcdda=disabled
 endif
 
 # libdvdnav
 ifeq ($(BR2_PACKAGE_LIBDVDNAV),y)
-MPV_CONF_OPTS += --enable-dvdnav
+MPV_CONF_OPTS += -Ddvdnav=enabled
 MPV_DEPENDENCIES += libdvdnav
 else
-MPV_CONF_OPTS += --disable-dvdnav
+MPV_CONF_OPTS += -Ddvdnav=disabled
 endif
 
 # libdrm
 ifeq ($(BR2_PACKAGE_LIBDRM),y)
-MPV_CONF_OPTS += --enable-drm
+MPV_CONF_OPTS += -Ddrm=enabled
 MPV_DEPENDENCIES += libdrm
 else
-MPV_CONF_OPTS += --disable-drm
+MPV_CONF_OPTS += -Ddrm=disabled
 endif
 
 # libvdpau
 ifeq ($(BR2_PACKAGE_LIBVDPAU),y)
-MPV_CONF_OPTS += --enable-vdpau
+MPV_CONF_OPTS += -Dvdpau=enabled
 MPV_DEPENDENCIES += libvdpau
 else
-MPV_CONF_OPTS += --disable-vdpau
+MPV_CONF_OPTS += -Dvdpau=disabled
 endif
 
 # LUA support, only for lua51/lua52/luajit
 # This enables the controller (OSD) together with libass
 ifeq ($(BR2_PACKAGE_LUA_5_1)$(BR2_PACKAGE_LUAJIT),y)
-MPV_CONF_OPTS += --enable-lua
+MPV_CONF_OPTS += -Dlua=enabled
 MPV_DEPENDENCIES += luainterpreter
 else
-MPV_CONF_OPTS += --disable-lua
+MPV_CONF_OPTS += -Dlua=disabled
 endif
 
 # OpenGL support
 ifeq ($(BR2_PACKAGE_HAS_LIBGL),y)
-MPV_CONF_OPTS += --enable-gl
+MPV_CONF_OPTS += -Dgl=enabled
 MPV_DEPENDENCIES += libgl
 else ifeq ($(BR2_PACKAGE_HAS_LIBGLES),y)
-MPV_CONF_OPTS += --enable-gl
+MPV_CONF_OPTS += -Dgl=enabled
 MPV_DEPENDENCIES += libgles
 else ifeq ($(BR2_PACKAGE_HAS_LIBEGL),y)
-MPV_CONF_OPTS += --enable-gl
+MPV_CONF_OPTS += -Dgl=enabled
 MPV_DEPENDENCIES += libegl
 else
-MPV_CONF_OPTS += --disable-gl
+MPV_CONF_OPTS += -Dgl=disabled
 endif
 
 # pulseaudio support
 ifeq ($(BR2_PACKAGE_PULSEAUDIO),y)
-MPV_CONF_OPTS += --enable-pulse
+MPV_CONF_OPTS += -Dpulse=enabled
 MPV_DEPENDENCIES += pulseaudio
 else
-MPV_CONF_OPTS += --disable-pulse
+MPV_CONF_OPTS += -Dpulse=disabled
 endif
 
 # SDL support
 # Sdl2 requires 64-bit sync intrinsics
 ifeq ($(BR2_TOOLCHAIN_HAS_SYNC_8)$(BR2_PACKAGE_SDL2),yy)
-MPV_CONF_OPTS += --enable-sdl2
+MPV_CONF_OPTS += -Dsdl2=enabled
 MPV_DEPENDENCIES += sdl2
 else
-MPV_CONF_OPTS += --disable-sdl2
+MPV_CONF_OPTS += -Dsdl2=disabled
 endif
 
 # Raspberry Pi support
 ifeq ($(BR2_PACKAGE_RPI_USERLAND),y)
-MPV_CONF_OPTS += --enable-rpi --enable-gl
+MPV_CONF_OPTS += -Drpi=enabled -Dgl=enabled
 MPV_DEPENDENCIES += rpi-userland
 else
-MPV_CONF_OPTS += --disable-rpi
+MPV_CONF_OPTS += -Drpi=disabled
 endif
 
 # va-api support
 ifeq ($(BR2_PACKAGE_LIBVA)$(BR2_PACKAGE_MPV_SUPPORTS_VAAPI),yy)
-MPV_CONF_OPTS += --enable-vaapi
+MPV_CONF_OPTS += -Dvaapi=enabled
 MPV_DEPENDENCIES += libva
 ifeq ($(BR2_PACKAGE_LIBDRM)$(BR2_PACKAGE_MESA3D_OPENGL_EGL),yy)
-MPV_CONF_OPTS += --enable-vaapi-drm
+MPV_CONF_OPTS += -Dvaapi-drm=enabled
 else
-MPV_CONF_OPTS += --disable-vaapi-drm
+MPV_CONF_OPTS += -Dvaapi-drm=disabled
 endif
 else
-MPV_CONF_OPTS += --disable-vaapi --disable-vaapi-drm
+MPV_CONF_OPTS += -Dvaapi=disabled -Dvaapi-drm=disabled
 endif
 
 # wayland support
 ifeq ($(BR2_PACKAGE_WAYLAND),y)
-MPV_CONF_OPTS += --enable-wayland
+MPV_CONF_OPTS += -Dwayland=enabled
 MPV_DEPENDENCIES += libxkbcommon wayland wayland-protocols
 else
-MPV_CONF_OPTS += --disable-wayland
+MPV_CONF_OPTS += -Dwayland=disabled
 endif
 
 # Base X11 support. Config.in ensures that if BR2_PACKAGE_XORG7 is
 # enabled, xlib_libX11, xlib_libXext, xlib_libXinerama,
 # xlib_libXrandr, xlib_libXScrnSaver.
 ifeq ($(BR2_PACKAGE_XORG7),y)
-MPV_CONF_OPTS += --enable-x11
+MPV_CONF_OPTS += -Dx11=enabled
 MPV_DEPENDENCIES += \
 	xlib_libX11 \
 	xlib_libXext \
@@ -214,17 +206,17 @@ MPV_DEPENDENCIES += \
 	xlib_libXScrnSaver
 # XVideo
 ifeq ($(BR2_PACKAGE_XLIB_LIBXV),y)
-MPV_CONF_OPTS += --enable-xv
+MPV_CONF_OPTS += -Dxv=enabled
 MPV_DEPENDENCIES += xlib_libXv
 else
-MPV_CONF_OPTS += --disable-xv
+MPV_CONF_OPTS += -Dxv=disabled
 endif
 else
-MPV_CONF_OPTS += --disable-x11
+MPV_CONF_OPTS += -Dx11=disabled
 endif
 
 ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
 MPV_CONF_ENV += LDFLAGS="$(TARGET_LDFLAGS) -latomic"
 endif
 
-$(eval $(waf-package))
+$(eval $(meson-package))

---
base-commit: 15dbc33be847c9b4d137e6311bb8ba68a51a3c27
change-id: 20241221-fix_mpv-a0a4fba5b044

Best regards,
-- 
Thomas Bonnefille <thomas.bonnefille@bootlin.com>

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH] package/mpv: use meson instead of waf
  2024-12-21 17:19 [Buildroot] [PATCH] package/mpv: use meson instead of waf Thomas Bonnefille via buildroot
@ 2024-12-22 15:18 ` Thomas Petazzoni via buildroot
  2024-12-22 18:25   ` Thomas Bonnefille
  2024-12-26 15:39 ` Thomas Petazzoni via buildroot
  1 sibling, 1 reply; 7+ messages in thread
From: Thomas Petazzoni via buildroot @ 2024-12-22 15:18 UTC (permalink / raw)
  To: Thomas Bonnefille via buildroot
  Cc: Thomas Bonnefille, Miquèl Raynal, Mahyar Koshkouei,
	Eric Le Bihan

Hello Thomas,

On Sat, 21 Dec 2024 18:19:21 +0100
Thomas Bonnefille via buildroot <buildroot@buildroot.org> wrote:

> Waf has been entirely removed from mpv since commit f2cce5f [1] in favor
> of meson.
> 
> This commit is a rework of mpv makefile to use meson.
> 
> Note that the meson package compilation itself supports the
> static/dynamic compilation setting.
> 
> Fixes:
>     https://autobuild.buildroot.org/results/68d42441fc0da34e1bf2a4247726f5f4ec3b8e77/
> 
> [1]: https://github.com/mpv-player/mpv/commit/f2cce5f38f4031bf1a4b4919ec90e4e8f8c66a77
> 
> Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>

Thanks for the fix. However, do you have an analysis of what the
problem is, and since when it started occurring?

Indeed, we want to know if we want to backport the fix to our stable
2024.02.x branch or not. Also, your fix is far from being "minimal":
such a conversion from waf-package to meson-package can potentially
introduce other regressions.

What was the issue with the waf build system?

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH] package/mpv: use meson instead of waf
  2024-12-22 15:18 ` Thomas Petazzoni via buildroot
@ 2024-12-22 18:25   ` Thomas Bonnefille
  2024-12-26 15:01     ` Thomas Petazzoni via buildroot
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Bonnefille @ 2024-12-22 18:25 UTC (permalink / raw)
  To: Thomas Petazzoni, Thomas Bonnefille via buildroot
  Cc: Miquèl Raynal, Mahyar Koshkouei, Eric Le Bihan

On Sun Dec 22, 2024 at 4:18 PM CET, Thomas Petazzoni wrote:
> Hello Thomas,
Hello Thomas,
>
> On Sat, 21 Dec 2024 18:19:21 +0100
> Thomas Bonnefille via buildroot <buildroot@buildroot.org> wrote:
>
> > Waf has been entirely removed from mpv since commit f2cce5f [1] in favor
> > of meson.
> > 
> > This commit is a rework of mpv makefile to use meson.
> > 
> > Note that the meson package compilation itself supports the
> > static/dynamic compilation setting.
> > 
> > Fixes:
> >     https://autobuild.buildroot.org/results/68d42441fc0da34e1bf2a4247726f5f4ec3b8e77/
> > 
> > [1]: https://github.com/mpv-player/mpv/commit/f2cce5f38f4031bf1a4b4919ec90e4e8f8c66a77
> > 
> > Signed-off-by: Thomas Bonnefille <thomas.bonnefille@bootlin.com>
>
> Thanks for the fix. However, do you have an analysis of what the
> problem is, and since when it started occurring?

I don't have an in depth analysis of the problem however, what I can say
is that the error appeared with commit
fd5623150dcc23441c4f0bd586378daeb91d66ea which bump waf from v2.0.26
(August 2023) to v2.1.1 (July 2024).
I would think that waf isn't backward compatible and as the mpv version
we're currently using is pretty old (November 2022), it broke with the
new Waf version.
>
> Indeed, we want to know if we want to backport the fix to our stable
> 2024.02.x branch or not. 

I think that, as far as the waf version get still stuck on
v2.0.26, the mpv package will not break (at least not this way).

> Also, your fix is far from being "minimal":
> such a conversion from waf-package to meson-package can potentially
> introduce other regressions.

Can you develop ? I'm not sure what kind of regression can be introduced
?
I don't know if it answers the question but the meson build system has
been added to mpv with "waf-compatibility" in mind and so all the
options were translated from waf to meson. [1]

>
> What was the issue with the waf build system?

I don't know why they get rid of waf but it was first deprecated and
then remove in favor of meson, so I thought this was a mandatory step to
operate this switch in Buildroot.

Thank you for your review,
Thomas

[1] : https://github.com/mpv-player/mpv/blob/release/0.35/DOCS/build-system-differences.md
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH] package/mpv: use meson instead of waf
  2024-12-22 18:25   ` Thomas Bonnefille
@ 2024-12-26 15:01     ` Thomas Petazzoni via buildroot
  2024-12-26 20:36       ` Thomas Bonnefille
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Petazzoni via buildroot @ 2024-12-26 15:01 UTC (permalink / raw)
  To: Thomas Bonnefille
  Cc: Thomas Bonnefille via buildroot, Miquèl Raynal,
	Mahyar Koshkouei, Eric Le Bihan

Hello Thomas,

On Sun, 22 Dec 2024 19:25:42 +0100
"Thomas Bonnefille" <thomas.bonnefille@bootlin.com> wrote:

> I don't have an in depth analysis of the problem however, what I can say
> is that the error appeared with commit
> fd5623150dcc23441c4f0bd586378daeb91d66ea which bump waf from v2.0.26
> (August 2023) to v2.1.1 (July 2024).

This is already a very good analysis: we know what broke the build, and
therefore since when it was broken.

> I would think that waf isn't backward compatible and as the mpv version
> we're currently using is pretty old (November 2022), it broke with the
> new Waf version.

Seems totally possible indeed.

And therefore, here is a more minimal fix:

diff --git a/package/mpv/mpv.hash b/package/mpv/mpv.hash
index a09015619d..44f2665a70 100644
--- a/package/mpv/mpv.hash
+++ b/package/mpv/mpv.hash
@@ -1,3 +1,4 @@
 # Locally calculated
 sha256  41df981b7b84e33a2ef4478aaf81d6f4f5c8b9cd2c0d337ac142fc20b387d1a9  mpv-0.35.1.tar.gz
+sha256  dcec3e179f9c33a66544f1b3d7d91f20f6373530510fa6a858cddb6bfdcde14b  waf-2.0.26
 sha256  8177f97513213526df2cf6184d8ff986c675afb514d4e68a404010521b880643  LICENSE.GPL
diff --git a/package/mpv/mpv.mk b/package/mpv/mpv.mk
index cf37091186..70283df274 100644
--- a/package/mpv/mpv.mk
+++ b/package/mpv/mpv.mk
@@ -6,6 +6,9 @@
 
 MPV_VERSION = 0.35.1
 MPV_SITE = $(call github,mpv-player,mpv,v$(MPV_VERSION))
+# The waf version from package/waf is too recent, so we download our
+# own
+MPV_EXTRA_DOWNLOADS = https://waf.io/waf-2.0.26
 MPV_DEPENDENCIES = \
 	host-pkgconf ffmpeg libass zlib \
 	$(if $(BR2_PACKAGE_LIBICONV),libiconv)
@@ -14,7 +17,10 @@ MPV_LICENSE_FILES = LICENSE.GPL
 MPV_CPE_ID_VENDOR = mpv
 MPV_INSTALL_STAGING = YES
 
-MPV_NEEDS_EXTERNAL_WAF = YES
+define MPV_INSTALL_WAF
+	$(INSTALL) -D -m 0755 $(MPV_DL_DIR)/waf-2.0.26 $(@D)/waf
+endef
+MPV_POST_PATCH_HOOKS += MPV_INSTALL_WAF
 
 # Some of these options need testing and/or tweaks
 MPV_CONF_OPTS = \

The idea is to have mpv download its own version of waf.

However, now that we understand that the issue was introduced by commit
fd5623150dcc23441c4f0bd586378daeb91d66ea, which got merged in 2024.08,
we know we don't need to backport the fix to our LTS branch. Therefore,
having a more "invasive" fix such as the one you proposed is acceptable.

> > Indeed, we want to know if we want to backport the fix to our stable
> > 2024.02.x branch or not.   
> 
> I think that, as far as the waf version get still stuck on
> v2.0.26, the mpv package will not break (at least not this way).

Right.

> Can you develop ? I'm not sure what kind of regression can be introduced
> ?
> I don't know if it answers the question but the meson build system has
> been added to mpv with "waf-compatibility" in mind and so all the
> options were translated from waf to meson. [1]

When you switch to one build system (waf) to another (meson), there is
a fairly high chance that the way dependencies are detected is going to
be slightly different, causing some subtle differences in behavior.
After all, all the dependency detection logic and all the build logic
is completely changed. We have regularly seen regressions when switch
from build system A to build system B. Of course, it doesn't mean it
shouldn't be done, but it means that considering such a switch as a fix
for our LTS branch is clearly not good.

But again, as the issue was introduced only in 2024.08, I think your
proposal to switch to meson-package is fine.

> I don't know why they get rid of waf but it was first deprecated and
> then remove in favor of meson, so I thought this was a mandatory step to
> operate this switch in Buildroot.

Oh, yes definitely the only solution moving forward is to switch to
meson. But as I've explained above: what we do moving forward (which
can be invasive) may need to be different than what we do more
conservatively in our LTS branch.

In addition to this migration to meson, what about bumping to a newer
version of mpv? Of course, in a separate commit.

Thanks!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH] package/mpv: use meson instead of waf
  2024-12-21 17:19 [Buildroot] [PATCH] package/mpv: use meson instead of waf Thomas Bonnefille via buildroot
  2024-12-22 15:18 ` Thomas Petazzoni via buildroot
@ 2024-12-26 15:39 ` Thomas Petazzoni via buildroot
  1 sibling, 0 replies; 7+ messages in thread
From: Thomas Petazzoni via buildroot @ 2024-12-26 15:39 UTC (permalink / raw)
  To: Thomas Bonnefille
  Cc: buildroot, Miquèl Raynal, Mahyar Koshkouei, Eric Le Bihan

Hello Thomas,

On Sat, 21 Dec 2024 18:19:21 +0100
Thomas Bonnefille <thomas.bonnefille@bootlin.com> wrote:

> Waf has been entirely removed from mpv since commit f2cce5f [1] in favor
> of meson.
> 
> This commit is a rework of mpv makefile to use meson.
> 
> Note that the meson package compilation itself supports the
> static/dynamic compilation setting.
> 
> Fixes:
>     https://autobuild.buildroot.org/results/68d42441fc0da34e1bf2a4247726f5f4ec3b8e77/
> 
> [1]: https://github.com/mpv-player/mpv/commit/f2cce5f38f4031bf1a4b4919ec90e4e8f8c66a77

This commit log needs to be extended with some details of since when
this starting happening, due to which commit (now that you have found
this information).

>  # Some of these options need testing and/or tweaks
>  MPV_CONF_OPTS = \
>  	--prefix=/usr \

--prefix /usr is already passed by meson-package, so keeping it here is
not needed.

>  ifeq ($(BR2_REPRODUCIBLE),y)
> -MPV_CONF_OPTS += --disable-build-date
> -endif
> -
> -ifeq ($(BR2_STATIC_LIBS),y)
> -MPV_CONF_OPTS += --disable-libmpv-shared --enable-libmpv-static
> -else
> -MPV_CONF_OPTS += --enable-libmpv-shared --disable-libmpv-static

Did you test a build of mpv with BR2_STATIC_LIBS=y after your change?

>  ifeq ($(BR2_TOOLCHAIN_HAS_LIBATOMIC),y)
>  MPV_CONF_ENV += LDFLAGS="$(TARGET_LDFLAGS) -latomic"
>  endif

Did you check that this still has the desired effect after the
conversion to meson?

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH] package/mpv: use meson instead of waf
  2024-12-26 15:01     ` Thomas Petazzoni via buildroot
@ 2024-12-26 20:36       ` Thomas Bonnefille
  2024-12-26 20:46         ` Thomas Petazzoni via buildroot
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Bonnefille @ 2024-12-26 20:36 UTC (permalink / raw)
  To: Thomas Petazzoni
  Cc: Thomas Bonnefille via buildroot, Miquèl Raynal,
	Mahyar Koshkouei, Eric Le Bihan

Hello Thomas,
>
> ...
>
> > Can you develop ? I'm not sure what kind of regression can be introduced
> > ?
> > I don't know if it answers the question but the meson build system has
> > been added to mpv with "waf-compatibility" in mind and so all the
> > options were translated from waf to meson. [1]
>
> When you switch to one build system (waf) to another (meson), there is
> a fairly high chance that the way dependencies are detected is going to
> be slightly different, causing some subtle differences in behavior.
> After all, all the dependency detection logic and all the build logic
> is completely changed. We have regularly seen regressions when switch
> from build system A to build system B. Of course, it doesn't mean it
> shouldn't be done, but it means that considering such a switch as a fix
> for our LTS branch is clearly not good.

Thank you for this explanation.

> In addition to this migration to meson, what about bumping to a newer
> version of mpv? Of course, in a separate commit.

Yes, it's totally possible but many things moved forward in mpv and a
bump is a little bit more complicated. 
I've already worked on this issue in my Buildroot's fork [1] and it
requires two more packages and therefore a significantly bigger patch
serie.

Thank you,
Thomas

[1] : https://github.com/Taumille/buildroot/tree/b4/bump_mpv
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Buildroot] [PATCH] package/mpv: use meson instead of waf
  2024-12-26 20:36       ` Thomas Bonnefille
@ 2024-12-26 20:46         ` Thomas Petazzoni via buildroot
  0 siblings, 0 replies; 7+ messages in thread
From: Thomas Petazzoni via buildroot @ 2024-12-26 20:46 UTC (permalink / raw)
  To: Thomas Bonnefille
  Cc: Thomas Bonnefille via buildroot, Miquèl Raynal,
	Mahyar Koshkouei, Eric Le Bihan

Hello,

On Thu, 26 Dec 2024 21:36:08 +0100
"Thomas Bonnefille" <thomas.bonnefille@bootlin.com> wrote:

> > When you switch to one build system (waf) to another (meson), there is
> > a fairly high chance that the way dependencies are detected is going to
> > be slightly different, causing some subtle differences in behavior.
> > After all, all the dependency detection logic and all the build logic
> > is completely changed. We have regularly seen regressions when switch
> > from build system A to build system B. Of course, it doesn't mean it
> > shouldn't be done, but it means that considering such a switch as a fix
> > for our LTS branch is clearly not good.  
> 
> Thank you for this explanation.

Could you resubmit a new iteration, with an improved commit log, and
taking into account the two other topics I raised in my review of the
patch today?

> Yes, it's totally possible but many things moved forward in mpv and a
> bump is a little bit more complicated. 
> I've already worked on this issue in my Buildroot's fork [1] and it
> requires two more packages and therefore a significantly bigger patch
> serie.

Glad to hear that you started working on this! No rush, you can
resubmit the bug fix separately from the version bump.

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-12-26 20:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-21 17:19 [Buildroot] [PATCH] package/mpv: use meson instead of waf Thomas Bonnefille via buildroot
2024-12-22 15:18 ` Thomas Petazzoni via buildroot
2024-12-22 18:25   ` Thomas Bonnefille
2024-12-26 15:01     ` Thomas Petazzoni via buildroot
2024-12-26 20:36       ` Thomas Bonnefille
2024-12-26 20:46         ` Thomas Petazzoni via buildroot
2024-12-26 15:39 ` Thomas Petazzoni via buildroot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox