gnome: Distinguish between internal and external linker flags #3463
Conversation
|
This patch doesn't fix all library path issues on FreeBSD. It is still possible to see build failure caused by wrong library path (#2881). Library path is always hard to get right ... |
lantw44
force-pushed
the
master
branch
2 times, most recently
from
99fceb1
to
8944571
Compare
|
I just fixed the line of code which caused test failure, and I hope my patch is good for review now. |
|
This fixes the issue we have building meson projects that bundle libgd when the graphical library libgd (https://libgd.github.io/) is also installed on the system (same name but totally different libs). |
|
Ping ... Can we get this reviewed? |
This should not happen. When linking built libraries we always link them by their file names. Thus |
|
|
This seems like a missing feature. |
|
Yes, everytime a project decided not to use automake and libtool, its gir build is very likely to be broken. It is hard to get right, and it usually takes me several days, or even weeks, to fix it in a reliable way. The solution usually looks like ugly workarounds, including changing the order of By the way, as you can find in Do you want me to open a gobject-introspection feature request? I don't really understand how |
|
Hello, what should I do to make progress on this issue? |
|
I think we can get this in, I agree we should not block on gobject-introspection fixing itself. I believe we need two things:
For the test, you will likely want to write it as a unit test; see |
|
@lantw44 were you able to find time for this? |
|
I remember this pull requeset is still open, but I spent most of my free time fixing GLib tests on FreeBSD in the last week. |
|
I just updated the commit to include a new project test called |
Thanks! Will do a review soon.
Could you open an upstream bug about this too and paste it here? I am optimistic about getting it fixed there too. |
|
It looks like that GLib is not available in Windows and macOS test environments, so I should skip the test when GLib cannot be found. |
run_project_tests.py
Outdated
| @@ -679,13 +679,18 @@ def detect_system_compiler(): | |||
| else: | |||
| raise RuntimeError("Could not find C compiler.") | |||
|
|
|||
| def setup_env(): | |||
| # Required by 'frameworks/22 gir link order' test | |||
| os.environ['PKG_CONFIG_SYSTEM_LIBRARY_PATH'] = '' | |||
There was a problem hiding this comment.
This should not be needed anymore. We call pkg-config with PKG_CONFIG_ALLOW_SYSTEM_LIBS=1, so it will always have -L flags in it.
After rebasing on the current master branch, I am no longer able to reproduce build failure in
I am surprised to see that g-ir-scanner seems to already support it. The pkg-config change committed to meson recently also affects the GNOME module, and full path to .so files are now passed directly to g-ir-scanner instead of being converted to -L and -l flags. Atk and Pango can be successfully built with meson master branch, and However, I cannot test for gtk-doc because of unrelated problems. Pango gtk-doc build fails with 'no such file or directory' and Atk gtk-doc build fails with include path problems. |
That was the intention of that PR, and I'm glad to see that it worked here too. Thanks for testing and verifying that! What's the gtk-doc issue? Could you please investigate a bit more and file an issue about it? We have a release on 1st, and would be good to know about possible regressions. |
Could you please rebase this PR to only include the test then? That should go in so we don't accidentally break that in the future. |
I am not sure what you meant here. I think the patch to GNOME module here is still nice to have. There are still a few -L flags being used when running g-ir-scanner. Even if all external libraries are referenced with full paths, I think we will still want to make sure that internal libraries come before external ones because sometimes RPATH or RUNPATH set on external libraries can also cause problems. |
Which ones are those?
I am not sure I understand. With that patch, we get rid of I also do not understand your concerns about RPATH or RUNPATH, could you expand on that a bit? |
I just found that I was wrong ... g-ir-scanner seems to consider these .so file arguments as source files. It seems to ignore them and doesn't pass them to the linker. The reason it sometimes works is that these libraries may also be pulled in as dependencies by the library itself. |
Well that's dumb. I'll test tomorrow how much breakage this is going to cause. |
|
As I suggested in issue #3768, I think the best long-term fix here is to let meson/ninja build generated binaries instead of letting external scripts doing it. The code in GNOME module that try to pass the flags to python wrappers is scary, it tries to replicate what meson backends already does to collect the right flags in the right order. Would be nice if those GNOME scripts could only generate .c files and let meson deal with compilation/execution of them. I started playing with that idea in https://gitlab.gnome.org/GNOME/gtk-doc/merge_requests/1 |
Yes, ideally it won't cause any problems, but I cannot be sure without testing 200 modules in JHBuild ... As what I said in the previous comment, g-ir-scanner doesn't really support passing libraries with full path. The pkg-config change already causes g-ir-scanner to fail for some modules in my JHBuild environment, including link-time failure (undefined reference to some_fribidi_symbol) and run-time failure (undefined symbol some_colord_symbol).
Even if we have no -L flags, ld still have to find dependencies of libraries we pass on the command line. The search path of dependent libraries are not constructed from -L flags but made from several command line arguments, environment variables and ELF file headers. If I understand correctly, GNU binutils manual says the search order is:
If we ignore I think I just found gnome-session now fails to link because of the |
I filed 3 issues for gtk-doc problems: |
I did a test with You can find 20 library path issues in the test result.
The most common error is: It is likely to be related to the |
When an older version of the library being built is installed in the same prefix as external dependencies, we have to be careful to construct the linker or compiler command line. If a -L flag from external dependencoes comes before a -L flag pointing to builddir, it is possible for the linker to load older libraries from the installation prefix instead of the newly built ones, which is likely to cause undefined reference error. Since the order of dependencies is not significant, we cannot expect internal dependencies to appear before external dependencies when recursively iterating the list of dependencies. To make it harder to make mistakes, linker flags come from internal and external dependencies are now stored in different order sets. Code using _get_dependencies_flags are expected to follow the order when constructing linker command line: 1. Internal linker flags 2. LDFLAGS set by users 3. External linker flags It is similar to what automake and libtool do for autotools projects.
Those tools use our arguments to build a file and execute it to introspect it at runtime. However, they do not know that you can pass the full path to the library to use, and ignore the arguments. The long-term fix for this is to have them output a .c file that Meson will build for them, which they can then run, but that will require upstream changes: https://gitlab.gnome.org/GNOME/gtk-doc/merge_requests/1 Closes mesonbuild#3774
|
@lantw44 I've added a commit that should revert the change that caused us to return absolute paths to libraries to Could you please test if this fixes things for you? I'll be merging this in a few hours, but if you find any issues we can fix it for the stable release. The long-term fix is to have |
Unfortunately, it only fixes gst-plugins-bad build. at-spi2-core, gstreamer, gst-plugins-base and issues unrelated to the meson GNOME module are still not fixed. All of these 4 projects mentioned here can be built successfully with meson 0.46.1 if the patch proposed in this pull request is applied. Building at-spi2-core with meson 0.46.1 + patch: Building at-spi2-core with the current meson master branch We can found that The problem of gstreamer is similar: Building gstreamer with meson 0.46.1 + patch: Building gstreamer with the current meson master branch:
I haven't checked why gst-plugins-base fails, but it looks like a library path issue again. Building gst-plugins-base with meson 0.46.1 + patch: Building gst-plugins-base with the current meson master branch:
Yes, it will be much easier to test and debug if we don't have two ways to generate linker flags. However, it may also be required to find a solution for #1635 because the reason which g-ir-scanner currently works is that it can set |
The patch in this PR is already merged, but if you can find a fix for these issues, I'll gladly apply it for the 0.47.1 stable release. FWICT these are FreeBSD-specific issues, so I have no way to test it. |
|
Another thing that would help, is if you could help us with setting up FreeBSD CI on Travis. I have tried in the past and failed, but Rust has a way to do it. I think for continued stability and regression testing, this is required. |
I meant applying the older version of the patch to meson 0.46.1. This is what currently used in FreeBSD ports.
Do you mean running FreeBSD in QEMU, as Travis doesn't support FreeBSD? |
Yes, I typed 'enter' too soon, my edit tries to make my intent clearer: I can't reproduce this because I don't have a FreeBSD instance to test things on, so if you could figure out what else needs changing we can get this into the 0.47.1 stable release.
Yes, Rust seems to do this: https://github.com/nbaksalyar/rust-libc/tree/master/ci#qemu-setup---freebsd |
This is a follow-up of #1718 and #1932. It fixes appstream-glib gir generation failure and gst-plugins-base gtk-doc build error for me in JHBuild on FreeBSD.
When an older version of the library being built is installed in the
same prefix as external dependencies, we have to be careful to construct
the linker or compiler command line. If a -L flag from external
dependencoes comes before a -L flag pointing to builddir, it is possible
for the linker to load older libraries from the installation prefix
instead of the newly built ones, which is likely to cause undefined
reference error.
Since the order of dependencies is not significant, we cannot expect
internal dependencies to appear before external dependencies when
recursively iterating the list of dependencies. To make it harder to
make mistakes, linker flags come from internal and external
dependencies are now stored in different order sets. Code using
_get_dependencies_flags are expected to follow the order when
constructing linker command line:
It is similar to what automake and libtool do for autotools projects.