Fix linking of shared/static libs with static libs

[nirbheek]

This commit contains the following fixes:

1. When a shared library A does link_with: to static library B, the
   parts of B used by A will be added to A, and so we don't need to
   return B in A.get_dependencies() for targets that link to A. This
   already is the behaviour when a shared library A does link_whole:
   on B.

2. In situation (1), when generating a pkg-config file for A, we must
   also not add B to Libs.private for A. This already is the behaviour
   when a shared library A does link_whole: on B.

3. When a static library A does link_whole: to static library B, we
   must add the objects in B to A.

4. When a static library A does link_with: to static library B, and
   B is not installed (which makes it an internal static library), we
   must add the objects in B to A, otherwise nothing can use A.

5. In situation (4), when generating a pkg-config file for A, we must
   also not add B to Libs.private for A.

All these situations are tested by the unit test added in this commit.

Closes #3934
Closes #3937

[nirbheek]

Ready for merging after review. This is a pretty important issue, without which static libraries in glib and gstreamer are completely unusable.

[nirbheek]

Tests fixed.

[jpakkane]

Has this been tested with gstreamer et al?

[nirbheek]

I've tested it with gstreamer and glib. I want to test more, but was focussing on the stable release.

[jpakkane]

Is this ready to merge now?

[nirbheek]

I'd like #4061 and #3894 merged first, to avoid conflicts and to make it easier for me to backport those to 0.47.

[textshell]

Sorry for just now noticing this.

Do we really want to change the output of the build depending on how a library is installed? Maybe a library is installed using a install script? Maybe a project doesn't even use meson installation support?

Point 4 and 5 seem like very implicit magic to me. At the very least this needs clear documentation for our users.

[jpakkane]

It seems more straightforward that if you "link whole" a static library to another, the end result should be the same as when plain linking a static library to another. That is: it should be "link whole"d into the final target that uses it.

[nirbheek]

Do we really want to change the output of the build depending on how a library is installed? Maybe a library is installed using a install script? Maybe a project doesn't even use meson installation support?

... this is how Autotools already works. See: #3937

Point 4 and 5 seem like very implicit magic to me. At the very least this needs clear documentation for our users.

Point 4 is also how Autotools already works, and point 5 is how people expect pkg-config files to work.

[jpakkane]

The fact that autotools does X is a very strong recommendation to do the exact opposite, regardless of the value of X.

[nirbheek]

The fact that autotools does X is a very strong recommendation to do the exact opposite, regardless of the value of X.

Is that why we copy Autotools behaviour in so many ways ranging from library names to option names? 😄

In any case, my point was that this is not unexpected behaviour, it's what users already expect. The reason why we haven't seen this yet is because glib and gstreamer are still shipping with Autotools. Without these patches the static libraries in those projects are missing a lot of objects.

[textshell]

It seems more straightforward that if you "link whole" a static library to another, the end result should be the same as when plain linking a static library to another. That is: it should be "link whole"d into the final target that uses it.

I'm not questioning the how "link whole" should work. I'm questioning that a normal link starts behaving more like a "link whole" depending on if the linked library is set to be installed.

I have to admit, that i just now notices this PR so i couldn't yet actually grok the code changes.

But this really seems like too implicit having it tied to the install argument. Users will not expect that to change how the actual libraries are build. They might even just add it when everything works. Which is does not, because they have not added it yet.

Point 4 is also how Autotools already works

This i certainly an non argument in meson land. Or even an indication to think really hard if this is something that really makes sense or just yet another quirk.

[textshell]

In any case, my point was that this is not unexpected behaviour, it's what users already expect. The reason why we haven't seen this yet is because glib and gstreamer are still shipping with Autotools. Without these patches the static libraries in those projects are missing a lot of objects.

I would certainly not expect this at all. I'd rather we force users to explicitly label internal only libraries than to tied it to something that surely is related but not really that closely. In my book "explicit is better than implicit" as long as it's not so much of a burden to make it impractical.

We do have choices here to make the meson builds for for the affected libraries.

[jpakkane]

So something like having a kwarg like link_with_internal? Would that work or would that get too confusing?

[jpakkane]

Then we could even verify and mandate that all those kinds of libraries must be static.

[textshell]

Then we could even verify and mandate that all those kinds of libraries must be static.

Yes, i was just going to ask what happens when that library is shared. I think erroring out might be a good start actually.

Now remind me, what is the difference between link_whole and link_with_internal?

For shared libraries? Seems like this only links in symbols (with function gc) / objects (without function gc) from the static lib that are needed?

For static libraries? Nothing? Could we later improve that to more closely match the shared case?

[jpakkane]

For static libraries? Nothing?

The only difference I can see is that it produces smaller binaries because the linker can strip out unneeded code.

[textshell]

For static libraries? Nothing?

The only difference I can see is that it produces smaller binaries because the linker can strip out unneeded code.

I think the current code does not do that, but i think we could define meson's semantics in a way that this could be added later.

On to the bike-shedding. Is link_with_internal the right name for the kwarg?

• internal seems to imply something is hidden from the outside. In this case it would be the dependency but not the actual content. (modulo -fdefault-visibility as usual for shared libraries)
• On the other hand this type of linking is some kind of inclusion of the (needed) objects, so that might be an option too.
• Something else?

[textshell]

Do we need something like link_with_internal for use with declare_dependency too?

I think when used with a both_libraries return object, this needs to know to take the static library (not the default shared one)

[nirbheek]

I think the behaviour implemented by this PR is absolutely the expected behaviour for people who have a setup where they link_with: a helper static library into their shared library. As shown by the build files written in glib and gstreamer, this is how people expected meson to work. We at gstreamer noticed it first because we started deploying meson for static-only builds intended for iOS and Android.

Of course you want the objects from the helper static library included into the main shared/static library. Nothing else makes sense, unless of course you're installing the static library and then you want the -lfoo arg in the pkg-config file so users know that they should link to the static library.

This is different from linking to a shared library because that adds an entry to DT_NEEDED, so the linker will search for that shared library automatically. That doesn't work for static libraries.

One theoretical problem here is if the user is installing the static library through some other means, such as an install script. For that we could add a kwarg to the library to specify that it's installed (and hence not internal, although we still won't know what arguments to add to the pc file, so that part can't work well).

I don't think it makes sense to define the relationship through a new link_*: kwarg. That will only be confusing for people who expect "link to static library" to work as-is. I also think the property resides on the static library, not in the relationship between the static library and its consumer. I doubt there's a case where you want to treat a static library as internal in one link and external in another.

[jpakkane]

Just as a clarification: this functionality (or something like it) is needed and we should provide for it. Maybe we will do what Autotools does, but we should not be beholden to it. If we can come up with an API that is better (possibly more explicit, easier for newcomers, what have you) then we should use that instead. This is a major change in behaviour so it is worth thinking about before committing (but not endlessly bikeshedding, obviously).

[textshell]

Having the information explicit on the internal library by a kwarg like "internal" or something else was my first impulse. So i think that is an alternative option to 'link_with_internal'.

Just thinking out loud, these internal libraries are not really much of libraries, but more collections of objects for later reuse that have independent compiler flags (which allows them to be reused). We also have constant requests for having independent compiler flags every once in a while and users are put of by telling them to use a library because of some connotations of that word. Maybe there is something common struggling to be discovered. Something like a "part". Or maybe not^^

[nirbheek]

I think it's a good idea to allow people to override the default of interpreting the install: kwarg to determine whether the library is internal or not.

One important deadline we have for this is that glib is planning on dropping Autotools entirely after their upcoming 2.58.0 release, and it would be good to have 0.48 out with this PR before that.

I also have the macOS compatibility_version issue on my list for targeting that.

[xclaesse]

I'll revise this PR next week.

ping? We are past the freeze already, but this is a serious issue, would be really appreciated to get it for 0.49.0 if you have the time ASAP.

[nirbheek]

I don't think it's a good idea to push this in last-minute, but yes I will find some time for this in the next couple of days so we can get it in after the release.

[Cogitri]

So...very sorry to ask for an ETA here, but I suppose this will be fixed soon-ish (not quite sure when 0.50.0 is scheduled to release). Glib 2.60.0 has been released and can't be used on any distro which needs its static libs due to them dropping autotools and this not being fixed yet which sort of blocks GNOME 3.32 at least on Void Linux :/

[ocrete]

Just one comment here, I don't think this should be specific to internal "convinience" libraries. I'd like to be able to have the same behaviour when linking in static libraries built with some other tool into my meson-built library.

[aduskett]

Where are we on this? I have just tested this patch series applied to meson .050.1 and GStreamer 1.16.0 with a static build using Buildroot and it fixes all the linking issues. It would be nice to not have to include a 406 line patch in Buildroot

[jpakkane]

Well this one ⚔️ conflicts currently so it needs a rebase at least. @nirbheek what is your opinion on this now? Is it ready for merging or does it still need work?

[aduskett]

For what it's worth, I did apply this merge request to the latest release and it does fix static linking with gstreamer 1.16.0 and glib 2.60.2

[nirbheek]

@nirbheek what is your opinion on this now? Is it ready for merging or does it still need work?

I don't think this PR can go in as-is anymore because projects have added workarounds that will cause errors now. I think the last time I checked, Mesa was one such project.

One part of this can go in, and that's the link_whole: behaviour with static libraries, point (3) in the PR description and (5) if it is still needed. The others are harder to get right. We'll have to break compatibility to add them, so we'll need a solid plan there.

[eli-schwartz]

Similar issue: #4984

Maybe A link_with/link_whole: B does not need to return B as a dependency, but sometimes it does need to return B's dependencies.

[xclaesse]

Glib 2.60.0 has been released and can't be used on any distro which needs its static libs

I don't think that's true, Glib merged a workaround for this and I even wrote a CI test that ensure that installed glib can be used to static link an app. But if you have a specific issue, please report.

[Cogitri]

I don't think that's true, Glib merged a workaround for this and I even wrote a CI test that ensure that installed glib can be used to static link an app. But if you have a specific issue, please report.

That may very well be the case now, it definitively was broken for Void Linux with GLib 2.60.0, but there have been some GLib release since that.

[xclaesse]

I made a new PR based on this one, based on the parts that I actually understand. Can everyone here give it a try in their project (especially Meson) and report any regression, please?

#5936

[xclaesse]

3., 4. and 5. should be fixed by #5936.

1. When a shared library A does `link_with:` to static library B, the
   parts of B used by A will be added to A, and so we don't need to
   return B in A.get_dependencies() for targets that link to A. This
   already is the behaviour when a shared library A does `link_whole:`
   on B.

I don't think this is what we want here. If we omit B from get_dependencies(), won't it break ninja dep graph too? Also if we have a library C that link_with A, since A is shared library C.get_dependencies() never included B. We never linked C to B. So I think it was already working as expected.

2. In situation (1), when generating a pkg-config file for A, we must
   also not add B to Libs.private for A. This already is the behaviour
   when a shared library A does `link_whole:` on B.

pkgconfig generator is a bit more complicated. If for A you use shared_library() then it will never include Libs.private anyway. If it's both_libraries() then it does need to add B into Libs.private otherwise the static A won't work. So I think this has always been working as expected.

[xclaesse]

#5936 is now merged, I think we can close this PR since I think all valid cases have been covered already. Any remaining issue can be tackled in a new PR. Thanks everyone who participated.