Re: Strange OSX make check-world failure

From: Samuel Cochran <sj26(at)sj26(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Strange OSX make check-world failure
Date: 2018-12-08 03:20:19
Message-ID: 1544239219.1522393.1602722240.45C739CA@webmail.messagingengine.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Dec 7, 2018, at 5:26 PM, Tom Lane wrote:
> Interesting proposal, but I think it needs work.

Absolutely! I only hacked it together to the point that it worked on my laptop and illustrated the approach. :-)

> * As coded, this only fixes the problem for references to libpq, not
> any of our other shared libraries.

None of the the other shared libraries are referenced by the modified binaries:

$ for bin in tmp_install/usr/local/pgsql/bin/*; do otool -L $bin; done | grep dylib | sort -u
.../tmp_install/usr/local/pgsql/lib/libpq.5.dylib (compatibility version 5.0.0, current version 5.12.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1252.200.5)
/usr/lib/libedit.3.dylib (compatibility version 2.0.0, current version 3.0.0)
/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.11)

But I agree it would be nice to make it work in potential future cases, too.

> * It's also unpleasant that it hard-wires knowledge of libpq's version
> numbering in a place pretty far removed from anywhere that should know
> that.

Ideally it would iterate the binaries, iterate the load commands, and rewrite each.

> * Just to be annoying, this won't work at all on 32-bit OSX versions
> unless we link everything with -headerpad_max_install_names. (I know
> Apple forgot about 32-bit machines long ago, but our buildfarm hasn't.)

We can make the references relative which would dramatically decrease the sizes.

> * Speaking of not working, I don't think this "find" invocation will
> report any failure exits from install_name_tool.

If we iterate more carefully, as above, then failures should be reported and cause an abort.

> * This doesn't fix anything for executables that never get installed,
> for instance isolationtester.
>
> We could probably fix the first four problems with some more sweat,
> but I'm not seeing a plausible answer to the last one. Overwriting
> isolationtester's rpath to make "make check" work would just break
> it for "make installcheck".

Ah, sorry, I'm not super familiar yet with the build process so missed this bit. But I think executable-relative paths will fix.

I tried using this line instead and `make check` and `make installcheck` both work for me. It's awful, I'm not super fluent in Makefile so I'm sure it could be 100X better, and probably isn't quoted correctly, but the approach itself works. I couldn't quickly figure out a portable way to generate a relative path from bindir to libdir which would be a great improvement.

$(if $(filter $(PORTNAME),darwin),for binary in $(abs_top_builddir)/tmp_install$(bindir)/*; do for dylib in $$(otool -L $$binary | tail +2 | awk '{ print $$1 }' | grep '$(libdir)'); do install_name_tool -change $$dylib @executable_path/../lib/$${dylib##*/} $$binary || exit $$?; done; done)

Cheers,
Sam

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2018-12-08 03:28:53 Re: pg_partition_tree crashes for a non-defined relation
Previous Message Peter Geoghegan 2018-12-08 01:58:19 Re: Fixing findDependentObjects()'s dependency on scan order (regressions in DROP diagnostic messages)