gobject-introspection: add libtool library path tweaks to scanner

[michaelld]

Description

add libtool library path tweaks to giscanner from gobject-introspection

Hopefully the final tweaks, since they now handle both libtool and non-libtool library references, both in-build and outside-build.

Now works work: gobject-introspection, poppler, gstreamer1-gst-plugins-base, py38-gobject3, gexiv2, and libsoup

Ref: https://trac.macports.org/ticket/61427

Type(s)

• bugfix
• enhancement
• security fix

Tested on

macOS 11.0.1 20B5012d
Xcode 12.2 12B5035g

will also test on macOS 10.15 latest once this PR is in place

Verification

Have you

• followed our Commit Message Guidelines?
• squashed and minimized your commits?
• checked that there aren't other open pull requests for the same change?
• referenced existing tickets on Trac with full URL?
• checked your Portfile with port lint?
• tried existing tests with sudo port test?
• tried a full install with sudo port -vst install?
• tested basic functionality of all binary files?

[macportsbot]

Notifying maintainers:
@dbevans for port gobject-introspection.

[macportsbot]

Travis Build #15126 Passed.

Lint results

--->  Verifying Portfile for gobject-introspection
--->  0 errors and 0 warnings found.

Port gobject-introspection success on xcode10.3. Log
Port gobject-introspection success on xcode9.4. Log
Port gobject-introspection success on xcode8.3. Log

[ryandesign]

By reinplacing the MacPorts library path into the source code, we're getting farther away from something that can be accepted by upstream. Ultimately, the fix needs to be in upstream code; the problem affects all Mac users, not just MacPorts users.

[michaelld]

By reinplacing the MacPorts library path into the source code, we're getting farther away from something that can be accepted by upstream. Ultimately, the fix needs to be in upstream code; the problem affects all Mac users, not just MacPorts users.

I know. I really tried to find a way to not do this, but there's no way I can ... oh ... I just thought of a way that might work! Let me see if it works ... 🤞

[kencu]

I had kind of hoped that each library path might be looked at, and all relative paths to libraries would go on the DYLD_LIBRARY_PATH list as that would mean they are in-tree, and all absolute paths would go on the normal LIBRARY_PATH list.

Would that approach work?

[michaelld]

I had kind of hoped that each library path might be looked at, and all relative paths to libraries would go on the DYLD_LIBRARY_PATH list as that would mean they are in-tree, and all absolute paths would go on the normal LIBRARY_PATH list.

Would that approach work?

Nope. There's a mix of both provided, some in-build and some out-of-build; for each type; all depends on the port as to the mix.

[michaelld]

The meta-issue is that we require ${MP_PREFIX}/lib to not be in any -L nor DYLD_LIBRARY_PATH. We require it as part of LIBRARY_PATH so that it is the very last path searched & hence linking can still find the legacy library and any other libraries installed there. The LIBRARY_PATH might hold other paths beyond the minimum required.

It's good that by default LIBRARY_PATH contains ${MP_PREFIX}/lib. My idea instead of the reinplace patch method currently in this PR is to retrieve the environment setting for LIBRARY_PATH and remove those paths from the merged this_L and self._options.library_paths. Should be pretty straight forward to implement & is a reasonably robust solution to both libtool and non-libtool based linking.

[michaelld]

Took a bit to get the string manipulation correct, but this change works nicely & doesn't involve a reinplace but instead uses the LIBRARY_PATH as provided. In my reading of the logfiles, this works very robustly for both libtool and non-libtool library linkage within giscanner.

[macportsbot]

Travis Build #15148 Passed.

Lint results

--->  Verifying Portfile for gobject-introspection
--->  0 errors and 0 warnings found.

Port gobject-introspection success on xcode10.3. Log
Port gobject-introspection success on xcode9.4. Log
Port gobject-introspection success on xcode8.3. Log
Port gobject-introspection success on xcode7.3. Log

[ryandesign]

We can merge this since it's an improvement over what we have, but it still seems MacPorts-specific if it relies on LIBRARY_PATH being set to ${prefix}/lib, because it's MacPorts that does that; users building outside of MacPorts wouldn't necessarily have that set.

What I think we want is for paths that are within the build directory to go into DYLD_LIBRARY_PATH and other paths not to.

[kencu]

my very simplistic approach would have been to see if the first character of the library path is "/", and if it is not, add it to DYLD_LIBRARY_PATH.

That is so utterly simple I can only think that must have been the first thing anybody tried, but just why that doesn't work, I'm not sure.

[idryzhov]

Another way to fix the gstreamer1-gst-plugins-base issue: #9117

[michaelld]

Another way to fix the gstreamer1-gst-plugins-base issue: #9117

Thanks for that PR. It might work for the specific port noted; I have tried it. It makes a lot more sense to fix the underlying issue, which is what this PR does.

While this PR’s changes are MacPorts oriented via the used of LIBRARY_PATH, it’s still a reasonably generic solution.

Further, since GO-I is a much older version than the current release, it is IMHO OK to do the fix this way, since this fix will never make it upstream anyway.

[michaelld]

my very simplistic approach would have been to see if the first character of the library path is "/", and if it is not, add it to DYLD_LIBRARY_PATH.

That is so utterly simple I can only think that must have been the first thing anybody tried, but just why that doesn't work, I'm not sure.

For some ports that I tested, this would work. For others it doesn’t. Just because the path is absolute doesn’t mean diddly.

[michaelld]

We can merge this since it's an improvement over what we have, but it still seems MacPorts-specific if it relies on LIBRARY_PATH being set to ${prefix}/lib, because it's MacPorts that does that; users building outside of MacPorts wouldn't necessarily have that set.

What I think we want is for paths that are within the build directory to go into DYLD_LIBRARY_PATH and other paths not to.

I agree with you all around here. Unfortunately I could find no easy way to determine which paths are from the build and which are not. Without a top level source and build directory like what cmake provides, deciding which path is which isn’t really doable. This PR is the best compromise is could come up with in short order. As you note: it’s an improvement over what we have right now!

[kencu]

For some ports that I tested, this would work. For others it doesn’t. Just because the path is absolute doesn’t mean diddly.

Please help me understand. Seems like: If the path to the library is absolute, then it's not an in-tree library. If it's relative, then it's obviously not a system library, and must be in tree.

The only way I can see this going wrong is if somehow the build system passes the full pathname to the in-tree library during the build, which seems impossible -- or at least, really unlikely.

How can this go wrong?

[kencu]

Yeah -- sorry, never mind my last comment. I'm quite sure you worked it all through, and I'm sure not able to do the python stuff like you, so I'll just go with the flow here ...

[idryzhov]

I suppose ticket 61471 may also be related to gobject-introspection, but this PR, unfortunately, doesn't fix it.

[ryandesign]

Please help me understand. Seems like: If the path to the library is absolute, then it's not an in-tree library.

That's not a given. Absolute paths could be to any directory.

If it's relative, then it's obviously not a system library, and must be in tree.

Correct.

The only way I can see this going wrong is if somehow the build system passes the full pathname to the in-tree library during the build, which seems impossible -- or at least, really unlikely.

Some build systems do this.

[ryandesign]

I suppose ticket 61471 may also be related to gobject-introspection, but this PR, unfortunately, doesn't fix it.

I can't see anything in 61471 that suggests any relationship to gobject-introspection or this PR. This PR is about a build-time issue; 61471 is about a runtime issue.

[ryandesign]

this fix will never make it upstream anyway

I don't know why you assume that.

[kencu]

Some build systems do this.

Luckily the builds where the gobject-introspection issue comes up are 99% meson, 1% cmake, and some dying autotools builds being converted to meson, so the build system list we need to worry about is really short with respect to gobject-introspection.

Funny enough, I have actually been trying to force meson to use the full path name to the in-tree build libraries, to fix builds on Tiger as it has no rpath support -- and I've been unable to make meson do that so far :>

meson does keep an internal list of in-tree libraries, so it can fix up their install names on installation. So meson does know exactly which directories should be placed on a DYLD_LIBRARY_PATH list...I don't know how you would access that list from gir-scanner exactly...but if testing for "/" doesn't pan out, that is an alternate idea.

In the end having the build system tell gobject-introspection on each invocation where to look for in-tree libraries, rather than having gobject-introspection try to sort it out on it's own, might be the only resilient solution, if these simple fixes don't pan out.

[michaelld]

this fix will never make it upstream anyway

I don't know why you assume that.

Badly phrased on my part; I was on my phone & trying to be as brief & concise as possible. Let me try again.

Upstream's giscanner code -- on my basic read through & not actual use -- looks more robust than that for the GO-I currently in MacPorts. It is still likely to have issues, since it uses DYLD_FALLBACK_LIBRARY_PATH ... but who knows? Until MacPorts' GO-I is updated to the current release, there's not a lot of point in trying to create a patch that can be pushed upstream since we have no good way to test / verify that it will work -- or at least I don't ; maybe others do. Hence, this fix is MP-specific and -- while we should try to make it as robust as reasonably possible -- there's only so far any code changes can be taken without knowledge of the top-level build and source directories (which to the best of my testing are not known during runtime from inside the giscanner code).

[michaelld]

Yeah -- sorry, never mind my last comment. I'm quite sure you worked it all through, and I'm sure not able to do the python stuff like you, so I'll just go with the flow here ...

It's not a matter of Python. It's multi-fold:

(1) to the best of my testing / understanding, CMake does not export its internal variable -- especially top-level BUILD and SOURCE directories -- to elsewhere e.g. Python unless directed to do so. Doing this on a port-by-port basis will be a total PITA.

(2) In my testing across various ports, some use absolute paths for all directories while others use a mix of relative and absolute. In my testing, all paths obtained from PKGCONFIG are absolute ... but I think that's just because that's the way those files are written rather than because PKGCONFIG interprets them into absolute form.

(3) With a mix of relative and absolute paths, and no knowledge of the top-level BUILD and SOURCE directories, there's no way -- good or hacky -- to determine which path should be directed to which environment variable.

Using the provided LIBRARY_PATH is a way to work around the reinplace I used before, and it is at least vaguely robust in the sense that any project could then use it. Hence, I think it's a reasonable way to determine the primary "MP system" library path versus the discouraged-unless-absolutely-necessary dropping in the ${mp_prefix} via reinplace.

[kencu]

every path returned by pkg-config has to be an absolute path, right? It would be useless otherwise.

And if a build is referencing libraries installed into the system, they all have to be absolute paths as well...

After I get clang-11 ready to roll, maybe I'll spend a week on this and learn some python! Thanks for taking it on -- clearly it was always going to be huge PITA, which is why it sat there for two years with nobody doing anything about it. :>

[michaelld]

every path returned by pkg-config has to be an absolute path, right? It would be useless otherwise.

Technically: no. I think that PKGCONFIG will return whatever was written into the file. We've had issues with projects writing "@rpath" before. That said, I would hope that any path returned is absolute, since otherwise it just doesn't make sense. But I don't think PKGCONFIG requires absolute paths nor enforces their use.

And if a build is referencing libraries installed into the system, they all have to be absolute paths as well...

Technically: no. It's very possible to use relative paths for this. That said, I would consider this use as obtuse at best for almost all cases. I'm sure there's some use-case where relative paths would actually make sense ... some strange corner-case ... maybe?

After I get clang-11 ready to roll, maybe I'll spend a week on this and learn some python!

Good idea! Python is a fun language, not too difficult to learn the basics of. I'm not Python expert -- as demonstrated that I couldn't work out the multifaceted list setting via multiple "if" statements ... it's not difficult once someone noted it to me ...

Thanks for taking it on -- clearly it was always going to be huge PITA, which is why it sat there for two years with nobody doing anything about it. :>

You're welcome! I hope I don't come off here as dismissive or overly general; that's not my intent. It just turns out that this is a complicated issue, and oftentimes complicated issues have complicated fixes.

[michaelld]

Any further comments / reviews here? Looking specifically at the port owner @dbevans ... who has yet to provide any feedback ... if not further comments / reviews in the next 24 hours then I'm moving ahead with merging per openmaintainer timeout.

[rseichter]

@michaelld When are you planning to merge this? It got bitten by this issue today, and it prevents me from building virt-manager on macOS 11. That, in turn, means that I have no GUI to access my libvirt-based virtual machines. 😬