Don't add dependencies recursively while linking by nirbheek · Pull Request #1545 · mesonbuild/meson

nirbheek · 2017-03-30T18:19:53Z

We were doing this on the basis of an old comment, but there was no test for it and I couldn't reproduce the issue with clang on Linux at all.

Let's add a test and see if it breaks anywhere if we stop doing this.

Halves the size of gstreamer's build.ninja from 19M to 8.5M

Closes #1057

tp-m · 2017-03-31T09:42:43Z

It appears that with this patch in the gtk4 meson build, the static gdk backend libs are no longer linked into the static lib libgdk.a now. They were passed to static_library(..., link_with: ...)

jpakkane · 2017-04-01T21:03:16Z

We really want this in as soon as possible. This may be the last chance to change this before we have too many users.

jpakkane · 2017-04-04T19:36:34Z

Apparently we don't have a test for exe -> static -> static, only one for exe -> static-> shared. We should add one.

nirbheek · 2017-04-04T19:48:39Z

Yeah, it's on my TODO list for this PR.

keszybz · 2017-04-13T00:59:41Z

FWIW, I'm building systemd with meson with this patch, and it seems to work nicely.

nirbheek · 2017-04-18T17:17:32Z

I should have some time to work on this tomorrow. If we have enough tests for various corner-cases, we could merge this for 0.40, and fix any breakage in a point release?

jpakkane · 2017-04-26T21:47:22Z

This message is posted to every outstanding merge request.

We have transitioned from Github wiki to our new documentation system that generates all documentation directly from the Git repo. This means that from now on every merge request must come with corresponding documentation changes.

If your MR requires documentation changes, do the necessary changes to files in docs/markdown.

If your MR adds a new feature, add a description to the release notes in docs/markdown/Release-notes-for-0.41.0.md.

If this commit is just a bugfix with no functional changes, you don't need to do anything.

Thank you

Linking dynamically against libudev will fail once mesonbuild/meson#1545 is merged and apparently already triggers a link failure on s390x. Make libshared provide by the udev symbols by including libudev_sources into libshared. This will cause those files to be compiled twice, but it actually reduces the installed size and is closer to what the autotools build system is doing. Closes systemd#5828

Linking dynamically against libudev will fail once mesonbuild/meson#1545 is merged and apparently already triggers a link failure on s390x. Make libshared provide the udev symbols by including libudev_sources into libshared. This will cause those files to be compiled twice, but it actually reduces the installed size and is closer to what the autotools build system is doing. Closes systemd#5828

Linking dynamically against libudev will fail once mesonbuild/meson#1545 is merged and apparently already triggers a link failure on s390x. Make libshared provide the udev symbols by including libudev_sources into libshared. This will cause those files to be compiled twice, but it actually reduces the installed size and is closer to what the autotools build system is doing. Closes #5828

jpakkane · 2017-05-13T21:47:13Z

What's the status of this? Is any help needed?

nirbheek · 2017-05-15T05:53:04Z

Dropped off my list accidentally, will look into this today!

We were doing this on the basis of an old comment, but there was no test for it and I couldn't reproduce the issue with clang on Linux at all. Let's add a (somewhat comprehensive) test and see if it breaks anywhere if we stop doing this. Halves the size of gstreamer's build.ninja from 20M to 8.7M Closes #1057

This significantly reduces the size of build.ninja for GStreamer.

Now we aggressively de-dup the list of libraries used while linking, and when linking with GNU ld we have to enclose all static libraries with -Wl,--start-group and -Wl,--end-group to force the linker to resolve all symbols recursively. This is needed when static libraries have circular deps on each other (see included test). The --start/end-group change is also needed for circular dependencies between static libraries because we no longer recursively list out all library dependencies. The size of build.ninja for GStreamer is now down to 6.1M from 20M, and yields a net reduction in configuration time of 10%

nirbheek · 2017-06-02T01:49:25Z

"Today" came 18 days late, but I have something that works for most cases now. Some people will get build failures because they under-specified the libraries that they need, but gstreamer itself didn't need any such fixes and that makes me think that people are generally disciplined about this.

Anyway, the fix in such cases is easy and is backwards-compatible so I think we shouldn't worry about this too much.

codecov-io · 2017-06-02T02:53:36Z

Codecov Report

Merging #1545 into master will increase coverage by 0.06%.
The diff coverage is 96.15%.

@@            Coverage Diff             @@
##           master    #1545      +/-   ##
==========================================
+ Coverage   63.01%   63.07%   +0.06%     
==========================================
  Files          58       58              
  Lines       16882    16900      +18     
  Branches     3456     3464       +8     
==========================================
+ Hits        10638    10660      +22     
+ Misses       5150     5148       -2     
+ Partials     1094     1092       -2

Impacted Files	Coverage Δ
mesonbuild/compilers.py	`74.92% <100%> (+0.34%)`	⬆️
mesonbuild/backend/ninjabackend.py	`88.6% <100%> (-0.01%)`	⬇️
mesonbuild/backend/backends.py	`89.92% <50%> (ø)`	⬆️
mesonbuild/interpreter.py	`73.93% <0%> (+0.1%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6662f36...4b42805. Read the comment docs.

This is more reliable, and more accurate. For instance, this means arguments in commands aren't surrounded by `'` on Linux unless that is actually needed by that specific argument. There is no equivalent helper for Windows, so we keep the old behaviour for that.

igor-raits · 2017-06-02T05:11:41Z

mesonbuild/backend/ninjabackend.py


 if mesonlib.is_windows():
-    quote_char = '"'
+    quote_func = lambda s: '"{}"'.format(s)


[flake8]

[E731] do not assign a lambda expression, use a def

igor-raits · 2017-06-02T05:11:42Z

mesonbuild/backend/ninjabackend.py

+                    quoter = ninja_quote
                else:
-                    templ = q_templ
+                    quoter = lambda x: ninja_quote(quote_func(x))


[flake8]

[E731] do not assign a lambda expression, use a def

nirbheek · 2017-06-02T05:12:27Z

Also, here's some stats:

glib's build.ninja went from 614KB to 531KB
gst-build's build.ninja went from 20MB to 6.0MB

All this with no significant changes in configure time (gst-build's configure time went down by 10% for me, but likely because of the smaller build.ninja file).

These numbers can be reduced a bunch more by using Ninja rules for each target so that build statements don't have to repeat the cflags over and over for each source file in the target. We could reduce this even more by putting compiler-specific cflags in the compiler rules instead of in each target's build statements.

Interestingly, the "use shlex.quote for quoting" commit in this PR also reduces the build.ninja size considerably: 100KB for gst-build.

jpakkane · 2017-06-03T19:01:42Z

 executable('circular', 'main.c', link_with : [st1, st2, st3])

I'm surprised that this works on platforms without groupings, mainly MSVC. IIRC it does something saner than LD by looking at past libraries for symbols. So the question is:

is MSVC guaranteed to do the right thing
do we ever need to support a platform that gets this wrong and does not have static library grouping?

If the latter is true (dunno, maybe Solaris or somesuch works like that) then having the dependencies a a set is problematic because you would need have something like [st1, st2, st3, st1, st2] go through to the link line unaltered.

We could also say that this is too niche and we don't care or will fix it later if the issue really arises.

nirbheek · 2017-06-04T01:37:25Z

I wrote the circular test specifically because I had read that only GNU ld is dumb while resolving symbols and I wanted to add a test for it. macOS and MSVC are saner, fwict.

Perhaps Sun Studio's ld has the same issue as GNU ld, but we've never tested anything with that, so we should deal with that when someone cares about it. I will note that OpenIndiana moved from Sun Studio to GCC last year, so we may not have to care about it ever.

because you would need have something like [st1, st2, st3, st1, st2] go through to the link line unaltered.

Specifying a library multiple times will now be ignored because we de-dup library names in the CompilerArgs class while constructing compiler arguments. This can be turned off for a broken linker in the future if needed.

nirbheek · 2017-06-04T01:51:49Z

Let's list the platforms that we actively support:

Linux (GNU ld)
Windows (MSVC link, GNU ld, Clang lld)
macOS and iOS (Apple ld, Clang lld)
Android (GNU ld)
*BSD (GNU ld, Clang lld(?))

Others:

OpenIndiana (GNU ld)
Haiku OS (GNU ld)

Is there anything else we want to support? I think most edge-platforms actually rely on GCC, so we shouldn't have much to worry about.

QuLogic · 2017-06-04T02:11:34Z

gold?

nirbheek · 2017-06-04T02:12:05Z

I tested gold, it behaves the same way as bfd at least on Fedora.

jpakkane · 2017-06-04T11:07:17Z

🏁 🎉 🎆 🥂

nirbheek added dependencies gstreamer mesa/wayland/xorg labels Mar 30, 2017

jpakkane mentioned this pull request Apr 7, 2017

meson: build systemd using meson systemd/systemd#5704

Merged

nirbheek added the WIP label Apr 20, 2017

keszybz mentioned this pull request Apr 21, 2017

Fix LTO + static libraries on GCC and Clang #1649

Merged

userwithuid mentioned this pull request Apr 27, 2017

meson: drop implicit libudev link dependencies systemd/systemd#5817

Closed

keszybz mentioned this pull request Apr 27, 2017

meson: drop implicit libudev link dependencies systemd/systemd#5818

Merged

mbiebl mentioned this pull request Apr 28, 2017

meson: do not link libshared dynamically against libudev systemd/systemd#5850

Merged

jpakkane mentioned this pull request May 19, 2017

Frameworks passed many times on the linker command line #1826

Closed

nirbheek added 3 commits June 2, 2017 05:32

ninja: Use a set for target deps and ordered deps

d2dc38a

This significantly reduces the size of build.ninja for GStreamer.

igor-raits reviewed Jun 2, 2017

View reviewed changes

nirbheek removed the WIP label Jun 4, 2017

jpakkane merged commit 6205162 into mesonbuild:master Jun 4, 2017

nirbheek deleted the dont-link-recursively branch June 4, 2017 11:44

This was referenced Jun 4, 2017

Explosion of library names and paths in subproject dependencies #671

Closed

-rpath $ORIGIN is ignored at link time with GNU ld #1897

Closed

nirbheek mentioned this pull request Jul 22, 2017

CompilerArgs: Put start/end-group around shared libs too #2097

Merged

Uh oh!

Conversation

nirbheek commented Mar 30, 2017

Uh oh!

tp-m commented Mar 31, 2017

Uh oh!

jpakkane commented Apr 1, 2017

Uh oh!

jpakkane commented Apr 4, 2017

Uh oh!

nirbheek commented Apr 4, 2017

Uh oh!

keszybz commented Apr 13, 2017

Uh oh!

nirbheek commented Apr 18, 2017

Uh oh!

jpakkane commented Apr 26, 2017

Uh oh!

jpakkane commented May 13, 2017

Uh oh!

nirbheek commented May 15, 2017

Uh oh!

nirbheek commented Jun 2, 2017

Uh oh!

codecov-io commented Jun 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

igor-raits Jun 2, 2017

Choose a reason for hiding this comment

Uh oh!

igor-raits Jun 2, 2017

Choose a reason for hiding this comment

Uh oh!

nirbheek commented Jun 2, 2017

Uh oh!

jpakkane commented Jun 3, 2017

Uh oh!

nirbheek commented Jun 4, 2017

Uh oh!

nirbheek commented Jun 4, 2017

Uh oh!

QuLogic commented Jun 4, 2017

Uh oh!

nirbheek commented Jun 4, 2017

Uh oh!

jpakkane commented Jun 4, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

codecov-io commented Jun 2, 2017 •

edited

Loading