Re: [Buildroot] [PATCH] package/mpv: use meson instead of waf

[Thomas Petazzoni via buildroot <buildroot@buildroot.org>]

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