Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

meson: need to handle linking to static libass #9478

Closed
DanOscarsson opened this issue Nov 19, 2021 · 17 comments
Closed

meson: need to handle linking to static libass #9478

DanOscarsson opened this issue Nov 19, 2021 · 17 comments

Comments

@DanOscarsson
Copy link
Contributor

meson build need to better support linking to static libraries

Important Information

Provide following Information:

  • mpv version: 0.34.0-51-g3d9c0eae9e-dirty
  • Linux Distribution and Version: opensuse Leap 15.3
  • Source of the mpv binary: compiled myself

Reproduction steps

Build ffmpeg and libass using mpv-build. This results in static libraries for them, no shared versions.

Build mpv using meson.
Build will fail in linking due to libraries that libass depends on, are not included.

My analysis of the problem is that as libass is only available as a static library, the needed libraries it depends on, are not included by meson as they are not included by pkg-config unless --static option is given.

I have two ways I can fix this, by adding static: true for the libass dependency in the meson.build file, or by changing the pkg-config libass.pc file.
But adding static: true in the meson.build file results in warnings from meson about not finding static version on the libraries libass depend. But most people will want them to be shared versions even if the have a static libass.
Also, those not needing the latest libass from mpv-build, would prefer to link to the shared version.
Changing libass.pc works but is an extra step and best if it can be avoided as the changed version is not suitable for a shared library.

Best would be if meson could handle this.
Looking at the meson source code I see that meson locates the full path for all libraries if locates. So it could easily see that a library is only available in static format and then add the needed libraries it depends on.

If you cannot get meson to fix this you should add some information to/about mpv-build of how to handle linking to libass with meson.
For a developer of mpv it is good to be able to use a static libass as you the can experiment with libass to better suit mpv, without disrupting other programs using libass. So using libraries like in mpv-build is needed.

@Dudemanguy
Copy link
Member

The sad answer is that there is currently not a good way to do this. Adding static: true does work like you said, but that has a side effect of spamming a ton of confusing warnings for people that aren't looking for static linking. I also do not think the upstream meson.build file project should enforce shared vs static when it's not needed.

The workaround I would go with in this case is passing an ugly c_link_args to specify all the needed static libraries. The scripts in mpv-build do not currently have any logic in them to deal with the newly added meson build. That's something that's on my TODO list, but the problem can be worked around.

@rubyFeedback
Copy link

I also do not think the upstream meson.build file project should enforce shared vs static when it's not needed.

My comment is not so much related to the discussion here as such - although it's cool that people pay more
attention to meson, perhaps one day we can leave waf behind - but in regards purely to shared versus static:
while I tend to use shared libraries, some software I use as static variants. Busybox, grep, sed, awk ... if possible
it would be useful if mpv could also be built as a static binary. Even if it is larger. I tend to break software a LOT
and move files all over everywhere. libass + waf sometimes gives me headaches; ultimately the end result is
the most important aspect for me. I think confusing warnings aren't really an obstacle (I reported some warnings
by meson to meson upstream and the finnish core dev guy tends to listen to good use cases, so these were
mostly resolved. I have had only good impressions with meson so far in general.)

@Dudemanguy
Copy link
Member

Nothing is stopping you from building it statically. It's just not as convenient as it should be.

@DanOscarsson
Copy link
Contributor Author

I have tested to add to meson so that if a library it looks up is detected to be static it will add the dependencies you get from pkg-config --static. This way you do not need to add static:true and you will not get warnings about not finding static extra libraries.
Using this with mpv works fine for me with no changes to standard meson.build file or libass.pc file.
As I have not studied the meson code before there may be more to think about when adding it to meson. The change to meson are a few lines of python code so somebody working with meson should be able to add it easily.

@Dudemanguy
Copy link
Member

I opened up a PR in meson that would solve this: mesonbuild/meson#9603.

@eli-schwartz
Copy link

Another interesting solution would be libass/libass#330 :)

You could then configure mpv to build a local static copy of libass, with free LTO thrown in (and it would work OOTB for people who don't have libass installed at all yet).

A global meson option is a pain point which I'd be happy to see solved though, of course.

@DanOscarsson
Copy link
Contributor Author

The PR by @Dudemanguy is not exactly the way I thought here.
In my case pkg-config --libs libass
gives: -L/specialdir -lass
which means that libass should be searched in /specialdir
As it happens there is only a libass.a in /specialdir so it will be linked statically.
And because it linked statically you need to call pkg-config --libs --static libass
to get all libraries you need to link also.
It is not that I prefer to use static libraries, it is that a library is only in static form and in that case you need to add the libraries it depends on (which meson misses).
While this is probably quite common for developers to use when experimenting, there can be standard libraries that are only available in static form.

So my solution is to have meson during pkg-config checking when it locates all libraries and it detects a library that need to be linked statically, it will call pkg-config with --static to add the libraries the static library depends on.
No option is needed for this (unless you want an option forbidding linking with static libraries). As different systems and cases have compiled libraries in different ways it would be good to handle as many cases without an option if that is possible.
As meson had code for how static libraries are named it was quite simple to add this.
An option prefer_static (or change static option to be true, prefer, false) could also be good for those that have several versions of a library and prefer to use the static version of that library.

@rorgoroth
Copy link

I opened up a PR in meson that would solve this: mesonbuild/meson#9603.

FWIW That also fixes the mingw static problem (aside from the mpv.com getting linked in too).

@Dudemanguy
Copy link
Member

Dudemanguy commented Nov 22, 2021

@DanOscarsson: I'm not sure I understand. The prefer_static option would cause meson to read the Requires.private field of the .pc file (aka the same thing as calling pkg-config with --static) which should cover the static linking you are talking about. You might have to use pkg_config_path or such to handle a library being in a different path but I think that covers what you are talking about. For every other library, it would just fall back to dynamic since it's not a strict "static-only" check.

FWIW That also fixes the mingw static problem (aside from the mpv.com getting linked in too).

Is this a bug in the meson build? I'm not exactly sure how this is supposed to work on Windows but looking at the .com, it doesn't look like it should be linked to anything but merely a separate executable.

@rorgoroth
Copy link

Is this a bug in the meson build? I'm not exactly sure how this is supposed to work on Windows but looking at the .com, it doesn't look like it should be linked to anything but merely a separate executable.

Idk, possibly?
The mpv.com is produced separately (generated/mpv.com) but gets combined in to mpv.exe - mpv.exe actually acts like mpv.com - if you double click it will flash open/closed, if you add a blank config with player-operation-mode=pseudo-gui it will stay open and open the actual mpv window separately.

@DanOscarsson
Copy link
Contributor Author

@Dudemanguy If I understand your prefer_static option, from the PR, you set it with the
meson configure command and it will result in static: true being used as default, or how do you set it?
But I do not want static linking and static versions of libraries by default.
Using shared libraries and linking shared by default is fine, but if a library mpv depends on is only available in static format, it should be used (which it is today) and as a static library do not automticaly link in the libraries it depends on, they have to be added (which meson fails to do today). So the case I want meson to handle is normal shared linking with one or more of depending libraries are only in a static version.

Also it looks like setting static: true on a dependency I would expect that to mean: use the static version of the dependency. But from the warnings meson gives it looks like it expects all libraries the static marked library depends on, to have a static version and be used. That is not what I would expect should happen if the default is to prefer shared.

@Dudemanguy
Copy link
Member

The mpv.com is produced separately (generated/mpv.com) but gets combined in to mpv.exe - mpv.exe actually acts like mpv.com - if you double click it will flash open/closed, if you add a blank config with player-operation-mode=pseudo-gui it will stay open and open the actual mpv window separately.

Okay yeah this sounds like a bug. I'm pretty sure waf doesn't do that.

So the case I want meson to handle is normal shared linking with one or more of depending libraries are only in a static version.

I think should still work? Admittedly I didn't test it, but what should happen is that the .a gets picked and since none of the static deps would be found in your case, then they just get linked shared instead. It's not the most elegant thing ever since it'll throw some warnings at you but it should work I believe.

@DanOscarsson
Copy link
Contributor Author

If I understands correctly the prefer_static means to request static everywhere.
That is not what I want. I want meson to create a normal mpv with shared libraries except in those cases where a shared library is missing but a static version is available and then use it.
I assume that is what most people want.
This does not work today as meson misses to add libraries the static library depends on.

That is what my small patch to meson fixes.
I will have to use it until something better comes up.

@Dudemanguy
Copy link
Member

This does not work today as meson misses to add libraries the static library depends on.

Right, that's exactly what prefer_static would solve. Unless you have a bunch of random static libraries in your pkg-config path that are duplicates of existing shared libraries, the other dependencies would just fallback to normal shared libraries.

@DanOscarsson
Copy link
Contributor Author

Tested by manually adding static:true to some of the dependencies i the build file.
I get a lot of warnings about libraries they depend on not being static.
I assume that will happen with your patch too.
Not very nice.
My patch detect when a library is only available in static format and then add the libraries it depends on without any warnings as in this case they are not preferred to be static.
While it is about 30-35 added/changed lines I do not think I will have time just now to make a PR to meson for them.

@Dudemanguy
Copy link
Member

Dudemanguy commented May 4, 2022

I know it's not exactly what you want, but the above mentioned PR has been merged. So the next meson release should make it a lot easier to do static linking. Wanting to handle only specific dependencies and such is still definitely a valid feature request. Feel free to make one upstream if such an issue isn't already open. I'll go ahead and close this now.

Note to self: look at the windows meson builds again since I think they are kind of bugged with the .exe/.com thing.

@rorgoroth
Copy link

Nice, I can build with upstream meson just fine now. The exe/com thing is indeed still an issue though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants