use more correct manual pkg-config command for libass

[kevmitch]

Actually ask for what static libass needs instead of guessing. Maybe
extra things get included since this assumes recursive dependencies are
also static, but I can't see how this would be a problem in practice. It
just explicitly includes some shared object dependencies that were
previously implicit. The resulting binary is the same size and has the
same shared object dependencies (although in slightly different order)
according to ldd.

[TheOneric]

[The resulting binary] has the same shared object dependencies (although in slightly different order) according to ldd

All libs changing order are additional ones added by --static --libs's recursive resolving of private Libs/Requires, so it appears (by chance?) all of them were already linked to before. However, it seems odd for mpv+libass+ffmpeg to link to eg libgraphite2 (lib to render Graphite fonts, optional dep of Harfbuzz). Any idea why? ~~In fact ldd --unused tells me those 7 dependencies aren't (directly) used by the resulting mpv+libass+ffmpeg binary.~~ (They only show up as unused if --static --libs was used. No idea what's going on here.)

As for how overlinking can be bad in general, see eg the following link for a short explanation and example of real problem(s) caused by this (If that's what “I can't see how this would be a problem in practice” was referring to.): https://lists.debian.org/debian-devel-announce/2005/11/msg00016.html For personal use only this is less problematic; as I don't exactly know what usecase mpv-build is targeting I can't say if that's a concern here. (-Wl,--as-needed is much more reliable now than when the mail linked above was written and will also be the default in Debian Bullseye onwards and already was default in Ubuntu for a while; setting this might be one possible lazy approach to avoid overlinking)

Unrelated: I noticed the build dependencies in debian/control list yasm which is no longer supported by libass (and FFmpeg?). This means you won't get ASM-speedup in a clean build based on this. Instead nasm should be utilised.

[TheOneric]

I made an oupsie in previous post by using ldd to check what's linked in: ldd also shows transitive dependecies (if they can be resolved), which explains the ""weirdness"". Checking the direct links properly with readelf -d mpv/build/mpv | grep NEEDED instead reveals that libgraphite2 is never linked in, even with the current form of this pr which uses:

$ pkg-config --static --libs libass
-lass -lm -lfontconfig -luuid -lexpat -lharfbuzz -lm -lglib-2.0 -pthread -lpcre -pthread -lgraphite2 -lfribidi -lfreetype -lpng16 -lm -lz -lm -lz -lbrotlidec -lbrotlicommon

I'm assuming something in the build ~~(perhaps waf itself)~~ already sets (-Wl,)--as-needed, so everything should be fine and more future proof with this patch. Although if the setups requires a compiler with gcc-argument-sytax anyway, it might be good to use the following instead for clarity:

diff --git a/scripts/mpv-config b/scripts/mpv-config
index 7acd085..ea97ed4 100755
--- a/scripts/mpv-config
+++ b/scripts/mpv-config
@@ -20,7 +20,7 @@ esac
 
 # add missing private dependencies from libass.pc
 # this is necessary due to the hybrid static / dynamic nature of the build
-export LDFLAGS="$LDFLAGS $(pkg-config --libs fontconfig harfbuzz fribidi)"
+export LDFLAGS="$LDFLAGS -Wl,--as-needed $(pkg-config --static --libs libass)"
 
 echo Using mpv options: $OPTIONS

[TheOneric]

That something being the compiler defaults itself as a test . It didn't do this when I first tested this iirc, but I upgraded my system since (I even wrote that it will become the default in Debian Bullseye in a previous post .. oh). In that case I would definitely recommend to explicitly set -Wl,--as-needed.

[avih]

For what it's worth, when building with mingw (tested: MXE on ubuntu), the global LDFLAGS makes mpv's configure considerably slower (every configure test links with these too).

In my mpv-build fork I wrote a statify-pc.sh which does what mpv-build needs - it modifies the .pc file so that it exposes the static (private) flags even when pkg-config was invoked without --static.

We need this (or the hack in this PR) because mpv doesn't support a per-package static. So either static build is enabled and then all the libs which are not available as static are not detected (like many system packages), or it's linked dynamically (the default) and the mpv-build libs .pc files are not invoked with --static.

So an alternative to this PR would be to add to this repository: https://github.com/avih/mpv-build/blob/mpv-build-rewrite/scripts/statify-pc.sh

And then add this line at the end of scripts/libass-build:

scripts/statify-pc.sh "$BUILD/build_libs/lib/pkgconfig/libass.pc"

[kevmitch]

whatever this was wasn't actually merged. I think I was adding --static to the pkg-config explicit command in scripts/mpv-config, but that idea (and the commits) were abandoned. This is only marked as merged because it was based out of my master branch which has been subsequently synced to mpv-player/mpv-build