Subject: [PATCH] pg_regress.c: Fix "make check" on Mac OS X: Pass DYLD_LIBRARY_PATH

From: Evan Jones <[email-masked]>
Date: Mon, 5 Jun 2023 09:47:30 -0400

This makes "make check" work on Mac OS X. Without this patch, on Mac OS X a
default "./configure; make; make check" fails with errors like:

dyld[65265]: Library not loaded: /usr/local/pgsql/lib/libpq.5.dylib
  Referenced from: <59A2EAF9-6298-3112-BEDB-EA9A62A9DB53>
/Users/evan.jones/postgresql-clean/tmp_install/usr/local/pgsql/bin/initdb
  Reason: tried: '/usr/local/pgsql/lib/libpq.5.dylib' (no such file),
'/System/Volumes/Preboot/Cryptexes/OS/usr/local/pgsql/lib/libpq.5.dylib'
(no such file), '/usr/local/pgsql/lib/libpq.5.dylib' (no such file),
'/usr/local/lib/libpq.5.dylib' (no such file), '/usr/lib/libpq.5.dylib' (no
such file, not in dyld cache)

The reason is that at some point, Mac OS X started removing the
DYLD_LIBRARY_PATH environment variable for "untrusted" executables [1]:
"Any dynamic linker (dyld) environment variables, such as
DYLD_LIBRARY_PATH, are purged when launching protected processes."

[1]
https://developer.apple.com/library/archive/documentation/Security/Conceptual/System_Integrity_Protection_Guide/RuntimeProtections/RuntimeProtections.html

One solution is to explicitly pass the DYLD_LIBRARY_PATH environment
variable to to the sub-process shell scripts that are run by pg_regress. To
do this, I created an extra_envvars global variable which is set to the
empty string "", but on Mac OS X, is filled in with "DYLD_LIBRARY_PATH=%s",
where the %s is the current environment variable. The "make check" Makefile
sets this environment variable to the temporary install directory, so this
fixes the above errors.

I tested this on Mac OS X and on Linux (Ubuntu 23.04).

Thanks!

Evan Jones

---

From: Julien Rouhaud <[email-masked]>
Date: Tue, 6 Jun 2023 10:25:08 +0800

Hi,

> sets this environment variable to the temporary install directory, so this

Note that this is a known issue and a workaround is documented in the macos
specific notes at
https://www.postgresql.org/docs/current/installation-platform-notes.html#INSTALLATION-NOTES-MACOS:

> macOS's “System Integrity Protection” (SIP) feature breaks make check,
> because it prevents passing the needed setting of DYLD_LIBRARY_PATH down to
> the executables being tested. You can work around that by doing make install
> before make check. Most PostgreSQL developers just turn off SIP, though.

---

From: David Zhang <[email-masked]>
Date: Fri, 16 Jun 2023 21:25:12 +0000

I have applied the patch to the latest master branch and successfully executed './configure && make && make check' on macOS Ventura. However, during the process, a warning was encountered: "mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]". Moving the declaration of 'result' to the beginning like below can resolve the warning, and it would be better to use a unique variable instead of 'result'. 

#ifdef __darwin__
static char extra_envvars[4096];
+int result = -1;
... ...
-int result = snprintf(extra_envvars, sizeof(extra_envvars), "DYLD_LIBRARY_PATH=%s",
+result = snprintf(extra_envvars, sizeof(extra_envvars), "DYLD_LIBRARY_PATH=%s",

---

From: David Zhang <[email-masked]>
Date: Thu, 22 Jun 2023 12:08:05 -0700

After conducting a further investigation into this issue, I have made 
some discoveries. The previous patch successfully resolves the problem 
when running the commands `./configure && make && make check` (without 
any previous sudo make install or make install). However, it stops at 
the 'isolation check' when using the commands `./configure 
--enable-tap-tests && make && make check-world`.

To address this, I attempted to apply a similar approach as the previous 
patch, resulting in an experimental patch (attached). This new patch 
helps progress the 'make-world' process and passes the 'isolation 
check', but there are still several remaining issues that need to be 
addressed.

Currently, there is a description suggesting a workaround by running a 
'make install' command first, but I find it to be somewhat inaccurate. 
It would be better to update the existing description to provide more 
precise instructions on how to overcome this issue. Here are the changes 
I would suggest.

from:
"You can work around that by doing make install before make check. Most 
PostgreSQL developers just turn off SIP, though."

to:
"You can execute sudo make install if you do not specify a prefix during 
the configure step, or make install without sudo if you do specify a 
prefix (assuming proper permissions) before make check. Most PostgreSQL 
developers just turn off SIP, though."

Otherwise, following the current description, if you run `./configure  
&& make install` you will get error: "mkdir: /usr/local/pgsql: 
Permission denied"

Below are the steps I took that led to the discovery of additional issues.

git apply  pg_regress_mac_os_x_dyld.patch
./configure
make
make check

... ...
# All 215 tests passed.

./configure --enable-tap-tests
make
make check-world

... ...
echo "# +++ isolation check in src/test/isolation +++"
... ...
dyld[32335]: Library not loaded: /usr/local/pgsql/lib/libpq.5.dylib
   Referenced from: <EB3758C5-A87B-36C5-AA29-C1E31AD89E70> 
/Users/david/hg/sandbox/postgres/src/test/isolation/isolationtester
   Reason: tried: '/usr/local/pgsql/lib/libpq.5.dylib' (no such file), 
'/System/Volumes/Preboot/Cryptexes/OS/usr/local/pgsql/lib/libpq.5.dylib' 
(no such file), '/usr/local/pgsql/lib/libpq.5.dylib' (no such file), 
'/usr/local/lib/libpq.5.dylib' (no such file), '/usr/lib/libpq.5.dylib' 
(no such file, not in dyld cache)
no data was returned by command 
""/Users/david/hg/sandbox/postgres/src/test/isolation/isolationtester" -V"

git apply pg_regress_mac_os_x_dyld_isolation_check_only.patch
./configure --enable-tap-tests
make
make check-world

... ...
# All 215 tests passed.
... ...
# +++ isolation check in src/test/isolation +++
... ...
# All 112 tests passed.

echo "# +++ tap check in src/test/modules/brin +++"
... ...
# +++ tap check in src/test/modules/brin +++
t/01_workitems.pl ........ Bailout called.  Further testing stopped:  
command "initdb -D 
/Users/david/hg/sandbox/postgres/src/test/modules/brin/tmp_check/t_01_workitems_tango_data/pgdata 
-A trust -N" died with signal 6
t/01_workitems.pl ........ Dubious, test returned 255 (wstat 65280, 0xff00)
No subtests run

Any thoughts ?

Thank you

David

> +result = snprintf(extra_envvars, sizeof(extra_envvars), "DYLD_LIBRARY_PATH=%s",

diff --git a/src/common/exec.c b/src/common/exec.c
index f209b934df..8cf2c21a66 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -74,6 +74,12 @@ static char *pg_realpath(const char *fname);
 static BOOL GetTokenUser(HANDLE hToken, PTOKEN_USER *ppTokenUser);
 #endif
 
+#ifdef __darwin__
+static char extra_envvars[4096];
+#else
+static const char extra_envvars[] = "";
+#endif 
+
 /*
  * validate_exec -- validate "path" as an executable file
  *
@@ -330,6 +336,15 @@ find_other_exec(const char *argv0, const char *target,
 	char		cmd[MAXPGPATH];
 	char		line[MAXPGPATH];
 
+#ifdef __darwin__
+        int result = 1;
+        result = snprintf(extra_envvars, sizeof(extra_envvars), "DYLD_LIBRARY_PATH=%s",
+                                                  getenv("DYLD_LIBRARY_PATH"));
+        if (result < 0 || result >= sizeof(extra_envvars))
+        {       
+                printf("extra_envars too small for DYLD_LIBRARY_PATH");
+        }
+#endif
 	if (find_my_exec(argv0, retpath) < 0)
 		return -1;
 
@@ -344,7 +359,7 @@ find_other_exec(const char *argv0, const char *target,
 	if (validate_exec(retpath) != 0)
 		return -1;
 
-	snprintf(cmd, sizeof(cmd), "\"%s\" -V", retpath);
+	snprintf(cmd, sizeof(cmd), "%s \"%s\" -V", extra_envvars, retpath);
 
 	if (!pipe_read_line(cmd, line, sizeof(line)))
 		return -1;

---

From: Peter Eisentraut <[email-masked]>
Date: Fri, 23 Jun 2023 23:05:52 +0200

On 22.06.23 21:08, David Zhang wrote:
> Otherwise, following the current description, if you run `./configure && 
> make install` you will get error: "mkdir: /usr/local/pgsql: Permission 
> denied"

I think you should interpret "doing make install" as "running make 
install or a similar command as described earlier in this chapter". 
Note also that the installation instructions don't use "sudo" anywhere 
right now, so throwing it in at this point would be weird.

> /Users/david/hg/sandbox/postgres/src/test/modules/brin/tmp_check/t_01_workitems_tango_data/pgdata -A trust -N" died with signal 6

As I mentioned earlier, you would need to find all uses of system() in 
the PostgreSQL source code and make your adjustments there.  IIRC, the 
TAP tests require pg_ctl, so maybe look there.