Re: Strange OSX make check-world failure
Hi folks 👋
Forgive me if I'm getting the mailing list etiquette wrong — first time poster.
I ended up sitting next to Thomas Munro at PGDU 2018 and talking about testing.
While trying to get `make check` running on my macbook, I think I may have
fixed this issue.
System Integrity Protection strips dynamic linker (dyld) environment variables,
such as DYLD_LIBRARY_PATH, during exec(2) [1] so we need to rewrite the load
paths inside binaries when relocating then during make temp-install before make
check on darwin. Homebrew does something similar [2]. I've attached a patch
which adjust the Makefile and gets make check working on my machine with SIP in
tact.
Cheers,
Sam
[1]:
https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html
[2]:
https://github.com/Homebrew/brew/blob/77e6a927504c51a1393a0a6ccaf6f2611ac4a9d5/Library/Homebrew/os/mac/keg.rb#L17-L30
On Tue, Sep 18, 2018, at 8:39 AM, Tom Lane wrote:
> Thomas Munro writes:
> > On Tue, Sep 18, 2018 at 2:14 AM Tom Lane wrote:
> >> "make check" generally won't work on OSX unless you've disabled SIP:
> >> https://www.howtogeek.com/230424/how-to-disable-system-integrity-protection-on-a-mac-and-why-you-shouldnt/
>
> > Aha! It looks like it was important to run "make install" before
> > running those tests.
>
> Right. If you don't want to disable SIP, you can work around it by always
> doing "make install" before "make check". Kind of a PITA though.
>
> regards, tom lane
>
>
From cdfe53a93453d8cdf12cfaaea13574365fbba66b Mon Sep 17 00:00:00 2001
From: Samuel Cochran
Date: Fri, 7 Dec 2018 15:27:30 +1100
Subject: [PATCH] Fix `make check` on darwin
System Integrity Protection strips dynamic linker (dyld) environment
variables, such as DYLD_LIBRARY_PATH, during exec(2), so we need to
rewrite the load paths inside binaries when relocating then during make
temp-install before make check on darwin.
https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 956fd274cd..48f2e2bcc7 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -390,6 +390,10 @@ ifeq ($(MAKELEVEL),0)
rm -rf '$(abs_top_builddir)'/tmp_install
$(MKDIR_P) '$(abs_top_builddir)'/tmp_install/log
$(MAKE) -C '$(top_builddir)' DESTDIR='$(abs_top_builddir)'/tmp_install
install >'$(abs_top_builddir)'/tmp_install/log/install.log 2>&1
+ # darwin doesn't propagate DYLD_* vars due to system integrity
+ # protection, so we need to rewrite the load commands inside the
+ # binaries when relocating them
+ $(if $(filter $(PORTNAME),darwin),find
'$(abs_top_builddir)/tmp_install$(bindir)' -type f -exec install_name_tool
-change '$(libdir)/libpq.5.dylib'
'$(abs_top_builddir)/tmp_install$(libdir)/libpq.5.dylib' {} \;)
endif
$(if $(EXTRA_INSTALL),for extra in $(EXTRA_INSTALL); do $(MAKE) -C
'$(top_builddir)'/$$extra DESTDIR='$(abs_top_builddir)'/tmp_install install
>>'$(abs_top_builddir)'/tmp_install/log/install.log || exit; done)
endif
Re: Strange OSX make check-world failure
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
