mpv-android / mpv-android

#mpv-android @ libera.chat
MIT License
1.81k stars 226 forks source link

Incorrect pkg-config path for Apple Silicon chips #907

Closed mukulrwt closed 2 weeks ago

mukulrwt commented 2 weeks ago

Important information

mpv-android version:

Which version of mpv-android introduced the problem (if known):

Description

The pkg-congig path being set in environment is incorrect for Apple Silicon mac.

This path should be different https://github.com/mpv-android/mpv-android/blob/master/buildscripts/include/path.sh#L24

it should be

# configure pkg-config paths if inside buildscripts
if [ -n "$ndk_triple" ]; then
    export PKG_CONFIG_SYSROOT_DIR="$prefix_dir"
    export PKG_CONFIG_LIBDIR="$PKG_CONFIG_SYSROOT_DIR/opt/homebrew/lib/pkgconfig" # path for homebrew
    unset PKG_CONFIG_PATH
fi

instead of

# configure pkg-config paths if inside buildscripts
if [ -n "$ndk_triple" ]; then
    export PKG_CONFIG_SYSROOT_DIR="$prefix_dir"
    export PKG_CONFIG_LIBDIR="$PKG_CONFIG_SYSROOT_DIR/lib/pkgconfig"
    unset PKG_CONFIG_PATH
fi

As per Homebrew documentation https://docs.brew.sh/Installation

The script installs Homebrew to its default, supported, best prefix (/opt/homebrew for Apple Silicon, /usr/local for macOS Intel

Log output

ERROR: dav1d >= 0.5.0 not found using pkg-config
sfan5 commented 2 weeks ago

$prefix_dir contains the sysroot of libraries/software built for the target Android, how your host system organizes stuff should be entirely irrelevant to that.

mukulrwt commented 2 weeks ago

@sfan5 I am completely new to this so I don't understand many things here. But this is what I get from this. I could be incorrect in setting up some things, but I followed the README for build steps and this where I am.

When running build, I get

ERROR: dav1d >= 0.5.0 not found using pkg-config

when PKG_CONFIG_LIBDIR="$PKG_CONFIG_SYSROOT_DIR/lib/pkgconfig"

When I set the PKG_CONFIG_LIBDIR to "$PKG_CONFIG_SYSROOT_DIR/opt/homebrew/lib/pkgconfig", this error goes away but then I get the error that libunibreak is not found.

The issue seems that these are installed in separate locations but the build is looking in only one.

e.g dav1d is in arm64/opt/homebrew/lib

Screenshot 2024-04-28 at 3 38 53 PM

But libunibreak is in arm64/lib

Screenshot 2024-04-28 at 3 36 57 PM
sfan5 commented 2 weeks ago

Looks like whatever Homebrew is doing to configure its path is incorrectly affecting the cross-compilation environment. Do you happen to know how Homebrew does that?

mukulrwt commented 2 weeks ago

Not sure about Homebrew path configuration. All I see is it creates a symlink to opt/homebrew. I am unable to find if some path variables being used are conflicting with build.

Also seems like the difference in install locations is between deps installed through make and ninja.

The deps through make are being installed in arm64/lib whereas through ninja they are being installed in amr64/opt/homebrew/lib, eventhough the destination is same in both.

sfan5 commented 2 weeks ago

We use ninja only in combination with meson. It appears Homebrew patches meson by blindly replacing /usr/local in a bunch of files with the homebrew prefix (/opt/homebrew), which notably also affects the default installation prefix.

At this point I'm not sure if this is some kind of joke because this is the most terrible idea I have seen in a while.

Either way, can you check if this solves it for you? (remember to delete the old prefix first)

diff --git a/buildscripts/buildall.sh b/buildscripts/buildall.sh
--- a/buildscripts/buildall.sh
+++ b/buildscripts/buildall.sh
@@ -76,6 +76,7 @@ setup_prefix () {
 buildtype = 'release'
 default_library = 'static'
 wrap_mode = 'nodownload'
+prefix = '/usr/local'
 [binaries]
 c = '$CC'
 cpp = '$CXX'
mukulrwt commented 2 weeks ago

Now the build is being created successfully but the package is not installing.

It says that the package appears to be invalid. I don't suppose this change could have caused this issue.

syphyr commented 2 weeks ago

Now the build is being created successfully but the package is not installing.

It says that the package appears to be invalid. I don't suppose this change could have caused this issue.

Did you sign it?

mukulrwt commented 2 weeks ago

@syphyr Thanks for pointing that out. I didn't know that it was required.

Thanks for your help @sfan5 and @syphyr.

sfan5 commented 2 weeks ago

edddc1d

linsui commented 1 week ago

It seems this fix breaks the build on debian. https://gitlab.com/linsui/fdroiddata/-/jobs/6779635181

sfan5 commented 1 week ago

Nah that's b9da79b5697bac740796bdaefd967b00d7b0407c. I suppose it wasn't wise to "fix" this deprecation warning since that means older meson versions won't work anymore.

linsui commented 1 week ago

Yep, just comfirmed this fix doesn't matter so the other one causes the failure... Which meson version should be used?

sfan5 commented 1 week ago

At least 1.3.0 understands this syntax but you can just revert the change for the time being.

linsui commented 1 week ago

It's not even available in Debian testing. I'll revert that change, thanks!